Skip to content

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

Merged
merged 9 commits into from
Jun 1, 2021
Merged

Conversation

sfkhalid
Copy link
Contributor

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

@github-actions github-actions bot added the VPR VPR FPGA Placement & Routing Tool label May 27, 2021
@sfkhalid sfkhalid mentioned this pull request May 28, 2021
sfkhalid and others added 4 commits May 31, 2021 12:01
…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
@sfkhalid
Copy link
Contributor Author

QoR data for this pull request:
large_constraints_updated_QoR.xlsx

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
@sfkhalid
Copy link
Contributor Author

sfkhalid commented Jun 1, 2021

@vaughnbetz Can this PR be merged now? I added the changes we discussed last week, as well as the new QoR results.

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.

Will merge, but see comment for us to discuss on Thursday.

@@ -48,6 +50,20 @@ PartitionRegion intersection(PartitionRegion& pr1, PartitionRegion& pr2) {
return pr;
}

Copy link
Contributor

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:

  1. 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.
  2. 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.

@vaughnbetz vaughnbetz merged commit 827170f into master Jun 1, 2021
@vaughnbetz vaughnbetz deleted the floorplan_constraints_tuning branch June 1, 2021 15:57
@@ -48,6 +50,20 @@ PartitionRegion intersection(PartitionRegion& pr1, PartitionRegion& pr2) {
return pr;
}

void update_cluster_part_reg(PartitionRegion& cluster_pr, const PartitionRegion& new_pr) {
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
VPR VPR FPGA Placement & Routing Tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants