Skip to content

Addition of switch_override feature to custom SB #2067

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 7 commits into from
Sep 1, 2022

Conversation

WhiteNinjaZ
Copy link
Contributor

@WhiteNinjaZ WhiteNinjaZ commented Jun 15, 2022

Description

This PR allows a user to specify a switch_override within the wireconn of a custom SB. The given switch will be used to override the wire_switch of the wire in the to set of the switch block. This PR will allow for more complicated wire shapes such as L and T shapes and provides the functionality needed to address issue #2043.

Motivation and Context

Allows for more complicated wire types in architecture description. Will close #2043 by allowing for diagonal interconnect as well as more complicated wire structures.

How Has This Been Tested?

Many tests have been run on a variety of custom architectures to ensure that the functionality of switch_override works properly. Tests have also been conducted to ensure that this new functionality does not interfere with older architectures.

I plan to update the Xilinx arch in #2053 to test diagonal wires after we have gotten different x/y channel distributions figured out with custom SB's.

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 libarchfpga Library for handling FPGA Architecture descriptions VPR VPR FPGA Placement & Routing Tool labels Jun 15, 2022
@WhiteNinjaZ WhiteNinjaZ force-pushed the diagonals branch 3 times, most recently from 3b2794d to 6a82958 Compare June 16, 2022 17:00
@WhiteNinjaZ WhiteNinjaZ marked this pull request as ready for review June 17, 2022 00:07
@WhiteNinjaZ
Copy link
Contributor Author

@vaughnbetz ready for review.

@vaughnbetz
Copy link
Contributor

  1. Can you confirm that the rr-graph created (when this feature is not used) is identical to what it used to be, and the QoR is also unchanged? I'd expect that to be the case, but it's good to confirm on the stratixiv arch used for the titan run. Easiest way to do this: write out the rr_graph.xml or rr_graph.echo before and after this change on that arch (which uses custom switch blocks) and confirm they are the same. This is a stronger test than the CI tests, which only check we're in a reasonable range for various metrics.
  2. Should add a test for the new feature (architecture file that uses it and at least one small design that runs on it). That could be done in a later PR if you prefer. If it's fast enough, you could add the test to vtr_reg_strong; otherwise add it to one of the nightly ones.

@WhiteNinjaZ
Copy link
Contributor Author

WhiteNinjaZ commented Jun 28, 2022

@vaughnbetz @amin1377 I have added a test case for the diagonal wires. Also, here are the results of the titan_quick_qor. I did do a comparison of the rr_graph using the stratixiv arch and the directrf_stratixiv_arch_timing.blif (one of the larger tests). I can confirm that the two are equivalent and that this PR has not changed the rr_graph.

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. I suggest a few commenting changes. None affect functionality, so we can merge this and you can do the commenting changes in a separate PR if you prefer -- your call.

@vaughnbetz
Copy link
Contributor

Looks like there are some conflicts to resolve too, so may as well make the commenting changes will fixing them.

@WhiteNinjaZ
Copy link
Contributor Author

@vaughnbetz all merge conflicts and requests resolved. Ready to merge.

@WhiteNinjaZ
Copy link
Contributor Author

@vaughnbetz CI is green, no conflicts, and all suggested changes have been implemented. Ready for merge.

@vaughnbetz vaughnbetz merged commit 8593839 into verilog-to-routing:master Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libarchfpga Library for handling FPGA Architecture descriptions VPR VPR FPGA Placement & Routing Tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to Describe Diagonal Wires with Current VPR architecture description
2 participants