llvm / llvm/circt

[circt-verilog][arcilator] End-to-end simulation mismatch on mixed clock/data signals

Open
#10,797 0 comments 0 reactions 0 assignees View on GitHub
Arc ImportVerilog
Dominant language
C++
Stars
2.2k
Forks
524
Avg merge
3d 2h
Merged PRs (30d)
46

Description

Consider
```SystemVerilog
module Gadget(input i, output reg o);
logic r1 = '0;
logic r2 = '0;

always @(posedge i) r1 <= 1'b1;
always @(posedge r1) r2 <= 1'b1;
always @(posedge r2) o <= r1;

endmodule
```

When transitioning `i` from zero to one, `o` should (deterministically) become one, since the posedge will propagate through `r1` and `r2`, and `r1` has to have become high for the process writing `o` to be unsuspended.
https://www.edaplayground.com/x/bt4m

Passing this through `circt-verilog` and adding an arcilator testbench we get:
```MLIR
hw.module @Gadget(in %i : i1, out o : i1) {
%true = hw.constant true
%0 = seq.to_clock %i
%r1 = seq.firreg %true clock %0 : i1
%1 = seq.to_clock %r1
%r2 = seq.firreg %true clock %1 : i1
%2 = seq.to_clock %r2
%o = seq.firreg %r1 clock %2 : i1
hw.output %o : i1
}

func.func @entry() {
%zero = hw.constant 0 : i1
%one = hw.constant 1 : i1
%tstep = hw.constant 0 : i64
arc.sim.instantiate @Gadget as %model {
arc.sim.set_input %model, "i" = %zero : i1, !arc.sim.instance<@Gadget>
arc.sim.step %model by %tstep : !arc.sim.instance<@Gadget>
arc.sim.set_input %model, "i" = %one : i1, !arc.sim.instance<@Gadget>
arc.sim.step %model by %tstep : !arc.sim.instance<@Gadget>
%o = arc.sim.get_port %model, "o" : i1, !arc.sim.instance<@Gadget>
arc.sim.emit "o", %o : i1
}
return
}
```
And running it yields:
```
o = 0
```

This is because Arcilator currently uses a different time model ("old" vs. "new" in LowerState) for clock inputs vs. data inputs on registers. Clock signals propagate immediately, while data signals are delayed. This allows it to detect an edge on `%r2` while the `%r1` on the data input still evaluates to zero.

There is a signal race condition in the core IR and it is unclear whether we consider the Arcilator behavior a valid refinement of it. But if we do, the core IR is not a correct lowering of the SV input.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start by reproducing the SystemVerilog example through circt-verilog and the Arcilator testbench shown in the issue, checking the emitted MLIR and resulting o value. Read Arcilator's LowerState time model and the handling of clock versus data inputs on registers. Done means the intended semantics of this lowering are decided and the mismatch is resolved or documented with a regression test.

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
Quiet
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.