Skip to content

Allow router to report successful routing even if RCV fails to resolve hold #1540

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
+111 −52
Open

Allow router to report successful routing even if RCV fails to resolve hold #1540

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
16bc19e
Patching RCV fail criteria, doesn't fail if routing is feasible
dpbaines Sep 13, 2020
7f00300
Move check to the is iteration complete function
dpbaines Sep 13, 2020
e62a48b
Refactored router code to store routing with best hold characteristic…
dpbaines Sep 20, 2020
2ef1474
Make format
dpbaines Sep 21, 2020
fc0fdfc
Changes to is_better_routing function in route_timing, easier to read…
dpbaines Sep 22, 2020
c45b4a7
Merge branch 'rcv_fail_fix' of https://github.com/verilog-to-routing/…
dpbaines Sep 22, 2020
d9babb0
Make format
dpbaines Sep 22, 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
161 changes: 111 additions & 50 deletions vpr/src/route/route_timing.cpp
View file
Original file line number Diff line number Diff line change
@@ -163,7 +163,8 @@ static t_bb calc_current_bb(const t_trace* head);
static bool is_better_quality_routing(const vtr::vector<ClusterNetId, t_traceback>& best_routing,
const RoutingMetrics& best_routing_metrics,
const WirelengthInfo& wirelength_info,
std::shared_ptr<const SetupHoldTimingInfo> timing_info);
std::shared_ptr<const SetupHoldTimingInfo> timing_info,
const t_router_opts& router_opts);

