apply PR comments

Author: soheilshahrouz

libs/libarchfpga/src/physical_types.cpp

@@ -86,10 +86,10 @@ bool t_rr_switch_inf::operator==(const t_rr_switch_inf& other) const {
 }
 
 std::size_t t_rr_switch_inf::Hasher::operator()(const t_rr_switch_inf& s) const {
-    std::size_t seed = 0;
+    std::size_t hash_val = 0;
 
-    auto hash_combine = [&seed](auto&& val) {
-        seed ^= std::hash<std::decay_t<decltype(val)>>{}(val) + 0x9e3779b9 + (seed << 6) + (seed >> 2);
+    auto hash_combine = [&hash_val](auto&& val) {
+        hash_val ^= std::hash<std::decay_t<decltype(val)>>{}(val) + 0x9e3779b9 + (hash_val << 6) + (hash_val >> 2);
     };
 
     hash_combine(s.R);
@@ -104,7 +104,7 @@ std::size_t t_rr_switch_inf::Hasher::operator()(const t_rr_switch_inf& s) const
     hash_combine(s.intra_tile);
     hash_combine(static_cast<int>(s.type()));
 
-    return seed;
+    return hash_val;
 }
 
 void t_rr_switch_inf::set_type(SwitchType type_val) {

libs/libarchfpga/src/physical_types.h

@@ -1929,6 +1929,12 @@ struct t_rr_switch_inf {
 
     bool operator==(const t_rr_switch_inf& other) const;
 
+    /**
+     * @brief Functor for computing a hash value for t_rr_switch_inf.
+     *
+     * This custom hasher enables the use of t_rr_switch_inf objects as keys
+     * in unordered containers such as std::unordered_map or std::unordered_set.
+     */
     struct Hasher {
         std::size_t operator()(const t_rr_switch_inf& s) const;
     };

libs/librrgraph/src/base/rr_graph_builder.h

@@ -264,7 +264,8 @@ class RRGraphBuilder {
         node_storage_.alloc_and_load_edges(rr_edges_to_create);
     }
 
-    /** @brief Overrides the associated switch for a given edge. */
+    /** @brief Overrides the associated switch for a given edge by
+     *         updating the edge to use the passed in switch. */
     inline void override_edge_switch(RREdgeId edge_id, RRSwitchId switch_id) {
         node_storage_.override_edge_switch(edge_id, switch_id);
     }

libs/librrgraph/src/base/rr_graph_storage.h

@@ -401,7 +401,7 @@ class t_rr_graph_storage {
     }
 
     /**
-     * @brief Retrieve the RREdgeId that connect the given source and sink nodes.
+     * @brief Retrieve the RREdgeId that connects the given source and sink nodes.
      *        If the given source/sink nodes are not connected, RREdgeId::INVALID() is returned.
      */
     RREdgeId edge_id(RRNodeId src, RRNodeId sink) const {
@@ -744,7 +744,8 @@ class t_rr_graph_storage {
      */
     void partition_edges(const vtr::vector<RRSwitchId, t_rr_switch_inf>& rr_switches);
 
-    /** @brief Overrides the associated switch for a given edge. */
+    /** @brief Overrides the associated switch for a given edge by
+     *         updating the edge to use the passed in switch. */
     void override_edge_switch(RREdgeId edge_id, RRSwitchId switch_id);
 
     /** @brief Validate that edge data is partitioned correctly.*/

libs/librrgraph/src/io/rr_graph_reader.cpp

@@ -20,6 +20,7 @@
 
 #include <fstream>
 #include <unordered_set>
+#include <utility>
 
 #include "vtr_time.h"
 #include "pugixml.hpp"
@@ -38,13 +39,11 @@
  *          (source_node_id, sink_node_id) Tdel
  *
  * @param line The line to parse.
- * @param overridden_Tdel Parsed override delay.
  * @param rr_graph The RR graph for edge lookup using source-sink nodes.
- * @return The RR edge whose attributes are to be overridden.
+ * @return A pair containing an RR edge and the overridden Tdel (intrinsic delay).
  */
-static RREdgeId process_rr_edge_override(const std::string& line,
-                                         float& overridden_Tdel,
-                                         const RRGraphView& rr_graph);
+static std::pair<RREdgeId, float> process_rr_edge_override(const std::string& line,
+                                                           const RRGraphView& rr_graph);
 
 /************************ Subroutine definitions ****************************/
 /* loads the given RR_graph file into the appropriate data structures
@@ -134,9 +133,8 @@ void load_rr_file(RRGraphBuilder* rr_graph_builder,
     }
 }
 
-static RREdgeId process_rr_edge_override(const std::string& line,
-                                         float& overridden_Tdel,
-                                         const RRGraphView& rr_graph) {
+static std::pair<RREdgeId, float> process_rr_edge_override(const std::string& line,
+                                                           const RRGraphView& rr_graph) {
     std::istringstream iss(line);
     char ch;
     RREdgeId edge_id;
@@ -165,11 +163,12 @@ static RREdgeId process_rr_edge_override(const std::string& line,
         VTR_LOG_ERROR("Invalid line format:  %s\n", line.c_str());
     }
 
+    float overridden_Tdel;
     if (!(iss >> overridden_Tdel)) {
         VTR_LOG_ERROR("Couldn't parse the overridden delay in this line:  %s\n", line.c_str());
     }
 
-    return edge_id;
+    return {edge_id, overridden_Tdel};
 }
 
 void load_rr_edge_delay_overrides(std::string_view filename,
@@ -184,20 +183,14 @@ void load_rr_edge_delay_overrides(std::string_view filename,
     }
 
     std::string line;
-    bool firstLine = true;
 
     while (std::getline(file, line)) {
-        if (firstLine) {
-            if (line.empty() || line[0] != '#') {
-                VTR_LOG_ERROR("Error: First line must start with #\n");
-            }
-            firstLine = false;
-            continue;  // Ignore first line
+        if (line[0] == '#') {
+            continue;  // Ignore lines starting with '#'
         }
 
         if (!line.empty()) {
-            float overridden_Tdel;
-            RREdgeId edge_id = process_rr_edge_override(line, overridden_Tdel, rr_graph);
+            const auto [edge_id, overridden_Tdel] = process_rr_edge_override(line, rr_graph);
             RRSwitchId curr_switch_id = (RRSwitchId)rr_graph.edge_switch(edge_id);
             t_rr_switch_inf switch_override_info = rr_graph.rr_switch_inf(curr_switch_id);
 

vpr/src/base/vpr_context.h

@@ -270,11 +270,13 @@ struct DeviceContext : public Context {
     t_clock_arch* clock_arch;
 
     /// @brief Name of rrgraph file read (if any).
-    /// Used to determine if the file is already loaded when reading rrgraph.
+    ///        Used to determine if the specified rr-graph file is already loaded,
+    /// so we can avoid redundant reading of the rr-graph
     std::string loaded_rr_graph_filename;
 
     /// @brief Name of rrgraph edge override file read (if any).
-    /// Used to determine if the file is already loaded when reading rrgraph edge overrides.
+    ///        Used to determine if the specified rr-graph edge override file is already loaded,
+    ///        so we can avoid redundant reading of the rr-graph
     std::string loaded_rr_edge_override_filename;
 
     /*******************************************************************

vpr/src/base/vpr_types.h

@@ -1315,7 +1315,7 @@ struct t_noc_opts {
  * Only important if the route_type is DETAILED.
  */
 struct t_det_routing_arch {
-    /// Should the tracks be uni-directional or bi-directional? (UDSD by AY)
+    /// Should the tracks be uni-directional or bi-directional?
     enum e_directionality directionality;
     int Fs;