[VTR][LOG] Improved Unused Arg Warning Suppression

Previously, VTR's logging used `sizeof` static casted to void in order
to suppress the unused argument warning when we want logs to be a NOP.

The `sizeof` operator may have some strange affects, but since the code
is NOP anyways thats not a huge deal. The reason for this change is that
intellisence that use clangd flag this as a potential issue causing
warnings.

This also exposes a potential issue where a LOG message is using a
function that may have memory effects, so it may not be cleaned up by
the compiler properly. The unused variables should be handled
explicitly.

Also moved static variables from the header file of vtr_log to the
implementation. No functional change, but just prevents these variables
from being present and accessible in every file that uses vtr_log (which
is alot).

Author: AlexandreSinger

libs/libvtrutil/src/vtr_log.cpp

@@ -1,6 +1,7 @@
-#include <string>
-#include <fstream>
 #include <cstdarg>
+#include <fstream>
+#include <string>
+#include <unordered_set>
 
 #include "vtr_util.h"
 #include "vtr_log.h"
@@ -19,6 +20,9 @@ void set_log_file(const char* filename) {
 
 } // namespace vtr
 
+static std::unordered_set<std::string> warnings_to_suppress;
+static std::string noisy_warn_log_file;
+
 void add_warnings_to_suppress(std::string function_name) {
     warnings_to_suppress.insert(function_name);
 }

libs/libvtrutil/src/vtr_log.h

@@ -1,7 +1,6 @@
 #ifndef VTR_LOG_H
 #define VTR_LOG_H
 #include <tuple>
-#include <unordered_set>
 #include <string>
 
 /**
@@ -99,21 +98,15 @@
     } while (false)
 
 /*
- * No-op version of logging macro which avoids unused parameter warnings.
+ * No-op version of logging macro. Drops all arguments so they have no
+ * performance effects.
  *
- * Note that to avoid unused parameter warnings we call sizeof() and cast
- * the result to void. sizeof is evaluated at compile time so there is no
- * run-time overhead.
- *
- * Also note the use of std::make_tuple to ensure all arguments in VA_ARGS
- * are used.
+ * Note: To prevent unused variable warnings, users of this logging feature
+ *       should handle the case when debugging is off. This should be done
+ *       explicitly to prevent hidden function calls that do not get optimized.
  */
 #define VTR_LOGVF_NOP(expr, file, line, ...)                     \
     do {                                                         \
-        static_cast<void>(sizeof(expr));                         \
-        static_cast<void>(sizeof(file));                         \
-        static_cast<void>(sizeof(line));                         \
-        static_cast<void>(sizeof(std::make_tuple(__VA_ARGS__))); \
     } while (false)
 
 // Debug logging macros
@@ -142,9 +135,6 @@ void set_log_file(const char* filename);
 
 } // namespace vtr
 
-static std::unordered_set<std::string> warnings_to_suppress;
-static std::string noisy_warn_log_file;
-
 /**
  * @brief The following data structure and functions allow to suppress noisy warnings and direct them into an external file, if specified.
  */

vpr/src/route/connection_router.cpp

@@ -193,7 +193,6 @@ t_heap* ConnectionRouter<Heap>::timing_driven_route_connection_from_heap(RRNodeI
         VTR_LOGV_DEBUG(router_debug_, "  Initial heap empty (no source)\n");
     }
 
-    const auto& device_ctx = g_vpr_ctx.device();
     auto& route_ctx = g_vpr_ctx.mutable_routing();
 
     t_heap* cheapest = nullptr;
@@ -217,6 +216,8 @@ t_heap* ConnectionRouter<Heap>::timing_driven_route_connection_from_heap(RRNodeI
             if (rcv_path_manager.is_enabled()) {
                 rcv_path_manager.insert_backwards_path_into_traceback(cheapest->path_data, cheapest->cost, cheapest->backward_path_cost, route_ctx);
             }
+            const auto& device_ctx = g_vpr_ctx.device();
+            static_cast<void>(device_ctx); // Ignore unused variable when logging is disabled.
             VTR_LOGV_DEBUG(router_debug_, "  Found target %8d (%s)\n", inode, describe_rr_node(device_ctx.rr_graph, device_ctx.grid, device_ctx.rr_indexed_data, inode, is_flat_).c_str());
             break;
         }
