RR Edge ID Verification #3164

amin1377 · 2025-06-24T22:41:26Z

Description

In a .route file, each edge between two nodes includes a switch ID, which is used to retrieve the electrical characteristics of that edge. When reading the .route file, this switch ID is compared against the corresponding switch ID from the RR Graph for consistency. If they do not match, an error is raised.

This consistency check is generally helpful. However, when analyzing multiple timing corners, each corner may have a different RR Graph with varying timing information (graph topologies are the same). As a result, the number and ordering of switch types may differ between corners. To allow reuse of the same .route file for fair comparisons across different timing corners, this strict verification can become a limitation.

To address this issue, a new CLI parameter verify_rr_switch_id has been introduced. By default, it is set to on, enforcing the consistency check. When set to off, the switch ID from the .route file is ignored, and the switch ID from the RR Graph is used instead.

soheilshahrouz

Thanks @amin1377.

I left a few comment for improving the readability a bit.

soheilshahrouz · 2025-06-25T15:33:53Z

vpr/src/base/read_options.cpp

@@ -1637,6 +1637,16 @@ argparse::ArgumentParser create_arg_parser(const std::string& prog_name, t_optio
        .default_value("on")
        .show_in(argparse::ShowIn::HELP_ONLY);

+    gen_grp.add_argument<bool, ParseOnOff>(args.verify_rr_switch_id, "--verify_rr_switch_id")
+        .help(
+            "Verify that the switch IDs in the routing file are consistent with those in the RR Graph."


Add a space or '\n' at the end of each line.

document this new option in the rst file

soheilshahrouz · 2025-06-25T15:36:43Z

vpr/src/base/read_options.cpp

@@ -1637,6 +1637,16 @@ argparse::ArgumentParser create_arg_parser(const std::string& prog_name, t_optio
        .default_value("on")
        .show_in(argparse::ShowIn::HELP_ONLY);

+    gen_grp.add_argument<bool, ParseOnOff>(args.verify_rr_switch_id, "--verify_rr_switch_id")


If this is a routing option, add it to route_grp

soheilshahrouz · 2025-06-25T15:38:03Z

vpr/src/base/read_options.h

@@ -73,6 +73,7 @@ struct t_options {
    argparse::ArgValue<e_timing_update_type> timing_update_type;
    argparse::ArgValue<bool> CreateEchoFile;
    argparse::ArgValue<bool> verify_file_digests;
+    argparse::ArgValue<bool> verify_rr_switch_id;


I think this option is only applicable to the routing stage and its variable should move there

soheilshahrouz · 2025-06-25T15:40:07Z

vpr/src/base/read_route.cpp

-static void process_route(const Netlist<>& net_list, std::ifstream& fp, const char* filename, int& lineno, bool is_flat);
-static void process_nodes(const Netlist<>& net_list, std::ifstream& fp, ClusterNetId inet, const char* filename, int& lineno);
-static void process_nets(const Netlist<>& net_list, std::ifstream& fp, ClusterNetId inet, std::string name, std::vector<std::string> input_tokens, const char* filename, int& lineno, bool is_flat);
+static void process_route(const Netlist<>& net_list,


The definition of some of these have doxygen comments. Can you move them to the declaration?

Thank you for breaking long lines. Can you do the same for the definition of these functions?

soheilshahrouz · 2025-06-25T15:41:27Z

vpr/src/base/read_route.cpp

+                         bool verify_rr_switch_id,
+                         bool is_flat);
+
+static void update_rr_switch_id(t_trace* trace, std::set<int>& seen_rr_nodes);


Doxygen comment

soheilshahrouz · 2025-06-25T16:27:58Z

vpr/src/base/read_route.cpp

+            //Verify that the next element (branch point) has been already seen in the traceback so far
+            if (!seen_rr_nodes.count(next->index)) {
+                VPR_FATAL_ERROR(VPR_ERROR_ROUTE, "Traceback branch point %d not found", next->index);
+            } else {


Is this else statement necessary?
I think we can remove it. Then, both branches of if (trace->iswitch == OPEN) call update_rr_switch_id(next, seen_rr_nodes). So, we can move it outside the conditional statement.

soheilshahrouz · 2025-06-25T16:30:24Z

vpr/src/base/read_route.cpp

+            //Check there is an edge connecting trace and next
+
+            auto& device_ctx = g_vpr_ctx.device();
+            const auto& rr_graph = device_ctx.rr_graph;


The only usage of device_ctx is to access rr_graph.
const auto& rr_graph = g_vpr_ctx.device().rr_graph would be more concise.

soheilshahrouz · 2025-06-25T16:32:24Z

vpr/src/base/read_route.cpp

+            }
+        } else { //Midway along branch
+
+            //Check there is an edge connecting trace and next


To be consistent with the coding style guide, add a space after //

soheilshahrouz · 2025-06-25T16:39:07Z

vpr/src/base/vpr_types.h

@@ -1343,6 +1343,8 @@ struct t_router_opts {

    bool with_timing_analysis;

+    bool verify_rr_switch_id;


doxygen comment

soheilshahrouz · 2025-06-25T16:45:35Z

vpr/src/base/read_route.cpp

+            if (!found) {
+                VPR_FATAL_ERROR(VPR_ERROR_ROUTE, "Traceback no RR edge between RR nodes %d -> %d\n", trace->index, next->index);
+            }
+            //Recurse


I think an iterative function that uses std::stack is easier to understand and eliminates the possiblity of stack overflow.
With an iterative function, you don't need to pass std::set as an argument, making the function self-contained.

amin1377 added 3 commits June 24, 2025 17:18

[vpr][CLI] add verify_rr_switch_id option

44b1948

[vpr][route] pass verify_rr_switch_id to relevant functions

c1f697e

[vpr][base] add update_rr_switch_id

a5899e3

amin1377 requested review from vaughnbetz and soheilshahrouz June 24, 2025 22:41

github-actions bot added VPR VPR FPGA Placement & Routing Tool lang-cpp C/C++ code labels Jun 24, 2025

soheilshahrouz requested changes Jun 25, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

RR Edge ID Verification #3164

RR Edge ID Verification #3164

Uh oh!

amin1377 commented Jun 24, 2025

Uh oh!

soheilshahrouz left a comment

Uh oh!

soheilshahrouz Jun 25, 2025

Uh oh!

soheilshahrouz Jun 25, 2025

Uh oh!

soheilshahrouz Jun 25, 2025

Uh oh!

soheilshahrouz Jun 25, 2025

Uh oh!

soheilshahrouz Jun 25, 2025

Uh oh!

soheilshahrouz Jun 25, 2025

Uh oh!

soheilshahrouz Jun 25, 2025

Uh oh!

soheilshahrouz Jun 25, 2025

Uh oh!

soheilshahrouz Jun 25, 2025

Uh oh!

soheilshahrouz Jun 25, 2025

Uh oh!

soheilshahrouz Jun 25, 2025

Uh oh!

soheilshahrouz Jun 25, 2025

Uh oh!

Uh oh!

		@@ -1343,6 +1343,8 @@ struct t_router_opts {

		bool with_timing_analysis;

		bool verify_rr_switch_id;

RR Edge ID Verification #3164

Are you sure you want to change the base?

RR Edge ID Verification #3164

Uh oh!

Conversation

amin1377 commented Jun 24, 2025

Description

Uh oh!

soheilshahrouz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!