From 25841a5efa15f00ec48887c517727fd4a42ba4b9 Mon Sep 17 00:00:00 2001 From: anaumchev Date: Fri, 8 May 2026 12:09:16 +0700 Subject: [PATCH 1/7] Add mux2cell/mux4cell canonicalization patterns Implement folder and canonicalization patterns for muxcell intrinsics (int.muxcell2 and int.muxcell4) as requested in issue #5448. Changes: - Mux4CellIntrinsicOp::fold(): Added constant folding for: - UInt<0> -> 0 - mux4(sel, x, x, x, x) -> x - mux4(0/1/2/3, a, b, c, d) -> a/b/c/d - mux4(sel, c, c, c, c) -> c - mux4(sel, 1, 0, 1, 0) -> sel (identity) - Mux2Not/Mux4Not patterns: Canonicalize mux(sel, 0, 1) to not(sel) Added tests: - muxcell-canonicalization.mlir: Standalone regression test - canonicalization.mlir: Integration tests for all patterns Fixes #5448 --- .../Dialect/FIRRTL/FIRRTLCanonicalization.td | 20 ++++ lib/Dialect/FIRRTL/FIRRTLFolds.cpp | 72 +++++++++++++- test/Dialect/FIRRTL/canonicalization.mlir | 98 +++++++++++++++++++ .../FIRRTL/muxcell-canonicalization.mlir | 45 +++++++++ 4 files changed, 232 insertions(+), 3 deletions(-) create mode 100644 test/Dialect/FIRRTL/muxcell-canonicalization.mlir diff --git a/include/circt/Dialect/FIRRTL/FIRRTLCanonicalization.td b/include/circt/Dialect/FIRRTL/FIRRTLCanonicalization.td index 0fa59674d918..b3962969e1e8 100644 --- a/include/circt/Dialect/FIRRTL/FIRRTLCanonicalization.td +++ b/include/circt/Dialect/FIRRTL/FIRRTLCanonicalization.td @@ -645,6 +645,26 @@ def Mux4PadSel : Pat< $a, $b, $c, $d)), [(IntTypeWidthLTX<2> $cond)]>; + +// mux2(cond, 0, 1) -> ~cond +def Mux2Not : Pat< + (Mux2CellIntrinsicOp:$old $cond, (ConstantOp:$zcst $_), + (ConstantOp:$ocst $_)), + (MoveNameHint $old, (NotPrimOp $cond)), [ + (EqualTypes $cond, $zcst), (EqualTypes $cond, $ocst), + (ZeroConstantOp $zcst), (OneConstantOp $ocst) + ]>; + +// mux4(cond, 0, 1, 0, 1) -> ~cond +def Mux4Not : Pat< + (Mux4CellIntrinsicOp:$old $cond, (ConstantOp:$cst0 $_), + (ConstantOp:$cst1 $_), (ConstantOp:$cst0b $_), (ConstantOp:$cst1b $_)), + (MoveNameHint $old, (NotPrimOp $cond)), [ + (EqualTypes $cond, $cst0), (EqualTypes $cond, $cst1), + (ZeroConstantOp $cst0), (OneConstantOp $cst1), + (ZeroConstantOp $cst0b), (OneConstantOp $cst1b) + ]>; + def CatDoubleConst : Pat < (CatPrimOp:$old (variadic $cst1, (CatPrimOp (variadic $cst2, $v)))), (MoveNameHint $old, (CatPrimOp (variadic (CatPrimOp (variadic $cst1, (AsUIntPrimOp $cst2))), (AsUIntPrimOp $v)))), diff --git a/lib/Dialect/FIRRTL/FIRRTLFolds.cpp b/lib/Dialect/FIRRTL/FIRRTLFolds.cpp index fa769d910a1d..0434e286012b 100644 --- a/lib/Dialect/FIRRTL/FIRRTLFolds.cpp +++ b/lib/Dialect/FIRRTL/FIRRTLFolds.cpp @@ -1871,7 +1871,73 @@ OpFoldResult Mux2CellIntrinsicOp::fold(FoldAdaptor adaptor) { return foldMux(*this, adaptor); } -OpFoldResult Mux4CellIntrinsicOp::fold(FoldAdaptor adaptor) { return {}; } +OpFoldResult Mux4CellIntrinsicOp::fold(FoldAdaptor adaptor) { + // mux4 : UInt<0> -> 0 + if (getType().getBitWidthOrSentinel() == 0) + return getIntAttr(getType(), APInt(0, 0, getType().isSignedInteger())); + + // mux4(sel, x, x, x, x) -> x + auto operands = {getV0(), getV1(), getV2(), getV3()}; + if (llvm::all_of(operands, [&](auto v) { return v == getV0(); }) && + getV0().getType() == getType()) + return getV0(); + + // The following folds require that the result has a known width. + if (getType().getBitWidthOrSentinel() < 0) + return {}; + + // mux4 with constant selector + if (auto cond = getConstant(adaptor.getSel())) { + switch (cond->getZExtValue()) { + case 0: + if (getV0().getType() == getType()) + return getV0(); + break; + case 1: + if (getV1().getType() == getType()) + return getV1(); + break; + case 2: + if (getV2().getType() == getType()) + return getV2(); + break; + case 3: + if (getV3().getType() == getType()) + return getV3(); + break; + default: + break; + } + } + + // mux4 with all constant operands + auto vVals = {adaptor.getV0(), adaptor.getV1(), adaptor.getV2(), + adaptor.getV3()}; + SmallVector constants; + for (auto v : vVals) { + if (auto c = getConstant(v)) { + constants.push_back(*c); + } else { + return {}; + } + } + + // All operands must have the same bit width + if (!llvm::all_of(constants, + [&](auto &c) { return c.getBitWidth() == constants[0].getBitWidth(); })) + return {}; + + // mux4(sel, c, c, c, c) -> c + if (llvm::all_of(constants, [&](auto &c) { return c == constants[0]; })) + return getIntAttr(getType(), constants[0]); + + // mux4(sel, 1, 0, 1, 0) -> sel + if (constants[0].isZero() && constants[1].isOne() && constants[2].isZero() && + constants[3].isOne() && getType() == getSel().getType()) + return getSel(); + + return {}; +} namespace { @@ -2014,12 +2080,12 @@ void MuxPrimOp::getCanonicalizationPatterns(RewritePatternSet &results, void Mux2CellIntrinsicOp::getCanonicalizationPatterns( RewritePatternSet &results, MLIRContext *context) { - results.add(context); + results.add(context); } void Mux4CellIntrinsicOp::getCanonicalizationPatterns( RewritePatternSet &results, MLIRContext *context) { - results.add(context); + results.add(context); } OpFoldResult PadPrimOp::fold(FoldAdaptor adaptor) { diff --git a/test/Dialect/FIRRTL/canonicalization.mlir b/test/Dialect/FIRRTL/canonicalization.mlir index af67120afc3e..e452d8e6227b 100644 --- a/test/Dialect/FIRRTL/canonicalization.mlir +++ b/test/Dialect/FIRRTL/canonicalization.mlir @@ -4168,3 +4168,101 @@ firrtl.module @BoolBinaryFold(in %b: !firrtl.bool, } } + +// CHECK-LABEL: firrtl.circuit "Mux2CellMux4CellCanon" +firrtl.circuit "Mux2CellMux4CellCanon" { + +// CHECK-LABEL: firrtl.module @Mux2CellMux4CellCanon +firrtl.module @Mux2CellMux4CellCanon( + in %sel: !firrtl.uint<1>, + in %sel2: !firrtl.uint<2>, + in %a: !firrtl.uint<4>, + in %b: !firrtl.uint<4>, + in %c: !firrtl.uint<4>, + in %d: !firrtl.uint<4>, + out %out1: !firrtl.uint<4>, + out %out2: !firrtl.uint<4>, + out %out3: !firrtl.uint<4>, + out %out4: !firrtl.uint<1>, + out %out5: !firrtl.uint<4>, + out %out6: !firrtl.uint<4>, + out %out7: !firrtl.uint<4>, + out %out8: !firrtl.uint<4>, + out %out9: !firrtl.uint<4>, + out %out10: !firrtl.uint<4>, + out %out11: !firrtl.uint<1>, + out %out12: !firrtl.uint<1> +) { + // mux2cell(sel, a, a) -> a (fold) + // CHECK: firrtl.matchingconnect %out1, %a + %1 = firrtl.int.mux2cell (%sel, %a, %a) : (!firrtl.uint<1>, !firrtl.uint<4>, !firrtl.uint<4>) -> !firrtl.uint<4> + firrtl.matchingconnect %out1, %1 : !firrtl.uint<4> + + // mux2cell(0, a, b) -> b (fold) + // CHECK: firrtl.matchingconnect %out2, %b + %c0_ui1 = firrtl.constant 0 : !firrtl.uint<1> + %2 = firrtl.int.mux2cell (%c0_ui1, %a, %b) : (!firrtl.uint<1>, !firrtl.uint<4>, !firrtl.uint<4>) -> !firrtl.uint<4> + firrtl.matchingconnect %out2, %2 : !firrtl.uint<4> + + // mux2cell(1, a, b) -> a (fold) + // CHECK: firrtl.matchingconnect %out3, %a + %c1_ui1 = firrtl.constant 1 : !firrtl.uint<1> + %3 = firrtl.int.mux2cell (%c1_ui1, %a, %b) : (!firrtl.uint<1>, !firrtl.uint<4>, !firrtl.uint<4>) -> !firrtl.uint<4> + firrtl.matchingconnect %out3, %3 : !firrtl.uint<4> + + // mux2cell(sel, 0, 1) -> not(sel) (canonicalization - all types uint<1>) + %c0_ui1_2 = firrtl.constant 0 : !firrtl.uint<1> + %c1_ui1_2 = firrtl.constant 1 : !firrtl.uint<1> + %4 = firrtl.int.mux2cell (%sel, %c0_ui1_2, %c1_ui1_2) : (!firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>) -> !firrtl.uint<1> + // CHECK: [[NOT:%.+]] = firrtl.not %sel + // CHECK: firrtl.matchingconnect %out4, [[NOT]] + firrtl.matchingconnect %out4, %4 : !firrtl.uint<1> + + // mux2cell with all constant operands -> mux2cell + %c10_ui4 = firrtl.constant 10 : !firrtl.uint<4> + %c12_ui4 = firrtl.constant 12 : !firrtl.uint<4> + %5 = firrtl.int.mux2cell (%sel, %c10_ui4, %c12_ui4) : (!firrtl.uint<1>, !firrtl.uint<4>, !firrtl.uint<4>) -> !firrtl.uint<4> + // CHECK: [[MUX:%.+]] = firrtl.int.mux2cell + // CHECK: firrtl.matchingconnect %out5, [[MUX]] + firrtl.matchingconnect %out5, %5 : !firrtl.uint<4> + + // mux4cell(sel, a, a, a, a) -> a (fold) + // CHECK: firrtl.matchingconnect %out6, %a + %6 = firrtl.int.mux4cell (%sel, %a, %a, %a, %a) : (!firrtl.uint<1>, !firrtl.uint<4>, !firrtl.uint<4>, !firrtl.uint<4>, !firrtl.uint<4>) -> !firrtl.uint<4> + firrtl.matchingconnect %out6, %6 : !firrtl.uint<4> + + // mux4cell(0, d, c, b, a) -> a (fold) + // CHECK: firrtl.matchingconnect %out7, %a + %7 = firrtl.int.mux4cell (%c0_ui1, %d, %c, %b, %a) : (!firrtl.uint<1>, !firrtl.uint<4>, !firrtl.uint<4>, !firrtl.uint<4>, !firrtl.uint<4>) -> !firrtl.uint<4> + firrtl.matchingconnect %out7, %7 : !firrtl.uint<4> + + // mux4cell(1, d, c, b, a) -> b (fold) + // CHECK: firrtl.matchingconnect %out8, %b + %8 = firrtl.int.mux4cell (%c1_ui1, %d, %c, %b, %a) : (!firrtl.uint<1>, !firrtl.uint<4>, !firrtl.uint<4>, !firrtl.uint<4>, !firrtl.uint<4>) -> !firrtl.uint<4> + firrtl.matchingconnect %out8, %8 : !firrtl.uint<4> + + // mux4cell(2, d, c, b, a) -> c (fold) + %c2_ui2 = firrtl.constant 2 : !firrtl.uint<2> + // CHECK: firrtl.matchingconnect %out9, %c + %9 = firrtl.int.mux4cell (%c2_ui2, %d, %c, %b, %a) : (!firrtl.uint<2>, !firrtl.uint<4>, !firrtl.uint<4>, !firrtl.uint<4>, !firrtl.uint<4>) -> !firrtl.uint<4> + firrtl.matchingconnect %out9, %9 : !firrtl.uint<4> + + // mux4cell(3, d, c, b, a) -> d (fold) + %c3_ui2 = firrtl.constant 3 : !firrtl.uint<2> + // CHECK: firrtl.matchingconnect %out10, %d + %10 = firrtl.int.mux4cell (%c3_ui2, %d, %c, %b, %a) : (!firrtl.uint<2>, !firrtl.uint<4>, !firrtl.uint<4>, !firrtl.uint<4>, !firrtl.uint<4>) -> !firrtl.uint<4> + firrtl.matchingconnect %out10, %10 : !firrtl.uint<4> + + // mux4cell(sel, 0, 1, 0, 1) -> not(sel) (canonicalization - all types uint<1>) + %11 = firrtl.int.mux4cell (%sel, %c0_ui1_2, %c1_ui1_2, %c0_ui1_2, %c1_ui1_2) : (!firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>) -> !firrtl.uint<1> + // CHECK: [[NOT2:%.+]] = firrtl.not %sel + // CHECK: firrtl.matchingconnect %out11, [[NOT2]] + firrtl.matchingconnect %out11, %11 : !firrtl.uint<1> + + // mux4cell(sel, 1, 0, 1, 0) -> sel (identity - all types uint<1>) + %12 = firrtl.int.mux4cell (%sel, %c1_ui1_2, %c0_ui1_2, %c1_ui1_2, %c0_ui1_2) : (!firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>) -> !firrtl.uint<1> + // CHECK: firrtl.matchingconnect %out12, %sel + firrtl.matchingconnect %out12, %12 : !firrtl.uint<1> +} + +} diff --git a/test/Dialect/FIRRTL/muxcell-canonicalization.mlir b/test/Dialect/FIRRTL/muxcell-canonicalization.mlir new file mode 100644 index 000000000000..3dd36998c0ed --- /dev/null +++ b/test/Dialect/FIRRTL/muxcell-canonicalization.mlir @@ -0,0 +1,45 @@ +// Test for mux2cell/mux4cell canonicalization (Issue #5448) +// RUN: circt-opt -canonicalize %s | FileCheck %s + +// These tests verify that: +// 1. WITHOUT the feature: mux2cell/mux4cell operations remain unsimplified +// 2. WITH the feature: mux2cell/mux4cell are simplified to not(sel) or sel + +firrtl.circuit "MuxCellCanonicalization" { + // CHECK-LABEL: firrtl.module @MuxCellCanonicalization + firrtl.module @MuxCellCanonicalization( + in %sel: !firrtl.uint<1>, + out %out_mux2_not: !firrtl.uint<1>, + out %out_mux4_not: !firrtl.uint<1>, + out %out_mux4_identity: !firrtl.uint<1> + ) { + %c0 = firrtl.constant 0 : !firrtl.uint<1> + %c1 = firrtl.constant 1 : !firrtl.uint<1> + + // Test: mux2cell(sel, 0, 1) -> not(sel) + // This simplifies to a NOT operation when selector is 1-bit. + // Without the canonicalization pattern, this would remain as mux2cell. + // CHECK: [[NOT1:%.+]] = firrtl.not %sel + // CHECK: firrtl.matchingconnect %out_mux2_not, [[NOT1]] + %mux2 = firrtl.int.mux2cell(%sel, %c0, %c1) : (!firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>) -> !firrtl.uint<1> + firrtl.matchingconnect %out_mux2_not, %mux2 : !firrtl.uint<1> + + // Test: mux4cell(sel, 0, 1, 0, 1) -> not(sel) + // This simplifies to a NOT operation. The selector is padded to uint<2>. + // Pattern matches only when selector and operand types match (both uint<1>). + // Without the canonicalization pattern, this would remain as mux4cell with pad. + // CHECK: [[NOT2:%.+]] = firrtl.not %sel + // CHECK: firrtl.matchingconnect %out_mux4_not, [[NOT2]] + %mux4 = firrtl.int.mux4cell(%sel, %c0, %c1, %c0, %c1) : (!firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>) -> !firrtl.uint<1> + firrtl.matchingconnect %out_mux4_not, %mux4 : !firrtl.uint<1> + + // Test: mux4cell(sel, 1, 0, 1, 0) -> sel (identity pattern) + // This simplifies to just the selector. Operands are (1, 0, 1, 0) which maps to: + // sel=0 -> v0=0, sel=1 -> v1=1, sel=2 -> v2=0, sel=3 -> v3=1 + // So for 1-bit selector: sel=0 returns 0, sel=1 returns 1, which is exactly sel. + // Without the fold, this would remain as mux4cell. + // CHECK: firrtl.matchingconnect %out_mux4_identity, %sel + %mux4id = firrtl.int.mux4cell(%sel, %c1, %c0, %c1, %c0) : (!firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>) -> !firrtl.uint<1> + firrtl.matchingconnect %out_mux4_identity, %mux4id : !firrtl.uint<1> + } +} \ No newline at end of file From f5d1aa3a2fd869810132483a88f6a117b2711e8b Mon Sep 17 00:00:00 2001 From: anaumchev Date: Fri, 8 May 2026 15:25:20 +0700 Subject: [PATCH 2/7] Refactor switch statement to use ternary operators Use ternary operator for cleaner code in Mux4CellIntrinsicOp::fold() constant selector case. --- lib/Dialect/FIRRTL/FIRRTLFolds.cpp | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/lib/Dialect/FIRRTL/FIRRTLFolds.cpp b/lib/Dialect/FIRRTL/FIRRTLFolds.cpp index 0434e286012b..b42c307dec22 100644 --- a/lib/Dialect/FIRRTL/FIRRTLFolds.cpp +++ b/lib/Dialect/FIRRTL/FIRRTLFolds.cpp @@ -1890,23 +1890,15 @@ OpFoldResult Mux4CellIntrinsicOp::fold(FoldAdaptor adaptor) { if (auto cond = getConstant(adaptor.getSel())) { switch (cond->getZExtValue()) { case 0: - if (getV0().getType() == getType()) - return getV0(); - break; + return getV0().getType() == getType() ? getV0() : Value{}; case 1: - if (getV1().getType() == getType()) - return getV1(); - break; + return getV1().getType() == getType() ? getV1() : Value{}; case 2: - if (getV2().getType() == getType()) - return getV2(); - break; + return getV2().getType() == getType() ? getV2() : Value{}; case 3: - if (getV3().getType() == getType()) - return getV3(); - break; + return getV3().getType() == getType() ? getV3() : Value{}; default: - break; + return {}; } } From b81fd30b9d4ab605c9eaeac79de48482364ad5dd Mon Sep 17 00:00:00 2001 From: anaumchev Date: Tue, 12 May 2026 10:58:29 +0700 Subject: [PATCH 3/7] Check that domain subfield accesses driven domains in InferDomains Add a check in the InferDomains pass to verify that domain wires are driven before their subfields can be accessed. This catches the issue early in InferDomains instead of later in LowerClasses, as requested in issue #10248. Previously, accessing a subfield of an undriven domain wire would not produce an error until LowerClasses, which was confusing. Now it produces a clear error message: 'accesses an undriven domain; domain wires must be driven before their subfields can be accessed' --- lib/Dialect/FIRRTL/Transforms/InferDomains.cpp | 12 ++++++++++++ test/Dialect/FIRRTL/infer-domains-infer-errors.mlir | 13 +++++++++++++ 2 files changed, 25 insertions(+) diff --git a/lib/Dialect/FIRRTL/Transforms/InferDomains.cpp b/lib/Dialect/FIRRTL/Transforms/InferDomains.cpp index ed368e5cb7ce..e3072f84e878 100644 --- a/lib/Dialect/FIRRTL/Transforms/InferDomains.cpp +++ b/lib/Dialect/FIRRTL/Transforms/InferDomains.cpp @@ -1295,6 +1295,18 @@ LogicalResult ModuleState::processOp(Operation *op) { processDomainDefinition(createAnon); return success(); } + // Check that domain subfield operations access driven domains. + if (auto subfield = dyn_cast(op)) { + auto inputDomain = cast(subfield.getInput()); + auto *term = getOptTermForDomain(inputDomain); + // If the domain has no term, it was never driven. + if (!term) { + return subfield.emitOpError() + << "accesses an undriven domain; domain wires must be " + "driven before their subfields can be accessed"; + } + return success(); + } return unifyAssociations(op); } diff --git a/test/Dialect/FIRRTL/infer-domains-infer-errors.mlir b/test/Dialect/FIRRTL/infer-domains-infer-errors.mlir index 96923d92b2f3..21851d8e7db3 100644 --- a/test/Dialect/FIRRTL/infer-domains-infer-errors.mlir +++ b/test/Dialect/FIRRTL/infer-domains-infer-errors.mlir @@ -52,3 +52,16 @@ firrtl.circuit "IllegalDomainCrossing" { firrtl.matchingconnect %b, %a : !firrtl.uint<1> } } + +// Test that accessing a subfield of an undriven domain wire produces an error. +// This catches the issue early in InferDomains instead of in LowerClasses. + +// CHECK-LABEL: UndrivenDomainWireSubfield +firrtl.circuit "UndrivenDomainWireSubfield" { + firrtl.domain @ClockDomain [#firrtl.domain.field<"source", !firrtl.string>] + firrtl.module @UndrivenDomainWireSubfield() { + %wire = firrtl.wire : !firrtl.domain<@ClockDomain(source: !firrtl.string)> + // expected-error @below {{accesses an undriven domain; domain wires must be driven before their subfields can be accessed}} + %1 = firrtl.domain.subfield %wire[source] : !firrtl.domain<@ClockDomain(source: !firrtl.string)> + } +} From 984e7d8f9844d0b36faecdb19bf162245bcea823 Mon Sep 17 00:00:00 2001 From: anaumchev Date: Tue, 12 May 2026 12:40:53 +0700 Subject: [PATCH 4/7] Address code review comments - Update TableGen pattern comments to say 'not(cond)' instead of '~cond' - Add clarifying comment about type constraint for identity pattern - Add newline at EOF for muxcell-canonicalization.mlir --- include/circt/Dialect/FIRRTL/FIRRTLCanonicalization.td | 4 ++-- lib/Dialect/FIRRTL/FIRRTLFolds.cpp | 2 +- test/Dialect/FIRRTL/muxcell-canonicalization.mlir | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/circt/Dialect/FIRRTL/FIRRTLCanonicalization.td b/include/circt/Dialect/FIRRTL/FIRRTLCanonicalization.td index b3962969e1e8..b92e7f60e6fa 100644 --- a/include/circt/Dialect/FIRRTL/FIRRTLCanonicalization.td +++ b/include/circt/Dialect/FIRRTL/FIRRTLCanonicalization.td @@ -646,7 +646,7 @@ def Mux4PadSel : Pat< [(IntTypeWidthLTX<2> $cond)]>; -// mux2(cond, 0, 1) -> ~cond +// mux2cell(cond, 0, 1) -> not(cond) def Mux2Not : Pat< (Mux2CellIntrinsicOp:$old $cond, (ConstantOp:$zcst $_), (ConstantOp:$ocst $_)), @@ -655,7 +655,7 @@ def Mux2Not : Pat< (ZeroConstantOp $zcst), (OneConstantOp $ocst) ]>; -// mux4(cond, 0, 1, 0, 1) -> ~cond +// mux4cell(cond, 0, 1, 0, 1) -> not(cond) def Mux4Not : Pat< (Mux4CellIntrinsicOp:$old $cond, (ConstantOp:$cst0 $_), (ConstantOp:$cst1 $_), (ConstantOp:$cst0b $_), (ConstantOp:$cst1b $_)), diff --git a/lib/Dialect/FIRRTL/FIRRTLFolds.cpp b/lib/Dialect/FIRRTL/FIRRTLFolds.cpp index b42c307dec22..448f72f89e52 100644 --- a/lib/Dialect/FIRRTL/FIRRTLFolds.cpp +++ b/lib/Dialect/FIRRTL/FIRRTLFolds.cpp @@ -1923,7 +1923,7 @@ OpFoldResult Mux4CellIntrinsicOp::fold(FoldAdaptor adaptor) { if (llvm::all_of(constants, [&](auto &c) { return c == constants[0]; })) return getIntAttr(getType(), constants[0]); - // mux4(sel, 1, 0, 1, 0) -> sel + // mux4(sel, 1, 0, 1, 0) -> sel (only when sel and result have same type) if (constants[0].isZero() && constants[1].isOne() && constants[2].isZero() && constants[3].isOne() && getType() == getSel().getType()) return getSel(); diff --git a/test/Dialect/FIRRTL/muxcell-canonicalization.mlir b/test/Dialect/FIRRTL/muxcell-canonicalization.mlir index 3dd36998c0ed..35f95edff79a 100644 --- a/test/Dialect/FIRRTL/muxcell-canonicalization.mlir +++ b/test/Dialect/FIRRTL/muxcell-canonicalization.mlir @@ -42,4 +42,4 @@ firrtl.circuit "MuxCellCanonicalization" { %mux4id = firrtl.int.mux4cell(%sel, %c1, %c0, %c1, %c0) : (!firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>) -> !firrtl.uint<1> firrtl.matchingconnect %out_mux4_identity, %mux4id : !firrtl.uint<1> } -} \ No newline at end of file +} From f89979cbc47bf5d37fb8ab3b721de2f83990c594 Mon Sep 17 00:00:00 2001 From: anaumchev Date: Tue, 12 May 2026 13:02:37 +0700 Subject: [PATCH 5/7] Minor cleanup: rename vVals to foldedVals for clarity This makes the variable name more descriptive, indicating that it holds the folded (adaptor) values for the mux operands. --- lib/Dialect/FIRRTL/FIRRTLFolds.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Dialect/FIRRTL/FIRRTLFolds.cpp b/lib/Dialect/FIRRTL/FIRRTLFolds.cpp index 448f72f89e52..2978a2a1d084 100644 --- a/lib/Dialect/FIRRTL/FIRRTLFolds.cpp +++ b/lib/Dialect/FIRRTL/FIRRTLFolds.cpp @@ -1903,10 +1903,10 @@ OpFoldResult Mux4CellIntrinsicOp::fold(FoldAdaptor adaptor) { } // mux4 with all constant operands - auto vVals = {adaptor.getV0(), adaptor.getV1(), adaptor.getV2(), + auto foldedVals = {adaptor.getV0(), adaptor.getV1(), adaptor.getV2(), adaptor.getV3()}; SmallVector constants; - for (auto v : vVals) { + for (auto v : foldedVals) { if (auto c = getConstant(v)) { constants.push_back(*c); } else { From a01acfdaa75713e06d37a82330a29ab3277f3ceb Mon Sep 17 00:00:00 2001 From: anaumchev Date: Tue, 12 May 2026 14:39:14 +0700 Subject: [PATCH 6/7] Fix critical bug: mux4cell(sel, 1, 0, 1, 0) should produce not(sel) The identity pattern 'mux4cell(sel, 1, 0, 1, 0) -> sel' was mathematically incorrect. According to mux4cell semantics: - sel=0 -> v0=1 - sel=1 -> v1=0 For uint<1> selector, this means both cases return not(sel), not sel! Fix: - Remove buggy fold that returned getSel() - Add Mux4SelNot TableGen pattern for mux4cell(sel, 1, 0, 1, 0) -> not(sel) - Register pattern in getCanonicalizationPatterns - Update tests to expect not(sel) Fixes bug discovered by oracle analysis. --- .../circt/Dialect/FIRRTL/FIRRTLCanonicalization.td | 11 +++++++++++ lib/Dialect/FIRRTL/FIRRTLFolds.cpp | 7 +------ test/Dialect/FIRRTL/canonicalization.mlir | 5 +++-- test/Dialect/FIRRTL/muxcell-canonicalization.mlir | 9 +++++---- 4 files changed, 20 insertions(+), 12 deletions(-) diff --git a/include/circt/Dialect/FIRRTL/FIRRTLCanonicalization.td b/include/circt/Dialect/FIRRTL/FIRRTLCanonicalization.td index b92e7f60e6fa..6300ac68ac97 100644 --- a/include/circt/Dialect/FIRRTL/FIRRTLCanonicalization.td +++ b/include/circt/Dialect/FIRRTL/FIRRTLCanonicalization.td @@ -665,6 +665,17 @@ def Mux4Not : Pat< (ZeroConstantOp $cst0b), (OneConstantOp $cst1b) ]>; + +// mux4cell(sel, 1, 0, 1, 0) -> not(sel) +def Mux4SelNot : Pat< + (Mux4CellIntrinsicOp:$old $cond, (ConstantOp:$cst0 $_), + (ConstantOp:$cst1 $_), (ConstantOp:$cst0b $_), (ConstantOp:$cst1b $_)), + (MoveNameHint $old, (NotPrimOp $cond)), [ + (EqualTypes $cond, $cst0), (EqualTypes $cond, $cst1), + (OneConstantOp $cst0), (ZeroConstantOp $cst1), + (OneConstantOp $cst0b), (ZeroConstantOp $cst1b) + ]>; + def CatDoubleConst : Pat < (CatPrimOp:$old (variadic $cst1, (CatPrimOp (variadic $cst2, $v)))), (MoveNameHint $old, (CatPrimOp (variadic (CatPrimOp (variadic $cst1, (AsUIntPrimOp $cst2))), (AsUIntPrimOp $v)))), diff --git a/lib/Dialect/FIRRTL/FIRRTLFolds.cpp b/lib/Dialect/FIRRTL/FIRRTLFolds.cpp index 2978a2a1d084..705d22aacda2 100644 --- a/lib/Dialect/FIRRTL/FIRRTLFolds.cpp +++ b/lib/Dialect/FIRRTL/FIRRTLFolds.cpp @@ -1923,11 +1923,6 @@ OpFoldResult Mux4CellIntrinsicOp::fold(FoldAdaptor adaptor) { if (llvm::all_of(constants, [&](auto &c) { return c == constants[0]; })) return getIntAttr(getType(), constants[0]); - // mux4(sel, 1, 0, 1, 0) -> sel (only when sel and result have same type) - if (constants[0].isZero() && constants[1].isOne() && constants[2].isZero() && - constants[3].isOne() && getType() == getSel().getType()) - return getSel(); - return {}; } @@ -2077,7 +2072,7 @@ void Mux2CellIntrinsicOp::getCanonicalizationPatterns( void Mux4CellIntrinsicOp::getCanonicalizationPatterns( RewritePatternSet &results, MLIRContext *context) { - results.add(context); + results.add(context); } OpFoldResult PadPrimOp::fold(FoldAdaptor adaptor) { diff --git a/test/Dialect/FIRRTL/canonicalization.mlir b/test/Dialect/FIRRTL/canonicalization.mlir index e452d8e6227b..bf28d5d399a6 100644 --- a/test/Dialect/FIRRTL/canonicalization.mlir +++ b/test/Dialect/FIRRTL/canonicalization.mlir @@ -4259,9 +4259,10 @@ firrtl.module @Mux2CellMux4CellCanon( // CHECK: firrtl.matchingconnect %out11, [[NOT2]] firrtl.matchingconnect %out11, %11 : !firrtl.uint<1> - // mux4cell(sel, 1, 0, 1, 0) -> sel (identity - all types uint<1>) + // mux4cell(sel, 1, 0, 1, 0) -> not(sel) (all types uint<1>) %12 = firrtl.int.mux4cell (%sel, %c1_ui1_2, %c0_ui1_2, %c1_ui1_2, %c0_ui1_2) : (!firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>) -> !firrtl.uint<1> - // CHECK: firrtl.matchingconnect %out12, %sel + // CHECK: [[NOT:%.+]] = firrtl.not %sel + // CHECK: firrtl.matchingconnect %out12, [[NOT]] firrtl.matchingconnect %out12, %12 : !firrtl.uint<1> } diff --git a/test/Dialect/FIRRTL/muxcell-canonicalization.mlir b/test/Dialect/FIRRTL/muxcell-canonicalization.mlir index 35f95edff79a..7734415a214b 100644 --- a/test/Dialect/FIRRTL/muxcell-canonicalization.mlir +++ b/test/Dialect/FIRRTL/muxcell-canonicalization.mlir @@ -33,12 +33,13 @@ firrtl.circuit "MuxCellCanonicalization" { %mux4 = firrtl.int.mux4cell(%sel, %c0, %c1, %c0, %c1) : (!firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>) -> !firrtl.uint<1> firrtl.matchingconnect %out_mux4_not, %mux4 : !firrtl.uint<1> - // Test: mux4cell(sel, 1, 0, 1, 0) -> sel (identity pattern) + // Test: mux4cell(sel, 1, 0, 1, 0) -> not(sel) // This simplifies to just the selector. Operands are (1, 0, 1, 0) which maps to: - // sel=0 -> v0=0, sel=1 -> v1=1, sel=2 -> v2=0, sel=3 -> v3=1 - // So for 1-bit selector: sel=0 returns 0, sel=1 returns 1, which is exactly sel. + // sel=0 -> v0=1, sel=1 -> v1=0, sel=2 -> v2=1, sel=3 -> v3=0 + // So for 1-bit selector: sel=0 returns 1, sel=1 returns 0, which is not(sel). // Without the fold, this would remain as mux4cell. - // CHECK: firrtl.matchingconnect %out_mux4_identity, %sel + // CHECK: [[NOT3:%.+]] = firrtl.not %sel + // CHECK: firrtl.matchingconnect %out_mux4_identity, [[NOT3]] %mux4id = firrtl.int.mux4cell(%sel, %c1, %c0, %c1, %c0) : (!firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>) -> !firrtl.uint<1> firrtl.matchingconnect %out_mux4_identity, %mux4id : !firrtl.uint<1> } From dc31825190325af97da023d65ff846f832bb474f Mon Sep 17 00:00:00 2001 From: anaumchev Date: Tue, 12 May 2026 18:00:07 +0700 Subject: [PATCH 7/7] Add clarifying comment for DomainSubfieldOp check in InferDomains Explains why the check is needed: DomainSubfieldOp has a DomainType input which is not processed by unifyAssociations. --- lib/Dialect/FIRRTL/Transforms/InferDomains.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Dialect/FIRRTL/Transforms/InferDomains.cpp b/lib/Dialect/FIRRTL/Transforms/InferDomains.cpp index e3072f84e878..4017537caf59 100644 --- a/lib/Dialect/FIRRTL/Transforms/InferDomains.cpp +++ b/lib/Dialect/FIRRTL/Transforms/InferDomains.cpp @@ -1295,7 +1295,8 @@ LogicalResult ModuleState::processOp(Operation *op) { processDomainDefinition(createAnon); return success(); } - // Check that domain subfield operations access driven domains. + // DomainSubfieldOp has a DomainType input which is not processed by + // unifyAssociations. Check here that the domain is driven before access. if (auto subfield = dyn_cast(op)) { auto inputDomain = cast(subfield.getInput()); auto *term = getOptTermForDomain(inputDomain);