verilator
Fix virtual interface "combinational region did not converge" error
Under some conditions I'm not quite sure about (but most likely involving interfaces and a class that drives the interface), a runtime error is erroneously triggered.
To reproduce, compile the following code with verilator --exe --build -j `nproc` --binary bug_ico.sv -o bug_ico (using the git master version):
interface INTF ();
logic field1;
logic field2;
endinterface
class intf_driver_class_unused;
virtual INTF intf;
function new(virtual INTF intf);
this.intf = intf;
endfunction
function void unused_func1();
intf.field1 = '0;
endfunction
function void unused_func2();
intf.field2 = '0;
endfunction
endclass
module idma_backend_top ();
typedef struct packed {
logic field1;
logic x;
} s1;
typedef struct packed {
logic field2;
logic y;
} s2;
// Bug happens with and without initializations
s1 struct1 = '{1'b0, 1'b0};
s2 struct2 = '{1'b0, 1'b0};
assign intf.field1 = struct1.field1;
assign struct2.field2 = intf.field2;
assign struct1.x = struct2.y;
INTF intf ();
// Doesn't change the outcome
// initial begin
// intf.field1 = '0;
// intf.field2 = '0;
// end
endmodule
(Note that the class is not even used - in the original project where this occurred it did, but while compressing it (~5k lines post-preprocessing :-( ) down to this reproducible example I found out that the class didn't even need instantiating)
The following error will show:
%Error: bug_ico.sv:22: Input combinational region did not converge.
Adding -CFLAGS -DVL_DEBUG=1 additionally prints out this line before the error message:
-V{t1,1} 'ico' region trigger index 1 is active: Internal 'ico' trigger - virtual interface: INTF
I would appreciate any insight into what exactly is going wrong here, happy to look into it as well at some point too though. I have a feeling it has something to do with some update trigger maybe being globally on the interface rather than the individual wires of the interface, or something like that. Though I have no idea why it needs that extra driver class.
Verilator considers all wires together in scheduling, even when they consists of structs/unions. Normally module/interface signals would not be likewise scheduled together, but virtual interfaces can do nearly anything so signals in a virt interface likewise schedules together.
So basically you have to verilator:
assign intf = struct1;
assign struct2 = intf;
assign struct1 = struct2;
Using this works:
s1 struct1 /*verilator split_var*/ = '{1'b0, 1'b0};
s2 struct2 /*verilator split_var*/ = '{1'b0, 1'b0};
Alright, thanks for the info. Unfortunately, in the original project I'm still getting the error, even after spraying /*verilator split_var*/ pretty much anywhere I could (both inside the modules and in their port lists)... Any suggestions on how to debug this more efficiently?
You could look at the .dot files that come with --debug but those are complicated. Maybe see if Version 4 of Verilator, if it works at all, gives UNOPTFLAT indication that helps you out?
OK, so what I did now is I now forced m_attrSplitVar to be true by default in V3AstNodeOther.h:1837 to ensure I didn't miss anything, but the ico error still comes up.
FYI, I also got an internal error on one of the assign statements (which I circumvented for testing the ico error by just assigning a constant instead), which might warrant a separate issue once I get around to it:
%Error: Internal Error: ../../rtl/idma_transport_layer_rw_axi.sv:207:40: ../V3SplitVar.cpp:1124: not enough split variables
: ... note: In instance 'idma_backend_top.i_idma_backend.i_idma_transport_layer'
207 | assign buffer_out_shifted = {buffer_out, buffer_out} >> (w_dp_req_i.shift*8);
| ^~~~~~~~~~
I'll try to create another minimal example from here where adding the split_var metacomment does not help and report back again then.
interface INTF();
logic x;
logic y;
endinterface
class intf_driver;
virtual INTF intf;
function new(virtual INTF intf);
this.intf = intf;
// Not really relevant what exactly is done with the fields,
// but they just have to be referenced for this bug to appear
while (intf.x != 1) begin end
intf.y = 1;
endfunction
endclass
module top();
logic s1 = '1;
logic s2 = '1;
INTF vintf ();
assign vintf.x = s1;
assign vintf.y = '1;
assign s2 = vintf.y;
assign s1 = s2;
initial begin
$finish;
end
endmodule
Note that in this example there aren't any structs involved, so I believe there is currently no workaround for this issue at the moment.
Also note that this is definitely related to the virtual interface stuff, as doing
module top();
logic s1 = '1;
logic s2 = '1;
// INTF vintf ();
logic vintf_x;
logic vintf_y;
assign vintf_x = s1;
assign vintf_y = '1;
assign s2 = vintf_y;
assign s1 = s2;
initial begin
$finish;
end
endmodule
instead works, and same if the intf_driver class is removed.
Any ideas on how to approach getting this to work?
Here is another example which is a bit more realistic and also shows a bit better what I'm trying to achieve. I have confirmed that it compiles and simulates correctly on QuestaSim and vcs. With verilator I get %Error: ico_bug.sv:54: Input combinational region did not converge - so I think this issue does warrant being reopened (or I can make a new issue, if you prefer).
`timescale 1ns/1ps
interface INTF();
logic clk;
logic [7:0] data;
logic valid;
logic ready;
endinterface
time TA = 1ns; // application time
class intf_driver;
virtual INTF intf;
function new(virtual INTF intf);
this.intf = intf;
endfunction
task cycle_start();
#TA;
endtask
task cycle_end();
@(posedge intf.clk);
endtask
task init_master();
intf.data = '0;
intf.valid = 0;
endtask
task init_slave();
intf.ready = 0;
endtask
task recv_data(output logic [7:0] data);
intf.ready <= #TA 1;
cycle_start();
while (intf.valid != 1) begin cycle_end(); cycle_start(); end
cycle_end();
data = intf.data;
intf.ready <= #TA 0;
endtask
task send_data(input logic [7:0] data);
intf.data <= #TA data;
intf.valid <= #TA 1;
cycle_start();
while (intf.ready != 1) begin cycle_end(); cycle_start(); end
cycle_end();
intf.valid <= #TA 0;
endtask
endclass
module top();
logic clk;
logic [7:0] data;
logic valid;
logic ready;
INTF read_intf();
assign read_intf.clk = clk;
assign read_intf.data = data;
assign read_intf.valid = valid;
assign ready = read_intf.ready;
INTF write_intf();
assign write_intf.clk = clk;
assign data = write_intf.data;
assign valid = write_intf.valid;
assign write_intf.ready = ready;
intf_driver driver_master;
intf_driver driver_slave;
initial begin
forever begin
clk = '1;
#10ns;
clk = '0;
#10ns;
end
end
initial begin
driver_master = new(write_intf);
driver_master.init_master();
#32ns;
driver_master.send_data(8'h42);
end
logic [7:0] recv_data;
initial begin
driver_slave = new(read_intf);
driver_slave.init_slave();
#22ns;
driver_slave.recv_data(recv_data);
$display("Got data: %02x", recv_data);
$finish;
end
endmodule
@wsnyder Is this area something that might be fixed / improved on in the future or am I better off trying to work around the issue with different SystemVerilog functionalities for now? (I could also try fixing it myself, but I'm guessing it would be quite involved...)
I agree this should be open. I'd suggest working around it - it's not clear to me how to fix this with the current scheduling, perhaps @kbieganski has ideas.
I think what would help in most (all?) of these cases is more granular virtual interface triggers, i.e. a separate trigger per interface member.
Right now, there's one trigger per virtual interface type which leads to spurious combinational loops. This:
assign vintf.x = s1;
assign s2 = vintf.y;
assign s1 = s2;
Becomes a loop like this:
... -> s1 -> vintf -> s2 -> s1 -> ...
If we had separate triggers for x and y, there would be no loop.
vintf.y -> s2 -> s1 -> vintf.x
Agreed, if this were to be implemented, I think it would solve these issues I'm having. I wonder if the "one trigger per virtual interface" approach was done because of performance reasons or just because it was easier to implement... either way, would be really cool to see this get worked on.
I also got this problem but never was able to create a small re-producible test case b/c of other failures like #5203
Hi gang, I wonder if there is an update pending on this issue. My environment is very dependent on virtual interfaces and I lack the skill to debug the verilator sources.
Based on looking at this thread and pull requests, I don't see anyone working on this.
We’d be happy to try and help support this issue. Any progress will be reported here. ;)
Great, I think they are all tagged with "lint" so any in the list would be great! https://github.com/verilator/verilator/issues?q=is%3Aissue%20state%3Aopen%20label%3A%22area%3A%20lint%22
e.g. #5876 for ANDs, #5681 for assign, etc.
Hello, Everyone
After the recent PR #6148 , I believe the "combinational region did not converge" issue has been resolved. We've adopted the member-level trigger approach mentioned in the issue to handle virtual interface assignments.
However, during development, I discovered another issue related to virtual interface. To distinguish it from this one, I’ve created a new issue #6175 describing the problem. I plan to work on it later, and any suggestions on how to address it would be greatly appreciated.