Address PR comments

Author: AmirhosseinPoolad

libs/librrgraph/src/base/rr_graph_storage.cpp

@@ -138,15 +138,12 @@ void t_rr_graph_storage::init_fan_in() {
 }
 
 
-// Functor for sorting edges according to destination node's ID.
+/// Functor for sorting edges according to destination node's ID.
 class edge_compare_dest_node {
 public:
     edge_compare_dest_node(const t_rr_graph_storage& rr_graph_storage) : rr_graph_storage_(rr_graph_storage) {}
 
-    bool operator()(size_t lhs_idx, size_t rhs_idx) const {
-        RREdgeId lhs = RREdgeId(lhs_idx);
-        RREdgeId rhs = RREdgeId(rhs_idx);
-
+    bool operator()(RREdgeId lhs, RREdgeId rhs) const {
         RRNodeId lhs_dest_node = rr_graph_storage_.edge_sink_node(lhs);
         RRNodeId rhs_dest_node = rr_graph_storage_.edge_sink_node(rhs);
 
@@ -262,26 +259,24 @@ void t_rr_graph_storage::mark_edges_as_rr_switch_ids() {
     remapped_edges_ = true;
 }
 
-// Functor for sorting edges according to source node, with configurable edges coming first
+/// Functor for sorting edges according to source node, with configurable edges coming first
 class edge_compare_src_node_and_configurable_first {
   public:
     edge_compare_src_node_and_configurable_first(const vtr::vector<RRSwitchId, t_rr_switch_inf>& rr_switch_inf, const t_rr_graph_storage& rr_graph_storage)
         : rr_switch_inf_(rr_switch_inf),
           rr_graph_storage_(rr_graph_storage) {}
 
-    bool operator()(size_t lhs_idx, size_t rhs_idx) const {
-        RREdgeId lhs = RREdgeId(lhs_idx);
-        RREdgeId rhs = RREdgeId(rhs_idx);
+    bool operator()(RREdgeId lhs, RREdgeId rhs) const {
 
         RRNodeId lhs_dest_node = rr_graph_storage_.edge_sink_node(lhs);
         RRNodeId lhs_src_node = rr_graph_storage_.edge_source_node(lhs);
         RRSwitchId lhs_switch_type = RRSwitchId(rr_graph_storage_.edge_switch(lhs));
-        bool lhs_is_configurable = rr_switch_inf_[RRSwitchId(lhs_switch_type)].configurable();
+        bool lhs_is_configurable = rr_switch_inf_[lhs_switch_type].configurable();
 
         RRNodeId rhs_dest_node = rr_graph_storage_.edge_sink_node(rhs);
         RRNodeId rhs_src_node = rr_graph_storage_.edge_source_node(rhs);
         RRSwitchId rhs_switch_type = RRSwitchId(rr_graph_storage_.edge_switch(rhs));
-        bool rhs_is_configurable = rr_switch_inf_[RRSwitchId(rhs_switch_type)].configurable();
+        bool rhs_is_configurable = rr_switch_inf_[rhs_switch_type].configurable();
 
         return std::make_tuple(lhs_src_node, !lhs_is_configurable, lhs_dest_node, lhs_switch_type) < std::make_tuple(rhs_src_node, !rhs_is_configurable, rhs_dest_node, rhs_switch_type);
     }

libs/librrgraph/src/base/rr_graph_storage.h

@@ -825,25 +825,26 @@ class t_rr_graph_storage {
     void sort_edges(t_comp_func comparison_function) {
 
         size_t num_edges = edge_src_node_.size();
-        std::vector<size_t> edge_indices(num_edges, 0);
-
-        std::iota(edge_indices.begin(), edge_indices.end(), 0);
+        vtr::StrongIdRange<RREdgeId> edge_range(RREdgeId(0), RREdgeId(num_edges));
+        std::vector<RREdgeId> edge_indices(edge_range.begin(), edge_range.end());
 
         std::ranges::stable_sort(edge_indices, comparison_function);
 
-
-
         vtr::vector<RREdgeId, RRNodeId> new_edge_src_node_(num_edges, RRNodeId::INVALID());
         vtr::vector<RREdgeId, RRNodeId> new_edge_dest_node_(num_edges, RRNodeId::INVALID());
         vtr::vector<RREdgeId, short> new_edge_switch_(num_edges, LIBRRGRAPH_UNDEFINED_VAL);
         vtr::vector<RREdgeId, bool> new_edge_remapped_(num_edges, false);
 
         size_t new_index = 0;
-        for (size_t edge_index : edge_indices) {
-            new_edge_src_node_[RREdgeId(new_index)] = edge_src_node_[RREdgeId(edge_index)];
-            new_edge_dest_node_[RREdgeId(new_index)] = edge_dest_node_[RREdgeId(edge_index)];
-            new_edge_switch_[RREdgeId(new_index)] = edge_switch_[RREdgeId(edge_index)];
-            new_edge_remapped_[RREdgeId(new_index)] = edge_remapped_[RREdgeId(edge_index)];
+        for (RREdgeId edge_index : edge_indices) {
+    
+            RREdgeId new_edge_index = RREdgeId(new_index);
+
+            new_edge_src_node_[new_edge_index] = edge_src_node_[edge_index];
+            new_edge_dest_node_[new_edge_index] = edge_dest_node_[edge_index];
+            new_edge_switch_[new_edge_index] = edge_switch_[edge_index];
+            new_edge_remapped_[new_edge_index] = edge_remapped_[edge_index];
+            
             new_index++;
         }
 
@@ -898,9 +899,6 @@ class t_rr_graph_storage {
     }
 
   private:
-    friend class edge_compare_dest_node;
-    friend class edge_compare_src_node_and_configurable_first;
-
     /** @brief
      * Take allocated edges in edge_src_node_/ edge_dest_node_ / edge_switch_
      * sort, and assign the first edge for each

vpr/src/analytical_place/ap_draw_manager.cpp

@@ -46,4 +46,4 @@ void APDrawManager::update_graphics(unsigned int iteration, enum APDrawType draw
     (void)iteration;
     (void)draw_type;
 #endif
-}
+}

vpr/src/analytical_place/ap_draw_manager.h

@@ -27,7 +27,7 @@ enum class APDrawType {
  * with NO_GRAPHICS conditional compilation directives.
  */
 class APDrawManager {
-public:
+  public:
     /**
      * @brief Constructor initializes the draw manager with a reference to the
      *        current partial placement.

vpr/src/analytical_place/global_placer.cpp

@@ -356,7 +356,7 @@ PartialPlacement SimPLGlobalPlacer::place() {
     // Initialize graphics for analytical placement, setting the reference in
     // the draw state.
     APDrawManager draw_manager(p_placement);
-    
+
     // Run the global placer.
     for (size_t i = 0; i < max_num_iterations_; i++) {
         float iter_start_time = runtime_timer.elapsed_sec();
@@ -366,7 +366,7 @@ PartialPlacement SimPLGlobalPlacer::place() {
         solver_->solve(i, p_placement);
         float solver_end_time = runtime_timer.elapsed_sec();
         double lb_hpwl = p_placement.get_hpwl(ap_netlist_);
-        
+
         // Update graphics after analytical solver
         draw_manager.update_graphics(i, APDrawType::Solver);
 
@@ -375,7 +375,7 @@ PartialPlacement SimPLGlobalPlacer::place() {
         partial_legalizer_->legalize(p_placement);
         float legalizer_end_time = runtime_timer.elapsed_sec();
         double ub_hpwl = p_placement.get_hpwl(ap_netlist_);
-        
+
         // Update graphics after legalizer
         draw_manager.update_graphics(i, APDrawType::Legalizer);
 

vpr/src/draw/draw.cpp

@@ -298,7 +298,7 @@ void update_screen(ScreenUpdatePriority priority,
      * value controls whether or not the Proceed button must be clicked to   *
      * continue.  Saves the pic_on_screen_val to allow pan and zoom redraws. */
     t_draw_state* draw_state = get_draw_state_vars();
-    
+
     strcpy(draw_state->default_message, msg);
 
     if (!draw_state->show_graphics)
@@ -529,7 +529,7 @@ void set_initial_world() {
     initial_world = ezgl::rectangle(
         {-VISIBLE_MARGIN * draw_width, -VISIBLE_MARGIN * draw_height},
         {(1. + VISIBLE_MARGIN) * draw_width, (1. + VISIBLE_MARGIN)
-                                                 * draw_height});    
+                                                 * draw_height});
 }
 
 void set_initial_world_ap() {
@@ -539,7 +539,6 @@ void set_initial_world_ap() {
     const size_t grid_w = device_ctx.grid.width();
     const size_t grid_h = device_ctx.grid.height();
 
-
     float draw_width = static_cast<float>(grid_w);
     float draw_height = static_cast<float>(grid_h);
 

vpr/src/draw/draw.h

@@ -56,10 +56,10 @@ void update_screen(ScreenUpdatePriority priority,
 void init_draw_coords(float clb_width, const BlkLocRegistry& blk_loc_registry);
 
 /**
-* @brief Set the intial_world ezgl::rectangle for analytical placement
-* 
-* This function sets graphic initial dimensions so there are no gaps between blocks
-*/
+ * @brief Set the intial_world ezgl::rectangle for analytical placement
+ * 
+ * This function sets graphic initial dimensions so there are no gaps between blocks
+ */
 void set_initial_world_ap();
 
 /**

vpr/src/draw/draw_interposer.cpp

@@ -24,8 +24,7 @@ void draw_interposer_cuts(ezgl::renderer* g) {
         {draw_coords->tile_x.front() - draw_coords->get_tile_width(),
          draw_coords->tile_y.front() - draw_coords->get_tile_height()},
         {draw_coords->tile_x.back() + 2 * draw_coords->get_tile_width(),
-         draw_coords->tile_y.back() + 2 * draw_coords->get_tile_height()}
-    };
+         draw_coords->tile_y.back() + 2 * draw_coords->get_tile_height()}};
 
     g->set_color(ezgl::BLACK, 255);
     g->set_line_dash(ezgl::line_dash::asymmetric_5_3);
@@ -80,4 +79,4 @@ void draw_interposer_cuts(ezgl::renderer* g) {
     }
 }
 
-#endif
+#endif

vpr/src/draw/draw_interposer.h

@@ -21,4 +21,4 @@ class renderer;
 /// which side of the cut owns the channel.
 void draw_interposer_cuts(ezgl::renderer* g);
 
-#endif
+#endif

vpr/src/draw/draw_types.h

@@ -415,7 +415,7 @@ struct t_draw_state {
      */
     std::optional<std::reference_wrapper<const PartialPlacement>> ap_partial_placement_ref_;
 
-public:
+  public:
     // Set/clear/get the AP partial placement reference used during AP drawing
     void set_ap_partial_placement_ref(const PartialPlacement& p) { ap_partial_placement_ref_ = std::cref(p); }
     void clear_ap_partial_placement_ref() { ap_partial_placement_ref_.reset(); }