Skip to content

Add check route option #1288

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
May 7, 2020
Merged

Add check route option #1288

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
fe9e236
vpr options: added option to disable check_route
acomodi May 7, 2019
752d99e
add --quick_check_route, disables slow non-configurable edge check
HackerFoo Dec 13, 2019
f99eb2a
use vtr::ScopedStartFinishTimer
HackerFoo Dec 13, 2019
99cedb8
check_route: add option to disable, quick check or full check
acomodi Apr 28, 2020
caf76ca
check_route: move option within check route function
acomodi Apr 29, 2020
0d5dc85
check_route: add regression test and golden results
acomodi Apr 29, 2020
44f8f7d
check_route: move default option to 'full'
acomodi Apr 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion vpr/src/base/SetupVPR.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,14 +373,15 @@ static void SetupRouterOpts(const t_options& Options, t_router_opts* RouterOpts)
RouterOpts->initial_timing = Options.router_initial_timing;
RouterOpts->update_lower_bound_delays = Options.router_update_lower_bound_delays;
RouterOpts->first_iteration_timing_report_file = Options.router_first_iteration_timing_report_file;

RouterOpts->strict_checks = Options.strict_checks;

RouterOpts->write_router_lookahead = Options.write_router_lookahead;
RouterOpts->read_router_lookahead = Options.read_router_lookahead;

RouterOpts->router_heap = Options.router_heap;
RouterOpts->exit_after_first_routing_iteration = Options.exit_after_first_routing_iteration;

RouterOpts->check_route = Options.check_route;
}

static void SetupAnnealSched(const t_options& Options,
Expand Down
44 changes: 44 additions & 0 deletions vpr/src/base/read_options.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -858,6 +858,41 @@ struct ParseRouterHeap {
}
};

struct ParseCheckRoute {
ConvertedValue<e_check_route_option> from_str(std::string str) {
ConvertedValue<e_check_route_option> conv_value;
if (str == "off")
conv_value.set_value(e_check_route_option::OFF);
else if (str == "quick")
conv_value.set_value(e_check_route_option::QUICK);
else if (str == "full")
conv_value.set_value(e_check_route_option::FULL);
else {
std::stringstream msg;
msg << "Invalid conversion from '" << str << "' to e_check_route_option (expected one of: " << argparse::join(default_choices(), ", ") << ")";
conv_value.set_error(msg.str());
}
return conv_value;
}

ConvertedValue<std::string> to_str(e_check_route_option val) {
ConvertedValue<std::string> conv_value;
if (val == e_check_route_option::OFF)
conv_value.set_value("off");
else if (val == e_check_route_option::QUICK)
conv_value.set_value("quick");
else {
VTR_ASSERT(val == e_check_route_option::FULL);
conv_value.set_value("full");
}
return conv_value;
}

std::vector<std::string> default_choices() {
return {"off", "quick", "full"};
}
};

