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 46 commits into
base: master
Choose a base branch
from
Open

Add tileable RR Graph #3134

wants to merge 46 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.


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.
Copy link
Contributor

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`.
Copy link
Contributor

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:
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?

<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:
Copy link
Contributor

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?

Copy link
Contributor

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
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?

default:
VTR_LOGF_ERROR(__FILE__, __LINE__,
"Invalid side index!\n");
exit(1);
Copy link
Contributor

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);
Copy link
Contributor

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);
Copy link
Contributor

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.

Copy link
Contributor

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
Copy link
Contributor

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);
Copy link
Contributor

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.

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.

2 participants