From 89363587128fbb9d9ff6d15c59924bc05842a006 Mon Sep 17 00:00:00 2001 From: Andrew Young Date: Wed, 28 Jun 2023 13:24:29 -0700 Subject: [PATCH] [LowerFIRRTLToHW] Use backedges over temporary wires, NFC This changes LowerFIRRTLToHW to use backedges instead of temporary wire operations. The temporary wires were guaranteed to be optimized away. Now we slightly more gracefully handle the, technically illegal, cases of these wires having no value driven to them. This change is NFC in the FIRRTL pipeline where these invariants have already been verified. We can remove the infrastructure for creating these temporary wires, as there are now more uses. --- lib/Conversion/FIRRTLToHW/LowerToHW.cpp | 59 +++++++++---------------- 1 file changed, 22 insertions(+), 37 deletions(-) diff --git a/lib/Conversion/FIRRTLToHW/LowerToHW.cpp b/lib/Conversion/FIRRTLToHW/LowerToHW.cpp index 29139b4e5c4..976f9c68033 100644 --- a/lib/Conversion/FIRRTLToHW/LowerToHW.cpp +++ b/lib/Conversion/FIRRTLToHW/LowerToHW.cpp @@ -1518,6 +1518,7 @@ struct FIRRTLLowering : public FIRRTLVisitor { LogicalResult setLoweringTo(Operation *orig, CtorArgTypes... args); template LogicalResult setLoweringToLTL(Operation *orig, CtorArgTypes... args); + Backedge createBackedge(Location loc, Type type); Backedge createBackedge(Value orig, Type type); bool updateIfBackedge(Value dest, Value src); @@ -1557,14 +1558,6 @@ struct FIRRTLLowering : public FIRRTLVisitor { return result; } - // Create a temporary wire to act as a destination for connect operations. - // These wires are removed after all connects have been lowered. - hw::WireOp createTmpHWWireOp(Type type, StringAttrOrRef name = {}) { - auto wire = builder.create(getOrCreateZConstant(type), name); - tmpWiresToRemove.push_back(wire); - return wire; - } - using FIRRTLVisitor::visitExpr; using FIRRTLVisitor::visitDecl; using FIRRTLVisitor::visitStmt; @@ -1795,11 +1788,6 @@ struct FIRRTLLowering : public FIRRTLVisitor { /// lowering. SmallVector tmpWiresToOptimize; - /// A list of wires created as a temporary destination to for connect-like - /// operations to hook up to. These wires are deleted again and replaced with - /// their connected value after all operations have been lowered. - SmallVector tmpWiresToRemove; - /// A namespace that can be used to generte new symbol names that are unique /// within this module. hw::ModuleNamespace moduleNamespace; @@ -1935,15 +1923,6 @@ LogicalResult FIRRTLLowering::run() { for (auto wire : tmpWiresToOptimize) optimizeTemporaryWire(wire); - // Remove all temporary wires created solely for the purpose of facilitating - // the lowering of connect-like operations. - for (auto wire : tmpWiresToRemove) { - if (wire.getInnerSymAttr()) - return wire.emitError("temporary wire was given an inner symbol"); - wire.replaceAllUsesWith(wire.getInput()); - wire.erase(); - } - // Determine the actual types of lowered LTL operations and remove any // intermediate wires among them. if (failed(fixupLTLOps())) @@ -2434,13 +2413,24 @@ LogicalResult FIRRTLLowering::setLoweringToLTL(Operation *orig, return setPossiblyFoldedLowering(orig->getResult(0), result); } +/// Creates a backedge of the specified result type. A backedge represents a +/// placeholder to be filled in later by a lowered value. If the backedge is not +/// updated with a real value by the end of the pass, it will be replaced with +/// an undriven wire. Backedges are allowed to be updated to other backedges. +/// If a chain of backedges forms a combinational loop, they will be replaced +/// with an undriven wire. +Backedge FIRRTLLowering::createBackedge(Location loc, Type type) { + auto backedge = backedgeBuilder.get(type, loc); + backedges.insert({backedge, backedge}); + return backedge; +} + /// Sets the lowering for a value to a backedge of the specified result type. /// This is useful for lowering types which cannot pass through a wire, or to /// directly materialize values in operations that violate the SSA dominance /// constraint. Backedge FIRRTLLowering::createBackedge(Value orig, Type type) { - auto backedge = backedgeBuilder.get(type, orig.getLoc()); - backedges.insert({backedge, backedge}); + auto backedge = createBackedge(orig.getLoc(), type); (void)setLowering(orig, backedge); return backedge; } @@ -3122,23 +3112,19 @@ LogicalResult FIRRTLLowering::visitDecl(MemOp op) { if (memportKind != op.getPortKind(i)) continue; - auto portName = op.getPortName(i).getValue(); - auto addInput = [&](StringRef portLabel, StringRef portLabel2, StringRef field, size_t width, StringRef field2 = "") { auto portType = IntegerType::get(op.getContext(), std::max((size_t)1, width)); - auto accesses = getAllFieldAccesses(op.getResult(i), field); - - Value wire = - createTmpHWWireOp(portType, "." + portName + "." + field + ".wire"); + Value backedge = createBackedge(builder.getLoc(), portType); + auto accesses = getAllFieldAccesses(op.getResult(i), field); for (auto a : accesses) { if (cast(a.getType()) .getPassiveType() .getBitWidthOrSentinel() > 0) - (void)setLowering(a, wire); + (void)setLowering(a, backedge); else a->eraseOperand(0); } @@ -3146,22 +3132,21 @@ LogicalResult FIRRTLLowering::visitDecl(MemOp op) { if (!field2.empty()) { // This handles the case, when the single bit mask field is removed, // and the enable is updated after 'And' with mask bit. - Value wire2 = createTmpHWWireOp(portType, "." + portName + "." + - field2 + ".wire"); + auto backedge2 = createBackedge(builder.getLoc(), portType); auto accesses2 = getAllFieldAccesses(op.getResult(i), field2); - for (auto a : accesses2) { if (cast(a.getType()) .getPassiveType() .getBitWidthOrSentinel() > 0) - (void)setLowering(a, wire2); + (void)setLowering(a, backedge2); else a->eraseOperand(0); } - wire = builder.createOrFold(wire, wire2, true); + backedge = + builder.createOrFold(backedge, backedge2, true); } - operands.push_back(wire); + operands.push_back(backedge); argNames.push_back( builder.getStringAttr(portLabel + Twine(portNumber) + portLabel2)); };