[Packer] Reorganized Iterative Packer Algorithm #3162

AlexandreSinger · 2025-06-24T15:54:24Z

The packer iteratively restarts itself with different options if the packer fails to find a dense enough clustering. The way this was written was very hard to follow and was error prone.

Rewrote the iterative packer code to be more like a state-machine. This makes it easier to add new states in the future and maintain the current states.

My goal here is to be accurate to what the packer was doing before (i.e. no results will change, the packer will behave exactly as it did before). So some of the state transitions may make you say "why do that, why not this?"; those changes will come in a future PR. This is just cleanup.

AmirhosseinPoolad

Thanks for the PR @AlexandreSinger, this really makes the try_pack loop much nicer. I had a couple of nitpicks but other than that, looks good.

Also regression_tests/vtr_reg_nightly_test5/vpr_tight_floorplan uses a ton of floorplan constraints so you should run that to make sure everything is fine.

vpr/src/pack/pack.cpp

AmirhosseinPoolad · 2025-06-24T22:42:42Z

vpr/src/pack/pack.cpp

+        }
+
+        // Set up for the next packer state.
+        switch (next_packer_state) {


There's a little bit of a safety issue here in that if in the future someone decides to skip one of the stages and go straight to the next stage (by modifying the previous section) this code might leave the packer in an invalid state. Essentially there's a coupling between the state diagram/transitions in the state machine and the effect of each state.

This isn't a massive issue and it's probably tricky to avoid it since I bet the attraction_groups methods are not idempotent so if you feel like it would be too much work, it's okay.

I agree, but this is not something I am able to fix right now. The options used for the next iteration of the packer requires information from the previous iteration. It is much cleaner to just set it up here. I am planning on leaving the way it is.

vaughnbetz

I suggest some changes.

vpr/src/pack/pack.cpp

vaughnbetz · 2025-06-24T22:49:42Z

vpr/src/pack/pack.cpp

+        /// @brief Region constraints: Turns on more attraction groups for all regions.
+        ATTRACTION_GROUPS_ALL_REGIONS,
+        /// @brief Region constraints: Turns on more attraction groups for all regions
+        ///        and increases the target density of clb blocks.


clb blocks? Probably should say "any overused block types"

The comment follows what is currently happening in this packer state. This one currently actually does do specifically "clb" blocks. This will change in a future PR, the goal of this PR is organization. It will be much easier to fix after this is in.

vaughnbetz · 2025-06-24T22:53:49Z

vpr/src/pack/pack.cpp

+                }
+            } else {
+                // If there are no overfilled region constraints.
+                switch (current_packer_state) {


What if I had a floorplan_not_fitting state that I've now resolved (with attraction groups), but then I still find I don't fit on the device. I think we only try turning UNRELATED clustering on if I didn't create any attraction groups (I was in state DEFAULT). Those shouldn't be coupled; we should try all density-improving options in that case.

Not sure if that is easiest to implement with more state checks in the else, or if two separate states (one monitoring attraction group status and another the overall density-targeting status) would be better.

I agree with you, this is something I noticed as well. Attraction groups should be turned on if we have overfilled region constraints, but we want unrelated clustering to turn on if we have too many of a given block type. I am planning on fixing this in the next PR.

OK, sounds good. It would be good to fix this!

vpr/src/pack/pack.cpp

vaughnbetz · 2025-06-24T22:58:03Z

vpr/src/pack/pack.cpp

@@ -224,9 +306,18 @@ bool try_pack(const t_packer_opts& packer_opts,
                //       do it for all block types. Doing it only for a clb
                //       string is dangerous -VB.
                cluster_legalizer.get_target_external_pin_util().set_block_pin_util("clb", pin_util);
+                break;


Yes, we should fix this.

We should really check which types of blocks are overused and increase their target_external_pin_util.

I already do this on the AP changes I am working on now. It will be very easy to bring in later in a future PR.

vaughnbetz · 2025-06-24T23:02:29Z

vpr/src/pack/pack.cpp

+                    case e_packer_state::ATTRACTION_GROUPS:
+                        next_packer_state = e_packer_state::MORE_ATTRACTION_GROUPS;
+                        break;
+                    case e_packer_state::MORE_ATTRACTION_GROUPS:


I think right now if we fail to fit on the grid after UNRELATED_AND_BALANCED we will go to the MORE_ATTRACTION_GROUPS state. That will eventually lead to turning on INCREASED_TARGET_DENSITY, which is good, but it will also result in at least a couple of useless clustering attempts (with MORE_ATTRACTION_GROUPS and ATTRACTION_GROUPS_ALL_REGIONS, which won't change anything if there are no attraction groups). Should change the control flow / state machine to avoid that.

Not exactly, I looked into this code to see exactly what it was doing. The current flow does the following:

Pack with default parameters (what I call default state)

If failed, if the region constraints are not overfilled (or if you do not have region constraints) turn on unrelated clustering and balanced block type usage. If you do have an overfilled region constraint it will turn on attraction groups.

If failed, if you have no overused block types it fails hard. If you have overfilled regions it creates more attraction groups.

If failed, again if you have no overused block types it fails hard, if you do it creates more attraction groups.

This repeats with the attraction groups until the last try where it raises the target pin utilization. But this only occurs when there is an overfilled region constraint.

The code that did this was incredibly confusing and what you are noticing now is the issues with the iterative packing algorithm (since this cleanup makes it more apparent what it is doing).

I am avoiding making any changes to this flow right now since it requires a lot of testing and its a lot easier to clean the code first and then make the changes. There are already issues raised tracking this issue.

vpr/src/pack/pack.cpp

The packer iteratively restarts itself with different options if the packer fails to find a dense enough clustering. The way this was written was very hard to follow and was error prone. Rewrote the iterative packer code to be more like a state-machine. This makes it easier to add new states in the future and maintain the current states.

github-actions bot added VPR VPR FPGA Placement & Routing Tool lang-cpp C/C++ code labels Jun 24, 2025

AlexandreSinger force-pushed the feature-packer-state branch from 913008b to 37709a0 Compare June 24, 2025 15:55

AlexandreSinger requested a review from AmirhosseinPoolad June 24, 2025 15:57

AlexandreSinger force-pushed the feature-packer-state branch from c9551f6 to 37709a0 Compare June 24, 2025 20:03

AmirhosseinPoolad reviewed Jun 24, 2025

View reviewed changes

vaughnbetz requested changes Jun 24, 2025

View reviewed changes

soheilshahrouz reviewed Jun 25, 2025

View reviewed changes

vpr/src/pack/pack.cpp Outdated Show resolved Hide resolved

vpr/src/pack/pack.cpp Outdated Show resolved Hide resolved

vpr/src/pack/pack.cpp Outdated Show resolved Hide resolved

AlexandreSinger added 2 commits June 26, 2025 12:20

[Packer] Updated Iterative Repacking Code

0c240d3

AlexandreSinger force-pushed the feature-packer-state branch from 37709a0 to 0c240d3 Compare June 26, 2025 17:29

AlexandreSinger merged commit ae57c3f into verilog-to-routing:master Jun 26, 2025
33 checks passed

AlexandreSinger deleted the feature-packer-state branch June 26, 2025 18:25

[Packer] Reorganized Iterative Packer Algorithm #3162

[Packer] Reorganized Iterative Packer Algorithm #3162

Uh oh!

Conversation

AlexandreSinger commented Jun 24, 2025

Uh oh!

AmirhosseinPoolad left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vaughnbetz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexandreSinger Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AlexandreSinger Jun 25, 2025 •

edited

Loading