@@ -525,7 +526,6 @@ void ConnectionRouter<Heap>::timing_driven_add_to_heap(const t_conn_cost_params&
                                                        RRNodeId to_node,
                                                        const RREdgeId from_edge,
                                                        RRNodeId target_node) {
-    const auto& device_ctx = g_vpr_ctx.device();
     t_heap next;
 
     // Initalize RCV data struct if needed, otherwise it's set to nullptr
@@ -557,6 +557,8 @@ void ConnectionRouter<Heap>::timing_driven_add_to_heap(const t_conn_cost_params&
     float new_back_cost = next.backward_path_cost;
 
     if (new_total_cost < best_total_cost && ((rcv_path_manager.is_enabled()) || (new_back_cost < best_back_cost))) {
+        const auto& device_ctx = g_vpr_ctx.device();
+        static_cast<void>(device_ctx); // Ignore unused variable when logging is disabled.
         VTR_LOGV_DEBUG(router_debug_, "      Expanding to node %d (%s)\n", to_node,
                        describe_rr_node(device_ctx.rr_graph,
                                         device_ctx.grid,
@@ -596,6 +598,8 @@ void ConnectionRouter<Heap>::timing_driven_add_to_heap(const t_conn_cost_params&
                             rr_graph_);
 
     } else {
+        const auto& device_ctx = g_vpr_ctx.device();
+        static_cast<void>(device_ctx); // Ignore unused variable when logging is disabled.
         VTR_LOGV_DEBUG(router_debug_, "      Didn't expand to %d (%s)\n", to_node, describe_rr_node(device_ctx.rr_graph, device_ctx.grid, device_ctx.rr_indexed_data, to_node, is_flat_).c_str());
         VTR_LOGV_DEBUG(router_debug_, "        Prev Total Cost %g Prev back Cost %g \n", best_total_cost, best_back_cost);
         VTR_LOGV_DEBUG(router_debug_, "        New Total Cost %g New back Cost %g \n", new_total_cost, new_back_cost);
@@ -783,12 +787,13 @@ void ConnectionRouter<Heap>::evaluate_timing_driven_node_costs(t_heap* to,
 
         total_cost = compute_node_cost_using_rcv(cost_params, to_node, target_node, to->path_data->backward_delay, to->path_data->backward_cong, to->R_upstream);
     } else {
-        const auto& device_ctx = g_vpr_ctx.device();
         //Update total cost
         float expected_cost = router_lookahead_.get_expected_cost(to_node,
                                                                   target_node,
                                                                   cost_params,
                                                                   to->R_upstream);
+        const auto& device_ctx = g_vpr_ctx.device();
+        static_cast<void>(device_ctx);  // Ignore unused variable when logging is disabled.
         VTR_LOGV_DEBUG(router_debug_ && !std::isfinite(expected_cost),
                        "        Lookahead from %s (%s) to %s (%s) is non-finite, expected_cost = %f, to->R_upstream = %f\n",
                        rr_node_arch_name(to_node, is_flat_).c_str(),
@@ -871,7 +876,6 @@ void ConnectionRouter<Heap>::add_route_tree_node_to_heap(
     RRNodeId target_node,
     const t_conn_cost_params& cost_params,
     const t_bb& net_bb) {
-    const auto& device_ctx = g_vpr_ctx.device();
     const RRNodeId inode = rt_node.inode;
     float backward_path_cost = cost_params.criticality * rt_node.Tdel;
     float R_upstream = rt_node.R_upstream;
@@ -894,6 +898,8 @@ void ConnectionRouter<Heap>::add_route_tree_node_to_heap(
                                                                      target_node,
                                                                      cost_params,
                                                                      R_upstream);
+        const auto& device_ctx = g_vpr_ctx.device();
+        static_cast<void>(device_ctx); // Ignore unused variable when logging is disabled.
         VTR_LOGV_DEBUG(router_debug_, "  Adding node %8d to heap from init route tree with cost %g (%s)\n",
                        inode,
                        tot_cost,

vpr/src/route/route_net.tpp

@@ -414,14 +414,15 @@ inline NetResultFlags route_sink(ConnectionRouter& router,
                                  const std::vector<std::unordered_map<RRNodeId, int>>& choking_spots,
                                  bool is_flat,
                                  const t_bb& net_bb) {
-    const auto& device_ctx = g_vpr_ctx.device();
     auto& route_ctx = g_vpr_ctx.mutable_routing();
 
     NetResultFlags flags;
 
     profiling::sink_criticality_start();
 
     RRNodeId sink_node = route_ctx.net_rr_terminals[net_id][target_pin];
+    const auto& device_ctx = g_vpr_ctx.device();
+    static_cast<void>(device_ctx);
     VTR_LOGV_DEBUG(f_router_debug, "Net %zu Target %d (%s)\n", size_t(net_id), itarget, describe_rr_node(device_ctx.rr_graph, device_ctx.grid, device_ctx.rr_indexed_data, sink_node, is_flat).c_str());
 
     router.clear_modified_rr_node_info();

vpr/src/route/router_lookahead_extended_map.cpp

@@ -243,10 +243,9 @@ std::pair<float, float> ExtendedMapLookahead::get_expected_delay_and_cong(RRNode
     float expected_cost = expected_delay_cost + expected_cong_cost;
 
     VTR_LOGV_DEBUG(f_router_debug, "Requested lookahead from node %d to %d\n", size_t(from_node), size_t(to_node));
-    const std::string& segment_name = rr_graph.rr_segments(RRSegmentId(from_seg_index)).name;
     VTR_LOGV_DEBUG(f_router_debug, "Lookahead returned %s (%d) with distance (%d, %d)\n",
-                   segment_name.c_str(), from_seg_index,
-                   dx, dy);
+                   rr_graph.rr_segments(RRSegmentId(from_seg_index)).name.c_str(),
+                   from_seg_index, dx, dy);
     VTR_LOGV_DEBUG(f_router_debug, "Lookahead delay: %g\n", expected_delay_cost);
     VTR_LOGV_DEBUG(f_router_debug, "Lookahead congestion: %g\n", expected_cong_cost);
     VTR_LOGV_DEBUG(f_router_debug, "Criticality: %g\n", params.criticality);

vpr/src/route/router_lookahead_map_utils.cpp

@@ -441,6 +441,7 @@ t_src_opin_delays compute_router_src_opin_lookahead(bool is_flat) {
                             }
                         }
                         if (reachable_wire_found) {
+                            static_cast<void>(is_flat); // Ignore unused variable when logging is disabled.
                             VTR_LOGV_DEBUG(f_router_debug, "Found no reachable wires from %s (%s) at (%d,%d,%d)\n",
                                            rr_node_typename[rr_type],
                                            rr_node_arch_name(node_id, is_flat).c_str(),