Skip to content

Commit c32f60b

Browse files
committed
Revert "[FIRRTL] Add folder for mul (#8222)"
This reverts commit a511fda. 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 cd02ca7 commit c32f60b

File tree

3 files changed

+3
-26
lines changed

3 files changed

+3
-26
lines changed

include/circt/Dialect/FIRRTL/FIRRTLExpressions.td

-2
Original file line numberDiff line numberDiff line change
@@ -1409,8 +1409,6 @@ def IntegerMulOp : IntegerBinaryPrimOp<"integer.mul", [Commutative]> {
14091409
!firrtl.integer
14101410
```
14111411
}];
1412-
1413-
let hasFolder = true;
14141412
}
14151413

14161414
def IntegerShrOp : IntegerBinaryPrimOp<"integer.shr"> {

lib/Dialect/FIRRTL/FIRRTLFolds.cpp

+2-13
Original file line numberDiff line numberDiff line change
@@ -953,19 +953,8 @@ OpFoldResult IntegerAddOp::fold(FoldAdaptor adaptor) {
953953
}
954954

955955
OpFoldResult IntegerMulOp::fold(FoldAdaptor adaptor) {
956-
if (auto rhsCst = getConstant(adaptor.getRhs())) {
957-
// Constant folding
958-
if (auto lhsCst = getConstant(adaptor.getLhs())) {
959-
auto resultWidth = lhsCst->getBitWidth() + rhsCst->getBitWidth();
960-
APSInt lhsCstExt = (*lhsCst).extend(resultWidth);
961-
APSInt rhsCstExt = (*rhsCst).extend(resultWidth);
962-
return IntegerAttr::get(IntegerType::get(getContext(), resultWidth),
963-
lhsCstExt * rhsCstExt);
964-
}
965-
// x * 1 -> x
966-
if (rhsCst->isOne())
967-
return getLhs();
968-
}
956+
// TODO: implement constant folding, etc.
957+
// Tracked in https://github.com/llvm/circt/issues/6724.
969958
return {};
970959
}
971960

test/Dialect/FIRRTL/canonicalization.mlir

+1-11
Original file line numberDiff line numberDiff line change
@@ -3576,14 +3576,12 @@ 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, out %out4: !firrtl.integer, out %out5: !firrtl.integer) {
3580-
// CHECK: [[C10000:%.+]] = firrtl.integer 10000
3579+
firrtl.class @PropertyArithmetic(in %in: !firrtl.integer, out %out0: !firrtl.integer, out %out1: !firrtl.integer, out %out2: !firrtl.integer, out %out3: !firrtl.integer) {
35813580
// CHECK: [[C3:%.+]] = firrtl.integer 3
35823581
// CHECK: [[C4:%.+]] = firrtl.integer 4
35833582
%0 = firrtl.integer 0
35843583
%1 = firrtl.integer 1
35853584
%2 = firrtl.integer 2
3586-
%100 = firrtl.integer 100
35873585

35883586
%res0 = firrtl.integer.shl %1, %2 : (!firrtl.integer, !firrtl.integer) -> !firrtl.integer
35893587
%res1 = firrtl.integer.shl %in, %0 : (!firrtl.integer, !firrtl.integer) -> !firrtl.integer
@@ -3600,14 +3598,6 @@ firrtl.class @PropertyArithmetic(in %in: !firrtl.integer, out %out0: !firrtl.int
36003598
// CHECK: firrtl.propassign %out3, %in
36013599
firrtl.propassign %out2, %res2 : !firrtl.integer
36023600
firrtl.propassign %out3, %res3 : !firrtl.integer
3603-
3604-
%res4 = firrtl.integer.mul %100, %100 : (!firrtl.integer, !firrtl.integer) -> !firrtl.integer
3605-
%res5 = firrtl.integer.mul %in, %1 : (!firrtl.integer, !firrtl.integer) -> !firrtl.integer
3606-
3607-
// CHECK: firrtl.propassign %out4, [[C10000]]
3608-
// CHECK: firrtl.propassign %out5, %in
3609-
firrtl.propassign %out4, %res4 : !firrtl.integer
3610-
firrtl.propassign %out5, %res5 : !firrtl.integer
36113601
}
36123602

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

0 commit comments

Comments
 (0)