Skip to content

Commit 9c6e160

Browse files
committed
Revert "[FIRRTL] Add folder for add (#8200)"
This reverts commit f2e38e8. In downstream testing, this folder appeared to cause assertions in upstream MLIR to fail with the following: ``` llvm::APSInt mlir::IntegerAttr::getAPSInt() const: Assertion `!getType().isSignlessInteger() && "Signless integers don't carry a sign for APSInt"' failed. ```
1 parent c32f60b commit 9c6e160

File tree

3 files changed

+7
-30
lines changed

3 files changed

+7
-30
lines changed

include/circt/Dialect/FIRRTL/FIRRTLExpressions.td

-2
Original file line numberDiff line numberDiff line change
@@ -1393,8 +1393,6 @@ def IntegerAddOp : IntegerBinaryPrimOp<"integer.add", [Commutative]> {
13931393
!firrtl.integer
13941394
```
13951395
}];
1396-
1397-
let hasFolder = true;
13981396
}
13991397

14001398
def IntegerMulOp : IntegerBinaryPrimOp<"integer.mul", [Commutative]> {

lib/Dialect/FIRRTL/FIRRTLFolds.cpp

+2-14
Original file line numberDiff line numberDiff line change
@@ -935,20 +935,8 @@ LogicalResult NEQPrimOp::canonicalize(NEQPrimOp op, PatternRewriter &rewriter) {
935935
}
936936

937937
OpFoldResult IntegerAddOp::fold(FoldAdaptor adaptor) {
938-
if (auto rhsCst = getConstant(adaptor.getRhs())) {
939-
// Constant folding
940-
if (auto lhsCst = getConstant(adaptor.getLhs())) {
941-
auto resultWidth =
942-
std::max(lhsCst->getBitWidth(), rhsCst->getBitWidth()) + 1;
943-
APSInt lhsCstExt = (*lhsCst).extend(resultWidth);
944-
APSInt rhsCstExt = (*rhsCst).extend(resultWidth);
945-
return IntegerAttr::get(IntegerType::get(getContext(), resultWidth),
946-
lhsCstExt + rhsCstExt);
947-
}
948-
// x + 0 -> x
949-
if (rhsCst->isZero())
950-
return getLhs();
951-
}
938+
// TODO: implement constant folding, etc.
939+
// Tracked in https://github.com/llvm/circt/issues/6696.
952940
return {};
953941
}
954942

test/Dialect/FIRRTL/canonicalization.mlir

+5-14
Original file line numberDiff line numberDiff line change
@@ -3576,28 +3576,19 @@ firrtl.module private @Issue7562(in %sel : !firrtl.uint<1>, in %a : !firrtl.cons
35763576
}
35773577

35783578
// CHECK-LABEL: firrtl.class @PropertyArithmetic
3579-
firrtl.class @PropertyArithmetic(in %in: !firrtl.integer, out %out0: !firrtl.integer, out %out1: !firrtl.integer, out %out2: !firrtl.integer, out %out3: !firrtl.integer) {
3580-
// CHECK: [[C3:%.+]] = firrtl.integer 3
3579+
firrtl.class @PropertyArithmetic(in %in: !firrtl.integer, out %out0: !firrtl.integer, out %out1: !firrtl.integer) {
35813580
// CHECK: [[C4:%.+]] = firrtl.integer 4
35823581
%0 = firrtl.integer 0
35833582
%1 = firrtl.integer 1
35843583
%2 = firrtl.integer 2
35853584

3586-
%res0 = firrtl.integer.shl %1, %2 : (!firrtl.integer, !firrtl.integer) -> !firrtl.integer
3587-
%res1 = firrtl.integer.shl %in, %0 : (!firrtl.integer, !firrtl.integer) -> !firrtl.integer
3585+
%3 = firrtl.integer.shl %1, %2 : (!firrtl.integer, !firrtl.integer) -> !firrtl.integer
3586+
%4 = firrtl.integer.shl %in, %0 : (!firrtl.integer, !firrtl.integer) -> !firrtl.integer
35883587

35893588
// CHECK: firrtl.propassign %out0, [[C4]]
35903589
// CHECK: firrtl.propassign %out1, %in
3591-
firrtl.propassign %out0, %res0 : !firrtl.integer
3592-
firrtl.propassign %out1, %res1 : !firrtl.integer
3593-
3594-
%res2 = firrtl.integer.add %1, %2 : (!firrtl.integer, !firrtl.integer) -> !firrtl.integer
3595-
%res3 = firrtl.integer.add %in, %0 : (!firrtl.integer, !firrtl.integer) -> !firrtl.integer
3596-
3597-
// CHECK: firrtl.propassign %out2, [[C3]]
3598-
// CHECK: firrtl.propassign %out3, %in
3599-
firrtl.propassign %out2, %res2 : !firrtl.integer
3600-
firrtl.propassign %out3, %res3 : !firrtl.integer
3590+
firrtl.propassign %out0, %3 : !firrtl.integer
3591+
firrtl.propassign %out1, %4 : !firrtl.integer
36013592
}
36023593

36033594
// CHECK-LABEL: firrtl.module private @LayerBlocks

0 commit comments

Comments
 (0)