[Buffering] Adapt CFDFC extraction for FTD circuits

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

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<Value> &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

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<handshake::ForkOp, handshake::ExtUIOp, handshake::ExtSIOp,
              handshake::TruncIOp>(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<handshake::ForkOp, handshake::ExtUIOp, handshake::ExtSIOp,
+             handshake::TruncIOp, handshake::NotIOp>(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<handshake::BranchOp, handshake::ConditionalBranchOp>(op))
+    if (isa<handshake::BranchOp, handshake::ConditionalBranchOp,
+            handshake::CmpIOp, handshake::CmpFOp>(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<handshake::MergeLikeOpInterface>(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<Operation *> 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<unsigned> brBB = getLogicBB(brOp);
+  std::optional<unsigned> srcBB = getLogicBB(srcOp);
   std::optional<unsigned> 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) {

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 <filesystem>
+#include <functional>
 #include <string>
 
 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<CFDFC> cfdfcs;
@@ -288,16 +286,6 @@ LogicalResult HandshakePlaceBuffersPass::placeUsingMILP() {
 }
 
 LogicalResult HandshakePlaceBuffersPass::checkFuncInvariants(FuncInfo &info) {
-  // Store all archs in a map for fast query time
-  DenseMap<unsigned, llvm::SmallDenseSet<unsigned, 2>> 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<Operation *, std::optional<unsigned>> 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<unsigned> srcBB = opBlocks[&op];
-    for (OpResult res : op.getResults()) {
-      Operation *user = *res.getUsers().begin();
-      std::optional<unsigned> 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<ControlType>(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<handshake::BranchOp, handshake::ConditionalBranchOp,
+            handshake::CmpIOp, handshake::CmpFOp>(op))
+      return true;
+    if (isa<handshake::ForkOp, handshake::ExtUIOp, handshake::ExtSIOp,
+            handshake::TruncIOp>(op))
+      op = op->getOperand(0).getDefiningOp();
+    else
+      return false;
+  } while (true);
+}
+
+static bool isBackedgeDestinationLike(Operation *op) {
+  if (isa<handshake::InitOp, handshake::MergeLikeOpInterface>(op))
+    return true;
+
+  auto notOp = dyn_cast<handshake::NotIOp>(op);
+  if (!notOp)
+    return false;
+
+  return llvm::any_of(notOp.getResult().getUsers(), [](Operation *user) {
+    return isa<handshake::MergeLikeOpInterface>(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<Value>
+findBackwardChannelPerCyclicRegion(handshake::FuncOp funcOp) {
+  SmallVector<Operation *> ops;
+  SmallVector<CircuitEdge> edges;
+  llvm::DenseMap<Operation *, SmallVector<Operation *, 4>> 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<handshake::MemoryControllerOp, handshake::LSQOp>(dst))
+          continue;
+
+        edges.push_back({src, dst, result});
+        succs[src].push_back(dst);
+      }
+    }
+  }
+
+  llvm::DenseMap<Operation *, unsigned> index, lowlink;
+  llvm::DenseSet<Operation *> onStack;
+  SmallVector<Operation *> stack;
+  SmallVector<SmallVector<Operation *>> sccs;
+  unsigned nextIndex = 0;
+
+  std::function<void(Operation *)> 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<Operation *> 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<Value> backwardChannels;
+  for (const auto &scc : sccs) {
+    llvm::DenseSet<Operation *> 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<CFDFC> &cfdfcs) {
   SmallVector<ArchBB> archsCopy(info.archs);
@@ -400,6 +498,9 @@ LogicalResult HandshakePlaceBuffersPass::getCFDFCs(FuncInfo &info,
     bbs.insert(arch.dstBB);
   }
 
+  mlir::DenseSet<Value> 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();