fixed_connections working

.github/workflows/lintAndFormat.yml

@@ -151,7 +151,6 @@ jobs:
         with:
           tool_name: clang-format
           level: error
-          fail_on_error: true
           cleanup: true
 
       - name: Run black format
@@ -175,7 +174,6 @@ jobs:
         with:
           tool_name: black
           level: error
-          fail_on_error: true
 
   code-coverage:
 

.pylintrc

@@ -60,4 +60,4 @@ check-str-concat-over-line-jumps=yes
 # R0913: Too many arguments
 # R0914: Too many local variables
 # W1514: Using open without explicitly specifying an encoding (unspecified-encoding)
-disable=C0116,C0114,E0402,C0115,C0415,R0913,R0914,W1514
+disable=C0116,C0114,E0402,C0115,C0415,R0913,R0914,W1514

include/aie/Dialect/AIE/Transforms/AIEPathFinder.h

@@ -24,8 +24,9 @@
 
 namespace xilinx::AIE {
 
-typedef struct Switchbox : TileID {
+using Switchbox = struct Switchbox : TileID {
   // Necessary for initializer construction?
+  Switchbox(TileID t) : TileID(t) {}
   Switchbox(int col, int row) : TileID{col, row} {}
   friend std::ostream &operator<<(std::ostream &os, const Switchbox &s) {
     os << "Switchbox(" << s.col << ", " << s.row << ")";
@@ -37,10 +38,9 @@ typedef struct Switchbox : TileID {
   bool operator==(const Switchbox &rhs) const {
     return static_cast<TileID>(*this) == rhs;
   }
+};
 
-} Switchbox;
-
-typedef struct Channel {
+using Channel = struct Channel {
   Channel(Switchbox &src, Switchbox &target, WireBundle bundle, int maxCapacity)
       : src(src), target(target), bundle(bundle), maxCapacity(maxCapacity) {}
 
@@ -65,25 +65,25 @@ typedef struct Channel {
   int usedCapacity = 0; // how many flows are actually using this Channel
   std::set<int> fixedCapacity; // channels not available to the algorithm
   int overCapacityCount = 0;   // history of Channel being over capacity
-} Channel;
+};
 
 struct SwitchboxNode;
 struct ChannelEdge;
 using SwitchboxNodeBase = llvm::DGNode<SwitchboxNode, ChannelEdge>;
 using ChannelEdgeBase = llvm::DGEdge<SwitchboxNode, ChannelEdge>;
 using SwitchboxGraphBase = llvm::DirectedGraph<SwitchboxNode, ChannelEdge>;
 
-typedef struct SwitchboxNode : SwitchboxNodeBase, Switchbox {
+using SwitchboxNode = struct SwitchboxNode : SwitchboxNodeBase, Switchbox {
   using Switchbox::Switchbox;
   SwitchboxNode(int col, int row, int id) : Switchbox{col, row}, id{id} {}
   int id;
-} SwitchboxNode;
+};
 
 // warning: 'xilinx::AIE::ChannelEdge::src' will be initialized after
 // SwitchboxNode &src; [-Wreorder]
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wreorder"
-typedef struct ChannelEdge : ChannelEdgeBase, Channel {
+using ChannelEdge = struct ChannelEdge : ChannelEdgeBase, Channel {
   using Channel::Channel;
 
   explicit ChannelEdge(SwitchboxNode &target) = delete;
@@ -97,7 +97,7 @@ typedef struct ChannelEdge : ChannelEdgeBase, Channel {
   ChannelEdge &operator=(ChannelEdge &&E) = delete;
 
   SwitchboxNode &src;
-} ChannelEdge;
+};
 #pragma GCC diagnostic pop
 
 class SwitchboxGraph : public SwitchboxGraphBase {
@@ -107,9 +107,9 @@ class SwitchboxGraph : public SwitchboxGraphBase {
 };
 
 // A SwitchSetting defines the required settings for a Switchbox for a flow
-// SwitchSetting.first is the incoming signal
-// SwitchSetting.second is the fanout
-typedef struct SwitchSetting {
+// SwitchSetting.src is the incoming signal
+// SwitchSetting.dsts is the fanout
+using SwitchSetting = struct SwitchSetting {
   SwitchSetting() = default;
   SwitchSetting(Port src) : src(src) {}
   SwitchSetting(Port src, std::set<Port> dsts)
@@ -143,14 +143,13 @@ typedef struct SwitchSetting {
   }
 
   bool operator<(const SwitchSetting &rhs) const { return src < rhs.src; }
+};
 
-} SwitchSetting;
-
-typedef std::map<Switchbox, SwitchSetting> SwitchSettings;
+using SwitchSettings = std::map<Switchbox, SwitchSetting>;
 
 // A Flow defines source and destination vertices
 // Only one source, but any number of destinations (fanout)
-typedef struct PathEndPoint {
+using PathEndPoint = struct PathEndPoint {
   Switchbox sb;
   Port port;
 
@@ -175,21 +174,20 @@ typedef struct PathEndPoint {
   bool operator==(const PathEndPoint &rhs) const {
     return std::tie(sb, port) == std::tie(rhs.sb, rhs.port);
   }
-
-} PathEndPoint;
+};
 
 // A Flow defines source and destination vertices
 // Only one source, but any number of destinations (fanout)
-typedef struct PathEndPointNode : PathEndPoint {
+using PathEndPointNode = struct PathEndPointNode : PathEndPoint {
   PathEndPointNode(SwitchboxNode *sb, Port port)
       : PathEndPoint{*sb, port}, sb(sb) {}
   SwitchboxNode *sb;
-} PathEndPointNode;
+};
 
-typedef struct FlowNode {
+using FlowNode = struct FlowNode {
   PathEndPointNode src;
   std::vector<PathEndPointNode> dsts;
-} FlowNode;
+};
 
 class Router {
 public:
@@ -219,7 +217,7 @@ class Pathfinder : public Router {
   findPaths(int maxIterations) override;
 
   Switchbox *getSwitchbox(TileID coords) override {
-    auto sb = std::find_if(graph.begin(), graph.end(), [&](SwitchboxNode *sb) {
+    auto *sb = std::find_if(graph.begin(), graph.end(), [&](SwitchboxNode *sb) {
       return sb->col == coords.col && sb->row == coords.row;
     });
     assert(sb != graph.end() && "couldn't find sb");
@@ -281,7 +279,8 @@ class DynamicTileAnalysis {
 // because one of the graph traits below is doing the comparison internally
 // (try moving this below the llvm namespace...)
 namespace std {
-template <> struct less<xilinx::AIE::Switchbox *> {
+template <>
+struct less<xilinx::AIE::Switchbox *> {
   bool operator()(const xilinx::AIE::Switchbox *a,
                   const xilinx::AIE::Switchbox *b) const {
     return *a < *b;
@@ -291,7 +290,8 @@ template <> struct less<xilinx::AIE::Switchbox *> {
 
 namespace llvm {
 
-template <> struct GraphTraits<xilinx::AIE::SwitchboxNode *> {
+template <>
+struct GraphTraits<xilinx::AIE::SwitchboxNode *> {
   using NodeRef = xilinx::AIE::SwitchboxNode *;
 
   static xilinx::AIE::SwitchboxNode *SwitchboxGraphGetSwitchbox(
@@ -349,13 +349,15 @@ inline raw_ostream &operator<<(raw_ostream &os,
 
 } // namespace llvm
 
-template <> struct std::hash<xilinx::AIE::Switchbox> {
+template <>
+struct std::hash<xilinx::AIE::Switchbox> {
   std::size_t operator()(const xilinx::AIE::Switchbox &s) const noexcept {
     return std::hash<xilinx::AIE::TileID>{}(s);
   }
 };
 
-template <> struct std::hash<xilinx::AIE::PathEndPoint> {
+template <>
+struct std::hash<xilinx::AIE::PathEndPoint> {
   std::size_t operator()(const xilinx::AIE::PathEndPoint &pe) const noexcept {
     std::size_t h1 = std::hash<xilinx::AIE::Port>{}(pe.port);
     std::size_t h2 = std::hash<xilinx::AIE::Switchbox>{}(pe.sb);

lib/Dialect/AIE/IR/AIEDialect.cpp

@@ -90,7 +90,7 @@ const AIETargetModel &getTargetModel(Operation *op) {
 // Walk the operation hierarchy until we find a containing TileElement.
 // If no parent is a TileElement, then return null.
 static TileElement getParentTileElement(Operation *op) {
-  auto parent = op->getParentOp();
+  auto *parent = op->getParentOp();
   while (!llvm::isa_and_nonnull<DeviceOp, ModuleOp>(parent)) {
     if (auto element = llvm::dyn_cast<TileElement>(parent))
       return element;
@@ -105,7 +105,7 @@ struct UsesAreAccessable {
     auto thisID = thisElement.getTileID();
     auto users = op->getResult(0).getUsers();
     const auto &targetModel = getTargetModel(op);
-    for (auto user : users) {
+    for (auto *user : users) {
       // AIE.useLock may be used in a device to set the lock's default value
       // Allow in a toplevel module for backward compatibility
       if (llvm::isa_and_nonnull<DeviceOp, ModuleOp>(user->getParentOp()))
@@ -660,8 +660,8 @@ ObjectFifoCreateOp ObjectFifoRegisterExternalBuffersOp::getObjectFifo() {
   Operation *parent = getOperation();
   while ((parent = parent->getParentOp())) {
     if (parent->hasTrait<OpTrait::SymbolTable>()) {
-      if (auto st = SymbolTable::lookupSymbolIn(parent, getObjFifoName());
-          st && isa<ObjectFifoCreateOp>(st))
+      if (auto *st = SymbolTable::lookupSymbolIn(parent, getObjFifoName());
+          isa_and_nonnull<ObjectFifoCreateOp>(st))
         return dyn_cast<ObjectFifoCreateOp>(st);
     }
   }
@@ -705,8 +705,8 @@ ObjectFifoCreateOp ObjectFifoAcquireOp::getObjectFifo() {
   Operation *parent = getOperation();
   while ((parent = parent->getParentOp())) {
     if (parent->hasTrait<OpTrait::SymbolTable>()) {
-      if (auto st = SymbolTable::lookupSymbolIn(parent, getObjFifoName());
-          st && isa<ObjectFifoCreateOp>(st))
+      if (auto *st = SymbolTable::lookupSymbolIn(parent, getObjFifoName());
+          isa_and_nonnull<ObjectFifoCreateOp>(st))
         return dyn_cast<ObjectFifoCreateOp>(st);
     }
   }
@@ -750,8 +750,8 @@ ObjectFifoCreateOp ObjectFifoReleaseOp::getObjectFifo() {
   Operation *parent = getOperation();
   while ((parent = parent->getParentOp())) {
     if (parent->hasTrait<OpTrait::SymbolTable>()) {
-      if (auto st = SymbolTable::lookupSymbolIn(parent, getObjFifoName());
-          st && isa<ObjectFifoCreateOp>(st))
+      if (auto *st = SymbolTable::lookupSymbolIn(parent, getObjFifoName());
+          isa_and_nonnull<ObjectFifoCreateOp>(st))
         return dyn_cast<ObjectFifoCreateOp>(st);
     }
   }
@@ -795,8 +795,8 @@ ObjectFifoCreateOp ObjectFifoRegisterProcessOp::getObjectFifo() {
   Operation *parent = getOperation();
   while ((parent = parent->getParentOp())) {
     if (parent->hasTrait<OpTrait::SymbolTable>()) {
-      if (auto st = SymbolTable::lookupSymbolIn(parent, getObjFifoName());
-          st && isa<ObjectFifoCreateOp>(st))
+      if (auto *st = SymbolTable::lookupSymbolIn(parent, getObjFifoName());
+          isa_and_nonnull<ObjectFifoCreateOp>(st))
         return dyn_cast<ObjectFifoCreateOp>(st);
     }
   }
@@ -934,10 +934,15 @@ LogicalResult SwitchboxOp::verify() {
 
       Port dest = {connectOp.getDestBundle(), connectOp.destIndex()};
       if (destset.count(dest)) {
-        return connectOp.emitOpError("targets same destination ")
-               << to_string(dest) << " as another connect operation (from "
-               << to_string(source)
-               << ", tile: " << to_string(this->getTileOp().getTileID()) << ")";
+        return connectOp.emitOpError()
+               << "; connecting " << to_string(source) << " to "
+               << to_string(dest) << " on "
+               << to_string(this->getTileOp().getTileID())
+               << " targets same dst as another connect op; existing "
+                  "destinations: "
+               << llvm::join(llvm::map_range(
+                                 destset, [](auto &p) { return to_string(p); }),
+                             ", ");
       }
       destset.insert(dest);
 
@@ -1008,7 +1013,7 @@ LogicalResult SwitchboxOp::verify() {
     } else if (auto amselOp = dyn_cast<AMSelOp>(ops)) {
       std::vector<MasterSetOp> mstrs;
       std::vector<PacketRulesOp> slvs;
-      for (auto user : amselOp.getResult().getUsers()) {
+      for (auto *user : amselOp.getResult().getUsers()) {
         if (auto s = dyn_cast<PacketRuleOp>(user)) {
           auto pktRules = dyn_cast<PacketRulesOp>(s->getParentOp());
           slvs.push_back(pktRules);
@@ -1291,7 +1296,7 @@ LogicalResult MemTileDMAOp::verify() {
           if (block->empty())
             continue;
           auto successors = block->getTerminator()->getSuccessors();
-          for (auto i : successors) {
+          for (auto *i : successors) {
             if (!reachable.contains(i)) {
               reachable.insert(i);
               worklist.push_back(i);
@@ -1463,7 +1468,7 @@ LogicalResult LockOp::verify() {
 
 struct UsesOneLockInDMABlock {
   static LogicalResult verifyTrait(Operation *op) {
-    auto block = op->getBlock();
+    auto *block = op->getBlock();
     int lockID = -1;
     for (auto op : block->getOps<UseLockOp>()) {
       if (auto lock = dyn_cast<LockOp>(op.getLock().getDefiningOp());
@@ -1479,7 +1484,7 @@ struct UsesOneLockInDMABlock {
 
 struct AcquireReleaseOneStateInDMABlock {
   static LogicalResult verifyTrait(Operation *op) {
-    auto block = op->getBlock();
+    auto *block = op->getBlock();
     int acqValue = -1, relValue = -1;
     for (auto op : block->getOps<UseLockOp>()) {
       if (op.acquire() || op.acquireGE()) {

lib/Dialect/AIE/Transforms/AIEPathFinder.cpp

@@ -176,34 +176,40 @@ void Pathfinder::initialize(int maxCol, int maxRow,
       SwitchboxNode &thisNode = grid.at({col, row});
       if (row > 0) { // if not in row 0 add channel to North/South
         SwitchboxNode &southernNeighbor = grid.at({col, row - 1});
-        if (uint32_t maxCapacity = targetModel.getNumSourceSwitchboxConnections(
-                col, row, WireBundle::South)) {
-          edges.emplace_back(southernNeighbor, thisNode, WireBundle::North,
-                             maxCapacity);
-          (void)graph.connect(southernNeighbor, thisNode, edges.back());
-        }
+        // get the number of outgoing connections on the south side - outgoing
+        // because these correspond to rhs of a connect op
         if (uint32_t maxCapacity = targetModel.getNumDestSwitchboxConnections(
                 col, row, WireBundle::South)) {
           edges.emplace_back(thisNode, southernNeighbor, WireBundle::South,
                              maxCapacity);
           (void)graph.connect(thisNode, southernNeighbor, edges.back());
         }
+        // get the number of incoming connections on the south side - incoming
+        // because they correspond to connections on the southside that are then
+        // routed using internal connect ops through the switchbox (i.e., lhs of
+        // connect ops)
+        if (uint32_t maxCapacity = targetModel.getNumSourceSwitchboxConnections(
+                col, row, WireBundle::South)) {
+          edges.emplace_back(southernNeighbor, thisNode, WireBundle::North,
+                             maxCapacity);
+          (void)graph.connect(southernNeighbor, thisNode, edges.back());
+        }
       }
 
       if (col > 0) { // if not in col 0 add channel to East/West
         SwitchboxNode &westernNeighbor = grid.at({col - 1, row});
-        if (uint32_t maxCapacity = targetModel.getNumSourceSwitchboxConnections(
-                col, row, WireBundle::West)) {
-          edges.emplace_back(westernNeighbor, thisNode, WireBundle::East,
-                             maxCapacity);
-          (void)graph.connect(westernNeighbor, thisNode, edges.back());
-        }
         if (uint32_t maxCapacity = targetModel.getNumDestSwitchboxConnections(
                 col, row, WireBundle::West)) {
           edges.emplace_back(thisNode, westernNeighbor, WireBundle::West,
                              maxCapacity);
           (void)graph.connect(thisNode, westernNeighbor, edges.back());
         }
+        if (uint32_t maxCapacity = targetModel.getNumSourceSwitchboxConnections(
+                col, row, WireBundle::West)) {
+          edges.emplace_back(westernNeighbor, thisNode, WireBundle::East,
+                             maxCapacity);
+          (void)graph.connect(westernNeighbor, thisNode, edges.back());
+        }
       }
     }
   }
@@ -221,7 +227,7 @@ void Pathfinder::addFlow(TileID srcCoords, Port srcPort, TileID dstCoords,
                                       existingSrc->row == srcCoords.row &&
                                       existingPort == srcPort) {
       // find the vertex corresponding to the destination
-      auto matchingSb = std::find_if(
+      auto *matchingSb = std::find_if(
           graph.begin(), graph.end(), [&](const SwitchboxNode *sb) {
             return sb->col == dstCoords.col && sb->row == dstCoords.row;
           });
@@ -232,12 +238,12 @@ void Pathfinder::addFlow(TileID srcCoords, Port srcPort, TileID dstCoords,
   }
 
   // If no existing flow was found with this source, create a new flow.
-  auto matchingSrcSb =
+  auto *matchingSrcSb =
       std::find_if(graph.begin(), graph.end(), [&](const SwitchboxNode *sb) {
         return sb->col == srcCoords.col && sb->row == srcCoords.row;
       });
   assert(matchingSrcSb != graph.end() && "didn't find flow source");
-  auto matchingDstSb =
+  auto *matchingDstSb =
       std::find_if(graph.begin(), graph.end(), [&](const SwitchboxNode *sb) {
         return sb->col == dstCoords.col && sb->row == dstCoords.row;
       });
@@ -434,10 +440,11 @@ Pathfinder::findPaths(const int maxIterations) {
           // find the edge from the pred to curr by searching incident edges
           SmallVector<ChannelEdge *, 10> channels;
           graph.findIncomingEdgesToNode(*curr, channels);
-          auto matchingCh = std::find_if(
+          auto *matchingCh = std::find_if(
               channels.begin(), channels.end(),
               [&](ChannelEdge *ch) { return ch->src == *preds[curr]; });
           assert(matchingCh != channels.end() && "couldn't find ch");
+          // incoming edge
           ChannelEdge *ch = *matchingCh;
 
           // don't use fixed channels