[VPR][Pack] Breaking Ties When Placing Primitives

[amin1377]

Description

In the packer, when attempting to place a primitive block within a cluster, the algorithm iterates over all compatible blocks, assigns a cost to each, and selects the one with the lowest cost.

If multiple blocks have the same lowest cost, the current behavior is to choose the one that appears more frequently in the cluster. However, there is no further mechanism to break ties when the frequency is also the same.

Since the list being iterated over is an unordered map, the iteration order can change depending on the order of blocks or modes in the architecture file. This means that even adding a new mode (which may not be used by the netlist) or simply reordering existing modes can impact the packing results.

In this PR, I resolve this by using the hierarchical block name (which is unique for each primitive within a cluster) as a tie-breaker. This ensures consistent packing results even when new modes are added or the mode order is modified.

[amin1377]

@AlexandreSinger I'd appreciate it if you could take a look at this PR. I know @AmirhosseinPoolad has some reservations about the proposed solution, and I understand his concerns. However, given that the hierarchical name is the only unique identifier we can reliably use, I couldn't think of a better alternative at this point.

The more elegant solution would be to refactor the code and data structures to avoid iterating over an unordered_map, which iteration order can vary. However, that would require significant code changes.

[vaughnbetz]

Lots of failures ... it seems there are some crashes.

[AlexandreSinger]

Hi Amin, I do not agree with this change. My main issue with it is that this is extremely hot code and adding a string comparison in here is not going to go very well. I also do not see this as a problem which will happen often enough to cause issues. The intra-cluster placement code is also the most complicated code in the packer, so I am not sure the affect this will have on the overall algorithm. If you are unlucky, it may actually make many circuits un-packable since there may be many ties.

I think this code is actually relying on the randomness from traversing unordered maps. I ran into issues with this when I was refactoring the packer code. If we really want to fix this, it would be better to use a seeded random number than using a deterministic tie-breaker in my opinion.

[AlexandreSinger]

To follow-up, the reason this may cause circuits to become un-packable is that this function is used to propose new candidate site locations. If you always tie-break with the same candidate site location, and it fails that site due to routability reasons, it may never propose another site. I am not 100% on this, but I would not be suprised if something like this happened.

[amin1377]

If you always tie-break with the same candidate site location, and it fails that site due to routability reasons, it may never propose another site

What you're saying makes sense, and with the current tie-breaker, the same site will likely be proposed repeatedly. I need to look more into the test failures to pinpoint the exact cause, but I agree with you. Thanks!