Skip to content

Support for link/router specific bandwidth/latency in architecture file #2562

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

Merged
merged 20 commits into from
May 31, 2024

Conversation

soheilshahrouz
Copy link
Contributor

This PR adds support for heterogeneous NoCs description in the architecture file. It is now possible to specify the latency of each NoC link and NoC router seperately. The user can also specify the bandwidth of each individual NoC link.

The architecture file reference needs to be updated to explaing how tag can be used. I think the code is done and the review process can begin. In meantime, I update the architecture file reference.

Description

The default latency and bandwidth for NoC links and routers are specified under tag with link_bandwidth, link_latency, and router_latency attributes. The user can override these default values for certain routers and links by adding an optional tag under tag.

Before this PR, some NoC-related cost calcualtion functions assumed the latency and bandwidth is the same for all NoC components. Now, these functions check if the latnecy or bandwidth is the same for all NoC components. When this not the case, thse functions query the bandwidth or the latency of NoC components separately.

Motivation and Context

Make the architecture description file syntax support heterogeneous NoC topologies.

How Has This Been Tested?

Has been tested with a toy architecture file.

Types of changes

  • Bug fix (change which fixes an issue)
  • New feature (change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

@github-actions github-actions bot added VPR VPR FPGA Placement & Routing Tool libarchfpga Library for handling FPGA Architecture descriptions lang-cpp C/C++ code libvtrutil libpugiutil labels May 27, 2024
Copy link
Contributor

@vaughnbetz vaughnbetz left a comment

Choose a reason for hiding this comment

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

Looks good. Only substantial thing to consider is if you can unify the code paths for the "all same latency" and detailed latency cases.

* @brief When set true, specifies that some NoC routers have different
* latencies than others. When set false, all the NoC routers have the same
* latency.
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment why these variables are useful / what they control (probably better to comment here than in the .cpp).

@vaughnbetz
Copy link
Contributor

Should also do a QoR test on the router suite just to be sure it is OK.

@soheilshahrouz
Copy link
Contributor Author

Link to QoR results for a subset of NoC benchmarks.

All QoR metrics are the same as the master branch. The placement time is almost the same.

@vaughnbetz vaughnbetz merged commit a8034e2 into master May 31, 2024
101 of 102 checks passed
@vaughnbetz vaughnbetz deleted the temp_arch_read_ branch May 31, 2024 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang-cpp C/C++ code libarchfpga Library for handling FPGA Architecture descriptions libpugiutil libvtrutil VPR VPR FPGA Placement & Routing Tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants