Wires driven through virtual interface traced improperly

[RootCubed]

If there is an interface and a class that uses this interface to drive wires, the driven wires remain at the same level throughout the simulation in the trace (though they seem to be simulated correctly).

To reproduce, compile the following code with verilator --trace --exe --build -j `nproc` --binary signal_driver.sv -o signal_driver (using the git master version):

interface INTF;
    wire signal;
endinterface

class sig_driver_class;
    virtual INTF i;

    function new(virtual INTF i);
        this.i = i;
    endfunction

    task clear_signal;
        i.signal = 0;
    endtask

    task set_signal;
        i.signal = 1;
    endtask
endclass

module top();
    INTF i();
    wire s;
    assign s = i.signal;

    sig_driver_class driver = new(i);

    initial begin
        $dumpfile("signal_driver.vcd");
        $dumpvars(0, top);

        #1ns;
        driver.set_signal();
        $display("Signal is %b", s);
        
        #1ns;
        driver.clear_signal();
        $display("Signal is %b", s);

        #1ns;
        // i.signal = 1;
        $finish;
    end
endmodule

Viewing the VCD file will show both s and i.signal as logic low throughout the simulation, even though the $display statements reflect the signal changing.

I also noticed that if the interface is at any point driven from outside of the class, the wire(s) will be traced correctly (remove the // before i.signal = 1; to reproduce).

Without driving the interface directly:

With driving the interface directly.

Comparing the verilated result with and without the i.signal = 1; line yields a large number of differences: verilated_diff.patch
One possible point of interest is that the file Vsignal_driver___024root__DepSet_hee9ce67b__0__Slow.cpp is generated, but not included anywhere in the rest of the files in the erroneous verilated files.

[kbieganski]

I believe tracing of signals driven through virtual interfaces is simply not implemented yet. Would you be willing to help out with this? It would probably involve something like this: #3579. Also see #4673 for a background on how scheduling works with virtual interfaces.

[RootCubed]

Sure, I'm happy to look further into the issue and hopefully come up with a fix / implement the missing feature.
Thanks for those two references, I'll make sure to look into those.

[RootCubed]

I'm still figuring my way around the large codebase, but I was able to get the trace to work without i.signal = 1; by hackily commenting out V3Trace.cpp:620 and doing const bool always = true; at V3Trace:653.
This obviously isn't a real fix, but it shows that Verilator does not detect that a function call might cause a signal to change through virtual interfaces.

I suppose the easiest solution would be to just make any interface signal be ACTIVITY_ALWAYS (which then should also propagate to the signals driven by that interface). I believe that this is also what happens when the interface is driven directly, but I might be wrong about that.

[wsnyder]

ACTIVITY_ALWAYS is expensive so best avoided unless we have no alternative. Perhaps the activityCode isn't getting tracked and set in the interface?

[RootCubed]

Yes, I noticed in the sample code that during the trace pass, the activity vertices for the signals s and i.signal don't have any in edges and thus have an empty actSet and marked as ACTIVITY_NEVER because of that.
I think that TraceVisitor::visit(AstStmtExpr *) needs to additionally handle AstCMethodCall instead of only AstCCall, but I haven't quite figured out what I would need to do there and whether there are any other places where something needs to be added...

[wsnyder]

Tracing method calls makes sense. The complication is there can be multiple possible virtual methods called, so the code will need to consider all of the possibilities.