steveicarus / iverilog

Icarus Verilog
https://steveicarus.github.io/iverilog/
GNU General Public License v2.0
2.85k stars 530 forks source link

Continuous assignment from functions #983

Open sifferman opened 1 year ago

sifferman commented 1 year ago

Hello, I discovered another issue with functions when submitting #980. I couldn't find anything in the 1800-2017 spec about this specific issue, but I assume my proposed changes will help iverilog more closely match synthesis tools.

Example

module test;

logic [7:0] test_internal;

function automatic [7:0] test_func;
    input logic unused;
    test_func = test_internal;
endfunction

wire [7:0] test_wire = test_func(0);

initial begin
    test_internal = 0;
    #1;
    $display("Expected %h, Recieved %h", test_func(0), test_wire);

    test_internal = 1;
    #1
    $display("Expected %h, Recieved %h", test_func(0), test_wire);

    $finish;
end

endmodule

In this test, test_func() is only run at time=0, and is not run when test_internal is updated. I would expect the function to be re-run if any RHS inside the function changes, no just if an input changes.

Survey

Pass:

Fail:

Solution

I'm not sure how to fix this issue. Anyone have ideas on where to get started?

larsclausen commented 1 year ago

This is an interesting corner case. Its not quite clear to me though what should be the right behavior.

What it comes down to is whether continuous assignment should behave like always @(*) or always_comb. always @(*) will only look at function arguments and only run when the function argument changes, whereas always_comb will also look at variables inside in the function and also run at 0 time.

E.g. if you put the following in your test you get the behavior you expect.

logic [7:0] test_wire;
always_comb test_wire = test_func(0);

The LRM is not particular clear on how this kind of function should be evaluated in a continuous assignment.

martinwhitaker commented 1 year ago

The LRM does state

Assignments on nets shall be continuous and automatic. In other words, whenever an operand in the right-hand expression changes value, the whole right-hand side shall be evaluated.

Variables accessed inside a function are not operands in the right-hand expression.

This is a well-known cause of simulation vs. synthesis mismatch, both when used in continuous assignments and when used in always @* constructs.

If you want implicit sensitivity to descend into functions, use the always_comb construct.

sifferman commented 1 year ago

That is very interesting. I figured that assign should act as always_comb, not always @*. I believe this issue is now resolved, unless anyone feels strongly about straying from the LRM to more closely match synthesis. If so, I'm appreciate a bit of guidance getting started since I'm unfamiliar with the code-base :)

larsclausen commented 1 year ago

One thing that could make sense is to add is a warning that is printed if we detect a situation like this.

martinwhitaker commented 1 year ago

Continuous assignments were part of the original Verilog language (circa 1984). always_comb was added as part of the SystemVerilog language extensions, specifically to support synthesis. For some background read this thread and specifically this comment.

caryr commented 1 year ago

A warning would be nice, but this is expected simulator behavior so we need to be careful what class of warning this is included in.