struct ParsePlaceEfforScaling {
ConvertedValue<e_place_effort_scaling> from_str(std::string str) {
ConvertedValue<e_place_effort_scaling> conv_value;
Expand Down Expand Up @@ -1887,6 +1922,15 @@ argparse::ArgumentParser create_arg_parser(std::string prog_name, t_options& arg
.default_value("off")
.show_in(argparse::ShowIn::HELP_ONLY);

route_timing_grp.add_argument<e_check_route_option, ParseCheckRoute>(args.check_route, "--check_route")
.help(
"Options to run check route in three different modes.\n"
" * off : check route is completely disabled.\n"
" * quick : runs check route with slow checks disabled.\n"
" * full : runs the full check route step.\n")
.default_value("quick")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd lean towards keeping the default for this option as full. That ensures we do the most thorough checking by default (which is what we want, for example in regular VTR). Particularly if #1289 narrows the run-time difference between quick and full.

If someone (e.g. symbiflow) want to use quick, I feel that should be an end-user/flow override.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, moved the option default back to full

.show_in(argparse::ShowIn::HELP_ONLY);

route_timing_grp.add_argument(args.router_debug_net, "--router_debug_net")
.help(
"Controls when router debugging is enabled for nets.\n"
Expand Down
1 change: 1 addition & 0 deletions vpr/src/base/read_options.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ struct t_options {
argparse::ArgValue<int> min_incremental_reroute_fanout;
argparse::ArgValue<bool> read_rr_edge_metadata;
argparse::ArgValue<bool> exit_after_first_routing_iteration;
argparse::ArgValue<e_check_route_option> check_route;

/* Timing-driven router options only */
argparse::ArgValue<float> astar_fac;
Expand Down
8 changes: 7 additions & 1 deletion vpr/src/base/vpr_api.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,13 @@ RouteStatus vpr_route_flow(t_vpr_setup& vpr_setup, const t_arch& arch) {
std::string graphics_msg;
if (route_status.success()) {
//Sanity check the routing
check_route(router_opts.route_type);
if (router_opts.check_route == e_check_route_option::QUICK) {
check_route(router_opts.route_type, /*quick=*/true);
} else if (router_opts.check_route == e_check_route_option::FULL) {
check_route(router_opts.route_type, /*quick=*/false);
} else {
VTR_LOG_WARN("The user disabled the check route step.");
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest moving this code inside check_route (passing in route_opts.check_route as an argument).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One reason I did put the check_route option check outside of check_route is that, if the option disables it, then check_route should not be called.

I thought it was cleaner to leave all the conditions of the check route option in one place

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's OK to put the check inside check_route and have it bail out early, something like:

void check_route(e_route_type route_type, e_check_route_option option) {
    if (option == e_check_route_option::OFF) {
        VTR_LOG_WARN("The user disabled the check route step.");
        return;
    }

   //Normal check route...
}

That keeps check_route() more re-usable and ensures the option is respected if more calls to check_route() are ever added in the future (e.g. after tweaking/fine-tuning an existing routing).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also like embedding the check inside check_route to make it all self contained.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, done. Also added a reg test with all the three options.

get_serial_num();

//Update status
Expand Down
8 changes: 8 additions & 0 deletions vpr/src/base/vpr_types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,12 @@ enum class e_balance_block_type_util {
AUTO
};

enum class e_check_route_option {
OFF,
QUICK,
FULL
};

/* Selection algorithm for selecting next seed */
enum class e_cluster_seed {
TIMING,
Expand Down Expand Up @@ -999,6 +1005,8 @@ struct t_router_opts {

e_heap_type router_heap;
bool exit_after_first_routing_iteration;

e_check_route_option check_route;
};

struct t_analysis_opts {
Expand Down
24 changes: 19 additions & 5 deletions vpr/src/route/check_route.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include "vtr_assert.h"
#include "vtr_log.h"
#include "vtr_memory.h"
#include "vtr_time.h"

#include "vpr_types.h"
#include "vpr_error.h"
Expand All @@ -27,12 +28,13 @@ static void reset_flags(ClusterNetId inet, bool* connected_to_route);
static void check_locally_used_clb_opins(const t_clb_opins_used& clb_opins_used_locally,
enum e_route_type route_type);

static void check_all_non_configurable_edges();
static bool check_non_configurable_edges(ClusterNetId net, const t_non_configurable_rr_sets& non_configurable_rr_sets);
static void check_net_for_stubs(ClusterNetId net);

/************************ Subroutine definitions ****************************/

void check_route(enum e_route_type route_type) {
void check_route(enum e_route_type route_type, bool quick) {
/* This routine checks that a routing: (1) Describes a properly *
* connected path for each net, (2) this path connects all the *
* pins spanned by that net, and (3) that no routing resources are *
Expand Down Expand Up @@ -66,8 +68,6 @@ void check_route(enum e_route_type route_type) {

check_locally_used_clb_opins(route_ctx.clb_opins_used_locally, route_type);

auto non_configurable_rr_sets = identify_non_configurable_rr_sets();

auto connected_to_route = std::make_unique<bool[]>(device_ctx.rr_nodes.size());
std::fill_n(connected_to_route.get(), device_ctx.rr_nodes.size(), false);

Expand Down Expand Up @@ -154,14 +154,16 @@ void check_route(enum e_route_type route_type) {
}
}

check_non_configurable_edges(net_id, non_configurable_rr_sets);

check_net_for_stubs(net_id);

reset_flags(net_id, connected_to_route.get());

} /* End for each net */

if (!quick) {
check_all_non_configurable_edges();
}

VTR_LOG("Completed routing consistency check successfully.\n");
VTR_LOG("\n");
}
Expand Down Expand Up @@ -622,6 +624,18 @@ static void check_node_and_range(int inode, enum e_route_type route_type) {
check_rr_node(inode, route_type, device_ctx);
}

//Checks that all non-configurable edges are in a legal configuration
//This check is slow, so it has been moved out of check_route()
static void check_all_non_configurable_edges() {
vtr::ScopedStartFinishTimer timer("Checking to ensure non-configurable edges are legal");
auto non_configurable_rr_sets = identify_non_configurable_rr_sets();
auto& cluster_ctx = g_vpr_ctx.clustering();

for (auto net_id : cluster_ctx.clb_nlist.nets()) {
check_non_configurable_edges(net_id, non_configurable_rr_sets);
}
}

//Checks that the specified routing is legal with respect to non-configurable edges
//
//For routing to be legal if *any* non-configurable edge is used, so must *all*
Expand Down
2 changes: 1 addition & 1 deletion vpr/src/route/check_route.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
#include "physical_types.h"
#include "route_common.h"

void check_route(enum e_route_type route_type);
void check_route(enum e_route_type route_type, bool quick);

void recompute_occupancy_from_scratch();

Expand Down