lookahead: addressed review comments

Signed-off-by: Alessandro Comodi <[email protected]>

Author: acomodi

libs/libvtrutil/src/vtr_geometry.h

@@ -149,6 +149,12 @@ Rect<T> bounding_box(const Rect<T>& lhs, const Rect<T>& rhs);
 template<typename T, typename std::enable_if<std::is_integral<T>::value>::type...>
 Point<T> sample(const vtr::Rect<T>& r, T x, T y, T d);
 
+// clamps v to be between low (lo) and high (hi), inclusive.
+template<class T>
+static constexpr const T& clamp(const T& v, const T& lo, const T& hi) {
+    return std::min(std::max(v, lo), hi);
+}
+
 //A 2D line
 template<class T>
 class Line {

vpr/src/route/router_lookahead_cost_map.cpp

@@ -3,6 +3,7 @@
 #include "router_lookahead_map_utils.h"
 #include "globals.h"
 #include "echo_files.h"
+#include "vtr_geometry.h"
 
 #ifdef VTR_ENABLE_CAPNPROTO
 #    include "capnp/serialize.h"
@@ -30,19 +31,13 @@ static int manhattan_distance(const vtr::Point<int>& a, const vtr::Point<int>& b
     return abs(b.x() - a.x()) + abs(b.y() - a.y());
 }
 
-// clamps v to be between low (lo) and high (hi), inclusive.
-template<class T>
-static constexpr const T& clamp(const T& v, const T& lo, const T& hi) {
-    return std::min(std::max(v, lo), hi);
-}
-
 template<typename T>
 static vtr::Point<T> closest_point_in_rect(const vtr::Rect<T>& r, const vtr::Point<T>& p) {
     if (r.empty()) {
         return vtr::Point<T>(0, 0);
     } else {
-        return vtr::Point<T>(clamp<T>(p.x(), r.xmin(), r.xmax() - 1),
-                             clamp<T>(p.y(), r.ymin(), r.ymax() - 1));
+        return vtr::Point<T>(vtr::clamp<T>(p.x(), r.xmin(), r.xmax() - 1),
+                             vtr::clamp<T>(p.y(), r.ymin(), r.ymax() - 1));
     }
 }
 
@@ -57,7 +52,11 @@ void CostMap::set_counts(size_t seg_count) {
     seg_count_ = seg_count;
 }
 
-// cached node -> segment map
+/**
+ * @brief Gets the segment index corresponding to a node index
+ * @param from_node_ind node index
+ * @return segment index corresponding to the input node
+ */
 int CostMap::node_to_segment(int from_node_ind) const {
     const auto& device_ctx = g_vpr_ctx.device();
 
@@ -67,6 +66,26 @@ int CostMap::node_to_segment(int from_node_ind) const {
     return device_ctx.rr_indexed_data[from_cost_index].seg_index;
 }
 
+/**
+ * @brief Penalizes a specific cost entry based on its distance to the cost map bounds.
+ * @param entry cost entry to penalize
+ * @param distance distance to the cost map bounds (can be zero in case the entry is within the bounds)
+ * @param penalty penalty factor relative to the current segment type
+ *
+ * If a specific (delta_x, delta_y) coordinates pair falls out of a segment bounding box, the returned cost is a result of the following equation:
+ *
+ * delay + distance * penalty * PENALTY_FACTOR
+ *
+ * delay    : delay of the closest point in the bounding box for the specific wire segment
+ * distance : this can assume two values:
+ *              - 0     : in case the deltas fall within the bounding box (in this case no penalty is added)
+ *              - non-0 : in case the point is out of the bounding box, the value is the manhattan distance between the point and the closest point within the bounding box.
+ * penalty  : value determined when building the lookahead and relative to a specific segment type (meaning there is one penalty value for each segment type).
+ *            The base delay_penalty value is a calculated as follows:
+ *              (max_delay - min_delay) / max(1, manhattan_distance(max_location, min_location))
+ *
+ * PENALTY_FACTOR: impact of the penalty on the total delay cost.
+ */
 static util::Cost_Entry penalize(const util::Cost_Entry& entry, int distance, float penalty) {
     penalty = std::max(penalty, PENALTY_MIN);
     return util::Cost_Entry(entry.delay + distance * penalty * PENALTY_FACTOR,
@@ -202,6 +221,15 @@ void CostMap::set_cost_map(const util::RoutingCosts& delay_costs, const util::Ro
     //
     // In case the lookahead is queried to return a cost that is outside of the bounding box, the closest
     // cost within the bounding box is returned, with the addition of a calculated penalty cost.
+    //
+    // The bounding boxes do have two dimensions and are specified as matrices.
+    // The first dimension though is unused and set to be constant as it is required to keep consistency
+    // with the lookahead map. The extended lookahead map and the normal one index the various costs as follows:
+    //      - Lookahead map         : [0..chan_index][seg_index][dx][dy]
+    //      - Extended lookahead map: [0][seg_index][dx][dy]
+    //
+    // The extended lookahead map does not require the first channel index distinction, therefore the first dimension
+    // remains unused.
     vtr::Matrix<vtr::Rect<int>> bounds({1, seg_count_});
     for (const auto& entry : delay_costs) {
         bounds[0][entry.first.seg_index].expand_bounding_box(vtr::Rect<int>(entry.first.delta));

vpr/src/route/router_lookahead_cost_map.h

@@ -63,6 +63,8 @@ class CostMap {
      * @param cy y location of the cost map entry that needs to be filled
      * @param bounds bounds of the cost map corresponding to the current segment
      * @return A pair containing the nearby cost entry and the distance from the current location to the narby cost entry found.
+     *
+     * The coordinates identifying the cost map location to fill (cx, cy) need to fall within the bounding box provided as input (bounds). If the (cx, cy) point falls out of the bounds, a default cost entry is returned instead.
      */
     std::pair<util::Cost_Entry, int> get_nearby_cost_entry(const vtr::NdMatrix<util::Cost_Entry, 2>& matrix, int cx, int cy, const vtr::Rect<int>& bounds);
 

vpr/src/route/router_lookahead_extended_map.cpp

@@ -45,7 +45,17 @@
 // Don't continue storing a path after hitting a lower-or-same cost entry.
 static constexpr bool BREAK_ON_MISS = false;
 
-///@brief Threshold indicating the minimum number of paths to sample for each point in a sample region
+/**
+ * @brief Threshold indicating the minimum number of paths to sample for each point in a sample region
+ *
+ * Each sample region contains a set of points containing the starting nodes from which the Dijkstra expansions
+ * are performed.
+ * Each node produces a number N of paths that are being explored exhaustively until no more expansions are possible,
+ * and the number of paths for each Dijkstra run is added to the total number of paths found when computing a sample region
+ * In case there are still points in a region that were not picked to perform the Dijkstra expansion, and the
+ * MIN_PATH_COUNT threshold has been reached, the sample region is intended to be complete and the computation of a next
+ * sample region begins.
+ */
 static constexpr int MIN_PATH_COUNT = 1000;
 
 template<typename Entry>
@@ -193,6 +203,16 @@ float ExtendedMapLookahead::get_map_cost(RRNodeId from_node,
     float expected_delay = cost_entry.delay;
     float expected_congestion = cost_entry.congestion;
 
+    // The CHAN -> IPIN delay was removed from the cost map calculation, as it might tamper the addition
+    // of a smaller cost to this node location. This might happen as not all the CHAN -> IPIN connection
+    // have a delay, therefore, a different cost than the correct one might have been added to the cost
+    // map location of the input node.
+    //
+    // The CHAN -> IPIN delay gets re-added to the final calculation as it effectively is a valid delay
+    // to reach the destination.
+    //
+    // TODO: Capture this delay as a funtion of both the current wire type and the ipin to have a more
+    //       realistic excpected cost returned.
     float site_pin_delay = this->get_chan_ipin_delays(to_node);
     expected_delay += site_pin_delay;
 
@@ -549,6 +569,8 @@ float ExtendedMapLookahead::get_expected_cost(
         return get_map_cost(
             RRNodeId(current_node), RRNodeId(target_node), params.criticality);
     } else if (rr_type == IPIN) { /* Change if you're allowing route-throughs */
+        // This is to return only the cost between the IPIN and SINK. No need to
+        // query the cost map, as the routing of this connection is almost done.
         return (device_ctx.rr_indexed_data[SINK_COST_INDEX].base_cost);
     } else { /* Change this if you want to investigate route-throughs */
         return (0.);

vpr/src/route/router_lookahead_extended_map.h

@@ -22,7 +22,7 @@ class ExtendedMapLookahead : public RouterLookahead {
      * @param from_node starting node
      * @param delta_x x delta value that is the distance between the current node to the target node
      * @param delta_y y delta value that is the distance between the current node to the target node
-     * @param criticality_fac criticality of the current connection
+     * @param criticality_fac criticality of the current connection between 0 (all congestion) and 1 (all timing)
      * @return expected cost to get to the destination
      */
     float get_src_opin_cost(RRNodeId from_node, int delta_x, int delta_y, float criticality_fac) const;
@@ -31,6 +31,14 @@ class ExtendedMapLookahead : public RouterLookahead {
      * @brief Returns the CHAN -> IPIN delay that gets added to the final expected delay
      * @param to_node target node for which we query the IPIN delay
      * @return IPIN delay relative to the input destination node
+     *
+     * The CHAN -> IPIN delay was removed from the cost map calculation, as it might tamper the addition
+     * of a smaller cost to this node location. This might happen as not all the CHAN -> IPIN connection
+     * have a delay, therefore, a different cost than the correct one might have been added to the cost
+     * map location of the input node.
+     * 
+     * The CHAN -> IPIN delay gets re-added to the final calculation as it effectively is a valid delay
+     * to reach the destination.
      */
     float get_chan_ipin_delays(RRNodeId to_node) const;