Skip to content
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

Specify a single location or a specific region for a custom switch block #2664

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

Conversation

saaramahmoudi
Copy link
Contributor

This PR generalize the custom switch block description to allow the user to specify a exact (x,y) location for the switch block by setting type="XY_SPECIFIED", the exact syntax is explained below:

  • You should set the switch block type to XY_SPECIFIED (upper case), the other options are: EVERYWHERE, CORNER, ..
  • Once you set the type to XY_SPECIFIED, you can put an exact location by setting a "x" and "y" attribute, for example: x="1" y="2"
  • If you want to specify a complete row, you can set "x" to -1, for example: x="-1" y="2"
  • if you want to specify a complete column, you can set "y" to -1, for example: x="2" y="-1"
  • If you want to specify a part of grid, you can specify a region using regular expression documented here: https://mithro-vtr.readthedocs.io/en/latest/arch/reference.html#tag-%3Cregiontype= (please search for word "region" in layout documentation)

How Has This Been Tested?

The x and y attribute is tested by @petergrossmann21, and the region description is tested by myself.

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

I will add a new test and update documentation later today, but I have created the PR to start the review process.

@vaughnbetz
Copy link
Contributor

Thanks @saaramahmoudi . I'll review shortly. Can you also please update the documentation and add it to this PR?

@saaramahmoudi
Copy link
Contributor Author

Thanks @saaramahmoudi . I'll review shortly. Can you also please update the documentation and add it to this PR?

Yes, I will update both documentation and a new CI test for this PR later today.

@github-actions github-actions bot added VPR VPR FPGA Placement & Routing Tool libarchfpga Library for handling FPGA Architecture descriptions lang-cpp C/C++ code labels Jul 25, 2024
libs/libarchfpga/src/physical_types.h Outdated Show resolved Hide resolved
libs/libarchfpga/src/physical_types.h Show resolved Hide resolved
libs/libarchfpga/src/physical_types.h Outdated Show resolved Hide resolved
libs/libarchfpga/src/read_xml_arch_file.cpp Show resolved Hide resolved
libs/libarchfpga/src/read_xml_arch_file.cpp Outdated Show resolved Hide resolved
vpr/src/route/build_switchblocks.cpp Outdated Show resolved Hide resolved
vpr/src/route/build_switchblocks.cpp Outdated Show resolved Hide resolved
vpr/src/route/build_switchblocks.cpp Outdated Show resolved Hide resolved
vpr/src/route/build_switchblocks.cpp Outdated Show resolved Hide resolved
libs/libarchfpga/src/read_xml_arch_file.cpp Outdated Show resolved Hide resolved
Copy link

@petergrossmann21 petergrossmann21 left a comment

Choose a reason for hiding this comment

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

@saaramahmoudi Can you include the tests that verify this feature in the PR?

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.

Some comments to address (mine partially overlap Soheil's).
Also, it would be good to make the documentation update now, as part of this PR as well. That also might help clarify the precise meaning of repeat and increment (as would comments on the structure data members storing the sb_loc information).

libs/libarchfpga/src/physical_types.h Outdated Show resolved Hide resolved
libs/libarchfpga/src/physical_types.h Outdated Show resolved Hide resolved
libs/libarchfpga/src/physical_types.h Show resolved Hide resolved
libs/libarchfpga/src/physical_types.h Outdated Show resolved Hide resolved
libs/libarchfpga/src/read_xml_arch_file.cpp Outdated Show resolved Hide resolved
vpr/src/route/build_switchblocks.cpp Outdated Show resolved Hide resolved
vpr/src/route/build_switchblocks.cpp Outdated Show resolved Hide resolved
vpr/src/route/build_switchblocks.cpp Outdated Show resolved Hide resolved
vpr/src/route/build_switchblocks.cpp Outdated Show resolved Hide resolved
vpr/src/route/build_switchblocks.cpp Outdated Show resolved Hide resolved
@petergrossmann21
Copy link

@saaramahmoudi I don't see any new files related to testing; is there something that still needs to be added?

@saaramahmoudi
Copy link
Contributor Author

@petergrossmann21 @vaughnbetz I am so sorry, but I have been busy doing a different task at work. I am trying my best to land this PR as soon as possible. It might take two-three weeks.

@vaughnbetz
Copy link
Contributor

@saaramahmoudi : I think this is ready to merge, but I just merged a libcatch2 update that caused a conflict (sorry!). If you fix that conflict I think this is good to go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external_libs lang-cpp C/C++ code 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.

4 participants