From e1fc1023d800dc4852b255a4e88dc017cc70559e Mon Sep 17 00:00:00 2001 From: Fabian Schuiki Date: Thu, 26 Jun 2025 13:29:02 -0700 Subject: [PATCH] [Comb] Enable cross-block folds on and/or/xor (#8607) Now that #8517 has landed, re-enable the canonicalizers and constant folders for `comb.and`, `comb.or`, and `comb.xor` that have operands defined in different blocks. --- lib/Dialect/Comb/CombFolds.cpp | 16 ---------------- test/Dialect/Calyx/remove-comb-groups.mlir | 9 +++------ test/Dialect/Seq/hw-memsim.mlir | 3 +-- 3 files changed, 4 insertions(+), 24 deletions(-) diff --git a/lib/Dialect/Comb/CombFolds.cpp b/lib/Dialect/Comb/CombFolds.cpp index 1d0f831cf0..69581f97d2 100644 --- a/lib/Dialect/Comb/CombFolds.cpp +++ b/lib/Dialect/Comb/CombFolds.cpp @@ -824,9 +824,6 @@ static bool canCombineOppositeBinCmpIntoConstant(OperandRange operands) { } OpFoldResult AndOp::fold(FoldAdaptor adaptor) { - if (hasOperandsOutsideOfBlock(getOperation())) - return {}; - APInt value = APInt::getAllOnes(cast(getType()).getWidth()); auto inputs = adaptor.getInputs(); @@ -977,8 +974,6 @@ LogicalResult AndOp::canonicalize(AndOp op, PatternRewriter &rewriter) { if (size > 1 && canonicalizeIdempotentInputs(op, rewriter)) return success(); - if (hasOperandsOutsideOfBlock(&*op)) - return failure(); assert(size > 1 && "expected 2 or more operands, `fold` should handle this"); // Patterns for and with a constant on RHS. @@ -1101,9 +1096,6 @@ LogicalResult AndOp::canonicalize(AndOp op, PatternRewriter &rewriter) { } OpFoldResult OrOp::fold(FoldAdaptor adaptor) { - if (hasOperandsOutsideOfBlock(getOperation())) - return {}; - auto value = APInt::getZero(cast(getType()).getWidth()); auto inputs = adaptor.getInputs(); // or(x, 10, 01) -> 11 @@ -1163,8 +1155,6 @@ LogicalResult OrOp::canonicalize(OrOp op, PatternRewriter &rewriter) { if (size > 1 && canonicalizeIdempotentInputs(op, rewriter)) return success(); - if (hasOperandsOutsideOfBlock(&*op)) - return failure(); assert(size > 1 && "expected 2 or more operands"); // Patterns for and with a constant on RHS. @@ -1243,9 +1233,6 @@ LogicalResult OrOp::canonicalize(OrOp op, PatternRewriter &rewriter) { } OpFoldResult XorOp::fold(FoldAdaptor adaptor) { - if (hasOperandsOutsideOfBlock(getOperation())) - return {}; - auto size = getInputs().size(); auto inputs = adaptor.getInputs(); @@ -1298,9 +1285,6 @@ static void canonicalizeXorIcmpTrue(XorOp op, unsigned icmpOperand, } LogicalResult XorOp::canonicalize(XorOp op, PatternRewriter &rewriter) { - if (hasOperandsOutsideOfBlock(&*op)) - return failure(); - auto inputs = op.getInputs(); auto size = inputs.size(); assert(size > 1 && "expected 2 or more operands"); diff --git a/test/Dialect/Calyx/remove-comb-groups.mlir b/test/Dialect/Calyx/remove-comb-groups.mlir index 3fb7039b04..ef6600d887 100644 --- a/test/Dialect/Calyx/remove-comb-groups.mlir +++ b/test/Dialect/Calyx/remove-comb-groups.mlir @@ -12,8 +12,7 @@ calyx.component @main(%go: i1 {go}, %clk: i1 {clk}, %reset: i1 {reset}) -> (%don // CHECK: calyx.assign %eq_reg.write_en = %true : i1 // CHECK: calyx.assign %eq.left = %true : i1 // CHECK: calyx.assign %eq.right = %true : i1 -// CHECK: %0 = comb.and %eq_reg.done : i1 -// CHECK: calyx.group_done %0 ? %true : i1 +// CHECK: calyx.group_done %eq_reg.done ? %true : i1 calyx.comb_group @Cond { calyx.assign %eq.left = %c1_1 : i1 calyx.assign %eq.right = %c1_1 : i1 @@ -60,8 +59,7 @@ calyx.component @main(%go: i1 {go}, %clk: i1 {clk}, %reset: i1 {reset}) -> (%don // CHECK: calyx.assign %eq_reg.write_en = %true : i1 // CHECK: calyx.assign %eq.left = %true : i1 // CHECK: calyx.assign %eq.right = %true : i1 -// CHECK: %0 = comb.and %eq_reg.done : i1 -// CHECK: calyx.group_done %0 ? %true : i1 +// CHECK: calyx.group_done %eq_reg.done ? %true : i1 // CHECK: } calyx.comb_group @Cond1 { calyx.assign %eq.left = %c1_1 : i1 @@ -74,8 +72,7 @@ calyx.component @main(%go: i1 {go}, %clk: i1 {clk}, %reset: i1 {reset}) -> (%don // CHECK: calyx.assign %eq_reg.write_en = %true : i1 // CHECK: calyx.assign %eq.left = %true : i1 // CHECK: calyx.assign %eq.right = %true : i1 -// CHECK: %0 = comb.and %eq_reg.done : i1 -// CHECK: calyx.group_done %0 ? %true : i1 +// CHECK: calyx.group_done %eq_reg.done ? %true : i1 // CHECK: } calyx.comb_group @Cond2 { calyx.assign %eq.left = %c1_1 : i1 diff --git a/test/Dialect/Seq/hw-memsim.mlir b/test/Dialect/Seq/hw-memsim.mlir index ddfb566e3d..61a4c96277 100644 --- a/test/Dialect/Seq/hw-memsim.mlir +++ b/test/Dialect/Seq/hw-memsim.mlir @@ -114,8 +114,7 @@ hw.module.generated @FIRRTLMem_1_1_1_16_10_0_1_0_0, @FIRRTLMem(in %ro_addr_0: i4 //CHECK-NEXT: } //CHECK-NEXT: %true_1 = hw.constant true //CHECK-NEXT: sv.always posedge %wo_clock_0 { -//CHECK-NEXT: %[[WO_EN:.+]] = comb.and %wo_en_0, %true_1 : i1 -//CHECK-NEXT: sv.if %[[WO_EN]] { +//CHECK-NEXT: sv.if %wo_en_0 { //CHECK-NEXT: %[[wslot:.+]] = sv.array_index_inout %Memory[%wo_addr_0] //CHECK-NEXT: %[[c0_i32:.+]] = hw.constant 0 : i32 //CHECK-NEXT: sv.passign %[[wslot]], %wo_data_0