-
Notifications
You must be signed in to change notification settings - Fork 418
Add tileable RR Graph #3134
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
base: master
Are you sure you want to change the base?
Add tileable RR Graph #3134
Conversation
…rilog-to-routing into add_tileable_rr_graph
…rilog-to-routing into add_tileable_rr_graph
…rilog-to-routing into add_tileable_rr_graph
Hi @AlexandreSinger, I think the PR is ready for your first round of review. I'd appreciate it if you could take a look. Thanks! |
Hi @soheilshahrouz, This PR is ready for your review. Since you're familiar with the RR Graph code, it would be great if you could take a look at the tileable RR Graph implementation. |
…rilog-to-routing into add_tileable_rr_graph
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 @amin1377 thanks for bringing this in! I recognize not all of this is your code. Overall the code is well structured however it needs some code style and data structure cleanup so it can fit in better with the rest of the VTR flow.
Some. of the data structure changes can be made into issues; however, the coding style things should probably be fixed now.
|
||
Technical details can be found in :cite:`XTang_FPT_2019`. | ||
|
||
.. note:: Strongly recommend to enable the tileable routing architecture when you want to PnR large FPGA fabrics, which can effectively reduce the runtime. |
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.
Strongly recommend -> It is strongly recommended to
|
||
.. option:: through_channel="<bool>" | ||
|
||
Allow routing channels to pass through multi-width and multi-height programable blocks. This is mainly used in heterogeneous FPGAs to increase routability, as illustrated in :numref:`fig_thru_channel`. |
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.
programable -> programmable
|
||
.. note:: These options are required | ||
|
||
In the OpenFPGA architecture file, you may define additional attributes for each VPR's direct connection: |
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.
Are we destinguishing OpenFPGA architecture files from regular architecture files?
<direct name="scff_chain" from_pin="clb.sc_out" to_pin="clb.sc_in" x_offset="0" y_offset="-1" z_offset="0"/> | ||
</directlist> | ||
|
||
In OpenFPGA architecture: |
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.
Same comment as above. Is our goal to maintain a separate architecture description file for OpenFPGA and VPR?
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.
I would prefer if we could limit this as much as possible such that an "OpenFPGA" architecture and VPR architecture are one in the same.
@@ -0,0 +1,254 @@ | |||
.. _VIB: | |||
|
|||
VIB Architecture |
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.
What does VIB. I think this file also needs some context. Why is it here? Who should be using this architecture?
default: | ||
VTR_LOGF_ERROR(__FILE__, __LINE__, | ||
"Invalid side index!\n"); | ||
exit(1); |
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.
exit(1) should not be used here.
default: | ||
VTR_LOGF_ERROR(__FILE__, __LINE__, | ||
"Invalid side index!\n"); | ||
exit(1); |
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.
exit(1) should not be used here.
if (from.type_name != vib->get_pbtype_name()) { | ||
VTR_LOGF_ERROR(__FILE__, __LINE__, | ||
"Wrong from type name!\n"); | ||
exit(1); |
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.
Another exit(1) here and just below.
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.
There are actually a lot in this function.
@@ -0,0 +1,57 @@ | |||
#ifndef TILEABLE_RR_GRAPH_NODE_BUILDER_H |
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.
Pragma once + file comment.
default: | ||
VTR_LOGF_ERROR(__FILE__, __LINE__, | ||
"Invalid routing resource node!\n"); | ||
exit(1); |
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.
exit(1) here and throughout this file.
Merging OpenFPGA branch into master branch. PR #2135 explains features of OpenFPGA.