error 10818 with simple VHDL construct under Quartus v 12.1 build 177

Because having the line:

result <= '0';

before the if statement does not match the "pattern" used to infer a register.

Drop that line and it'll synthesize fine.

Your process line should be process(clk,rst). Sure, all works, but it does not provide your "intent" for the logic. Explicitly indicating you wanted the process only to be sensitive to clock and reset helps the next person maintaining your code understand it better.

Cheers,

Dave

Ok, thank you very much Dave for the insight. Here is the full story of this construct.

While working on sdc_controller from OpenCores (SD card controller) I was getting Error 10818. Never seen it with my own code - I've been indoctrinated well to avoid this :) It seems, that authors of that code were using Actel tools, that should be comfortable with the above coding style (which I still don't like). For example one of the fragments looks like this:

always @(posedge clk or posedge rst )
begin
   new_bw <=0;    
  if (rst) begin
    m_wr_pnt<=0;
    write_cnt<=0;
    new_bw <=0; 
    read_cnt<=0;
   end
  else if (we_m) begin    
    if (free_bd >0) begin
      write_cnt <=write_cnt+1;
      m_wr_pnt<=m_wr_pnt+1;
      if (!write_cnt) begin      //First write indicate source buffer addr (2x16)
        bd_mem<=dat_in_m;             
      end
      else begin        //Second write indicate SD card block addr (2x16)
        bd_mem<=dat_in_m;
        new_bw <=write_cnt;      //Second 16 bytes writen, complete BD
      end
     end
  end   
end

Now, if I want to take out assignment new_bw<=0 before the 'if' assignment, I need to put 'else new_bw<='0'' to several places.

And this is prone to error. And if I miss one, the D-trigger will be inferred and the design will not operate as intended.

Thus the question is:

what is the proper pattern to assign some output signal in the process on clock edge, but only till next clock and only if some (maybe complex) condition is met, and else set it do default value?

Your problem in converting the code to VHDL is that Verilog has blocking and non-blocking assignment operators. Download the SystemVerilog spec here:

http://standards.ieee.org/getieee/1800/download/1800-2012.pdf

and look at the section on "Assignment Statements".

My SystemVerilog skills are limited, so I'll leave it to someone else to provide insight as to what is occurring.

Cheers,

Dave

--- Quote Start ---

What is the proper pattern to assign some output signal in the process on clock edge, but only till next clock and only if some (maybe complex) condition is met, and else set it do default value?

--- Quote End ---

Hmm, I normally only use that construct in a combinatorial process. I'd expect it to be something like:

D1: process (all)
begin
  if rst then
     result <='0';
  elsif rising_edge(clk) then
     -- Default
     result <= '0';
     -- Override
     if (some other condition) then
         result <= ena;
     end if;
  end if;
end process;

Cheers,

Dave

--- Quote Start ---

Thus the question is:

What is the proper pattern to assign some output signal in the process on clock edge, but only till next clock and only if some (maybe complex) condition is met, and else set it do default value?

--- Quote End ---

you can use default statement:

if reset ...

elsif rising_edge(clk) then

data <= '0';

if condition1 then

data <= '1';

end if;

end if;

condition1 will be checked at every clock edge and if not true then '0' is assigned on data

As a first comment, the unconditional assignment "new_bw <=0" in the quoted Verilog code is not corresponding to the standard for synthesizable Verilog (IEEE Std 1364.1). I'm not sure, if it will be accepted by all tools and achieves consistency between simulation and hardware behaviour. You can hardly translate bad Verilog code to synthesizable VHDL without some rework.

To resolve the bad code issue, you have to analyze the original code for the usage of the reg variable new_bw in other places and determine the intended behaviour. The problem can't be clarified by only looking at the quoted always block.

signal new_bw is used in another always block, with same sensitivity list, only once for reading

This "bad" code is known to by synthesizable with Actel tools. I wish all tools were more consistent in this respect....

--- Quote Start ---

Your process line should be process(clk,rst). Sure, all works, but it does not provide your "intent" for the logic. Explicitly indicating you wanted the process only to be sensitive to clock and reset helps the next person maintaining your code understand it better.

--- Quote End ---

Well - before VHDL2008 emerged I was, or course, using the explicit sensitivity list. But blame it on my stupidity, but it's been certainly more then once, when simulation result was different from synthesized circuit operation

due to omitting some arguments from the sensitivity list (which synthesis tools ignore).

Thus nowdays I prefer to use the form that is consistent between simulation and synthesis.

--- Quote Start ---

Well - before VHDL2008 emerged I was, or course, using the explicit sensitivity list. But blame it on my stupidity, but it's been certainly more then once, when simulation result was different from synthesized circuit operation

due to omitting some arguments from the sensitivity list (which synthesis tools ignore).

Thus nowdays I prefer to use the form that is consistent between simulation and synthesis.

--- Quote End ---

I would argue that this is a bad approach for a clocked process. In the case of a clocked process, just list the clock and reset. For a combinatorial process, sure use all to save yourself forgetting a signal. But even then, Quartus will warn you if you have missed a signal in the sensitivity list of a process, so just make sure to read the warning messages. Similarly, your Modelsim testbench should fail if you've forgotten a signal in the sensitivity list, and your testbench provides full code coverage.

Bottom-line, if you have a good test and verification policy in place, you don't need to be "lazy" and use process(all).

These are just my preferences ...

Cheers,

Dave

I just verified, that Quartus is also complaining about the original Verilog code, which was expectable.

In fact you can change it to regular synthesizable Verilog by exchanging a few lines, corresponding to the VHDL version shown in post# 5 and# 6. In so far it's completely misterious why the author used the starnge construct.

Regarding sensitivity lists, in my understanding, they have been introduced to reduce the effort of a simulator that has to interprete the code. Using process(all) in VHDL (or always @* in Verilog) means that the simulator has to compute all expressions for each delta cycle, but also avoids all possible simulation mismatches related to sensitivity lists.

You can argue that sensitivity lists are ignored in synthesis anyway, so if you are only simulating synthesizable code, or not simulating it at all, you can use process(all) without changing anything.

P.S. The corrected Verilog code

always @(posedge clk or posedge rst )
  if (rst) begin
    m_wr_pnt<=0;
    write_cnt<=0;
    new_bw <=0; 
    read_cnt<=0;
   end
  else
  begin
   new_bw <=0;    
   if (we_m) begin    
    if (free_bd >0) begin
      write_cnt <=write_cnt+1;
      m_wr_pnt<=m_wr_pnt+1;
      if (!write_cnt) begin      //First write indicate source buffer addr (2x16)
        bd_mem<=dat_in_m;             
      end
      else begin        //Second write indicate SD card block addr (2x16)
        bd_mem<=dat_in_m;
        new_bw <=write_cnt;      //Second 16 bytes writen, complete BD
      end
     end
  end   
end