ILP for pathfinding/routing (#761)

This PR implements routing/path-finding using ILP (using Gurobi/or-tools) through the python pass registration mechanism landed in #710.

.github/workflows/buildAndTest.yml

@@ -13,6 +13,13 @@ env:
   # Otherwise, on Ubuntu 20.04 the installation of tzdata asking question
   DEBIAN_FRONTEND: noninteractive
 
+concurrency:
+  # A PR number if a pull request and otherwise the commit hash. This cancels
+  # queued and in-progress runs for the same PR (presubmit) or commit
+  # (postsubmit).
+  group: ci-build-test-cpp-linux-${{ github.event.number || github.sha }}
+  cancel-in-progress: true
+
 jobs:
   build-repo:
     name: Build and Test
@@ -45,6 +52,7 @@ jobs:
       - name: Install Python packages
         run: |
           pip install cmake numpy psutil pybind11 rich pkginfo lit PyYAML
+          pip install -r python/requirements.txt
 
       - name: Install packages
         run: sudo apt-get install -y ninja-build clang lld

.github/workflows/lintAndFormat.yml

@@ -40,6 +40,7 @@ jobs:
       - name: Install Python packages
         run: |
           pip install cmake numpy psutil pybind11 rich pkginfo lit PyYAML requests
+          pip install -r python/requirements.txt
 
       - name: Get MLIR
         id: mlir-wheels
@@ -150,7 +151,6 @@ jobs:
         with:
           tool_name: clang-format
           level: error
-          fail_on_error: true
           cleanup: true
 
       - name: Run black format
@@ -174,7 +174,6 @@ jobs:
         with:
           tool_name: black
           level: error
-          fail_on_error: true
 
   code-coverage:
 
@@ -200,6 +199,7 @@ jobs:
       - name: Install Python and other packages
         run: |
           pip install cmake numpy psutil pybind11 rich lit
+          pip install -r python/requirements.txt
 
       - name: Install Ninja
         run: sudo apt-get install -y ninja-build clang lld llvm

.pylintrc

@@ -56,4 +56,8 @@ check-str-concat-over-line-jumps=yes
 # C0114: Missing module docstring (missing-module-docstring)
 # E0402: Attempted relative import beyond top-level package (relative-beyond-top-level)
 # C0115: Missing class docstring (missing-class-docstring)
-disable=C0116,C0114,E0402,C0115
+# C0415: Import outside toplevel
+# 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

include/aie/Dialect/AIE/IR/AIETargetModel.h

@@ -19,7 +19,7 @@
 
 namespace xilinx::AIE {
 
-typedef struct TileID {
+using TileID = struct TileID {
   // friend definition (will define the function as a non-member function in the
   // namespace surrounding the class).
   friend std::ostream &operator<<(std::ostream &os, const TileID &s) {
@@ -50,7 +50,7 @@ typedef struct TileID {
   bool operator!=(const TileID &rhs) const { return !(*this == rhs); }
 
   int col, row;
-} TileID;
+};
 
 class AIETargetModel {
 public:
@@ -155,7 +155,7 @@ class AIETargetModel {
   virtual bool isLegalMemAffinity(int coreCol, int coreRow, int memCol,
                                   int memRow) const = 0;
 
-  /// Return the base address in the local address map of differnet memories.
+  /// Return the base address in the local address map of different memories.
   virtual uint32_t getMemInternalBaseAddress(TileID src) const = 0;
   virtual uint32_t getMemSouthBaseAddress() const = 0;
   virtual uint32_t getMemWestBaseAddress() const = 0;
@@ -321,7 +321,7 @@ class AIE2TargetModel : public AIETargetModel {
 };
 
 class VC1902TargetModel : public AIE1TargetModel {
-  llvm::SmallDenseSet<unsigned, 16> noc_columns = {
+  llvm::SmallDenseSet<unsigned, 16> nocColumns = {
       2, 3, 6, 7, 10, 11, 18, 19, 26, 27, 34, 35, 42, 43, 46, 47};
 
 public:
@@ -332,11 +332,11 @@ class VC1902TargetModel : public AIE1TargetModel {
   int rows() const override { return 9; /* One Shim row and 8 Core rows. */ }
 
   bool isShimNOCTile(int col, int row) const override {
-    return row == 0 && noc_columns.contains(col);
+    return row == 0 && nocColumns.contains(col);
   }
 
   bool isShimPLTile(int col, int row) const override {
-    return row == 0 && !noc_columns.contains(col);
+    return row == 0 && !nocColumns.contains(col);
   }
 
   bool isShimNOCorPLTile(int col, int row) const override {
@@ -504,7 +504,8 @@ class IPUTargetModel : public AIE2TargetModel {
 } // namespace xilinx::AIE
 
 namespace llvm {
-template <> struct DenseMapInfo<xilinx::AIE::TileID> {
+template <>
+struct DenseMapInfo<xilinx::AIE::TileID> {
   using FirstInfo = DenseMapInfo<int>;
   using SecondInfo = DenseMapInfo<int>;
 
@@ -528,7 +529,8 @@ template <> struct DenseMapInfo<xilinx::AIE::TileID> {
 };
 } // namespace llvm
 
-template <> struct std::hash<xilinx::AIE::TileID> {
+template <>
+struct std::hash<xilinx::AIE::TileID> {
   std::size_t operator()(const xilinx::AIE::TileID &s) const noexcept {
     std::size_t h1 = std::hash<int>{}(s.col);
     std::size_t h2 = std::hash<int>{}(s.row);

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,11 +107,13 @@ 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)
+      : src(src), dsts(std::move(dsts)) {}
   Port src;
   std::set<Port> dsts;
 
@@ -141,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;
 
@@ -173,48 +174,63 @@ 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 Pathfinder {
-  SwitchboxGraph graph;
-  std::vector<FlowNode> flows;
-  std::map<TileID, SwitchboxNode> grid;
-  // Use a list instead of a vector because nodes have an edge list of raw
-  // pointers to edges (so growing a vector would invalidate the pointers).
-  std::list<ChannelEdge> edges;
+};
 
+class Router {
 public:
-  Pathfinder() = default;
+  Router() = default;
+  // This has to go first so it can serve as a key function.
+  // https://lld.llvm.org/missingkeyfunction
+  virtual ~Router() = default;
   virtual void initialize(int maxCol, int maxRow,
-                          const AIETargetModel &targetModel);
+                          const AIETargetModel &targetModel) = 0;
   virtual void addFlow(TileID srcCoords, Port srcPort, TileID dstCoords,
-                       Port dstPort);
-  virtual bool addFixedConnection(ConnectOp connectOp);
+                       Port dstPort) = 0;
+  virtual bool addFixedConnection(ConnectOp connectOp) = 0;
   virtual std::optional<std::map<PathEndPoint, SwitchSettings>>
-  findPaths(int maxIterations);
+  findPaths(int maxIterations) = 0;
+  virtual Switchbox *getSwitchbox(TileID coords) = 0;
+};
 
-  virtual Switchbox *getSwitchbox(TileID coords) {
-    auto sb = std::find_if(graph.begin(), graph.end(), [&](SwitchboxNode *sb) {
+class Pathfinder : public Router {
+public:
+  Pathfinder() = default;
+  void initialize(int maxCol, int maxRow,
+                  const AIETargetModel &targetModel) override;
+  void addFlow(TileID srcCoords, Port srcPort, TileID dstCoords,
+               Port dstPort) override;
+  bool addFixedConnection(ConnectOp connectOp) override;
+  std::optional<std::map<PathEndPoint, SwitchSettings>>
+  findPaths(int maxIterations) override;
+
+  Switchbox *getSwitchbox(TileID coords) override {
+    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");
     return *sb;
   }
-  virtual ~Pathfinder() = default;
+
+private:
+  SwitchboxGraph graph;
+  std::vector<FlowNode> flows;
+  std::map<TileID, SwitchboxNode> grid;
+  // Use a list instead of a vector because nodes have an edge list of raw
+  // pointers to edges (so growing a vector would invalidate the pointers).
+  std::list<ChannelEdge> edges;
 };
 
 WireBundle getConnectingBundle(WireBundle dir);
@@ -226,7 +242,7 @@ WireBundle getConnectingBundle(WireBundle dir);
 class DynamicTileAnalysis {
 public:
   int maxCol, maxRow;
-  std::shared_ptr<Pathfinder> pathfinder;
+  std::shared_ptr<Router> pathfinder;
   std::map<PathEndPoint, SwitchSettings> flowSolutions;
   std::map<PathEndPoint, bool> processedFlows;
 
@@ -238,8 +254,7 @@ class DynamicTileAnalysis {
   const int maxIterations = 1000; // how long until declared unroutable
 
   DynamicTileAnalysis() : pathfinder(std::make_shared<Pathfinder>()) {}
-  DynamicTileAnalysis(std::shared_ptr<Pathfinder> p)
-      : pathfinder(std::move(p)) {}
+  DynamicTileAnalysis(std::shared_ptr<Router> p) : pathfinder(std::move(p)) {}
 
   mlir::LogicalResult runAnalysis(DeviceOp &device);
 
@@ -264,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;
@@ -274,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(
@@ -332,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);