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

[Clustering] Independent Clustering Verification #2771

Conversation

AlexandreSinger
Copy link
Contributor

Created a method that would independently verify the clustering in the VPR flow. If a clustering passes this verification, it is assumed that it can be used in placement and routing without issue.

By design, this method does not use any global variables (everything needs to be passed in) and it recomputes everything that it does not assume. This allows it to be independent of the packing flow, so this method can also be used in the AP flow.

@github-actions github-actions bot added VPR VPR FPGA Placement & Routing Tool lang-cpp C/C++ code labels Oct 14, 2024
@AlexandreSinger
Copy link
Contributor Author

@vaughnbetz This is a complementary PR to the Independent Placement Verification PR I raised yesterday (PR #2769 ). I could not find anywhere in the clustering flow that checked these invariants directly so this should improve test coverage. This checks the clustering to ensure that every atom is in a cluster, every cluster has correct floorplanning constraints, and the cluster pbs are correct (for the most part). Please review.

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; merge when you see fit.

clb_atoms,
cluster_constraints,
constraints);
// TODO: There exists more checks for the clustering in base/check_netlist.cpp
Copy link
Contributor

Choose a reason for hiding this comment

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

I think general netlist features like mapping consistency are good to put in the netlist verify routine if possible. But I guess given that the atom_lookup clb mapping is not in the netlist or tied to it, it's OK to have the checks here instead. Just suggesting you think about whether we should have a lookup verifier that can be called by this routine for checks that make sense ...

Created a method that would independently verify the clustering in the
VPR flow. If a clustering passes this verification, it is assumed that
it can be used in placement and routing without issue.

By design, this method does not use any global variables (everything
needs to be passed in) and it recomputes everything that it does not
assume. This allows it to be independent of the packing flow, so this
method can also be used in the AP flow.
The verification and printing of the clustered netlsit made more sense
inside of the load_packing method. Moved.
@AlexandreSinger AlexandreSinger force-pushed the feature-verify-clustering branch from 693becf to 88f581d Compare November 7, 2024 03:09
@AlexandreSinger AlexandreSinger force-pushed the feature-verify-clustering branch from 5d373d8 to 58ea2c0 Compare November 7, 2024 14:51
@AlexandreSinger AlexandreSinger merged commit 1876d05 into verilog-to-routing:master Nov 7, 2024
37 checks passed
@AlexandreSinger AlexandreSinger deleted the feature-verify-clustering branch November 27, 2024 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang-cpp C/C++ code VPR VPR FPGA Placement & Routing Tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants