Skip to content

Commit

Permalink
[FIRRTL] Fix sign bit truncation in constant parser (#6794)
Browse files Browse the repository at this point in the history
Fix an issue in the parser of the `firrtl.constant` op, which would
truncate the sign bit of negative constants without reporting an error.
This would for example accept -2 as a valid 1-bit constant, truncating
the `...11111110` bit pattern to `0`.
  • Loading branch information
fabianschuiki authored Mar 8, 2024
1 parent a4a1eb6 commit 99968ff
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 4 deletions.
6 changes: 4 additions & 2 deletions lib/Dialect/FIRRTL/FIRRTLOps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3933,8 +3933,10 @@ ParseResult ConstantOp::parse(OpAsmParser &parser, OperationState &result) {
} else if (width < value.getBitWidth()) {
// The parser can return an unnecessarily wide result with leading
// zeros. This isn't a problem, but truncating off bits is bad.
if (value.getNumSignBits() < value.getBitWidth() - width)
return parser.emitError(loc, "constant too large for result type ")
unsigned neededBits = value.isNegative() ? value.getSignificantBits()
: value.getActiveBits();
if (width < neededBits)
return parser.emitError(loc, "constant out of range for result type ")
<< resultType;
value = value.trunc(width);
}
Expand Down
13 changes: 11 additions & 2 deletions test/Dialect/FIRRTL/errors.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ firrtl.module @Foo() {

firrtl.circuit "Foo" {
firrtl.module @Foo() {
// expected-error @+1 {{constant too large for result type}}
// expected-error @+1 {{constant out of range for result type}}
firrtl.constant 100 : !firrtl.uint<4>
}
}
Expand All @@ -148,13 +148,22 @@ firrtl.module @Foo() {

firrtl.circuit "Foo" {
firrtl.module @Foo() {
// expected-error @+1 {{constant too large for result type}}
// expected-error @+1 {{constant out of range for result type}}
firrtl.constant -100 : !firrtl.sint<4>
}
}

// -----

firrtl.circuit "Foo" {
firrtl.module @Foo() {
// expected-error @+1 {{constant out of range for result type}}
firrtl.constant -2 : !firrtl.sint<1>
}
}

// -----

firrtl.circuit "Foo" {
firrtl.module @Foo() {
// expected-error @+1 {{special constants can only be 0 or 1}}
Expand Down

0 comments on commit 99968ff

Please sign in to comment.