use (x, y) convention for CHANX instead of (y, x)

soheilshahrouz · soheilshahrouz · commit a6c0049ac163 · 2025-05-14T18:05:19.000-04:00
diff --git a/libs/librrgraph/src/base/rr_graph_builder.cpp b/libs/librrgraph/src/base/rr_graph_builder.cpp
@@ -31,31 +31,29 @@ void RRGraphBuilder::add_node_to_all_locs(RRNodeId node) {
     short node_layer = node_storage_.node_layer(node);
     short node_twist = node_storage_.node_ptc_twist(node);
     int node_offset = 0;
+
     for (int ix = node_storage_.node_xlow(node); ix <= node_storage_.node_xhigh(node); ix++) {
         for (int iy = node_storage_.node_ylow(node); iy <= node_storage_.node_yhigh(node); iy++) {
             node_ptc_num += node_twist * node_offset;
             node_offset++;
+
             switch (node_type) {
                 case e_rr_type::SOURCE:
                 case e_rr_type::SINK:
                 case e_rr_type::CHANY:
-                    node_lookup_.add_node(node, node_layer, ix, iy, node_type, node_ptc_num, TOTAL_2D_SIDES[0]);
-                    break;
                 case e_rr_type::CHANX:
-                    /* Currently need to swap x and y for CHANX because of chan, seg convention 
-                     * TODO: Once the builders is reworked for use consistent (x, y) convention,
-                     * the following swapping can be removed
-                     */
-                    node_lookup_.add_node(node, node_layer, iy, ix, node_type, node_ptc_num, TOTAL_2D_SIDES[0]);
+                    node_lookup_.add_node(node, node_layer, ix, iy, node_type, node_ptc_num, TOTAL_2D_SIDES[0]);
                     break;
+
                 case e_rr_type::OPIN:
                 case e_rr_type::IPIN:
-                    for (const e_side& side : TOTAL_2D_SIDES) {
+                    for (const e_side side : TOTAL_2D_SIDES) {
                         if (node_storage_.is_node_on_specific_side(node, side)) {
                             node_lookup_.add_node(node,node_layer, ix, iy, node_type, node_ptc_num, side);
                         }
                     }
                     break;
+
                 default:
                     VTR_LOG_ERROR("Invalid node type for node '%lu' in the routing resource graph file", size_t(node));
                     break;
diff --git a/libs/librrgraph/src/base/rr_spatial_lookup.cpp b/libs/librrgraph/src/base/rr_spatial_lookup.cpp
@@ -33,18 +33,6 @@ RRNodeId RRSpatialLookup::find_node(int layer,
         return RRNodeId::INVALID();
     }
 
-    /* Currently need to swap x and y for CHANX because of chan, seg convention 
-     * This is due to that the fast look-up builders uses (y, x) coordinate when
-     * registering a CHANX node in the look-up
-     * TODO: Once the builders is reworked for use consistent (x, y) convention,
-     * the following swapping can be removed
-     */
-    size_t node_x = x;
-    size_t node_y = y;
-    if (type == e_rr_type::CHANX) {
-        std::swap(node_x, node_y);
-    }
-
     VTR_ASSERT_SAFE(4 == rr_node_indices_[type].ndims());
 
     /* Sanity check to ensure the layer, x, y, side and ptc are in range
@@ -59,23 +47,23 @@ RRNodeId RRSpatialLookup::find_node(int layer,
         return RRNodeId::INVALID();
     }
 
-    if (node_x >= rr_node_indices_[type].dim_size(1)) {
+    if (x >= rr_node_indices_[type].dim_size(1)) {
         return RRNodeId::INVALID();
     }
 
-    if(node_y >= rr_node_indices_[type].dim_size(2)){
+    if(y >= rr_node_indices_[type].dim_size(2)){
         return RRNodeId::INVALID();
     }
 
     if (node_side >= rr_node_indices_[type].dim_size(3)) {
         return RRNodeId::INVALID();
     }
 
-    if (size_t(ptc) >= rr_node_indices_[type][layer][node_x][node_y][node_side].size()) {
+    if (size_t(ptc) >= rr_node_indices_[type][layer][x][y][node_side].size()) {
         return RRNodeId::INVALID();
     }
 
-    return rr_node_indices_[type][layer][node_x][node_y][node_side][ptc];
+    return rr_node_indices_[type][layer][x][y][node_side][ptc];
 }
 
 std::vector<RRNodeId> RRSpatialLookup::find_nodes_in_range(int layer,
@@ -114,18 +102,6 @@ std::vector<RRNodeId> RRSpatialLookup::find_nodes(int layer,
         return nodes;
     }
 
-    /* Currently need to swap x and y for CHANX because of chan, seg convention 
-     * This is due to that the fast look-up builders uses (y, x) coordinate when
-     * registering a CHANX node in the look-up
-     * TODO: Once the builders is reworked for use consistent (x, y) convention,
-     * the following swapping can be removed
-     */
-    size_t node_x = x;
-    size_t node_y = y;
-    if (type == e_rr_type::CHANX) {
-        std::swap(node_x, node_y);
-    }
-
     VTR_ASSERT_SAFE(4 == rr_node_indices_[type].ndims());
 
     /* Sanity check to ensure the x, y, side are in range 
@@ -140,11 +116,11 @@ std::vector<RRNodeId> RRSpatialLookup::find_nodes(int layer,
         return nodes;
     }
 
-    if (node_x >= rr_node_indices_[type].dim_size(1)) {
+    if (x >= rr_node_indices_[type].dim_size(1)) {
         return nodes;
     }
 
-    if(node_y >= rr_node_indices_[type].dim_size(2)){
+    if(y >= rr_node_indices_[type].dim_size(2)){
         return nodes;
     }
 
@@ -154,14 +130,14 @@ std::vector<RRNodeId> RRSpatialLookup::find_nodes(int layer,
 
     /* Reserve space to avoid memory fragmentation */
     size_t num_nodes = 0;
-    for (const auto& node : rr_node_indices_[type][layer][node_x][node_y][side]) {
+    for (const auto& node : rr_node_indices_[type][layer][x][y][side]) {
         if (node.is_valid()) {
             num_nodes++;
         }
     }
 
     nodes.reserve(num_nodes);
-    for (const auto& node : rr_node_indices_[type][layer][node_x][node_y][side]) {
+    for (const auto& node : rr_node_indices_[type][layer][x][y][side]) {
         if (node.is_valid()) {
             nodes.emplace_back(node);
         }
diff --git a/vpr/src/route/clock_network_builders.cpp b/vpr/src/route/clock_network_builders.cpp
@@ -372,8 +372,7 @@ int ClockRib::create_chanx_wire(int layer,
     /* TODO: Will replace these codes with an API add_node_to_all_locs() of RRGraphBuilder */
     for (int ix = rr_graph.node_xlow(chanx_node); ix <= rr_graph.node_xhigh(chanx_node); ++ix) {
         for (int iy = rr_graph.node_ylow(chanx_node); iy <= rr_graph.node_yhigh(chanx_node); ++iy) {
-            //TODO: CHANX uses odd swapped x/y indices here. Will rework once rr_node_indices is shadowed
-            rr_graph_builder.node_lookup().add_node(chanx_node, layer, iy, ix, rr_graph.node_type(chanx_node), rr_graph.node_track_num(chanx_node));
+            rr_graph_builder.node_lookup().add_node(chanx_node, layer, ix, iy, rr_graph.node_type(chanx_node), rr_graph.node_track_num(chanx_node));
         }
     }
 
diff --git a/vpr/src/route/rr_graph.cpp b/vpr/src/route/rr_graph.cpp
@@ -1336,11 +1336,10 @@ static void build_rr_graph(e_graph_type graph_type,
      */
     if (grid.get_num_layers() > 1 && sb_type == CUSTOM) {
         //keep how many nodes each switchblock requires for each x,y location
-        auto extra_nodes_per_switchblock = get_number_track_to_track_inter_die_conn(sb_conn_map, custom_3d_sb_fanin_fanout, device_ctx.rr_graph_builder);
+        vtr::NdMatrix<int, 2> extra_nodes_per_switchblock = get_number_track_to_track_inter_die_conn(sb_conn_map, custom_3d_sb_fanin_fanout, device_ctx.rr_graph_builder);
         //allocate new nodes in each switchblocks
-        alloc_and_load_inter_die_rr_node_indices(device_ctx.rr_graph_builder, &nodes_per_chan, grid, extra_nodes_per_switchblock, &num_rr_nodes);
+        alloc_and_load_inter_die_rr_node_indices(device_ctx.rr_graph_builder, nodes_per_chan, grid, extra_nodes_per_switchblock, &num_rr_nodes);
         device_ctx.rr_graph_builder.resize_nodes(num_rr_nodes);
-        extra_nodes_per_switchblock.clear();
     }
 
     /* START IPIN MAP */
diff --git a/vpr/src/route/rr_graph2.cpp b/vpr/src/route/rr_graph2.cpp
@@ -1103,19 +1103,19 @@ static void load_chan_rr_indices(const int max_chan_width,
     const auto& device_ctx = g_vpr_ctx.device();
 
     for (int layer = 0; layer < grid.get_num_layers(); layer++) {
-        /* Skip the current die if architecture file specifies that it doesn't require global resource routing */
+        // Skip the current die if architecture file specifies that it doesn't require global resource routing
         if (!device_ctx.inter_cluster_prog_routing_resources.at(layer)) {
             continue;
         }
 
         for (int chan = 0; chan < num_chans - 1; ++chan) {
             for (int seg = 1; seg < chan_len - 1; ++seg) {
-                /* Assign an inode to the starts of tracks */
-                int x = type == e_rr_type::CHANX ? seg : chan;
-                int y = type == e_rr_type::CHANX ? chan : seg;
+                // Assign an inode to the starts of tracks
+                const int x = (type == e_rr_type::CHANX) ? seg : chan;
+                const int y = (type == e_rr_type::CHANX) ? chan : seg;
                 const t_chan_seg_details* seg_details = chan_details[x][y].data();
 
-                /* Reserve nodes in lookup to save memory */
+                // Reserve nodes in lookup to save memory
                 rr_graph_builder.node_lookup().reserve_nodes(layer, chan, seg, type, max_chan_width);
 
                 for (int track = 0; track < max_chan_width; ++track) {
@@ -1124,24 +1124,19 @@ static void load_chan_rr_indices(const int max_chan_width,
                         continue;
 
                     int start = get_seg_start(seg_details, track, chan, seg);
+                    int node_start_x = (type == e_rr_type::CHANX) ? start : chan;
+                    int node_start_y = (type == e_rr_type::CHANX) ? chan : start;
 
-                    /* TODO: Now we still use the (y, x) convention here for CHANX. Should rework later */
-                    int node_x = chan;
-                    int node_y = start;
-                    if (e_rr_type::CHANX == type) {
-                        std::swap(node_x, node_y);
-                    }
-
-                    // If the start of the wire doesn't have an inode, assign one to it.
-                    RRNodeId inode = rr_graph_builder.node_lookup().find_node(layer, node_x, node_y, type, track);
+                    // If the start of the wire doesn't have an RRNodeId, assign one to it.
+                    RRNodeId inode = rr_graph_builder.node_lookup().find_node(layer, node_start_x, node_start_y, type, track);
                     if (!inode) {
                         inode = RRNodeId(*index);
                         ++(*index);
                         rr_graph_builder.node_lookup().add_node(inode, layer, chan, start, type, track);
                     }
 
-                    /* Assign inode of start of wire to current position */
-                    rr_graph_builder.node_lookup().add_node(inode, layer, chan, seg, type, track);
+                    // Assign RRNodeId of start of wire to current position
+                    rr_graph_builder.node_lookup().add_node(inode, layer, x, y, type, track);
                 }
             }
         }
@@ -1219,7 +1214,7 @@ vtr::NdMatrix<int, 2> get_number_track_to_track_inter_die_conn(t_sb_connection_m
 }
 
 void alloc_and_load_inter_die_rr_node_indices(RRGraphBuilder& rr_graph_builder,
-                                              const t_chan_width* nodes_per_chan,
+                                              const t_chan_width& nodes_per_chan,
                                               const DeviceGrid& grid,
                                               const vtr::NdMatrix<int, 2>& extra_nodes_per_switchblock,
                                               int* index) {
@@ -1232,31 +1227,32 @@ void alloc_and_load_inter_die_rr_node_indices(RRGraphBuilder& rr_graph_builder,
      *  3) ptc = [max_chanx_width:max_chanx_width+number_of_connection-1]
      *  4) direction = NONE
      */
-    auto& device_ctx = g_vpr_ctx.device();
+    const auto& device_ctx = g_vpr_ctx.device();
 
     for (int layer = 0; layer < grid.get_num_layers(); layer++) {
         /* Skip the current die if architecture file specifies that it doesn't have global resource routing */
         if (!device_ctx.inter_cluster_prog_routing_resources.at(layer)) {
             continue;
         }
+
         for (size_t y = 0; y < grid.height() - 1; ++y) {
             for (size_t x = 1; x < grid.width() - 1; ++x) {
-                //count how many track-to-track connection go from current layer to other layers
+                // count how many track-to-track connection go from current layer to other layers
                 int conn_count = extra_nodes_per_switchblock[x][y];
 
-                //skip if no connection is required
+                // skip if no connection is required
                 if (conn_count == 0) {
                     continue;
                 }
 
-                //reserve extra nodes for inter-die track-to-track connection
-                rr_graph_builder.node_lookup().reserve_nodes(layer, y, x, e_rr_type::CHANX, conn_count + nodes_per_chan->max);
+                // reserve extra nodes for inter-die track-to-track connection
+                rr_graph_builder.node_lookup().reserve_nodes(layer, x, y, e_rr_type::CHANX, conn_count + nodes_per_chan.max);
                 for (int rr_node_offset = 0; rr_node_offset < conn_count; rr_node_offset++) {
-                    RRNodeId inode = rr_graph_builder.node_lookup().find_node(layer, x, y, e_rr_type::CHANX, nodes_per_chan->max + rr_node_offset);
+                    RRNodeId inode = rr_graph_builder.node_lookup().find_node(layer, x, y, e_rr_type::CHANX, nodes_per_chan.max + rr_node_offset);
                     if (!inode) {
                         inode = RRNodeId(*index);
                         ++(*index);
-                        rr_graph_builder.node_lookup().add_node(inode, layer, y, x, e_rr_type::CHANX, nodes_per_chan->max + rr_node_offset);
+                        rr_graph_builder.node_lookup().add_node(inode, layer, x, y, e_rr_type::CHANX, nodes_per_chan.max + rr_node_offset);
                     }
                 }
             }
diff --git a/vpr/src/route/rr_graph2.h b/vpr/src/route/rr_graph2.h
@@ -29,7 +29,7 @@ void alloc_and_load_rr_node_indices(RRGraphBuilder& rr_graph_builder,
 
 
 /**
- * @brief allocates extra nodes within the RR graph to support 3D custom switch blocks for multi-die FPGAs
+ * @brief Allocates extra nodes within the RR graph to support 3D custom switch blocks for multi-die FPGAs
  *
  *  @param rr_graph_builder RRGraphBuilder data structure which allows data modification on a routing resource graph
  *  @param nodes_per_chan number of tracks per channel (x, y)
@@ -39,7 +39,7 @@ void alloc_and_load_rr_node_indices(RRGraphBuilder& rr_graph_builder,
  *  @param index RRNodeId that should be assigned to add a new RR node to the RR graph
  */
 void alloc_and_load_inter_die_rr_node_indices(RRGraphBuilder& rr_graph_builder,
-                                              const t_chan_width* nodes_per_chan,
+                                              const t_chan_width& nodes_per_chan,
                                               const DeviceGrid& grid,
                                               const vtr::NdMatrix<int, 2>& extra_nodes_per_switchblock,
                                               int* index);

Original file line number	Diff line number	Diff line change
`@@ -372,8 +372,7 @@ int ClockRib::create_chanx_wire(int layer,`
`372`	`372`	`/* TODO: Will replace these codes with an API add_node_to_all_locs() of RRGraphBuilder */`
`373`	`373`	`for (int ix = rr_graph.node_xlow(chanx_node); ix <= rr_graph.node_xhigh(chanx_node); ++ix) {`
`374`	`374`	`for (int iy = rr_graph.node_ylow(chanx_node); iy <= rr_graph.node_yhigh(chanx_node); ++iy) {`
`375`		`- //TODO: CHANX uses odd swapped x/y indices here. Will rework once rr_node_indices is shadowed`
`376`		`- rr_graph_builder.node_lookup().add_node(chanx_node, layer, iy, ix, rr_graph.node_type(chanx_node), rr_graph.node_track_num(chanx_node));`
	`375`	`+ rr_graph_builder.node_lookup().add_node(chanx_node, layer, ix, iy, rr_graph.node_type(chanx_node), rr_graph.node_track_num(chanx_node));`
`377`	`376`	`}`
`378`	`377`	`}`
`379`	`378`