diff --git a/include/circt/Dialect/FIRRTL/Passes.td b/include/circt/Dialect/FIRRTL/Passes.td index 744758656113..1ca66de0590a 100644 --- a/include/circt/Dialect/FIRRTL/Passes.td +++ b/include/circt/Dialect/FIRRTL/Passes.td @@ -124,22 +124,6 @@ def RegisterOptimizer : Pass<"firrtl-register-optimizer", "firrtl::FModuleOp"> { }]; } -def RemoveUnusedPorts : Pass<"firrtl-remove-unused-ports", "firrtl::CircuitOp"> { - let summary = "Remove unused ports"; - let description = [{ - This pass removes unused ports without annotations or symbols. Implementation - wise, this pass iterates over the instance graph in a topological order from - leaves to the top so that we can remove unused ports optimally. - }]; - let options = [ - Option<"ignoreDontTouch", "ignore-dont-touch", "bool", "false", - "remove unused ports even if they have a symbol or annotation"> - ]; - let statistics = [ - Statistic<"numRemovedPorts", "num-removed-ports", "Number of ports erased">, - ]; -} - def IMDeadCodeElim : Pass<"firrtl-imdeadcodeelim", "mlir::ModuleOp"> { let summary = "Intermodule dead code elimination"; let description = [{ @@ -148,6 +132,10 @@ def IMDeadCodeElim : Pass<"firrtl-imdeadcodeelim", "mlir::ModuleOp"> { of public modules or a value with a symbol. We first populate alive values into a set, and then propagate the liveness by looking at their dataflow. }]; + let options = [ + Option<"removePortsOnly", "remove-ports-only", "bool", "false", + "only remove dead module ports and preserve other operations"> + ]; let statistics = [ Statistic<"numErasedOps", "num-erased-ops", "Number of operations erased">, Statistic<"numErasedModules", "num-erased-modules", "Number of modules erased">, diff --git a/lib/Dialect/FIRRTL/FIRRTLReductions.cpp b/lib/Dialect/FIRRTL/FIRRTLReductions.cpp index c7cb08cbbea8..3a1e157b2b33 100644 --- a/lib/Dialect/FIRRTL/FIRRTLReductions.cpp +++ b/lib/Dialect/FIRRTL/FIRRTLReductions.cpp @@ -2663,6 +2663,16 @@ struct ExtmoduleConventionRemover : public OpReduction { bool isOneShot() const override { return true; } }; +struct IMDCEPortReduction : public PassReduction { + IMDCEPortReduction(MLIRContext *context) + : PassReduction( + context, firrtl::createIMDeadCodeElim({/*removePortsOnly=*/true})) { + } + std::string getName() const override { + return "firrtl-imdeadcodeelim-remove-ports"; + } +}; + //===----------------------------------------------------------------------===// // Reduction Registration //===----------------------------------------------------------------------===// @@ -2702,9 +2712,7 @@ void firrtl::FIRRTLReducePatternDialectInterface::populateReducePatterns( true, true); patterns.add(getContext(), firrtl::createInliner()); patterns.add(getContext(), firrtl::createIMConstProp()); - patterns.add( - getContext(), - firrtl::createRemoveUnusedPorts({/*ignoreDontTouch=*/true})); + patterns.add(getContext()); patterns.add(); patterns.add(getContext(), firrtl::createIMDeadCodeElim()); patterns.add(); diff --git a/lib/Dialect/FIRRTL/Transforms/CMakeLists.txt b/lib/Dialect/FIRRTL/Transforms/CMakeLists.txt index 70b8e3f780f0..9dd930292a48 100755 --- a/lib/Dialect/FIRRTL/Transforms/CMakeLists.txt +++ b/lib/Dialect/FIRRTL/Transforms/CMakeLists.txt @@ -56,7 +56,6 @@ add_circt_dialect_library(CIRCTFIRRTLTransforms ProbesToSignals.cpp RandomizeRegisterInit.cpp RegisterOptimizer.cpp - RemoveUnusedPorts.cpp ResolvePaths.cpp ResolveTraces.cpp SFCCompat.cpp diff --git a/lib/Dialect/FIRRTL/Transforms/IMDeadCodeElim.cpp b/lib/Dialect/FIRRTL/Transforms/IMDeadCodeElim.cpp index a396e4a65e4b..0e0fdcc14fa8 100644 --- a/lib/Dialect/FIRRTL/Transforms/IMDeadCodeElim.cpp +++ b/lib/Dialect/FIRRTL/Transforms/IMDeadCodeElim.cpp @@ -56,6 +56,8 @@ static bool isDeletableDeclaration(Operation *op) { namespace { struct IMDeadCodeElimPass : public circt::firrtl::impl::IMDeadCodeElimBase { + using Base::Base; + void runOnOperation() override; void rewriteModuleSignature(FModuleOp module); @@ -200,6 +202,12 @@ void IMDeadCodeElimPass::markUnknownSideEffectOp(Operation *op) { for (auto operand : op->getOperands()) markAlive(operand); markBlockUndeletable(op); + + // Recursively mark any blocks contained within these operations as + // executable. + for (auto ®ion : op->getRegions()) + for (auto &block : region.getBlocks()) + markBlockExecutable(&block); } void IMDeadCodeElimPass::visitUser(Operation *op) { @@ -266,7 +274,7 @@ void IMDeadCodeElimPass::markBlockExecutable(Block *block) { return; // Already executable. auto fmodule = dyn_cast(block->getParentOp()); - if (fmodule && fmodule.isPublic()) + if (fmodule && (fmodule.isPublic() || removePortsOnly)) markAlive(fmodule); // Mark ports with don't touch as alive. @@ -278,29 +286,36 @@ void IMDeadCodeElimPass::markBlockExecutable(Block *block) { } for (auto &op : *block) { - if (isDeclaration(&op)) - markDeclaration(&op); - else if (auto instance = dyn_cast(op)) + if (auto instance = dyn_cast(op)) { markFInstanceLikeOp(instance); - else if (isa(op)) - // Skip connect op. continue; - else if (hasUnknownSideEffect(&op)) { + } + + // Skip connects in both modes. + if (isa(op)) + continue; + + if (removePortsOnly) { + // In port-only mode, all non-instance, non-connect ops are alive. markUnknownSideEffectOp(&op); - // Recursively mark any blocks contained within these operations as - // executable. - for (auto ®ion : op.getRegions()) - for (auto &block : region.getBlocks()) - markBlockExecutable(&block); + continue; } + // Full IMDCE mode: handle declarations and side effects. + if (isDeclaration(&op)) + markDeclaration(&op); + else if (hasUnknownSideEffect(&op)) + markUnknownSideEffectOp(&op); + // TODO: Handle attach etc. } } void IMDeadCodeElimPass::forwardConstantOutputPort(FModuleOp module) { - // This tracks constant values of output ports. - SmallVector> constantPortIndicesAndValues; + // This tracks constant values of output ports. std::nullopt represents an + // invalid value. + SmallVector>> + constantPortIndicesAndValues; auto ports = module.getPorts(); auto *instanceGraphNode = instanceGraph->lookup(module); @@ -314,12 +329,15 @@ void IMDeadCodeElimPass::forwardConstantOutputPort(FModuleOp module) { continue; // Remember the index and constant value connected to an output port. - if (auto connect = getSingleConnectUserOf(arg)) + if (auto connect = getSingleConnectUserOf(arg)) { if (auto constant = connect.getSrc().getDefiningOp()) constantPortIndicesAndValues.push_back({index, constant.getValue()}); + else if (connect.getSrc().getDefiningOp()) + constantPortIndicesAndValues.push_back({index, std::nullopt}); + } } - // If there is no constant port, abort. + // If there is no constant or invalid port, abort. if (constantPortIndicesAndValues.empty()) return; @@ -334,8 +352,13 @@ void IMDeadCodeElimPass::forwardConstantOutputPort(FModuleOp module) { auto result = instance.getResult(index); assert(ports[index].isOutput() && "must be an output port"); - // Replace the port with the constant. - result.replaceAllUsesWith(ConstantOp::create(builder, constant)); + // Replace the port with the constant or invalid value. + Value replacement; + if (constant) + replacement = ConstantOp::create(builder, *constant); + else + replacement = InvalidValueOp::create(builder, result.getType()); + result.replaceAllUsesWith(replacement); } } } @@ -435,11 +458,13 @@ void IMDeadCodeElimPass::runOnOperation() { forwardConstantOutputPort(module); for (auto module : circuit.getBodyBlock()->getOps()) { - // Mark the ports of public modules as alive. - if (module.isPublic()) { + bool isPublic = module.isPublic(); + if (isPublic || removePortsOnly) { markBlockExecutable(module.getBodyBlock()); - for (auto port : module.getBodyBlock()->getArguments()) - markAlive(port); + // Mark the ports of public modules as alive. + if (isPublic) + for (auto port : module.getBodyBlock()->getArguments()) + markAlive(port); } // Walk annotations and populate a map from hierpath to attached annotation @@ -502,8 +527,9 @@ void IMDeadCodeElimPass::runOnOperation() { if (!liveElements.count(op)) op.erase(); - for (auto module : modules) - eraseEmptyModule(module); + if (!removePortsOnly) + for (auto module : modules) + eraseEmptyModule(module); // Clean up data structures. executableBlocks.clear(); diff --git a/lib/Dialect/FIRRTL/Transforms/RemoveUnusedPorts.cpp b/lib/Dialect/FIRRTL/Transforms/RemoveUnusedPorts.cpp deleted file mode 100644 index d98d48f18895..000000000000 --- a/lib/Dialect/FIRRTL/Transforms/RemoveUnusedPorts.cpp +++ /dev/null @@ -1,221 +0,0 @@ -//===- RemoveUnusedPorts.cpp - Remove Dead Ports ----------------*- C++ -*-===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#include "circt/Dialect/FIRRTL/FIRRTLAnnotations.h" -#include "circt/Dialect/FIRRTL/FIRRTLInstanceGraph.h" -#include "circt/Dialect/FIRRTL/FIRRTLOps.h" -#include "circt/Dialect/FIRRTL/FIRRTLUtils.h" -#include "circt/Dialect/FIRRTL/Passes.h" -#include "circt/Support/Debug.h" -#include "mlir/IR/ImplicitLocOpBuilder.h" -#include "mlir/Pass/Pass.h" -#include "llvm/ADT/BitVector.h" -#include "llvm/ADT/PostOrderIterator.h" -#include "llvm/Support/Debug.h" - -#define DEBUG_TYPE "firrtl-remove-unused-ports" - -namespace circt { -namespace firrtl { -#define GEN_PASS_DEF_REMOVEUNUSEDPORTS -#include "circt/Dialect/FIRRTL/Passes.h.inc" -} // namespace firrtl -} // namespace circt - -using namespace circt; -using namespace firrtl; - -namespace { - -struct RemoveUnusedPortsPass - : public circt::firrtl::impl::RemoveUnusedPortsBase { - using Base::Base; - - void runOnOperation() override; - void removeUnusedModulePorts(FModuleOp module, - InstanceGraphNode *instanceGraphNode); - - /// If true, the pass will remove unused ports even if they have carry a - /// symbol or annotations. This is likely to break the IR, but may be useful - /// for `circt-reduce` where preserving functional correctness of the IR is - /// not important. - bool ignoreDontTouch = false; -}; -} // namespace - -void RemoveUnusedPortsPass::runOnOperation() { - CIRCT_DEBUG_SCOPED_PASS_LOGGER(this); - - auto &instanceGraph = getAnalysis(); - // Iterate in the reverse order of instance graph iterator, i.e. from leaves - // to top. - for (auto *node : llvm::post_order(&instanceGraph)) - if (auto module = dyn_cast(*node->getModule())) - // Don't prune the main module. - if (!module.isPublic()) - removeUnusedModulePorts(module, node); -} - -void RemoveUnusedPortsPass::removeUnusedModulePorts( - FModuleOp module, InstanceGraphNode *instanceGraphNode) { - LLVM_DEBUG(llvm::dbgs() << "Prune ports of module: " << module.getName() - << "\n"); - // This tracks constant values of output ports. None indicates an invalid - // value. - SmallVector> outputPortConstants; - auto ports = module.getPorts(); - // This tracks port indexes that can be erased. - llvm::BitVector removalPortIndexes(ports.size()); - - for (const auto &e : llvm::enumerate(ports)) { - unsigned index = e.index(); - auto port = e.value(); - auto arg = module.getArgument(index); - - // If the port is don't touch or has unprocessed annotations, we cannot - // remove the port. - if ((hasDontTouch(arg) || !port.annotations.empty()) && !ignoreDontTouch) - continue; - - // TODO: Handle inout ports. - if (port.isInOut()) - continue; - - // If the port is input and has an user, we cannot remove the - // port. - if (port.isInput() && !arg.use_empty()) - continue; - - auto portIsUnused = [&](InstanceRecord *a) -> bool { - auto port = a->getInstance()->getResult(arg.getArgNumber()); - return port.getUses().empty(); - }; - - // Output port. - if (port.isOutput()) { - if (arg.use_empty()) { - // Sometimes the connection is already removed possibly by IMCP. - // In that case, regard the port value as an invalid value. - outputPortConstants.push_back(std::nullopt); - } else if (llvm::all_of(instanceGraphNode->uses(), portIsUnused)) { - // Replace the port with a wire if it is unused. - auto builder = ImplicitLocOpBuilder::atBlockBegin( - arg.getLoc(), module.getBodyBlock()); - auto wire = WireOp::create(builder, arg.getType()); - arg.replaceAllUsesWith(wire.getResult()); - outputPortConstants.push_back(std::nullopt); - } else if (arg.hasOneUse()) { - // If the port has a single use, check the port is only connected to - // invalid or constant - Operation *op = arg.use_begin().getUser(); - auto connectLike = dyn_cast(op); - if (!connectLike) - continue; - auto *srcOp = connectLike.getSrc().getDefiningOp(); - if (!isa_and_nonnull(srcOp)) - continue; - - if (auto constant = dyn_cast(srcOp)) - outputPortConstants.push_back(constant.getValue()); - else { - assert(isa(srcOp) && "only expect invalid"); - outputPortConstants.push_back(std::nullopt); - } - - // Erase connect op because we are going to remove this output ports. - op->erase(); - - if (srcOp->use_empty()) - srcOp->erase(); - } else { - // Otherwise, we cannot remove the port. - continue; - } - } - - removalPortIndexes.set(index); - } - - // If there is nothing to remove, abort. - if (removalPortIndexes.none()) - return; - - // Delete ports from the module. - module.erasePorts(removalPortIndexes); - LLVM_DEBUG(llvm::for_each(removalPortIndexes.set_bits(), [&](unsigned index) { - llvm::dbgs() << "Delete port: " << ports[index].name << "\n"; - });); - - // Rewrite all uses. - for (auto *use : instanceGraphNode->uses()) { - auto instance = ::cast(*use->getInstance()); - ImplicitLocOpBuilder builder(instance.getLoc(), instance); - TieOffCache tieOffCache(builder); - unsigned outputPortIndex = 0; - for (auto index : removalPortIndexes.set_bits()) { - auto result = instance.getResult(index); - assert(!ports[index].isInOut() && "don't expect inout ports"); - - // If the port is input, replace the port with an unwritten wire - // so that we can remove use-chains in SV dialect canonicalization. - if (ports[index].isInput()) { - WireOp wire = WireOp::create(builder, result.getType()); - - // Check that the input port is only written. Sometimes input ports are - // used as temporary wires. In that case, we cannot erase connections. - bool onlyWritten = llvm::all_of(result.getUsers(), [&](Operation *op) { - if (auto connect = dyn_cast(op)) - return connect.getDest() == result; - return false; - }); - - result.replaceUsesWithIf(wire.getResult(), [&](OpOperand &op) -> bool { - // Connects can be deleted directly. - if (onlyWritten && isa(op.getOwner())) { - op.getOwner()->erase(); - return false; - } - return true; - }); - - // If the wire doesn't have an user, just erase it. - if (wire.use_empty()) - wire.erase(); - - continue; - } - - // Output port. Replace with the output port with an invalid or constant - // value. - auto portConstant = outputPortConstants[outputPortIndex++]; - Value value; - if (portConstant) - value = ConstantOp::create(builder, *portConstant); - else { - // Use TieOffCache to create tie-off values for output ports. - if (auto baseType = - dyn_cast(result.getType())) { - value = tieOffCache.getInvalid(baseType); - } else { - // For non-base types like ref types, we cannot create an invalid - // value. Skip replacing uses for these types. - continue; - } - } - - result.replaceAllUsesWith(value); - } - - // Create a new instance op without unused ports. - instance.cloneWithErasedPortsAndReplaceUses(removalPortIndexes); - // Remove old one. - instance.erase(); - } - - numRemovedPorts += removalPortIndexes.count(); -} diff --git a/test/Dialect/FIRRTL/Reduction/port-pruner.mlir b/test/Dialect/FIRRTL/Reduction/imdce-remove-ports.mlir similarity index 75% rename from test/Dialect/FIRRTL/Reduction/port-pruner.mlir rename to test/Dialect/FIRRTL/Reduction/imdce-remove-ports.mlir index ca699d8201f8..f2e8317968e8 100644 --- a/test/Dialect/FIRRTL/Reduction/port-pruner.mlir +++ b/test/Dialect/FIRRTL/Reduction/imdce-remove-ports.mlir @@ -1,6 +1,6 @@ // UNSUPPORTED: system-windows // See https://github.com/llvm/circt/issues/4129 -// RUN: circt-reduce %s --test /usr/bin/env --test-arg grep --test-arg -q --test-arg "firrtl.module private @Bar" --keep-best=0 --include firrtl-remove-unused-ports | FileCheck %s +// RUN: circt-reduce %s --test /usr/bin/env --test-arg grep --test-arg -q --test-arg "firrtl.module private @Bar" --keep-best=0 --include firrtl-imdeadcodeelim-remove-ports | FileCheck %s firrtl.circuit "Foo" { // CHECK-LABEL: firrtl.module @Foo @@ -10,11 +10,11 @@ firrtl.circuit "Foo" { // CHECK-NOT: %bar_e // CHECK: %bar_b, %bar_d = firrtl.instance bar @Bar %bar_a, %bar_b, %bar_c, %bar_d, %bar_e = firrtl.instance bar @Bar (in a: !firrtl.uint<1>, in b: !firrtl.uint<1>, out c: !firrtl.uint<1>, out d: !firrtl.uint<1>, out e: !firrtl.uint<1>) - firrtl.connect %bar_a, %x : !firrtl.uint<1>, !firrtl.uint<1> - firrtl.connect %bar_b, %x : !firrtl.uint<1>, !firrtl.uint<1> + firrtl.matchingconnect %bar_a, %x : !firrtl.uint<1> + firrtl.matchingconnect %bar_b, %x : !firrtl.uint<1> %0 = firrtl.add %bar_c, %bar_d : (!firrtl.uint<1>, !firrtl.uint<1>) -> !firrtl.uint<2> %1 = firrtl.add %0, %bar_e : (!firrtl.uint<2>, !firrtl.uint<1>) -> !firrtl.uint<3> - firrtl.connect %y, %1 : !firrtl.uint<3>, !firrtl.uint<3> + firrtl.matchingconnect %y, %1 : !firrtl.uint<3> } // We're only ever using ports %b and %d -- the rest should be stripped. @@ -33,8 +33,8 @@ firrtl.circuit "Foo" { ) { %invalid_ui1 = firrtl.invalidvalue : !firrtl.uint<1> %0 = firrtl.not %b : (!firrtl.uint<1>) -> !firrtl.uint<1> - firrtl.connect %c, %invalid_ui1 : !firrtl.uint<1>, !firrtl.uint<1> - firrtl.connect %d, %0 : !firrtl.uint<1>, !firrtl.uint<1> - firrtl.connect %e, %invalid_ui1 : !firrtl.uint<1>, !firrtl.uint<1> + firrtl.matchingconnect %c, %invalid_ui1 : !firrtl.uint<1> + firrtl.matchingconnect %d, %0 : !firrtl.uint<1> + firrtl.matchingconnect %e, %invalid_ui1 : !firrtl.uint<1> } } diff --git a/test/Dialect/FIRRTL/Reduction/pattern-registration.mlir b/test/Dialect/FIRRTL/Reduction/pattern-registration.mlir index 46e2eb5fd5f1..483b21508b85 100644 --- a/test/Dialect/FIRRTL/Reduction/pattern-registration.mlir +++ b/test/Dialect/FIRRTL/Reduction/pattern-registration.mlir @@ -33,7 +33,7 @@ // CHECK-DAG: firrtl-operand0-forwarder // CHECK-DAG: firrtl-operand1-forwarder // CHECK-DAG: firrtl-operand2-forwarder -// CHECK-DAG: firrtl-remove-unused-ports +// CHECK-DAG: firrtl-imdeadcodeelim-remove-ports // CHECK-DAG: hw-constantifier // CHECK-DAG: hw-module-externalizer // CHECK-DAG: hw-module-internal-name-sanitizer diff --git a/test/Dialect/FIRRTL/remove-unused-ports.mlir b/test/Dialect/FIRRTL/imdce-remove-ports.mlir similarity index 77% rename from test/Dialect/FIRRTL/remove-unused-ports.mlir rename to test/Dialect/FIRRTL/imdce-remove-ports.mlir index 6e213f41762d..34dc9c8bca53 100644 --- a/test/Dialect/FIRRTL/remove-unused-ports.mlir +++ b/test/Dialect/FIRRTL/imdce-remove-ports.mlir @@ -1,4 +1,4 @@ -// RUN: circt-opt -pass-pipeline='builtin.module(firrtl.circuit(firrtl-remove-unused-ports))' %s -split-input-file | FileCheck %s +// RUN: circt-opt -pass-pipeline='builtin.module(firrtl-imdeadcodeelim{remove-ports-only})' %s -split-input-file | FileCheck %s firrtl.circuit "Top" { // CHECK-LABEL: firrtl.module @Top(in %a: !firrtl.uint<1>, in %b: !firrtl.uint<1>, out %c: !firrtl.uint<1>, // CHECK-SAME: out %d_unused: !firrtl.uint<1>, out %d_invalid: !firrtl.uint<1>, out %d_constant: !firrtl.uint<1>) @@ -6,31 +6,33 @@ firrtl.circuit "Top" { out %d_unused: !firrtl.uint<1>, out %d_invalid: !firrtl.uint<1>, out %d_constant: !firrtl.uint<1>) { %A_a, %A_b, %A_c, %A_d_unused, %A_d_invalid, %A_d_constant = firrtl.instance A @UseBar(in a: !firrtl.uint<1>, in b: !firrtl.uint<1>, out c: !firrtl.uint<1>, out d_unused: !firrtl.uint<1>, out d_invalid: !firrtl.uint<1>, out d_constant: !firrtl.uint<1>) // CHECK: %A_b, %A_c = firrtl.instance A @UseBar(in b: !firrtl.uint<1>, out c: !firrtl.uint<1>) - // CHECK-NEXT: firrtl.connect %A_b, %b - // CHECK-NEXT: firrtl.connect %c, %A_c - // CHECK-NEXT: firrtl.connect %d_unused, %{{invalid_ui1.*}} - // CHECK-NEXT: firrtl.connect %d_invalid, %{{invalid_ui1.*}} - // CHECK-NEXT: firrtl.connect %d_constant, %{{c1_ui1.*}} - firrtl.connect %A_a, %a : !firrtl.uint<1>, !firrtl.uint<1> - firrtl.connect %A_b, %b : !firrtl.uint<1>, !firrtl.uint<1> - firrtl.connect %c, %A_c : !firrtl.uint<1>, !firrtl.uint<1> - firrtl.connect %d_unused, %A_d_unused : !firrtl.uint<1>, !firrtl.uint<1> - firrtl.connect %d_invalid, %A_d_invalid : !firrtl.uint<1>, !firrtl.uint<1> - firrtl.connect %d_constant, %A_d_constant : !firrtl.uint<1>, !firrtl.uint<1> + // CHECK-NEXT: firrtl.matchingconnect %A_b, %b + // CHECK-NEXT: firrtl.matchingconnect %c, %A_c + // CHECK-NEXT: firrtl.matchingconnect %d_unused, %{{invalid_ui1.*}} + // CHECK-NEXT: firrtl.matchingconnect %d_invalid, %{{invalid_ui1.*}} + // CHECK-NEXT: firrtl.matchingconnect %d_constant, %{{c1_ui1.*}} + firrtl.matchingconnect %A_a, %a : !firrtl.uint<1> + firrtl.matchingconnect %A_b, %b : !firrtl.uint<1> + firrtl.matchingconnect %c, %A_c : !firrtl.uint<1> + firrtl.matchingconnect %d_unused, %A_d_unused : !firrtl.uint<1> + firrtl.matchingconnect %d_invalid, %A_d_invalid : !firrtl.uint<1> + firrtl.matchingconnect %d_constant, %A_d_constant : !firrtl.uint<1> } // Check that %a, %d_unused, %d_invalid and %d_constant are removed. // CHECK-LABEL: firrtl.module private @Bar(in %b: !firrtl.uint<1>, out %c: !firrtl.uint<1>) - // CHECK-NEXT: firrtl.connect %c, %b - // CHECK-NEXT: } + // CHECK-NEXT: firrtl.matchingconnect %c, %b + // CHECK: } firrtl.module private @Bar(in %a: !firrtl.uint<1>, in %b: !firrtl.uint<1>, out %c: !firrtl.uint<1>, out %d_unused: !firrtl.uint<1>, out %d_invalid: !firrtl.uint<1>, out %d_constant: !firrtl.uint<1>) { - firrtl.connect %c, %b : !firrtl.uint<1>, !firrtl.uint<1> + firrtl.matchingconnect %c, %b : !firrtl.uint<1> %invalid_ui1 = firrtl.invalidvalue : !firrtl.uint<1> - firrtl.connect %d_invalid, %invalid_ui1 : !firrtl.uint<1>, !firrtl.uint<1> + firrtl.matchingconnect %d_unused, %invalid_ui1 : !firrtl.uint<1> + %invalid_ui1_2 = firrtl.invalidvalue : !firrtl.uint<1> + firrtl.matchingconnect %d_invalid, %invalid_ui1_2 : !firrtl.uint<1> %c1_i1 = firrtl.constant 1 : !firrtl.uint<1> - firrtl.connect %d_constant, %c1_i1 : !firrtl.uint<1>, !firrtl.uint<1> + firrtl.matchingconnect %d_constant, %c1_i1 : !firrtl.uint<1> } // Check that %a, %d_unused, %d_invalid and %d_constant are removed. @@ -38,13 +40,13 @@ firrtl.circuit "Top" { firrtl.module private @UseBar(in %a: !firrtl.uint<1>, in %b: !firrtl.uint<1>, out %c: !firrtl.uint<1>, out %d_unused: !firrtl.uint<1>, out %d_invalid: !firrtl.uint<1>, out %d_constant: !firrtl.uint<1>) { %A_a, %A_b, %A_c, %A_d_unused, %A_d_invalid, %A_d_constant = firrtl.instance A @Bar(in a: !firrtl.uint<1>, in b: !firrtl.uint<1>, out c: !firrtl.uint<1>, out d_unused: !firrtl.uint<1>, out d_invalid: !firrtl.uint<1>, out d_constant: !firrtl.uint<1>) - firrtl.connect %A_a, %a : !firrtl.uint<1>, !firrtl.uint<1> + firrtl.matchingconnect %A_a, %a : !firrtl.uint<1> // CHECK: %A_b, %A_c = firrtl.instance A @Bar(in b: !firrtl.uint<1>, out c: !firrtl.uint<1>) - firrtl.connect %A_b, %b : !firrtl.uint<1>, !firrtl.uint<1> - firrtl.connect %c, %A_c : !firrtl.uint<1>, !firrtl.uint<1> - firrtl.connect %d_unused, %A_d_unused : !firrtl.uint<1>, !firrtl.uint<1> - firrtl.connect %d_invalid, %A_d_invalid : !firrtl.uint<1>, !firrtl.uint<1> - firrtl.connect %d_constant, %A_d_constant : !firrtl.uint<1>, !firrtl.uint<1> + firrtl.matchingconnect %A_b, %b : !firrtl.uint<1> + firrtl.matchingconnect %c, %A_c : !firrtl.uint<1> + firrtl.matchingconnect %d_unused, %A_d_unused : !firrtl.uint<1> + firrtl.matchingconnect %d_invalid, %A_d_invalid : !firrtl.uint<1> + firrtl.matchingconnect %d_constant, %A_d_constant : !firrtl.uint<1> } // Make sure that %a, %b and %c are not erased because they have an annotation or a symbol. @@ -52,18 +54,18 @@ firrtl.circuit "Top" { firrtl.module private @Foo(in %a: !firrtl.uint<1> sym @dntSym, in %b: !firrtl.uint<1>, out %c: !firrtl.uint<1> sym @dntSym2) attributes { portAnnotations = [[], [{a = "a"}], []]} { - // CHECK: firrtl.connect %c, %{{invalid_ui1.*}} + // CHECK: firrtl.matchingconnect %c, %{{invalid_ui1.*}} %invalid_ui1 = firrtl.invalidvalue : !firrtl.uint<1> - firrtl.connect %c, %invalid_ui1 : !firrtl.uint<1>, !firrtl.uint<1> + firrtl.matchingconnect %c, %invalid_ui1 : !firrtl.uint<1> } - // CHECK-LABEL: firrtl.module private @UseFoo(in %a: !firrtl.uint<1>, in %b: !firrtl.uint<1>, out %c: !firrtl.uint<1>) + // CHECK-LABEL: firrtl.module private @UseFoo(in %a: !firrtl.uint<1>, in %b: !firrtl.uint<1>) firrtl.module private @UseFoo(in %a: !firrtl.uint<1>, in %b: !firrtl.uint<1>, out %c: !firrtl.uint<1>) { %A_a, %A_b, %A_c = firrtl.instance A @Foo(in a: !firrtl.uint<1>, in b: !firrtl.uint<1>, out c: !firrtl.uint<1>) // CHECK: %A_a, %A_b, %A_c = firrtl.instance A @Foo(in a: !firrtl.uint<1>, in b: !firrtl.uint<1>, out c: !firrtl.uint<1>) - firrtl.connect %A_a, %a : !firrtl.uint<1>, !firrtl.uint<1> - firrtl.connect %A_b, %b : !firrtl.uint<1>, !firrtl.uint<1> - firrtl.connect %c, %A_c : !firrtl.uint<1>, !firrtl.uint<1> + firrtl.matchingconnect %A_a, %a : !firrtl.uint<1> + firrtl.matchingconnect %A_b, %b : !firrtl.uint<1> + firrtl.matchingconnect %c, %A_c : !firrtl.uint<1> } } @@ -93,13 +95,15 @@ firrtl.circuit "Top" { // Check that %a, %d_unused, %d_invalid and %d_constant are removed. // CHECK-LABEL: firrtl.module private @Bar(in %b: !firrtl.uint<1>, out %c: !firrtl.uint<1>) // CHECK-NEXT: firrtl.matchingconnect %c, %b - // CHECK-NEXT: } + // CHECK: } firrtl.module private @Bar(in %a: !firrtl.uint<1>, in %b: !firrtl.uint<1>, out %c: !firrtl.uint<1>, out %d_unused: !firrtl.uint<1>, out %d_invalid: !firrtl.uint<1>, out %d_constant: !firrtl.uint<1>) { firrtl.matchingconnect %c, %b : !firrtl.uint<1> %invalid_ui1 = firrtl.invalidvalue : !firrtl.uint<1> - firrtl.matchingconnect %d_invalid, %invalid_ui1 : !firrtl.uint<1> + firrtl.matchingconnect %d_unused, %invalid_ui1 : !firrtl.uint<1> + %invalid_ui1_2 = firrtl.invalidvalue : !firrtl.uint<1> + firrtl.matchingconnect %d_invalid, %invalid_ui1_2 : !firrtl.uint<1> %c1_i1 = firrtl.constant 1 : !firrtl.uint<1> firrtl.matchingconnect %d_constant, %c1_i1 : !firrtl.uint<1> } @@ -128,7 +132,7 @@ firrtl.circuit "Top" { firrtl.matchingconnect %c, %invalid_ui1 : !firrtl.uint<1> } - // CHECK-LABEL: firrtl.module private @UseFoo(in %a: !firrtl.uint<1>, in %b: !firrtl.uint<1>, out %c: !firrtl.uint<1>) + // CHECK-LABEL: firrtl.module private @UseFoo(in %a: !firrtl.uint<1>, in %b: !firrtl.uint<1>) firrtl.module private @UseFoo(in %a: !firrtl.uint<1>, in %b: !firrtl.uint<1>, out %c: !firrtl.uint<1>) { %A_a, %A_b, %A_c = firrtl.instance A @Foo(in a: !firrtl.uint<1>, in b: !firrtl.uint<1>, out c: !firrtl.uint<1>) // CHECK: %A_a, %A_b, %A_c = firrtl.instance A @Foo(in a: !firrtl.uint<1>, in b: !firrtl.uint<1>, out c: !firrtl.uint<1>) @@ -140,9 +144,9 @@ firrtl.circuit "Top" { // ----- -// Ensure that the "output_file" attribute isn't destroyed by RemoveUnusedPorts. +// Ensure that the "output_file" attribute isn't destroyed by IMDCE port-only mode. // This matters for interactions between Grand Central (which sets these) and -// RemoveUnusedPorts which may clone modules with stripped ports. +// IMDCE port-only mode which may clone modules with stripped ports. // // CHECK-LABEL: "PreserveOutputFile" firrtl.circuit "PreserveOutputFile" { @@ -198,3 +202,21 @@ firrtl.circuit "ProbeTypes" { firrtl.module private @Bar(out %ref: !firrtl.probe) { } } + +// ----- + +// Test case for issue #10504. Property types with instances should not crash +// This tests that unused property ports can be removed without assertion failures +firrtl.circuit "PropertyTypes" { + // CHECK-LABEL: firrtl.module @PropertyTypes + firrtl.module @PropertyTypes() { + // CHECK: firrtl.instance Bar @Bar() + %Bar_a = firrtl.instance Bar @Bar(out a: !firrtl.list) + } + + // The unused property output port should be removed without crashing. + // CHECK-LABEL: firrtl.module private @Bar() { + // CHECK-NEXT: } + firrtl.module private @Bar(out %a: !firrtl.list) { + } +}