Skip to content

[Place] Moved PlaceMacros Out of BlkLocRegistry #2896

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

Conversation

AlexandreSinger
Copy link
Contributor

@AlexandreSinger AlexandreSinger commented Feb 13, 2025

The PlaceMacros only require the Clustering and atom/arch info to be constructed; after which they are never modified. Since they are unchanged by the placement, they were out of place in the BlkLocRegistry.

Moved them to the placement context.

This will make it easier to separate out the initial placement from the SA placer.

@github-actions github-actions bot added VPR VPR FPGA Placement & Routing Tool lang-cpp C/C++ code labels Feb 13, 2025
@vaughnbetz
Copy link
Contributor

I'm OK with passing around place_macros more if you want, but not sure why this would be a member of the clustering context. That seems like a step backward.

@vaughnbetz
Copy link
Contributor

Sounds good, thanks.

The PlaceMacros only require the Clustering and atom/arch info to be
constructed; after which they are never modified. Since they are
unchanged by the placement, they were out of place in the
BlkLocRegistry.

Moved them to the clustering context since they only make sense once the
clusters are loaded.

This will make it easier to separate out the initial placement from the
SA placer.
Vaughn thought that the place macros made more sense in the placement
context instead of the clustering context.
@AlexandreSinger AlexandreSinger force-pushed the feature-ap-detailed-placer branch from b53162c to 38b09e2 Compare February 15, 2025 18:53
@AlexandreSinger
Copy link
Contributor Author

I have updated the location of the PlaceMacros to the placement context. @soheilshahrouz do you have any comments?

@AlexandreSinger
Copy link
Contributor Author

Thanks for the review @soheilshahrouz ! @vaughnbetz Do you have any further comments?

@vaughnbetz
Copy link
Contributor

LGTM, thanks.

@vaughnbetz vaughnbetz merged commit 4c81b1d into verilog-to-routing:master Feb 19, 2025
36 checks passed
@AlexandreSinger AlexandreSinger deleted the feature-ap-detailed-placer branch February 19, 2025 15:42
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.

3 participants