verilog-to-routing / vtr-verilog-to-routing

Verilog to Routing -- Open Source CAD Flow for FPGA Research
https://verilogtorouting.org
Other
1.02k stars 396 forks source link

[SPECRand] Uninitialized Induction Variable in spec_rand.cpp #2831

Closed AlexandreSinger closed 3 days ago

AlexandreSinger commented 4 days ago

In the GCC13 build of VTR, we are getting the following warning from spec_rand.cpp:

image

This code can be found in VTR here: https://github.com/verilog-to-routing/vtr-verilog-to-routing/blob/ec23fea1f56d52a0dcd94787f3ee81517636671c/libs/libvtrutil/src/specrand.cpp#L122-L125

Leaving the induction variable uninitialized is undefined behavior, I believe; however, the compiler is likely initializing it to zero for us. The fix for this is to simply update it to size_t kk = 0 in the preamble of the for loop; however, since this file is from outside of the VTR repo I am not sure if we can change this file.

AlexandreSinger commented 4 days ago

@heshpdx Are we able to modify this file on the VTR side with this fix? Or does the change need to be done elsewhere first?

heshpdx commented 4 days ago

Feel free to edit. This is already fixed in my tree. Sorry for not sharing!

heshpdx commented 4 days ago

I dug up these edits I made to fix other warnings. Sharing in case it helps you.

diff --git a/src/vtr-vpr/libs/EXTERNAL/libtatum/libtatum/tatum/tags/TimingTags.hpp b/src/vtr-vpr/libs/EXTERNAL/libtatum/libtatum/tatum/tags/TimingTags.hpp
index a52ff1d..ee53efe 100644
--- a/src/vtr-vpr/libs/EXTERNAL/libtatum/libtatum/tatum/tags/TimingTags.hpp
+++ b/src/vtr-vpr/libs/EXTERNAL/libtatum/libtatum/tatum/tags/TimingTags.hpp
@@ -123,7 +123,7 @@ class TimingTags {
                 friend bool operator!=(Iterator a, Iterator b) { return a.p_ != b.p_; }

                 reference operator*() { return *p_; }
-                const reference operator*() const { return *p_; } //Required for MSVC (gcc/clang are fine with only the non-cost version)
+                reference operator*() const { return *p_; } //Required for MSVC (gcc/clang are fine with only the non-const version)
                 pointer operator->() { return p_; }
                 reference operator[](size_t n) { return *(p_ + n); }

diff --git a/src/vtr-vpr/vpr/src/base/netlist_writer.cpp b/src/vtr-vpr/vpr/src/base/netlist_writer.cpp
index e14457c..2065a4d 100644
--- a/src/vtr-vpr/vpr/src/base/netlist_writer.cpp
+++ b/src/vtr-vpr/vpr/src/base/netlist_writer.cpp
@@ -2263,7 +2263,7 @@ class MergedNetlistWriterVisitor : public NetlistWriterVisitor {
         return io_name;
     }

-    void print_primary_io(int depth) {
+    void print_primary_io(int depth) override {
         //Primary Inputs
         for (auto iter = inputs_.begin(); iter != inputs_.end(); ++iter) {
             //verilog_os_ << indent(depth + 1) << "input " << escape_verilog_identifier(*iter);
@@ -2292,7 +2292,7 @@ class MergedNetlistWriterVisitor : public NetlistWriterVisitor {
         }
     }

-    void print_assignments(int depth) {
+    void print_assignments(int depth) override {
         verilog_os_ << "\n";
         verilog_os_ << indent(depth + 1) << "//IO assignments\n";
         for (auto& assign : assignments_) {

diff --git a/src/vtr-vpr/vpr/src/base/setup_noc.cpp b/src/vtr-vpr/vpr/src/base/setup_noc.cpp
index 3bc3672..50d25cc 100644
--- a/src/vtr-vpr/vpr/src/base/setup_noc.cpp
+++ b/src/vtr-vpr/vpr/src/base/setup_noc.cpp
@@ -148,7 +148,7 @@ void create_noc_routers(const t_noc_inf& noc_info, NocStorage* noc_model, std::v
     // go through each user desctibed router in the arch file and assign it to a physical router on the FPGA
     for (auto logical_router = noc_info.router_list.begin(); logical_router != noc_info.router_list.end(); logical_router++) {
         // assign the shortest distance to a large value (this is done so that the first distance calculated and we can replace this)
-        shortest_distance = LLONG_MAX;
+        shortest_distance = (double)LLONG_MAX;

         // get position of the current logical router
         curr_logical_router_position_x = logical_router->device_x_position;

diff --git a/src/vtr-vpr/libs/librrgraph/src/base/rr_graph_utils.cpp b/src/vtr-vpr/libs/librrgraph/src/base/rr_graph_utils.cpp
index 0641e75..e57c9fa 100644
--- a/src/vtr-vpr/libs/librrgraph/src/base/rr_graph_utils.cpp
+++ b/src/vtr-vpr/libs/librrgraph/src/base/rr_graph_utils.cpp
@@ -115,6 +115,7 @@ vtr::vector<RRNodeId, std::vector<RREdgeId>> get_fan_in_list(const RRGraphView&
     //Walk the graph and increment fanin on all dwnstream nodes
     rr_graph.rr_nodes().for_each_edge(
         [&](RREdgeId edge, RRNodeId src, RRNodeId sink) -> void {
+            (void) src;
             node_fan_in_list[sink].push_back(edge);
         });
AlexandreSinger commented 4 days ago

Thank you so much Mahesh! I have updated spec_rand.cpp file and I went through all of your changes above and ensured that upstream has these changes as well!