Forum Discussion

Altera_Forum's avatar
Altera_Forum
Icon for Honored Contributor rankHonored Contributor
13 years ago

synthesis error 10818

Hi,

while working with OpenCores SD card controller ( http://opencores.org/project,sdcard_mass_storage_controller )

I got error 10818 while trying to synthesize this for Cyclone IV (DE2-115).

The issue boils down to this not synthesizable construct: 


module sd_bd (
	input clk,
	input rst,
	input ena,
	output reg result
);
 
always @(posedge clk, posedge rst )
begin
	result <=0;  
	if (rst) 
		result <=0; 
	else if (ena) 
		result <= 1;
end
endmodule

gives this error message when trying to synthesize it for Quartus 12.1 Build 177: 


Critical Warning (10237): Verilog HDL warning at sd_bd.v(10): 
can't infer register for assignment in edge-triggered always construct because the clock isn't obvious. Generated combinational logic instead
Error (10818): Can't infer register for "result" at sd_bd.v(13) because it does not hold its value outside the clock edge 
File: /commonroot/home/vleo/000_JOB/EISU_OR1200/sdc_test_on_DE2_115/vhdl/sd_controller/sd_bd.v Line: 13

What is described there - seems like a D-trigger, i.e. the ena input is feteched on the clk edge, then held till next clock.

This is certainly not a good way to code what's intended, but this construct is used over and over in sdc_controller project. i wonder what is the workaround, i.e. minimal patch that will allow this to function.

Actual code contains other signals on input and output, but the root cause of Quartus error is this ill-shaped D-trigger.

I'm attaching ModelSim archive that shows that this code simulates as expected and Quartus project, that gives error 10818

p.s. I first thought that this illustrates Verilog language deficiency - none of this should happen in VHDL. But unfortunately I got the very same error with VHDL version of the example as well.

In Verilog though, it works, when rst is removed from sensitivity list. So - Verilog seems to win here :)

7 Replies

  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor
    13 years ago

    try this:

    always @(posedge clk, posedge rst )

    begin

    //result <=0;

    if (rst)

    result <=0;

    else if (ena)

    result <= 1;

    else

    result <= 0;

    end
  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor
    13 years ago

    Great! It works with assignment moved to the else clause - builds in Quartus - and Modelsim shows the same result.

    Thank you so much. I'm not good with Verilog+Quartus, this is valuable lesson.

    V.L.
  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor
    13 years ago

    Here is an update - although this workaround worked well for the pruned test case, it also works for the "real" code: 

    
always @(posedge clk or posedge rst )
begin
   new_bw <=0;  
  
  if (rst) begin
    m_wr_pnt<=0;
    
    write_cnt<=0;
    new_bw <=0; 
   
  end
  else if (we_m) begin    
    if (free_bd >0) begin
      write_cnt <=~ write_cnt;
      m_wr_pnt<=m_wr_pnt+1;
      if (!write_cnt) begin  //First write indicate source buffer addr
        bd_mem<=dat_in_m;            
      end
      else begin        //Second write indicate SD card block addr
        bd_mem<=dat_in_m;
        new_bw <=1;     
      end
     end
  end   
       
end

    but the issue now is - moving new_bw to an 'else' clause requires putting it there several times. It's far from mechanical patch. The pattern of 

    always ... begin 
xyz<=0; 
if ... if ..  if .. then xyz <=1  ... 
end

    is rather frequent in the sdc_controller code.

    So, although this is a workaround, the resulting code is difficult to understand and VERY prone to errors - if you miss one branch, you'll get a register that holds value longer then 1 clock cycle, which is not what is intended.

    I still wonder if there is option or way to make Quartus understand this rather simple preposition - a default value on the signal in the always block.

    So, what about simply removing 'rst' from the always sensitivity list? This is not quite the same function, but if reset is simply a reset there, not cigar pretending to be a reset :-) it may work.
  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor
    13 years ago

    is this below any good? 

    
always @(posedge clk, posedge rst )
begin
    //result <=0;  
    if (rst) 
        result <=0;
    else if (~ena) 
        result <= 0;
    else
       result <= 1;
end
  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor
    13 years ago

    This IS good, but in real code, there are far more branches and you'll end up with many 'else result <= 0'.

    When it's not just one 'result' signal, but say 5 signals, it becomes a real mess.

    For now, I'm removing 'posedge rst' - and hope that sync. reset vs. async. will not matter. At least it builds now, we'll see if the whole design works with that.
  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor
    13 years ago

    You can use something like 

    always @ (posedge clk, posedge rst) begin
if (rst) begin
 // Reset code; compile time only constants.
 xyz <= 0;
else if (enable) begin 
 // Synchronous code
 xyz <= 0;
 if (somecondition)
  xyz <= 1;
 else if (someother condition)
  xyz <= 2;
end
end

    The problem in your first code snippet was that you have an action outside the first "if/else" block, so the behavior wasn't clear to the tool.
  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor
    11 years ago

    --- Quote Start ---

    is this below any good? 

    
always @(posedge clk, posedge rst )
begin
    //result <=0;  
    if (rst) 
        result <=0;
    else if (~ena) 
        result <= 0;
    else
       result <= 1;
end

    --- Quote End ---

    I believe most of the FPGA synthesis tools have a problem understanding the structure of a MUX and a FF when the "always @(.... " structure has a default assignment outside the if/else-if/else structure. When I have to add a default assignment, I generally do it as follows. 

    
reg result_comb;
reg result;
always @(*)
begin
  result_combo = 0;
  if(cond1)
    result_combo = 1;
  else if (cond2)
    result_combo = 0;
end
always @(posedge clk, posedge rst)
begin
  if(rst)
    result <= 0;
  else
    result <= result_combo
end

    This may not satisfy the "minimal patch" requirement, but it works