Clean up confusing rr-node lookup convention for x/y location of CHANX nodes

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;

libs/librrgraph/src/base/rr_graph_view.h

@@ -351,7 +351,7 @@ class RRGraphView {
 
                 start_x = " (" + std::to_string(node_xhigh(node)) + ","; //start coordinates have large value
                 start_y = std::to_string(node_yhigh(node)) + ",";
-                start_layer_str = std::to_string(node_layer_num);
+                start_layer_str = std::to_string(node_layer_num) + ")";
                 end_x = " (" + std::to_string(node_xlow(node)) + ","; //end coordinates have smaller value
                 end_y = std::to_string(node_ylow(node)) + ",";
                 end_layer_str = std::to_string(node_layer_num) + ")";
@@ -360,7 +360,7 @@ class RRGraphView {
             else {                                                      // signal travels in increasing direction, stays at same point, or can travel both directions
                 start_x = " (" + std::to_string(node_xlow(node)) + ","; //start coordinates have smaller value
                 start_y = std::to_string(node_ylow(node)) + ",";
-                start_layer_str = std::to_string(node_layer_num);
+                start_layer_str = std::to_string(node_layer_num) + ")";
                 end_x = " (" + std::to_string(node_xhigh(node)) + ","; //end coordinates have larger value
                 end_y = std::to_string(node_yhigh(node)) + ",";
                 end_layer_str = std::to_string(node_layer_num) + ")"; //layer number

libs/librrgraph/src/base/rr_spatial_lookup.cpp

@@ -1,9 +1,6 @@
 #include "vtr_assert.h"
 #include "rr_spatial_lookup.h"
 
-RRSpatialLookup::RRSpatialLookup() {
-}
-
 RRNodeId RRSpatialLookup::find_node(int layer,
                                     int x,
                                     int y,
@@ -33,18 +30,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 +44,23 @@ RRNodeId RRSpatialLookup::find_node(int layer,
         return RRNodeId::INVALID();
     }
 
-    if (node_x >= rr_node_indices_[type].dim_size(1)) {
+    if (size_t(x) >= rr_node_indices_[type].dim_size(1)) {
         return RRNodeId::INVALID();
     }
 
-    if(node_y >= rr_node_indices_[type].dim_size(2)){
+    if (size_t(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 +99,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 +113,11 @@ std::vector<RRNodeId> RRSpatialLookup::find_nodes(int layer,
         return nodes;
     }
 
-    if (node_x >= rr_node_indices_[type].dim_size(1)) {
+    if (size_t(x) >= rr_node_indices_[type].dim_size(1)) {
         return nodes;
     }
 
-    if(node_y >= rr_node_indices_[type].dim_size(2)){
+    if (size_t(y) >= rr_node_indices_[type].dim_size(2)){
         return nodes;
     }
 
@@ -154,14 +127,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 (RRNodeId 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 (RRNodeId node : rr_node_indices_[type][layer][x][y][side]) {
         if (node.is_valid()) {
             nodes.emplace_back(node);
         }
@@ -338,7 +311,7 @@ void RRSpatialLookup::resize_nodes(int layer,
         || (x >= int(rr_node_indices_[type].dim_size(1)))
         || (y >= int(rr_node_indices_[type].dim_size(2)))
         || (size_t(side) >= rr_node_indices_[type].dim_size(3))) {
-        rr_node_indices_[type].resize({std::max(rr_node_indices_[type].dim_size(0),size_t(layer)+1),
+        rr_node_indices_[type].resize({std::max(rr_node_indices_[type].dim_size(0), size_t(layer)+1),
                                        std::max(rr_node_indices_[type].dim_size(1), size_t(x) + 1),
                                        std::max(rr_node_indices_[type].dim_size(2), size_t(y) + 1),
                                        std::max(rr_node_indices_[type].dim_size(3), size_t(side) + 1)});
@@ -352,7 +325,7 @@ void RRSpatialLookup::reorder(const vtr::vector<RRNodeId, RRNodeId>& dest_order)
             for (size_t x = 0; x < grid.dim_size(1); x++) {
                 for (size_t y = 0; y < grid.dim_size(2); y++) {
                     for (size_t s = 0; s < grid.dim_size(3); s++) {
-                        for (auto &node: grid[l][x][y][s]) {
+                        for (RRNodeId &node: grid[l][x][y][s]) {
                             if (node.is_valid()) {
                                 node = dest_order[node];
                             }

libs/librrgraph/src/base/rr_spatial_lookup.h

@@ -1,5 +1,4 @@
-#ifndef RR_SPATIAL_LOOKUP_H
-#define RR_SPATIAL_LOOKUP_H
+#pragma once
 
 /** 
  * @file
@@ -25,7 +24,7 @@ class RRSpatialLookup {
     /* -- Constructors -- */
   public:
     /* Explicitly define the only way to create an object */
-    explicit RRSpatialLookup();
+    explicit RRSpatialLookup() = default;
 
     /* Disable copy constructors and copy assignment operator
      * This is to avoid accidental copy because it could be an expensive operation considering that the 
@@ -293,5 +292,3 @@ class RRSpatialLookup {
     /* Fast look-up: TODO: Should rework the data type. Currently it is based on a 3-dimensional array mater where some dimensions must always be accessed with a specific index. Such limitation should be overcome */
     t_rr_node_indices rr_node_indices_;
 };
-
-#endif

libs/librrgraph/src/io/rr_graph_uxsdcxx_serializer.h

@@ -1848,16 +1848,11 @@ class RrGraphSerializer final : public uxsd::RrGraphBase<RrGraphContextTypes> {
 
         /* Alloc the lookup table */
         for (e_rr_type rr_type : RR_TYPES) {
-            if (rr_type == e_rr_type::CHANX) {
-                rr_graph_builder.node_lookup().resize_nodes(grid_.get_num_layers(), grid_.height(), grid_.width(), rr_type, NUM_2D_SIDES);
-            } else {
-                rr_graph_builder.node_lookup().resize_nodes(grid_.get_num_layers(), grid_.width(), grid_.height(), rr_type, NUM_2D_SIDES);
-            }
+            rr_graph_builder.node_lookup().resize_nodes(grid_.get_num_layers(), grid_.width(), grid_.height(), rr_type, NUM_2D_SIDES);
         }
 
         /* Add the correct node into the vector */
-        for (size_t inode = 0; inode < rr_nodes_->size(); inode++) {
-            auto node = (*rr_nodes_)[inode];
+        for (const t_rr_node& node : *rr_nodes_) {
             rr_graph_builder.add_node_to_all_locs(node.id());
         }
     }

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));
         }
     }
 

vpr/src/route/rr_graph.cpp

@@ -1250,12 +1250,11 @@ static void build_rr_graph(e_graph_type graph_type,
 
     // Add routing resources to rr_graph lookup table
     alloc_and_load_rr_node_indices(device_ctx.rr_graph_builder,
-                                   &nodes_per_chan,
+                                   nodes_per_chan,
                                    grid,
                                    &num_rr_nodes,
                                    chan_details_x,
-                                   chan_details_y,
-                                   is_flat);
+                                   chan_details_y);
 
     size_t expected_node_count = num_rr_nodes;
     if (clock_modeling == DEDICATED_NETWORK) {
@@ -1337,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 */