Skip to content

Commit cdda01b

Browse files
committed
[Router] Fixed Code Review Comments and Cleanup Codebase
Added more explanation to the command-line options messages and code comments. Cleaned up ParallelConnectionRouter-related codebase.
1 parent daa56e9 commit cdda01b

File tree

6 files changed

+27
-20
lines changed

6 files changed

+27
-20
lines changed

doc/src/vpr/command_line_usage.rst

+8-2
Original file line numberDiff line numberDiff line change
@@ -1576,8 +1576,14 @@ The following options are only valid when the router is in timing-driven mode (t
15761576

15771577
Controls whether to enable queue draining optimization for MultiQueue-based parallel connection router.
15781578

1579-
When enabled, queues can be emptied quickly by draining all elements if no further solutions need to be explored in the
1580-
path search to guarantee optimality or determinism after reaching the target.
1579+
When enabled, queues can be emptied quickly by draining all elements if no further solutions need to be explored after
1580+
the target is reached in the path search.
1581+
1582+
Note: For this optimization to maintain optimality and deterministic results, the 'ordering heuristic' (calculated by
1583+
:option:`--astar_fac` and :option:`--astar_offset`) must be admissible to ensure emptying queues of entries with higher
1584+
costs does not prune possibly superior solutions. However, you can still enable this optimization regardless of whether
1585+
optimality and determinism are required for your specific use case (in such cases, the 'ordering heuristic' can be
1586+
inadmissible).
15811587

15821588
This parameter has no effect if :option:`--enable_parallel_connection_router` is not set.
15831589

utils/route_diag/src/main.cpp

-1
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,6 @@ static void do_one_route(const Netlist<>& net_list,
9797
segment_inf,
9898
is_flat);
9999

100-
// TODO: adding tests for parallel connection router
101100
SerialConnectionRouter<FourAryHeap> router(
102101
device_ctx.grid,
103102
*router_lookahead,

vpr/src/route/NestedNetlistRouter.h

+6-6
Original file line numberDiff line numberDiff line change
@@ -70,15 +70,15 @@ class NestedNetlistRouter : public NetlistRouter {
7070
/** Route all nets in a PartitionTree node and add its children to the task queue. */
7171
void route_partition_tree_node(PartitionTreeNode& node);
7272

73-
std::shared_ptr<ConnectionRouterInterface> _make_router(const RouterLookahead* router_lookahead,
73+
std::unique_ptr<ConnectionRouterInterface> _make_router(const RouterLookahead* router_lookahead,
7474
const t_router_opts& router_opts,
7575
bool is_flat) {
7676
auto& device_ctx = g_vpr_ctx.device();
7777
auto& route_ctx = g_vpr_ctx.mutable_routing();
7878

7979
if (!router_opts.enable_parallel_connection_router) {
8080
// Serial Connection Router
81-
return std::make_shared<SerialConnectionRouter<HeapType>>(
81+
return std::make_unique<SerialConnectionRouter<HeapType>>(
8282
device_ctx.grid,
8383
*router_lookahead,
8484
device_ctx.rr_graph.rr_nodes(),
@@ -89,7 +89,7 @@ class NestedNetlistRouter : public NetlistRouter {
8989
is_flat);
9090
} else {
9191
// Parallel Connection Router
92-
return std::make_shared<ParallelConnectionRouter<HeapType>>(
92+
return std::make_unique<ParallelConnectionRouter<HeapType>>(
9393
device_ctx.grid,
9494
*router_lookahead,
9595
device_ctx.rr_graph.rr_nodes(),
@@ -131,19 +131,19 @@ class NestedNetlistRouter : public NetlistRouter {
131131

132132
/* Thread-local storage.
133133
* These are maps because thread::id is a random integer instead of 1, 2, ... */
134-
std::unordered_map<std::thread::id, std::shared_ptr<ConnectionRouterInterface>> _routers_th;
134+
std::unordered_map<std::thread::id, std::unique_ptr<ConnectionRouterInterface>> _routers_th;
135135
std::unordered_map<std::thread::id, RouteIterResults> _results_th;
136136
std::mutex _storage_mutex;
137137

138138
/** Get a thread-local ConnectionRouter. We lock the id->router lookup, but this is
139139
* accessed once per partition so the overhead should be small */
140-
std::shared_ptr<ConnectionRouterInterface> get_thread_router() {
140+
ConnectionRouterInterface& get_thread_router() {
141141
auto id = std::this_thread::get_id();
142142
std::lock_guard<std::mutex> lock(_storage_mutex);
143143
if (!_routers_th.count(id)) {
144144
_routers_th.emplace(id, _make_router(_router_lookahead, _router_opts, _is_flat));
145145
}
146-
return _routers_th.at(id);
146+
return *_routers_th.at(id);
147147
}
148148

149149
RouteIterResults& get_thread_results() {

vpr/src/route/NestedNetlistRouter.tpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ void NestedNetlistRouter<HeapType>::route_partition_tree_node(PartitionTreeNode&
6868
auto& results = get_thread_results();
6969

7070
auto flags = route_net(
71-
*get_thread_router(),
71+
get_thread_router(),
7272
_net_list,
7373
net_id,
7474
_itry,

vpr/src/route/multi_queue_d_ary_heap.h

+9-7
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,12 @@
2626
#include <tuple>
2727
#include <memory>
2828

29-
using MQHeapNode = std::tuple<HeapNodePriority, uint32_t /*FIXME*/>;
29+
// FIXME: Use unified heap node struct (HeapNodeId) and comparator (HeapNodeComparator)
30+
// defined in heap_type.h. Currently, the MQ_IO is not compatible with them. Need a lot
31+
// of refactoring in MQ_IO to make it work, which is left for another PR to clean it up.
32+
using MQHeapNode = std::tuple<HeapNodePriority, uint32_t /* FIXME: Use HeapNodeId */>;
3033

31-
// FIXME: use unified heap node struct and comparator in heap_type.h
32-
struct MQHeapNodeTupleComparator {
34+
struct MQHeapNodeTupleComparator /* FIXME: Use HeapNodeComparator */ {
3335
bool operator()(const MQHeapNode& u, const MQHeapNode& v) {
3436
return std::get<0>(u) > std::get<0>(v);
3537
}
@@ -102,15 +104,15 @@ class MultiQueueDAryHeap {
102104
return (bool)(pq_->empty());
103105
}
104106

105-
uint64_t getNumPushes() const {
107+
uint64_t get_num_pushes() const {
106108
return pq_->getNumPushes();
107109
}
108110

109-
uint64_t getNumPops() const {
111+
uint64_t get_num_pops() const {
110112
return pq_->getNumPops();
111113
}
112114

113-
uint64_t getHeapOccupancy() const {
115+
uint64_t get_heap_occupancy() const {
114116
return pq_->getQueueOccupancy();
115117
}
116118

@@ -119,7 +121,7 @@ class MultiQueueDAryHeap {
119121
}
120122

121123
#ifdef MQ_IO_ENABLE_CLEAR_FOR_POP
122-
void setMinPrioForPop(const HeapNodePriority& minPrio) {
124+
void set_min_priority_for_pop(const HeapNodePriority& minPrio) {
123125
pq_->setMinPrioForPop(minPrio);
124126
}
125127
#endif

vpr/src/route/parallel_connection_router.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -181,8 +181,8 @@ void ParallelConnectionRouter<Heap>::timing_driven_find_single_shortest_path_fro
181181
this->thread_barrier_.wait();
182182

183183
// Collect the number of heap pushes and pops
184-
this->router_stats_->heap_pushes += this->heap_.getNumPushes();
185-
this->router_stats_->heap_pops += this->heap_.getNumPops();
184+
this->router_stats_->heap_pushes += this->heap_.get_num_pushes();
185+
this->router_stats_->heap_pops += this->heap_.get_num_pops();
186186

187187
// Reset the heap for the next connection
188188
this->heap_.reset();
@@ -394,7 +394,7 @@ void ParallelConnectionRouter<Heap>::timing_driven_add_to_heap(const t_conn_cost
394394
if (to_node == target_node) {
395395
#ifdef MQ_IO_ENABLE_CLEAR_FOR_POP
396396
if (multi_queue_direct_draining_) {
397-
this->heap_.setMinPrioForPop(new_total_cost);
397+
this->heap_.set_min_priority_for_pop(new_total_cost);
398398
}
399399
#endif
400400
return;

0 commit comments

Comments
 (0)