Skip to content

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

Open
wants to merge 62 commits into
base: master
Choose a base branch
from
Open

Add tileable RR Graph #3134

wants to merge 62 commits into from

Conversation

amin1377
Copy link
Contributor

@amin1377 amin1377 commented Jun 11, 2025

Merging OpenFPGA branch into master branch. PR #2135 explains features of OpenFPGA.

@github-actions github-actions bot added VPR VPR FPGA Placement & Routing Tool libarchfpga Library for handling FPGA Architecture descriptions docs Documentation lang-cpp C/C++ code libvtrutil labels Jun 11, 2025
@amin1377
Copy link
Contributor Author

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!

@amin1377 amin1377 requested a review from AlexandreSinger June 19, 2025 22:36
@amin1377
Copy link
Contributor Author

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.

@amin1377 amin1377 requested a review from soheilshahrouz June 19, 2025 22:37
Copy link
Contributor

@AlexandreSinger AlexandreSinger left a 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.


.. note:: These options are required

In the OpenFPGA architecture file, you may define additional attributes for each VPR's direct connection:
Copy link
Contributor

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on the meeting we had before, I guess the conclusion was to not support all OpenFPGA features in the standard RR Graph generation (at least for now). As a result, I’m creating a separate section to clearly outline the additional features that are exclusively supported by the tileable RR Graph.

@@ -0,0 +1,254 @@
.. _VIB:

VIB Architecture
Copy link
Contributor

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I moved it under the Tileable Architecture section. The vib.rst file explains what VIB is. In short, VIB combines the connection block (CB), switch block (SB), and intra-block crossbar into a single block. As a result, each tile consists of two sub-blocks: the functional block and the VIB (Versatile Interconnect Block). This design facilitates switch architecture exploration and ideally improves router flexibility.

@amin1377
Copy link
Contributor Author

QoR comparison:
Titan: Link
Large VTR: Link

@amin1377
Copy link
Contributor Author

Thanks @AlexandreSinger; reviewing this amount of code is no small task, and I really appreciate your time. I've addressed your comments. Regarding your suggestions about replacing the vector of vectors with VTR data structures: while I agree with them in principle, I didn’t apply all of them since this part of the code is not performance-critical.

I think the code is now ready for the next round of reviews. I’ve also added @AmirhosseinPoolad to help with it.

Copy link
Contributor

@soheilshahrouz soheilshahrouz left a comment

Choose a reason for hiding this comment

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

Thanks for this PR.
Since it's a large one, I’ve reviewed some files for now and will go through the rest soon.

@amin1377
Copy link
Contributor Author

@soheilshahrouz: Thank you for taking the time to review this PR. I’ve addressed all your comments, and I think this PR is now ready for the next round of review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation lang-cpp C/C++ code libarchfpga Library for handling FPGA Architecture descriptions libvtrutil VPR VPR FPGA Placement & Routing Tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants