zachjs / sv2v

SystemVerilog to Verilog conversion
BSD 3-Clause "New" or "Revised" License
498 stars 50 forks source link

`always_comb` doesn't generate a sensitivity list: just puts `@(*)` #199

Closed hughperkins closed 2 years ago

hughperkins commented 2 years ago

If I put:

module foo(input clk, input a, input b, output reg c, output reg d);
    task blah(output reg out1);
        out1 = a;
    endtask

    always_comb begin
        blah(d);
    end
endmodule

... then I expect a sensitivity list to be generated containing a, because task blah reads from a.

What actually happens: sv2v generates:

module foo (
    clk,
    a,
    b,
    c,
    d
);
    input clk;
    input a;
    input b;
    output reg c;
    output reg d;
    task blah;
        output reg out1;
        out1 = a;
    endtask
    always @(*)
        blah(d);
endmodule

This is simply a standard verilog @(*) sensitivity list. verilog compilers often do not take into account called functions and tasks when evaluating such lists. One of the benefits of systemverilog is thtat hte sensitivity list takes into account called functions and tasks. However, currently sv2v simply converts the always_comb into the verilog style @(*), and so we miss out on one of the key benefits of moving to systemverilog.

zachjs commented 2 years ago

Thank you for bringing this to my attention! I agree there is an issue here.

The standard is clear that the implicit sensitivities of always_comb blocks should include the contents of called functions. However, Section 9.2.2.2.1 of IEEE 1800-2017 states:

Task calls are allowed in an always_comb, but the contents of the tasks do not add anything to the sensitivity list.

Because of this, I believe the behavior for the above example is accurate in this regard because it uses a task and not a function. That said, I would like to implement the analysis to fix the issue with called functions. I have some ideas, and I'm aiming to make some progress this weekend.

hughperkins commented 2 years ago

Ah, interesting. Good point about only function contents being used to add to the sensitivity list. And tasks which do not consume time can be simply rewritten as void functions. (In reality, neither iverilog nor yosys appear to accept void functions, but that's not your own problem; they should simply start supporting void functions).

hughperkins commented 2 years ago

(Oh... I could use a void function, pass that systemverilog to sv2v. sv2v would use the void function contents for senstivities, then convert to a verilog task, which iverilog or yosys could read, I see :) )

zachjs commented 2 years ago

Indeed, void functions should affect the sensitivity list.

I've pushed an implementation which generates explicit sensitivity lists when necessary. I'm very interested to hear if it works the way you expect!

hughperkins commented 2 years ago

Thanks! Will take a look the next time I come across an issue in verilog which pushes me to try systemverilog again :)

(Every so often I come across something which seems like sv will solve, but so far, I've always found a way of solving it within verilog for now :) )