Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn on mixing non-blocking and blocking assignments to the same variable #1400

Open
AndreasLoow opened this issue Sep 23, 2019 · 5 comments

Comments

@AndreasLoow
Copy link

Steps to reproduce the issue

Hi, I noticed the following miscompilation bug. Not sure if it is considered interesting or not:

Consider the following Verilog program:

> cat small.v
module small(input clk, output reg out);

   always @ (posedge clk) begin
      out <= 0;
      out = 1;
   end

endmodule

Compile using the following:

> read_verilog small.v; proc; clean; write_verilog

With this, I get the following program as output:

/* Generated by Yosys 0.9+431 (git sha1 057dae4, clang 10.0.1 -fPIC -Os) */
Dumping module `\small'.

(* cells_not_processed =  1  *)
(* src = "small.v:1" *)
module \small (clk, out);
  (* src = "small.v:1" *)
  input clk;
  (* src = "small.v:1" *)
  output out;
  reg out;
  always @(posedge clk)
      out <= 1'h1;
endmodule

Expected behaviour

I expected the output Verilog program to have the same behaviour as the input Verilog program, in particular I expected out to evaluate to 0 each clock cycle.

Actual behaviour

The output Verilog program sets out to 1 (instead of 0).

I am aware that the input program is strange/ugly (style guides often mention not to mix different assignments to the same variable etc.), but I expected some kind of warning from the compiler rather than the compiler silently miscompiling the program (or simply that the compiler would correctly compile the program).

@whitequark
Copy link
Member

Quoting IEEE 1364.1-2002:
Screenshot_20190923_220030

Your variable out is not temporarily assigned and used within the always statement, so your program is not valid synthesizable Verilog. In this case, quite similar to other languages designed in a similar way, such as C, the compiler is allowed to produce any output it wants without emitting any diagnostics.

@AndreasLoow
Copy link
Author

I see, sorry, I was not aware that Yosys is following (the superseded) 1364.1-2002 (which I am not familiar with). Instead, I had SystemVerilog in mind. Just taking a quick look into 1800-2017, I found one example (Example 3, on p. 239) containing assignments of both kinds to the same variable (in an initial block), indicating that such programs have well-defined semantics at least under (System)Verilog's simulation semantics.

But of course, this does not matter much as SystemVerilog is a different language than the language Yosys claims to support... because I guess this is out of scope for read_verilog's -sv flag? (I get the same behaviour with and without the -sv flag, i.e. an output Verilog program assigning 1 to out.) Things become more fuzzy in SystemVerilog I suppose, given that the standard does not give us any notion of synthesizable... resulting in some custom-made mix of 1364.1-2002 and 1800-2017.

@daveshah1
Copy link

I don't think that example is relevant, it's not two examples in the same always block and seeing which one "wins" but instead one initial block (that runs at the first timestep) and an always block (which runs later on) so there is no conflict.

FWIW, trying the design in a commercial SystemVerilog tool, after renaming the module as small is a keyword, it produces the error ERROR: small.v:5: mixed blocking and non-blocking assignments on 'out' is not supported

@AndreasLoow
Copy link
Author

I'll copy the example here to make sure we are talking about the same example:

module nonblock2;
  logic a, b;

  initial begin
    a = 0;
    b = 1;
    a <= b; // evaluates, schedules,
    b <= a; // and executes in two steps
  end

  initial begin
    $monitor ($time, ,"a = %b b = %b", a, b);
    #100 $finish;
  end
endmodule

According to (System)Verilog's simulation semantics, a must become 1, and b must become 0 in this example. That we have an initial block instead of an always block is as far as I can tell not relevant. To be clear: I'm comparing the contents of the initial block with the contents of the always block from my example; the only difference being that the initial block will execute once, and the always block will execute forever. The always block in the example from the standard is not important here.

As for the error generated by the (non-Yosys) SystemVerilog tool: I think this is reasonable behaviour. I think either producing an error or compiling the program in a semantics-preserving way according to the simulation semantics are both valid options. Silently miscompiling the program is more confusing behaviour than those two in my opinion (there are probably a lot of other (simple) Verilog programs where the compiler will not preserve semantics... but still).

But this is of course a design decision: I.e., is it the user's responsibility to not write strange programs, or should the compiler warn the user if it is given strange programs. (Seems that Yosys approach is to try to follow 1364.1-2002 and provide fire missiles semantics (undefined behaviour in C terminology) for strange programs?)

@mwkmwkmwk
Copy link
Member

Please note that Yosys is not a simulator, and thus is not really a full implementation of Verilog semantics — by design, it can only support synthesisable code, and non-blocking assignment to an output port in a clocked always block can not be represented by synthesis.

While this is not a priority of yosys (see http://www.clifford.at/yosys/faq.html point 5), we can add an error or warning for this case (mixed blocking and non-blocking assignments).

@whitequark whitequark changed the title Non-blocking assignments followed by blocking assignments are not compiled correctly Warn on mixing non-blocking and blocking assignments to the same variable Nov 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants