[VPR] Remove the hard requirement on side option when adding a node; Add comments about TODO action items;

Author: tangxifan

vpr/src/device/rr_spatial_lookup.cpp

@@ -162,12 +162,13 @@ std::vector<RRNodeId> RRSpatialLookup::find_sink_nodes(int x,
         return sink_nodes;
     }
 
-    /* By default, we always added the channel nodes to the TOP side (to save memory) */
+    /* By default, we always added the sink nodes to the TOP side (to save memory) */
     e_side node_side = TOP;
     if (node_side >= rr_node_indices_[SINK].dim_size(2)) {
         return sink_nodes;
     }
 
+    /* Only insert valid ids in the returned vector */
     for (const auto& node : rr_node_indices_[SINK][x][y][node_side]) {
         if (RRNodeId(node)) {
             sink_nodes.push_back(RRNodeId(node));
@@ -212,6 +213,11 @@ void RRSpatialLookup::add_node(RRNodeId node,
     VTR_ASSERT(node); /* Must have a valid node id to be added */
     VTR_ASSERT_SAFE(3 == rr_node_indices_[type].ndims());
 
+    /* For non-IPIN/OPIN nodes, the side should always be the TOP side which follows the convention in find_node() API! */
+    if (type != IPIN && type != OPIN) {
+        VTR_ASSERT(side == SIDES[0]);
+    }
+
     resize_nodes(x, y, type, side);
 
     if (size_t(ptc) >= rr_node_indices_[type][x][y][side].size()) {

vpr/src/device/rr_spatial_lookup.h

@@ -88,7 +88,7 @@ class RRSpatialLookup {
      *
      * Note: 
      * - Return an empty list if there are no sinks at the given (x, y) location
-     * - The node list returned only contain valid ids
+     * - The node list returned only contains valid ids
      */
     std::vector<RRNodeId> find_sink_nodes(int x,
                                           int y) const;
@@ -130,7 +130,7 @@ class RRSpatialLookup {
                   int y,
                   t_rr_type type,
                   int ptc,
-                  e_side side);
+                  e_side side = SIDES[0]);
 
     /**
      * Mirror the last dimension of a look-up, i.e., a list of nodes, from a source coordinate to 
@@ -191,7 +191,7 @@ class RRSpatialLookup {
      * or inside the data structures to be changed later.
      * That explains why the reference is used here temporarily
      */
-    /* Fast look-up */
+    /* Fast look-up: TODO: Should rework the data type. Currently it is based on a 3-dimensional arrqay mater where some dimensions must always be accessed with a specific index. Such limitation should be overcome */
     t_rr_node_indices& rr_node_indices_;
 };
 

vpr/src/route/clock_connection_builders.cpp

@@ -54,10 +54,10 @@ void RoutingToClockConnection::create_switches(const ClockRRGraphBuilder& clock_
     auto& device_ctx = g_vpr_ctx.device();
     const auto& node_lookup = device_ctx.rr_graph.node_lookup();
 
-    int virtual_clock_network_root_idx = create_virtual_clock_network_sink_node(switch_location.x, switch_location.y);
+    RRNodeId virtual_clock_network_root_idx = create_virtual_clock_network_sink_node(switch_location.x, switch_location.y);
     {
         auto& mut_device_ctx = g_vpr_ctx.mutable_device();
-        mut_device_ctx.virtual_clock_network_root_idx = virtual_clock_network_root_idx;
+        mut_device_ctx.virtual_clock_network_root_idx = size_t(virtual_clock_network_root_idx);
     }
 
     // rr_node indices for x and y channel routing wires and clock wires to connect to
@@ -85,13 +85,11 @@ void RoutingToClockConnection::create_switches(const ClockRRGraphBuilder& clock_
 
         // Connect to virtual clock sink node
         // used by the two stage router
-        clock_graph.add_edge(rr_edges_to_create, clock_index, virtual_clock_network_root_idx, arch_switch_idx);
+        clock_graph.add_edge(rr_edges_to_create, clock_index, size_t(virtual_clock_network_root_idx), arch_switch_idx);
     }
 }
 
-int RoutingToClockConnection::create_virtual_clock_network_sink_node(
-    int x,
-    int y) {
+RRNodeId RoutingToClockConnection::create_virtual_clock_network_sink_node(int x, int y) {
     auto& device_ctx = g_vpr_ctx.mutable_device();
     auto& rr_graph = device_ctx.rr_nodes;
     auto& node_lookup = device_ctx.rr_graph_builder.node_lookup();
@@ -120,11 +118,11 @@ int RoutingToClockConnection::create_virtual_clock_network_sink_node(
     // However, since the SINK node has the same xhigh/xlow as well as yhigh/ylow, we can probably use a shortcut
     for (int ix = rr_graph.node_xlow(node_index); ix <= rr_graph.node_xhigh(node_index); ++ix) {
         for (int iy = rr_graph.node_ylow(node_index); iy <= rr_graph.node_yhigh(node_index); ++iy) {
-            node_lookup.add_node(node_index, ix, iy, rr_graph.node_type(node_index), rr_graph.node_ptc_num(node_index), SIDES[0]);
+            node_lookup.add_node(node_index, ix, iy, rr_graph.node_type(node_index), rr_graph.node_ptc_num(node_index));
         }
     }
 
-    return size_t(node_index);
+    return node_index;
 }
 
 /*

vpr/src/route/clock_connection_builders.h

@@ -57,7 +57,7 @@ class RoutingToClockConnection : public ClockConnection {
     /* Connects the inter-block routing to the clock source at the specified coordinates */
     void create_switches(const ClockRRGraphBuilder& clock_graph, t_rr_edge_info_set* rr_edges_to_create) override;
     size_t estimate_additional_nodes() override;
-    int create_virtual_clock_network_sink_node(int x, int y);
+    RRNodeId create_virtual_clock_network_sink_node(int x, int y);
 };
 
 class ClockToClockConneciton : public ClockConnection {

vpr/src/route/clock_network_builders.cpp

@@ -346,10 +346,11 @@ int ClockRib::create_chanx_wire(int x_start,
     /* Add the node to spatial lookup */
     auto& rr_graph = (*rr_nodes);
     RRNodeId chanx_node = RRNodeId(node_index);
+    /* 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, iy, ix, rr_graph.node_type(chanx_node), rr_graph.node_ptc_num(chanx_node), SIDES[0]);
+            rr_graph_builder.node_lookup().add_node(chanx_node, iy, ix, rr_graph.node_type(chanx_node), rr_graph.node_ptc_num(chanx_node));
         }
     }
 
@@ -651,9 +652,10 @@ int ClockSpine::create_chany_wire(int y_start,
     /* Add the node to spatial lookup */
     auto& rr_graph = (*rr_nodes);
     RRNodeId chany_node = RRNodeId(node_index);
+    /* TODO: Will replace these codes with an API add_node_to_all_locs() of RRGraphBuilder */
     for (int ix = rr_graph.node_xlow(chany_node); ix <= rr_graph.node_xhigh(chany_node); ++ix) {
         for (int iy = rr_graph.node_ylow(chany_node); iy <= rr_graph.node_yhigh(chany_node); ++iy) {
-            rr_graph_builder.node_lookup().add_node(chany_node, ix, iy, rr_graph.node_type(chany_node), rr_graph.node_ptc_num(chany_node), SIDES[0]);
+            rr_graph_builder.node_lookup().add_node(chany_node, ix, iy, rr_graph.node_type(chany_node), rr_graph.node_ptc_num(chany_node));
         }
     }
 

vpr/src/route/rr_edge.h

@@ -1,6 +1,7 @@
 #ifndef RR_EDGE_H
 #define RR_EDGE_H
 
+/* TODO: MUST change the node id to RRNodeId before refactoring is finished! */
 struct t_rr_edge_info {
     t_rr_edge_info(int from, int to, short type) noexcept
         : from_node(from)
@@ -22,4 +23,4 @@ struct t_rr_edge_info {
 
 typedef std::vector<t_rr_edge_info> t_rr_edge_info_set;
 
-#endif /* RR_EDGE */
+#endif /* RR_EDGE */