diff --git a/include/dynamatic/Support/CFG.h b/include/dynamatic/Support/CFG.h index 0ef76046c..c275f3186 100644 --- a/include/dynamatic/Support/CFG.h +++ b/include/dynamatic/Support/CFG.h @@ -98,12 +98,12 @@ bool getBBEndpoints(Value val, Operation *user, BBEndpoints &endpoints); bool getBBEndpoints(Value val, BBEndpoints &endpoints); /// Determines whether the value is a backedge i.e., whether the channel -/// corresponding to the value is located between a branch-like operation and a -/// merge-like operation, where the merge-like operation happens semantically -/// "before" the branch-like operation. This function can only correctly -/// identify backedges if the circuit's branches and merges are associated to -/// basic blocks (otherwise it will always return false). `user` must be one of -/// `val`'s users. +/// corresponding to the value is located between a loop-feedback source +/// (branch-like or compare-like within a block) and a merge-like operation, +/// where the merge-like operation happens semantically "before" that source. +/// This function can only correctly identify backedges if the circuit's +/// branches, compares, and merges are associated to basic blocks (otherwise it +/// will always return false). `user` must be one of `val`'s users. bool isBackedge(Value val, Operation *user, BBEndpoints *endpoints = nullptr); /// Determines whether the value is a backedge. The value must have a single diff --git a/include/dynamatic/Transforms/BufferPlacement/Utils/CFDFC.h b/include/dynamatic/Transforms/BufferPlacement/Utils/CFDFC.h index 186c3aaab..0dfe7a6fb 100644 --- a/include/dynamatic/Transforms/BufferPlacement/Utils/CFDFC.h +++ b/include/dynamatic/Transforms/BufferPlacement/Utils/CFDFC.h @@ -59,7 +59,12 @@ struct CFDFC { /// Constructs a CFDFC from a set of selected archs and basic blocks in the /// function. Assumes that every value in the function is used exactly once. - CFDFC(handshake::FuncOp funcOp, ArchSet &archs, unsigned numExec); + CFDFC(handshake::FuncOp funcOp, ArchSet &archs, unsigned numExec, + const DenseSet &backwardChannels); + + /// Returns whether the channel between the two blocks has a corresponding + /// edge in this CFDFC's CFG cycle. + bool isCFGCompliant(unsigned srcBB, unsigned dstBB) const; // Determines whether the channel is a "CFDFC backedge" i.e., the first // channel along a sequence of backedges from a source block to a destination diff --git a/lib/Support/CFG.cpp b/lib/Support/CFG.cpp index 09bbd1c81..1b7f5e91f 100644 --- a/lib/Support/CFG.cpp +++ b/lib/Support/CFG.cpp @@ -150,23 +150,31 @@ static bool followToBlock(Operation *op, unsigned &bb, } /// Determines whether the operation is of a nature which can be traversed -/// outside blocks during backedge identification. -static inline bool canGoThroughOutsideBlocks(Operation *op) { +/// backwards during backedge source identification. +static inline bool canBacktrackToLoopSourceThrough(Operation *op) { return isa(op); } -/// Attempts to backtrack through forks and bitwidth modification operations -/// till reaching a branch-like operation. On success, returns the branch-like -/// operation that was backtracked to (or the passed operation if it was itself -/// branch-like); otherwise, returns nullptr. -static Operation *backtrackToBranch(Operation *op) { +/// Determines whether the operation is of a nature which can be traversed +/// forwards during backedge destination identification. +static inline bool canFollowToMergeThrough(Operation *op) { + return isa(op); +} + +/// Attempts to backtrack through source-transparent operations till reaching +/// an operation that can act as the source of loop feedback within a block. On +/// success, returns that operation (or the passed operation if it was itself +/// such a source); otherwise, returns nullptr. +static Operation *backtrackToLoopSource(Operation *op) { do { if (!op) break; - if (isa(op)) + if (isa(op)) return op; - if (canGoThroughOutsideBlocks(op)) + if (canBacktrackToLoopSourceThrough(op)) op = op->getOperand(0).getDefiningOp(); else break; @@ -175,14 +183,14 @@ static Operation *backtrackToBranch(Operation *op) { } /// Attempts to follow the def-use chains of all the operation's results through -/// forks and bitwidth modification operations till reaching merge-like -/// operations that all belong to the same basic block. On success, returns one -/// of the merge-like operations reached by a def-use chain (or the passed -/// operation if it was itself merge-like); otherwise, returns nullptr. +/// destination-transparent operations till reaching merge-like operations that +/// all belong to the same basic block. On success, returns one of the +/// merge-like operations reached by a def-use chain (or the passed operation if +/// it was itself merge-like); otherwise, returns nullptr. static Operation *followToMerge(Operation *op) { if (isa(op)) return op; - if (canGoThroughOutsideBlocks(op)) { + if (canFollowToMergeThrough(op)) { // All users of the operation's results must lead to merges within a unique // block SmallVector mergeOps; @@ -245,20 +253,20 @@ bool dynamatic::isBackedge(Value val, Operation *user, BBEndpoints *endpoints) { return false; // If both source and destination blocks are identical, the edge must be - // located between a branch-like operation and a merge-like operation - Operation *brOp = backtrackToBranch(val.getDefiningOp()); - if (!brOp) + // located between a loop-feedback source and a merge-like operation. + Operation *srcOp = backtrackToLoopSource(val.getDefiningOp()); + if (!srcOp) return false; Operation *mergeOp = followToMerge(user); if (!mergeOp) return false; - // Check that the branch and merge are part of the same block indicated by the + // Check that the source and merge are part of the same block indicated by the // edge's BB endpoints (should be the case in all non-degenerate cases) - std::optional brBB = getLogicBB(brOp); + std::optional srcBB = getLogicBB(srcOp); std::optional mergeBB = getLogicBB(mergeOp); - return brBB.has_value() && mergeBB.has_value() && *brBB == *mergeBB && - *brBB == bbs.srcBB; + return srcBB.has_value() && mergeBB.has_value() && *srcBB == *mergeBB && + *srcBB == bbs.srcBB; } bool dynamatic::isBackedge(Value val, BBEndpoints *endpoints) { diff --git a/lib/Transforms/BufferPlacement/HandshakePlaceBuffers.cpp b/lib/Transforms/BufferPlacement/HandshakePlaceBuffers.cpp index e8230b7cf..1c9683938 100644 --- a/lib/Transforms/BufferPlacement/HandshakePlaceBuffers.cpp +++ b/lib/Transforms/BufferPlacement/HandshakePlaceBuffers.cpp @@ -17,7 +17,6 @@ #include "dynamatic/Dialect/Handshake/HandshakeAttributes.h" #include "dynamatic/Dialect/Handshake/HandshakeEnums.h" #include "dynamatic/Dialect/Handshake/HandshakeOps.h" -#include "dynamatic/Dialect/Handshake/HandshakeTypes.h" #include "dynamatic/Support/Attribute.h" #include "dynamatic/Support/CFG.h" #include "dynamatic/Transforms/BufferPlacement/CostAwareBuffers.h" @@ -32,10 +31,13 @@ #include "mlir/IR/BuiltinOps.h" #include "mlir/IR/OperationSupport.h" #include "mlir/Support/IndentedOstream.h" +#include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/DenseSet.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Casting.h" #include "llvm/Support/Path.h" #include +#include #include using namespace mlir; @@ -228,10 +230,6 @@ LogicalResult HandshakePlaceBuffersPass::placeUsingMILP() { << "Failed to read profiling information from CSV"; } - // Check IR invariants and parse basic block archs from disk - if (failed(checkFuncInvariants(info))) - return failure(); - // Get CFDFCs from the function unless the functions has no archs (i.e., // it has a single block) in which case there are no CFDFCs std::vector cfdfcs; @@ -288,16 +286,6 @@ LogicalResult HandshakePlaceBuffersPass::placeUsingMILP() { } LogicalResult HandshakePlaceBuffersPass::checkFuncInvariants(FuncInfo &info) { - // Store all archs in a map for fast query time - DenseMap> transitions; - for (ArchBB &arch : info.archs) - transitions[arch.srcBB].insert(arch.dstBB); - - // Store the BB to which each block belongs for quick access later - DenseMap> opBlocks; - for (Operation &op : info.funcOp.getOps()) - opBlocks[&op] = getLogicBB(&op); - for (Operation &op : info.funcOp.getOps()) { // Most operations should belong to a basic block for buffer placement to // work correctly. Don't outright fail in case one operation is outside of @@ -314,32 +302,6 @@ LogicalResult HandshakePlaceBuffersPass::checkFuncInvariants(FuncInfo &info) { "behavior may be suboptimal or incorrect."; } } - - std::optional srcBB = opBlocks[&op]; - for (OpResult res : op.getResults()) { - Operation *user = *res.getUsers().begin(); - std::optional dstBB = opBlocks[user]; - - // All transitions between blocks must exist in the original CFG - if (srcBB && dstBB && *srcBB != *dstBB && - !transitions[*srcBB].contains(*dstBB)) { - auto endBB = *opBlocks.at(info.funcOp.getBodyBlock()->getTerminator()); - if (isa(res.getType()) && srcBB == ENTRY_BB && - dstBB == endBB) { - /// NOTE: (lucas-rami) This is probably the start->end control channel - /// which goes from the entry block to the exit block. This is fine in - /// general so we let this pass without triggering a warning or error - continue; - } - - return op.emitError() - << "Result " << res.getResultNumber() << " defined in block " - << *srcBB << " is used in block " << *dstBB - << ". This connection does not exist according to the CFG " - "graph. Solving the buffer placement MILP would yield an " - "incorrect placement."; - } - } } return success(); } @@ -382,6 +344,142 @@ static void logFuncInfo(FuncInfo &info) { os.flush(); } +namespace { +struct CircuitEdge { + Operation *src; + Operation *dst; + Value channel; +}; + +static bool isBackedgeSourceLike(Operation *op) { + do { + if (!op) + return false; + if (isa(op)) + return true; + if (isa(op)) + op = op->getOperand(0).getDefiningOp(); + else + return false; + } while (true); +} + +static bool isBackedgeDestinationLike(Operation *op) { + if (isa(op)) + return true; + + auto notOp = dyn_cast(op); + if (!notOp) + return false; + + return llvm::any_of(notOp.getResult().getUsers(), [](Operation *user) { + return isa(user); + }); +} + +/// Finds all loop-feedback-source -> merge-like backward channels per cyclic +/// SCC in the handshake graph. Grouping by SCC remains more stable than trying +/// to assign channels to every simple cycle when cycles overlap. +static mlir::DenseSet +findBackwardChannelPerCyclicRegion(handshake::FuncOp funcOp) { + SmallVector ops; + SmallVector edges; + llvm::DenseMap> succs; + + for (Operation &op : funcOp.getOps()) { + ops.push_back(&op); + succs[&op] = {}; + } + + for (Operation *src : ops) { + for (Value result : src->getResults()) { + for (OpOperand &use : result.getUses()) { + Operation *dst = use.getOwner(); + + if (isa(dst)) + continue; + + edges.push_back({src, dst, result}); + succs[src].push_back(dst); + } + } + } + + llvm::DenseMap index, lowlink; + llvm::DenseSet onStack; + SmallVector stack; + SmallVector> sccs; + unsigned nextIndex = 0; + + std::function strongConnect = [&](Operation *op) { + index[op] = nextIndex; + lowlink[op] = nextIndex; + ++nextIndex; + stack.push_back(op); + onStack.insert(op); + + for (Operation *succ : succs[op]) { + if (!index.contains(succ)) { + strongConnect(succ); + lowlink[op] = std::min(lowlink[op], lowlink[succ]); + } else if (onStack.contains(succ)) { + lowlink[op] = std::min(lowlink[op], index[succ]); + } + } + + if (lowlink[op] != index[op]) + return; + + SmallVector scc; + while (true) { + Operation *top = stack.pop_back_val(); + onStack.erase(top); + scc.push_back(top); + if (top == op) + break; + } + sccs.push_back(std::move(scc)); + }; + + for (Operation *op : ops) { + if (!index.contains(op)) + strongConnect(op); + } + + mlir::DenseSet backwardChannels; + for (const auto &scc : sccs) { + llvm::DenseSet sccNodes(scc.begin(), scc.end()); + + bool isCyclic = scc.size() > 1; + if (!isCyclic) { + Operation *only = scc.front(); + isCyclic = llvm::any_of(edges, [&](const CircuitEdge &edge) { + return edge.src == only && edge.dst == only; + }); + } + if (!isCyclic) + continue; + + for (const CircuitEdge &edge : edges) { + if (!sccNodes.contains(edge.src) || !sccNodes.contains(edge.dst)) + continue; + if (!isBackedge(edge.channel)) + continue; + if (!isBackedgeSourceLike(edge.src)) + continue; + if (!isBackedgeDestinationLike(edge.dst)) + continue; + backwardChannels.insert(edge.channel); + } + } + + return backwardChannels; +} + +} // namespace + LogicalResult HandshakePlaceBuffersPass::getCFDFCs(FuncInfo &info, std::vector &cfdfcs) { SmallVector archsCopy(info.archs); @@ -400,6 +498,9 @@ LogicalResult HandshakePlaceBuffersPass::getCFDFCs(FuncInfo &info, bbs.insert(arch.dstBB); } + mlir::DenseSet backwardChannels = + findBackwardChannelPerCyclicRegion(info.funcOp); + // Set of selected archs ArchSet selectedArchs; // Number of executions @@ -424,7 +525,7 @@ LogicalResult HandshakePlaceBuffersPass::getCFDFCs(FuncInfo &info, break; // Create the CFDFC from the set of selected archs and BBs - cfdfcs.emplace_back(info.funcOp, selectedArchs, numExecs); + cfdfcs.emplace_back(info.funcOp, selectedArchs, numExecs, backwardChannels); } while (!firstCFDFC); return success(); diff --git a/lib/Transforms/BufferPlacement/Utils/CFDFC.cpp b/lib/Transforms/BufferPlacement/Utils/CFDFC.cpp index 2ff251404..d1031457b 100644 --- a/lib/Transforms/BufferPlacement/Utils/CFDFC.cpp +++ b/lib/Transforms/BufferPlacement/Utils/CFDFC.cpp @@ -155,7 +155,8 @@ static void setBBConstraints(std::unique_ptr &model, MILPVars &vars) { } } -CFDFC::CFDFC(handshake::FuncOp funcOp, ArchSet &archs, unsigned numExec) +CFDFC::CFDFC(handshake::FuncOp funcOp, ArchSet &archs, unsigned numExec, + const DenseSet &backwardChannels) : numExecs(numExec) { // Identify the block that starts the CFDFC; it's the only one that is both @@ -214,33 +215,32 @@ CFDFC::CFDFC(handshake::FuncOp funcOp, ArchSet &archs, unsigned numExec) else dstBB = *optBB; - if (srcBB != dstBB) { - // The channel is in the CFDFC if it belongs belong to a selected arch - // between two basic blocks - for (size_t i = 0; i < cycle.size(); ++i) { - unsigned nextBB = i == cycle.size() - 1 ? 0 : i + 1; - if (srcBB == cycle[i] && dstBB == cycle[nextBB]) { - channels.insert(res); - if (isCFDFCBackedge(res)) - backedges.insert(res); - break; - } - } - } else if (cycle.size() == 1) { - // The channel is in the CFDFC if its producer/consumer belong to the - // same basic block and the CFDFC is just a block looping to itself + if (!cycle.contains(dstBB)) + continue; + + // FTD circuits may contain channels between blocks without a matching CFG + // edge. Include every non-backward channel whose endpoints belong to the + // cycle. Include backward channels only when they correspond to an edge + // in the CFG cycle currently being modeled. + if (!backwardChannels.contains(res)) channels.insert(res); - if (isCFDFCBackedge(res)) - backedges.insert(res); - } else if (!isBackedge(res)) { - // The channel is in the CFDFC if its producer/consumer belong to the - // same basic block and the channel is not a backedge + else if (isCFGCompliant(srcBB, dstBB)) { channels.insert(res); + backedges.insert(res); } } } } +bool CFDFC::isCFGCompliant(unsigned srcBB, unsigned dstBB) const { + for (size_t i = 0; i < cycle.size(); ++i) { + unsigned nextBB = i == cycle.size() - 1 ? 0 : i + 1; + if (srcBB == cycle[i] && dstBB == cycle[nextBB]) + return true; + } + return false; +} + bool CFDFC::isCFDFCBackedge(Value val) { // A CFDFC backedge is a backedge if (!isBackedge(val))