diff --git a/include/circt/Dialect/Comb/Combinational.td b/include/circt/Dialect/Comb/Combinational.td index caa1636cc1..de707ce2b3 100644 --- a/include/circt/Dialect/Comb/Combinational.td +++ b/include/circt/Dialect/Comb/Combinational.td @@ -85,21 +85,6 @@ def XorOp : UTVariadicOp<"xor", [Commutative]> { }]; } -def MergeOp : UTVariadicOp<"merge", [Commutative]> { - let summary ="Electrically merge signals together as if connected by a wire."; - let description = [{ - This operation merges multiple signals together as if they were connected to - a single wire. Presumably at most one of them should be a Z state at any - given time otherwise you'd get a short circuit. - - This operation allows modeling multiconnect semantics in SSA. - ``` - %result = hw.merge %a, %b, %c : t1 - ``` - }]; - let hasCanonicalizeMethod = false; -} - //===----------------------------------------------------------------------===// // Comparisons //===----------------------------------------------------------------------===// diff --git a/integration_test/Bindings/Python/dialects/comb.py b/integration_test/Bindings/Python/dialects/comb.py index d5fcd0956b..f536217c2a 100644 --- a/integration_test/Bindings/Python/dialects/comb.py +++ b/integration_test/Bindings/Python/dialects/comb.py @@ -172,9 +172,6 @@ with Context() as ctx, Location.unknown(): # CHECK: comb.xor %[[CONST]], %[[CONST]] comb.XorOp.create(const.result, const.result) - # CHECK: comb.merge %[[CONST]], %[[CONST]] - comb.MergeOp.create(const.result, const.result) - # CHECK: comb.concat %[[CONST]], %[[CONST]] comb.ConcatOp.create(i32, const.result, const.result) diff --git a/lib/Bindings/Python/circt/dialects/_comb_ops_ext.py b/lib/Bindings/Python/circt/dialects/_comb_ops_ext.py index 35a0ebb980..a7042fad75 100644 --- a/lib/Bindings/Python/circt/dialects/_comb_ops_ext.py +++ b/lib/Bindings/Python/circt/dialects/_comb_ops_ext.py @@ -185,11 +185,6 @@ class XorOp: pass -@VariadicOp -class MergeOp: - pass - - # Sugar classes for miscellaneous ops. class ConcatOp: diff --git a/lib/Conversion/FIRRTLToHW/LowerToHW.cpp b/lib/Conversion/FIRRTLToHW/LowerToHW.cpp index fc5d27d6bb..12e2059e66 100644 --- a/lib/Conversion/FIRRTLToHW/LowerToHW.cpp +++ b/lib/Conversion/FIRRTLToHW/LowerToHW.cpp @@ -779,19 +779,21 @@ static Value tryEliminatingConnectsToValue(Value flipValue, if (flipValue.getType().isa()) return tryEliminatingAttachesToAnalogValue(flipValue, insertPoint); - SmallVector connects; + ConnectOp theConnect; for (auto *use : flipValue.getUsers()) { // We only know about 'connect' uses, where this is the destination. auto connect = dyn_cast(use); - if (!connect || connect.src() == flipValue) + if (!connect || connect.src() == flipValue || + // We only support things with a single connect. + theConnect) return {}; - connects.push_back(connect); + theConnect = connect; } // We don't have an HW equivalent of "poison" so just don't special case // the case where there are no connects other uses of an output. - if (connects.empty()) + if (!theConnect) return {}; // TODO: Emit an sv.constantz here since it is unconnected. // Don't special case zero-bit results. @@ -801,40 +803,31 @@ static Value tryEliminatingConnectsToValue(Value flipValue, // Convert each connect into an extended version of its operand being // output. - SmallVector results; ImplicitLocOpBuilder builder(insertPoint->getLoc(), insertPoint); - for (auto connect : connects) { - auto connectSrc = connect.src(); + auto connectSrc = theConnect.src(); - // Convert fliped sources to passive sources. - if (!connectSrc.getType().cast().isPassive()) - connectSrc = builder.createOrFold(connectSrc); + // Convert fliped sources to passive sources. + if (!connectSrc.getType().cast().isPassive()) + connectSrc = builder.createOrFold(connectSrc); - // We know it must be the destination operand due to the types, but the - // source may not match the destination width. - auto destTy = flipValue.getType().cast().getPassiveType(); - if (destTy.getBitWidthOrSentinel() != - connectSrc.getType().cast().getBitWidthOrSentinel()) { - // The only type mismatchs we care about is due to integer width - // differences. - auto destWidth = destTy.getBitWidthOrSentinel(); - assert(destWidth != -1 && "must know integer widths"); - connectSrc = - builder.createOrFold(destTy, connectSrc, destWidth); - } - - // Remove the connect and use its source as the value for the output. - connect.erase(); - results.push_back(connectSrc); + // We know it must be the destination operand due to the types, but the + // source may not match the destination width. + auto destTy = flipValue.getType().cast().getPassiveType(); + if (destTy.getBitWidthOrSentinel() != + connectSrc.getType().cast().getBitWidthOrSentinel()) { + // The only type mismatchs we care about is due to integer width + // differences. + auto destWidth = destTy.getBitWidthOrSentinel(); + assert(destWidth != -1 && "must know integer widths"); + connectSrc = builder.createOrFold(destTy, connectSrc, destWidth); } - // Convert from FIRRTL type to builtin type to do the merge. - for (auto &result : results) - result = castFromFIRRTLType(result, loweredType, builder); + // Remove the connect and use its source as the value for the output. + theConnect.erase(); - // Folding merge of one value just returns the value. - return builder.createOrFold(results); + // Convert from FIRRTL type to builtin type. + return castFromFIRRTLType(connectSrc, loweredType, builder); } static SmallVector getAllFieldAccesses(Value structValue, diff --git a/lib/Dialect/Comb/CombFolds.cpp b/lib/Dialect/Comb/CombFolds.cpp index 395a69d37c..127327e78a 100644 --- a/lib/Dialect/Comb/CombFolds.cpp +++ b/lib/Dialect/Comb/CombFolds.cpp @@ -962,14 +962,6 @@ LogicalResult XorOp::canonicalize(XorOp op, PatternRewriter &rewriter) { return failure(); } -OpFoldResult MergeOp::fold(ArrayRef constants) { - // hw.merge(x, x, x) -> x. - if (llvm::all_of(inputs(), [&](auto in) { return in == this->inputs()[0]; })) - return inputs()[0]; - - return {}; -} - OpFoldResult SubOp::fold(ArrayRef constants) { APInt value; // sub(x - 0) -> x diff --git a/lib/Translation/ExportVerilog/ExportVerilog.cpp b/lib/Translation/ExportVerilog/ExportVerilog.cpp index 3b29560eb4..4ba3cf18a2 100644 --- a/lib/Translation/ExportVerilog/ExportVerilog.cpp +++ b/lib/Translation/ExportVerilog/ExportVerilog.cpp @@ -3063,29 +3063,6 @@ static void lowerAlwaysInlineOperation(Operation *op) { return; } -/// We lower the Merge operation to a wire at the top level along with -/// assigns to it and a ReadInOut. -static Value lowerMergeOp(MergeOp merge) { - auto module = merge->getParentOfType(); - assert(module && "merges should only be in a module"); - - // Start with the wire at the top level. - ImplicitLocOpBuilder b(merge.getLoc(), &module.getBodyBlock()->front()); - auto wire = b.create(merge.getType()); - - // Each of the operands is an assign or passign into the wire. - b.setInsertionPoint(merge); - if (merge->getParentOp()->hasTrait()) { - for (auto op : merge.getOperands()) - b.create(wire, op); - } else { - for (auto op : merge.getOperands()) - b.create(wire, op); - } - - return b.create(wire); -} - /// Lower a commutative operation into an expression tree. This enables /// long-line splitting to work with them. static Value lowerVariadicCommutativeOp(Operation &op, OperandRange operands) { @@ -3353,14 +3330,6 @@ static void prepareHWModule(Block &block, ModuleNameManager &names, continue; } - // Lower 'merge' operations to wires and connects. - if (auto merge = dyn_cast(op)) { - auto result = lowerMergeOp(merge); - op.getResult(0).replaceAllUsesWith(result); - op.erase(); - continue; - } - // Lower commutative variadic operations with more than two operands into // balanced operand trees so we can split long lines across multiple // statements. diff --git a/test/Conversion/FIRRTLToHW/lower-to-hw-module.mlir b/test/Conversion/FIRRTLToHW/lower-to-hw-module.mlir index 786ab68821..110dfae979 100644 --- a/test/Conversion/FIRRTLToHW/lower-to-hw-module.mlir +++ b/test/Conversion/FIRRTLToHW/lower-to-hw-module.mlir @@ -100,6 +100,8 @@ firrtl.circuit "Simple" { out %outD: !firrtl.uint<4>, in %inE: !firrtl.uint<3>, out %outE: !firrtl.uint<4>) { + // CHECK: %.outB.output = sv.wire : !hw.inout + // CHECK: [[OUTBR:%.+]] = sv.read_inout %.outB.output // CHECK: [[OUTC:%.+]] = sv.wire : !hw.inout // CHECK: [[OUTCR:%.+]] = sv.read_inout %.outC.output // CHECK: [[OUTD:%.+]] = sv.wire : !hw.inout @@ -110,7 +112,9 @@ firrtl.circuit "Simple" { // Multi connect firrtl.connect %outB, %inA : !firrtl.uint<4>, !firrtl.uint<4> + // CHECK: sv.assign %.outB.output, %inA : i4 firrtl.connect %outB, %inB : !firrtl.uint<4>, !firrtl.uint<4> + // CHECK: sv.assign %.outB.output, %inB : i4 %0 = firrtl.sub %inA, %outC : (!firrtl.uint<4>, !firrtl.uint<4>) -> !firrtl.uint<5> @@ -118,11 +122,9 @@ firrtl.circuit "Simple" { firrtl.connect %outE, %inE : !firrtl.uint<4>, !firrtl.uint<3> - // CHECK: [[OUTBY:%.+]] = comb.merge %inB, %inA : i4 - // Extension for outE // CHECK: [[OUTE:%.+]] = comb.concat %false, %inE : (i1, i3) -> i4 - // CHECK: hw.output %inA, [[OUTBY]], [[OUTCR]], [[OUTDR]], [[OUTE]] + // CHECK: hw.output %inA, [[OUTBR]], [[OUTCR]], [[OUTDR]], [[OUTE]] } // CHECK-LABEL: hw.module @Analog(%a1: !hw.inout) -> (%outClock: i1) { diff --git a/test/Dialect/HW/canonicalization.mlir b/test/Dialect/HW/canonicalization.mlir index 294aba82b3..55411df82d 100644 --- a/test/Dialect/HW/canonicalization.mlir +++ b/test/Dialect/HW/canonicalization.mlir @@ -56,16 +56,6 @@ hw.module @xor_cstfold(%arg0: i7) -> (i7) { hw.output %0 : i7 } -// CHECK-LABEL: hw.module @merge_fold -// CHECK-NEXT: %0 = comb.merge %arg0, %arg0, %arg1 : i7 -// CHECK-NEXT: hw.output %arg0, %arg0, %0 : i7, i7, i7 -hw.module @merge_fold(%arg0: i7, %arg1: i7) -> (i7, i7, i7) { - %a = comb.merge %arg0 : i7 - %b = comb.merge %arg0, %arg0, %arg0 : i7 - %c = comb.merge %arg0, %arg0, %arg1 : i7 - hw.output %a, %b, %c: i7, i7, i7 -} - // CHECK-LABEL: hw.module @add_cstfold(%arg0: i7) -> (i7) { // CHECK-NEXT: %c15_i7 = hw.constant 15 : i7 // CHECK-NEXT: %0 = comb.add %arg0, %c15_i7 : i7 diff --git a/test/ExportVerilog/hw-dialect.mlir b/test/ExportVerilog/hw-dialect.mlir index d59528ffc1..07eef3dd4a 100644 --- a/test/ExportVerilog/hw-dialect.mlir +++ b/test/ExportVerilog/hw-dialect.mlir @@ -351,21 +351,6 @@ hw.module @wires(%in4: i4, %in8: i8) -> (%a: i4, %b: i8, %c: i8) { hw.output %wireout, %memout1, %memout2 : i4, i8, i8 } -// CHECK-LABEL: module merge -hw.module @merge(%in1: i4, %in2: i4, %in3: i4, %in4: i4) -> (%x: i4) { - // CHECK: wire [3:0] _T; - // CHECK: assign _T = in1 + in2; - %a = comb.add %in1, %in2 : i4 - - // CHECK-NEXT: assign _T = in2; - // CHECK-NEXT: assign _T = in3; - %b = comb.merge %a, %in2, %in3 : i4 - - // CHECK: assign x = _T + in4 + in4; - %c = comb.add %b, %in4, %in4 : i4 - hw.output %c : i4 -} - // CHECK-LABEL: module signs hw.module @signs(%in1: i4, %in2: i4, %in3: i4, %in4: i4) { %awire = sv.wire : !hw.inout diff --git a/test/ExportVerilog/sv-dialect.mlir b/test/ExportVerilog/sv-dialect.mlir index be0aaf88a8..dc5c935c9a 100644 --- a/test/ExportVerilog/sv-dialect.mlir +++ b/test/ExportVerilog/sv-dialect.mlir @@ -361,35 +361,6 @@ hw.module @exprInlineTestIssue439(%clk: i1) { } } -// CHECK-LABEL: module issue439( -// https://github.com/llvm/circt/issues/439 -hw.module @issue439(%in1: i1, %in2: i1) { - // CHECK: wire _T; - // CHECK: wire _T_0 = in1 | in2; - %clock = comb.or %in1, %in2 : i1 - - // CHECK-NEXT: always @(posedge _T_0) - sv.always posedge %clock { - // CHECK-NEXT: _T <= in1; - // CHECK-NEXT: _T <= in2; - %merged = comb.merge %in1, %in2 : i1 - // CHECK-NEXT: $fwrite(32'h80000002, "Bye %x\n", _T); - sv.fwrite "Bye %x\n"(%merged) : i1 - } -} - -// CHECK-LABEL: module issue726( -// https://github.com/llvm/circt/issues/726 -hw.module @issue726(%in1: i1, %in2: i1) -> (%out: i1) { - // CHECK: wire _T; - // CHECK: assign _T = in1; - // CHECK: assign _T = in2; - %merged = comb.merge %in1, %in2 : i1 - - // CHECK: assign out = _T; - hw.output %merged : i1 -} - // https://github.com/llvm/circt/issues/595 // CHECK-LABEL: module issue595 hw.module @issue595(%arr: !hw.array<128xi1>) {