static bool early_reconvergence_exit_heuristic(const t_router_opts& router_opts,
int itry_since_last_convergence,
@@ -406,6 +407,10 @@ bool try_timing_driven_route_tmpl(const t_router_opts& router_opts,

int rcv_finished_count = RCV_FINISH_EARLY_COUNTDOWN;

// If tHNS is less than zero and the rcv_finished_count isn't zero, then hold isn't legal
// In this case don't allow the router to break early if RCV is enabled
bool hold_legal = true;

print_route_status_header();
for (itry = 1; itry <= router_opts.max_router_iterations; ++itry) {
RouterStats router_iteration_stats;
@@ -533,10 +538,10 @@ bool try_timing_driven_route_tmpl(const t_router_opts& router_opts,
/*
* Are we finished?
*/
if (is_iteration_complete(routing_is_feasible, router_opts, itry, timing_info, rcv_finished_count == 0)) {
if (routing_is_feasible) {
auto& router_ctx = g_vpr_ctx.routing();

if (is_better_quality_routing(best_routing, best_routing_metrics, wirelength_info, timing_info)) {
if (is_better_quality_routing(best_routing, best_routing_metrics, wirelength_info, timing_info, router_opts)) {
//Save routing
best_routing = router_ctx.trace;
best_clb_opins_used_locally = router_ctx.clb_opins_used_locally;
@@ -556,14 +561,18 @@ bool try_timing_driven_route_tmpl(const t_router_opts& router_opts,
best_routing_metrics.used_wirelength = wirelength_info.used_wirelength();
}

//Decrease pres_fac so that critical connections will take more direct routes
//Note that we use first_iter_pres_fac here (typically zero), and switch to
//use initial_pres_fac on the next iteration.
pres_fac = update_pres_fac(router_opts.first_iter_pres_fac);
if (router_opts.routing_budgets_algorithm != YOYO) {
// Only relax/reset routing parameters if RCV is inactive

//Decrease pres_fac so that critical connections will take more direct routes
//Note that we use first_iter_pres_fac here (typically zero), and switch to
//use initial_pres_fac on the next iteration.
pres_fac = update_pres_fac(router_opts.first_iter_pres_fac);

//Reduce timing tolerances to re-route more delay-suboptimal signals
connections_inf.set_connection_criticality_tolerance(0.7);
connections_inf.set_connection_delay_tolerance(1.01);
//Reduce timing tolerances to re-route more delay-suboptimal signals
connections_inf.set_connection_criticality_tolerance(0.7);
connections_inf.set_connection_delay_tolerance(1.01);
}

++legal_convergence_count;
itry_since_last_convergence = 0;
@@ -579,11 +588,17 @@ bool try_timing_driven_route_tmpl(const t_router_opts& router_opts,
pres_fac = update_pres_fac(router_opts.initial_pres_fac);
}

if (budgeting_inf.if_set()) {
// If total negative slack is greater than or equal to 0, or RCV is finished, allow router to complete
hold_legal = timing_info->hold_total_negative_slack() >= 0 || rcv_finished_count == 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

If this variable doesn't really mean hold is legal (it seems to mean RCV should stop trying) then you should give it a different name.

}

//Have we converged the maximum number of times, did not make any changes, or does it seem
//unlikely additional convergences will improve QoR?
if (legal_convergence_count >= router_opts.max_convergence_count
|| router_iteration_stats.connections_routed == 0
|| early_reconvergence_exit_heuristic(router_opts, itry_since_last_convergence, timing_info, best_routing_metrics)) {
if ((legal_convergence_count >= router_opts.max_convergence_count
|| router_iteration_stats.connections_routed == 0
|| early_reconvergence_exit_heuristic(router_opts, itry_since_last_convergence, timing_info, best_routing_metrics))
Copy link
Contributor

Choose a reason for hiding this comment

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

Could move hold_legal into the early_reconvergence_exit_heuristic routine, which I think would be easier to read by simplifying this condition and giving you a place to comment why you're not saying you're exiting early.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that would be easier to read, but because the default max_convergence_count is 1, the router would stop early regardless. What I could do is to move all of these conditions into a seperate function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Still outstanding

Copy link
Contributor

Choose a reason for hiding this comment

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

When RCV is selected you could change the legal_convergence_count default to some larger number (we can have dependent option defaults; take a look at the option parsing code and you should find an example). Then if the early_reconvergence_exit_heuristic understands hold and rcv it can do the right thing. The if statement won't look as long.

You could also move the whole if into a control routine; that would also work. But I think changing legal_convergence_count makes sense no matter what, as I believe you've found RCV wants a higher value for this setting. Better to do that by actually changing the value than writing if statements to work around it.

&& hold_legal) {
#ifndef NO_GRAPHICS
update_router_info_and_check_bp(BP_ROUTE_ITER, -1);
#endif
@@ -1980,22 +1995,6 @@ void enable_router_debug(
#endif
}

bool is_iteration_complete(bool routing_is_feasible, const t_router_opts& router_opts, int itry, std::shared_ptr<const SetupHoldTimingInfo> timing_info, bool rcv_finished) {
//This function checks if a routing iteration has completed.
//When VPR is run normally, we check if routing_budgets_algorithm is disabled, and if the routing is legal
//With the introduction of yoyo budgeting algorithm, we must check if there are no hold violations
//in addition to routing being legal and the correct budgeting algorithm being set.

if (routing_is_feasible) {
if (router_opts.routing_budgets_algorithm != YOYO) {
return true;
} else if (router_opts.routing_budgets_algorithm == YOYO && (timing_info->hold_worst_negative_slack() == 0 || rcv_finished) && itry != 1) {
return true;
}
}
return false;
}

bool should_setup_lower_bound_connection_delays(int itry, const t_router_opts& /*router_opts*/) {
/* Checks to see if router should (re)calculate route budgets
* It's currently set to only calculate after the first routing iteration */
@@ -2004,38 +2003,100 @@ bool should_setup_lower_bound_connection_delays(int itry, const t_router_opts& /
return false;
}

// Compares the worst and total setup slacks of the current routing with that of the best routing so far
// Returns -1 if the new routing is worse, 0 if tied, and 1 if its improved
static int compare_setup(const RoutingMetrics& best_routing_metrics, std::shared_ptr<const SetupHoldTimingInfo> timing_info) {
// In general we don't care about ties or improvements to positive slack, as once timing constraints have been met there is no point in trying harder
// Because of this, we only check if timing has either improved, or degraded, in which case we report the routing as better or worse respectively

// Worst negative setup slack is prioritized over total negative slack, since this metric is directly correlated to the CPD, and thus Fmax
if (timing_info->setup_worst_negative_slack() > best_routing_metrics.sWNS) {
return 1;
} else if (timing_info->setup_worst_negative_slack() < best_routing_metrics.sWNS) {
return -1;
}

if (timing_info->setup_total_negative_slack() > best_routing_metrics.sTNS) {
return 1;
} else if (timing_info->setup_total_negative_slack() < best_routing_metrics.sTNS) {
return -1;
}

// Return a tie
return 0;
}

// Compares the worst and total hold slacks of the current routing with that of the best routing so far
// Returns -1 if the new routing is worse, 0 if tied, and 1 if its improved
static int compare_hold(const RoutingMetrics& best_routing_metrics, std::shared_ptr<const SetupHoldTimingInfo> timing_info) {
// In general we don't care about ties or improvements to positive slack, as once timing constraints have been met there is no point in trying harder
// Because of this, we only check if timing has either improved, or degraded, in which case we report the routing as better or worse respectively

// Worst negative hold slack is prioritized since getting this as close to zero as possible increases the odds of the routing still working on a physical device
// this is because there are margins of error on timing calculations
if (timing_info->hold_worst_negative_slack() > best_routing_metrics.sWNS) {
return 1;
} else if (timing_info->hold_worst_negative_slack() < best_routing_metrics.sWNS) {
return -1;
}

if (timing_info->hold_total_negative_slack() > best_routing_metrics.sTNS) {
return 1;
} else if (timing_info->hold_total_negative_slack() < best_routing_metrics.sTNS) {
return -1;
}

// Report a tie
return 0;
}

static bool is_better_quality_routing(const vtr::vector<ClusterNetId, t_traceback>& best_routing,
const RoutingMetrics& best_routing_metrics,
const WirelengthInfo& wirelength_info,
std::shared_ptr<const SetupHoldTimingInfo> timing_info) {
std::shared_ptr<const SetupHoldTimingInfo> timing_info,
const t_router_opts& router_opts) {
if (best_routing.empty()) {
return true; //First legal routing
}

//Rank first based on sWNS, followed by other timing metrics
if (timing_info) {
if (timing_info->setup_worst_negative_slack() > best_routing_metrics.sWNS) {
return true;
} else if (timing_info->setup_worst_negative_slack() < best_routing_metrics.sWNS) {
return false;
}
if (router_opts.routing_budgets_algorithm != YOYO) {
// If RCV is disabled prioritize setup slack improvements
int setup_comparison = compare_setup(best_routing_metrics, timing_info);

if (setup_comparison > 0) {
// Setup has improved
return true;
} else if (setup_comparison < 0) {
// Setup has degraded
return false;
}
// Setup slack tied
} else {
// If RCV is enabled prioritize hold over setup, but still return true if setup slack improves

if (timing_info->setup_total_negative_slack() > best_routing_metrics.sTNS) {
return true;
} else if (timing_info->setup_total_negative_slack() < best_routing_metrics.sTNS) {
return false;
}
// Compare hold of current routing to best routing
int hold_comparison = compare_hold(best_routing_metrics, timing_info);

if (timing_info->hold_worst_negative_slack() > best_routing_metrics.hWNS) {
return true;
} else if (timing_info->hold_worst_negative_slack() > best_routing_metrics.hWNS) {
return false;
}
if (hold_comparison > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This code could be shortened by checking if YOYO is enabled, and if so, checking hold and returning whatever is better, or doing nothing in a tie. Then check setup (no matter what), then wirelength.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, I should have seen this! Will fix thanks.

// Hold has improved
return true;
} else if (hold_comparison < 0) {
// Hold has degraded
return false;
}

if (timing_info->hold_total_negative_slack() > best_routing_metrics.hTNS) {
return true;
} else if (timing_info->hold_total_negative_slack() > best_routing_metrics.hTNS) {
return false;
int setup_comparison = compare_setup(best_routing_metrics, timing_info);

if (setup_comparison > 0) {
// Setup has improved
return true;
} else if (setup_comparison < 0) {
// Setup has degraded
return false;
}

// Setup and Hold have tied
}
}

2 changes: 0 additions & 2 deletions vpr/src/route/route_timing.h
View file
Original file line number Diff line number Diff line change
@@ -69,8 +69,6 @@ void free_timing_driven_route_structs(float* pin_criticality, int* sink_order, t

void enable_router_debug(const t_router_opts& router_opts, ClusterNetId net, int sink_rr, int router_iteration, ConnectionRouterInterface* router);

bool is_iteration_complete(bool routing_is_feasible, const t_router_opts& router_opts, int itry, std::shared_ptr<const SetupHoldTimingInfo> timing_info, bool rcv_finished);

bool should_setup_lower_bound_connection_delays(int itry, const t_router_opts& router_opts);

std::vector<t_heap> timing_driven_find_all_shortest_paths_from_route_tree(t_rt_node* rt_root,