[ExportVerilog] always_comb ordering issue
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 2.2k
- Forks
- 524
- Avg merge
- 3d 2h
- Merged PRs (30d)
- 46
Description
The following MLIR (bear with me there's potentially a lot of un-important parts of this IR):
```mlir
module attributes {circt.loweringOptions = "locationInfoStyle=none"} {
hw.module @Main(%IA: i2, %IB: i2, %IS: i1) -> (O: i2) {
%0 = hw.constant 0 : i2
%1 = hw.constant 1 : i2
%2 = hw.constant 0 : i1
%3 = comb.extract %IA from 0 : (i2) -> i1
%4 = comb.extract %IA from 1 : (i2) -> i1
%5 = comb.extract %IB from 0 : (i2) -> i1
%6 = hw.constant 1 : i1
%7 = comb.extract %IB from 1 : (i2) -> i1
%14 = sv.reg : !hw.inout
%8 = sv.read_inout %14 : !hw.inout
%15 = sv.reg : !hw.inout
%9 = sv.read_inout %15 : !hw.inout
%16 = sv.reg : !hw.inout
%10 = sv.read_inout %16 : !hw.inout
%17 = sv.reg : !hw.inout
%11 = sv.read_inout %17 : !hw.inout
%18 = sv.reg : !hw.inout
%12 = sv.read_inout %18 : !hw.inout
%19 = sv.reg : !hw.inout
%13 = sv.read_inout %19 : !hw.inout
sv.alwayscomb {
sv.if %IS {
sv.bpassign %16, %2 : i1
sv.bpassign %17, %2 : i1
sv.bpassign %18, %5 : i1
sv.bpassign %19, %7 : i1
} else {
sv.bpassign %16, %3 : i1
sv.bpassign %17, %4 : i1
sv.bpassign %18, %6 : i1
sv.bpassign %19, %2 : i1
}
}
%20 = comb.concat %11, %11, %10 : i1, i1, i1
%21 = comb.concat %13, %13, %12 : i1, i1, i1
%22 = comb.add %20, %21 : i3
%23 = comb.extract %22 from 1 : (i3) -> i1
%24 = comb.extract %22 from 2 : (i3) -> i1
%25 = comb.concat %24, %23 : i1, i1
%26 = hw.constant 1 : i2
%27 = comb.icmp eq %25, %26 : i2
%28 = hw.constant 1 : i2
%29 = comb.concat %24, %23 : i1, i1
%30 = hw.constant -2 : i2
%31 = comb.icmp eq %29, %30 : i2
%32 = hw.constant 2 : i2
%33 = hw.constant 1 : i1
%34 = hw.constant 0 : i1
%35 = comb.extract %22 from 0 : (i3) -> i1
%39 = sv.reg : !hw.inout
%36 = sv.read_inout %39 : !hw.inout
%40 = sv.reg : !hw.inout
%37 = sv.read_inout %40 : !hw.inout
%41 = sv.reg : !hw.inout
%38 = sv.read_inout %41 : !hw.inout
sv.alwayscomb {
sv.if %27 {
sv.bpassign %40, %33 : i1
sv.bpassign %41, %34 : i1
} else {
sv.if %31 {
sv.bpassign %40, %34 : i1
sv.bpassign %41, %33 : i1
} else {
sv.bpassign %40, %35 : i1
sv.bpassign %41, %23 : i1
}
}
}
%42 = comb.concat %38, %37 : i1, i1
hw.output %42 : i2
}
}
```
produces the following Verilog:
```verilog
// Generated by CIRCT circtorg-0.0.0-1018-g3a39b339f
module Main(
input [1:0] IA,
IB,
input IS,
output [1:0] O);
reg _GEN;
reg _GEN_0;
reg _GEN_1;
reg _GEN_2;
reg _GEN_3;
reg _GEN_4;
always_comb begin
automatic logic [2:0] _GEN_5;
_GEN_5 = {{2{_GEN_0}}, _GEN} + {{2{_GEN_2}}, _GEN_1};
if (IS) begin
_GEN = 1'h0;
_GEN_0 = 1'h0;
_GEN_1 = IB[0];
_GEN_2 = IB[1];
end
else begin
_GEN = IA[0];
_GEN_0 = IA[1];
_GEN_1 = 1'h1;
_GEN_2 = 1'h0;
end
if (_GEN_5[2:1] == 2'h1) begin
_GEN_3 = 1'h1;
_GEN_4 = 1'h0;
end
else if (_GEN_5[2:1] == 2'h2) begin
_GEN_3 = 1'h0;
_GEN_4 = 1'h1;
end
else begin
_GEN_3 = _GEN_5[0];
_GEN_4 = _GEN_5[1];
end
end // always_comb
assign O = {_GEN_4, _GEN_3};
endmodule
```
The issue here is that the add (`%22 = comb.add %20, %21 : i3`) which is outside of the first `always_comb` gets pushed into the *beginning* of the `always_comb` (`_GEN_5 = ... + ...`) in the Verilog, although it depends on register assignments later in that same `always_comb`. This results in a simulation where the add uses old values of the relevant registers.
I believe that potentially using non-block assignments, *or* moving the add outside of the `always_comb` would fix this issue.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the ExportVerilog path and reproduce the issue from the supplied MLIR and generated Verilog, focusing on the two always_comb regions and the comb.add dependency. Compare the emitted statement order with the IR order and verify in simulation that the add observes the assignments from the same always_comb; done means the generated behavior no longer uses stale register values.
Written by the indexing model from the issue text.
Assessment
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100