[ImportVerilog] Detect and fixup two-state-exhaustive case statements #8628
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Case statements often occur in the following form in real-world Verilog:
```
module Foo(input logic [1:0] a, output logic [3:0] z);
always_comb begin
case (a)
2'd0: z = 4'b0001;
2'd1: z = 4'b0010;
2'd2: z = 4'b0100;
2'd3: z = 4'b1000;
endcase
end
endmodule
```
The intention being that the case statement exhaustively covers all
possible `a` input values, and assigns a corresponding `z` output value.
This case statement _is_ exhaustive assuming two-state logic. However,
the Verilog `logic` type is four-state, where the bits in `a` can also
have the value X and Z. If `a` contains X or Z, none of the case
statements would execute and `z` would not be assigned, essentially
creating a latch.
In practice, logic synthesizers fold X and Z values to 0 or 1 however
they see fit, often in an attempt to minimize logic. For this case
statement the obvious assumption is that the input is two-state and the
match is exhaustive. This does not work in simulation, though.
This commit makes ImportVerilog detect if a case statement is exhaustive
assuming two-state logic. If it is, the last case item is treated as the
default, being executed regardless of its case value. As a result, the
above example generates a CFG where all possible paths through the case
statement assign the output `z`. This then ensures that the case
statement can be lowered to properly in circt-verilog, without
generating conditional drives or latches.
The core dialects of CIRCT currently don't represent X and Z well, and
most of ImportVerilog and the circt-verilog pipeline assumes all values
are two-state. Once we add X and Z handling to the core dialects, we
will have to revisit this behavior and make it opt-out.
Fixes #8626.
maerhart
approved these changes
Jul 1, 2025
uenoku
reviewed
Jul 1, 2025
| auto twoStateExhaustive = false; | ||
| if (auto intType = dyn_cast<moore::IntType>(caseExpr.getType()); | ||
| intType && intType.getWidth() < 32 && | ||
| itemConsts.size() == (1 << intType.getWidth())) { |
There was a problem hiding this comment.
Is itemConsts guaranteed to be unique?
There was a problem hiding this comment.
I think not necessarily. If I recall correctly, you can have multiple case items match in Verilog, but only the first match as listed in the source text is executed. In that case this change wouldn't treat that case statement as exhaustive and instead just generate the regular default/no-match case in the control flow.
We might want a dedicated moore.switch control flow statement in the long run, especially once we need to handle X/Z bits properly. cf.switch doesn't really work too well there, since it uses plain old integers. There's also this batshit crazy Verilog pattern to express a one-hot match (constant in the case expr, dynamic values in the individual case items):
unique case (1)
a[0]: doStuff();
a[1]: doStuff();
a[2]: doStuff();
a[3]: doStuff();
endcaseNo idea if we'd want to capture this in the IR somehow. Verilog makes me sad.
|
@maerhart Yeah this makes me sad, too 😞. Especially because Verilog goes out of its way so much just to look and feel like C/C++, without realizing that its variable mutation is definitely not a good model to describe hardware. This |
TaoBi22
pushed a commit
to TaoBi22/circt
that referenced
this pull request
Jul 17, 2025
…llvm#8628) Case statements often occur in the following form in real-world Verilog: ``` module Foo(input logic [1:0] a, output logic [3:0] z); always_comb begin case (a) 2'd0: z = 4'b0001; 2'd1: z = 4'b0010; 2'd2: z = 4'b0100; 2'd3: z = 4'b1000; endcase end endmodule ``` The intention being that the case statement exhaustively covers all possible `a` input values, and assigns a corresponding `z` output value. This case statement _is_ exhaustive assuming two-state logic. However, the Verilog `logic` type is four-state, where the bits in `a` can also have the value X and Z. If `a` contains X or Z, none of the case statements would execute and `z` would not be assigned, essentially creating a latch. In practice, logic synthesizers fold X and Z values to 0 or 1 however they see fit, often in an attempt to minimize logic. For this case statement the obvious assumption is that the input is two-state and the match is exhaustive. This does not work in simulation, though. This commit makes ImportVerilog detect if a case statement is exhaustive assuming two-state logic. If it is, the last case item is treated as the default, being executed regardless of its case value. As a result, the above example generates a CFG where all possible paths through the case statement assign the output `z`. This then ensures that the case statement can be lowered to properly in circt-verilog, without generating conditional drives or latches. The core dialects of CIRCT currently don't represent X and Z well, and most of ImportVerilog and the circt-verilog pipeline assumes all values are two-state. Once we add X and Z handling to the core dialects, we will have to revisit this behavior and make it opt-out. Fixes llvm#8626.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Case statements often occur in the following form in real-world Verilog:
The intention being that the case statement exhaustively covers all possible
ainput values, and assigns a correspondingzoutput value. This case statement is exhaustive assuming two-state logic. However, the Veriloglogictype is four-state, where the bits inacan also have the value X and Z. Ifacontains X or Z, none of the case statements would execute andzwould not be assigned, essentially creating a latch.In practice, logic synthesizers fold X and Z values to 0 or 1 however they see fit, often in an attempt to minimize logic. For this case statement the obvious assumption is that the input is two-state and the match is exhaustive. This does not work in simulation, though.
This commit makes ImportVerilog detect if a case statement is exhaustive assuming two-state logic. If it is, the last case item is treated as the default, being executed regardless of its case value. As a result, the above example generates a CFG where all possible paths through the case statement assign the output
z. This then ensures that the case statement can be lowered to properly in circt-verilog, without generating conditional drives or latches.The core dialects of CIRCT currently don't represent X and Z well, and most of ImportVerilog and the circt-verilog pipeline assumes all values are two-state. Once we add X and Z handling to the core dialects, we will have to revisit this behavior and make it opt-out.
Fixes #8626.