-
Notifications
You must be signed in to change notification settings - Fork 415
Floorplan constraints tuning #1741
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
…Changes made to atom_cluster_floorplanning_check and related functions
…cluded that file in any file which uses NO_SUBTILE
… passed in, rather than returning a new PartitionRegion. This function is a more efficient version of the PartitionRegion intersection function, and is now used during the atom floorplanning checks that happen during clustering
QoR data for this pull request: To summarize the above - it was found that using the vpr constraints option to specify large floorplan constraints for blocks was increasing pack time. The changes made on this pull request got rid of the pack time increase that was occurring when specifying large constraints. The spreadsheet above shows the results of running three circuits with no constraints specified vs. large constraints specified, and it is shown that there is no significant increase in pack time anymore. |
…on of the Region class. This change was causing the vpr unit test to fail, and so it will be moved to another pull request where the unit test will also be updated accordingly with the change. The change is not necessary on this pull request
@vaughnbetz Can this PR be merged now? I added the changes we discussed last week, as well as the new QoR results. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will merge, but see comment for us to discuss on Thursday.
@@ -48,6 +50,20 @@ PartitionRegion intersection(PartitionRegion& pr1, PartitionRegion& pr2) { | |||
return pr; | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's talk about this in our meeting this week. I can merge this to get it going, but this update_cluster_part_reg has a few problems:
- name is overly specific; this is an "in-place" intersection, rather than having anything to do with clustering specifically. We should at least change the name to reflect what the routine does, rather than who calls it right now.
- As written it is probably not faster than the other intersection routine, since it still creates a new partition_region and assigns it to the cluster_pr. So if the other routine is leading to a negligible cluster time increase we can delete this one. I know I requested an in-place version for speed, but looking at this I don't think it is "in place" enough to be faster.
@@ -48,6 +50,20 @@ PartitionRegion intersection(PartitionRegion& pr1, PartitionRegion& pr2) { | |||
return pr; | |||
} | |||
|
|||
void update_cluster_part_reg(PartitionRegion& cluster_pr, const PartitionRegion& new_pr) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just delete this one and keep the original
Description
This branch will have changes to improve the usability and performance of user-defined floorplan constraints in vpr.
Changes were made to floorplan-related routines in clustering like atom_cluster_floorplan_check to reduce the pack time, which was increasing when floorplan constraints were specified. Now the pack time is similar when running vpr with no constraints or large constraints.
Changes will also be made to the clustering process to improve performance with tight floorplan constraints. Attraction groups will be used to ensure that molecules with similar floorplan regions are packed together.
Related Issue
This is part of a series of PRs to add floorplan placement constraints to VPR. It is related to issue #932