-
Notifications
You must be signed in to change notification settings - Fork 408
[Router] Upstream Fine-Grained Parallel Router (FPT'24) #2920
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.
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
ueqri
wants to merge
16
commits into
verilog-to-routing:master
Choose a base branch
from
ueqri:upstream-fpt24-fine-grained-parallel-router
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
[Router] Upstream Fine-Grained Parallel Router (FPT'24) #2920
ueqri
wants to merge
16
commits into
verilog-to-routing:master
from
ueqri:upstream-fpt24-fine-grained-parallel-router
+3,407
−1,609
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Upstreamed the fine-grained parallel router implementation into the VTR master. The original branch is https://github.com/verilog-to-routing/vtr-verilog-to-routing/tree/mq-parallel-router. Modified the MultiQueue (SPAA'24) implementation and integrated it into the VTR codebase.
The original FG parallel router used to use boost. VTR does not install boost by default. Moved to STL instead.
Added a partial abstract class for ConnectionRouter, derived from the pure abstract ConnectionRouterInterface. The SerialConnectionRouter and ParallelConnectionRouter classes are now derived from the ConnectionRouter class, utilizing the common class members and helper functions to reduce code duplication.
Added Doxygen-style code comments and documentation for connection routers, including the ConnectionRouter abstract class, the Parallel- ConnectionRouter concrete class, and the SerialConnectionRouter concrete class. Updated the helper messages for command-line options added for parallel connection router.
Fixed the interface issues of ConnectionRouter in NestedNetlistRouter. Fixed code formats. Fixed typo in read_options.cpp.
Updated the command-line usage for parallel connection router in both Read the Docs and read_options.cpp.
Added regression tests for parallel connection router by appending extra sets of configurations to those VTR flow regression tests previously selected by Fahri for testing coarse-grained parallel router. Removed VPR connection router test (vpr/test/test_connection_router.cpp), since it has been out-dated for a very long time and has caused lots of trouble for running VPR C++ tests locally.
Fixed a weird code formatting issue in libs/librtlnumber/src/include/ internal_bits.hpp. GitHub CI said the file failed dev/check-format.sh, however, the same script runs perfectly in my local environment. Double checked the version of clang-format, which seemed to be the same as CI. Directly copied the file from the GitHub repo to resolve this issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ueqri
These changes are looking great! I have left some comments.
You mentioned to me that the parallel router code was not 100% commented yet, so I will review that after you give me the go ahead.
The `No source in route tree` bug in ParallelConnectionRouter (since commit 875b98e) has been fixed. It turns out that putting another member variable `MultiQueueDAryHeap<HeapImplementation::arg_D> heap_` in the derived class ParallelConnectionRouter together with the existing `HeapImplementation heap_` in the base class ConnectionRouter causes the issue. The solution is to keep `heap_` only in the base class and use `ConnectionRouter<MultiQueueDAryHeap<HeapImplementation::arg_D>>` rather than `ConnectionRouter<HeapImplementation>` for deriving the parallel connection router. Please note that ParallelConnectionRouter still has some bugs (i.e., getting stuck in the MultiQueue pop). This commit is not fully working. Please do not use it for any experiments. Updated the previously incorrect command-line options for the parallel connection router in the regression tests.
Fixed the weird bug in parallel connection router as mentioned in commit f73212c. The bug occurred because two function parameters 'num_threads' and 'num_queues' have been misplaced when instantiating the MQ_IO. This took two weeks to figure out exactly. The VTR benchmark (`vtr_reg_qor_chain` task) has been tested/passed for different cases (1) 'serial mode' 1T+2Q (1 thread, 2 queues), (2) 2T+4Q, and (3) 4T+2Q. The determinism has also been verified for the VTR benchmark.
Added more explanation to the command-line options messages and code comments. Cleaned up ParallelConnectionRouter-related codebase.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Upstreamed the fine-grained parallel router implementation into the VTR master. The original branch is https://github.com/verilog-to-routing/vtr-verilog-to-routing/tree/mq-parallel-router.
Modified the MultiQueue (SPAA'24) implementation and integrated it into the VTR codebase.
VTR strong regression tests passed.
TODOs