[Place] Expand search range for sparse blocks #2960
Conversation
github-actions
bot
added
VPR
|
Does the described problem happen only when IO blocks are located at the top/bottom rows? What if they are initially placed at left/right sides of the device? |
I'm not entirely sure, but given that the x-axis of the compressed grid is fully dense, I don't think this happens for the x-axis. |
Upon further discussion with @soheilshahrouz, it seems likely that the same issue occurs along the x-axis. So, it would make sense to expand the x-axis range limit if the number of compatible blocks in a given row falls below a certain threshold. Otherwise, there’s a bias toward placing IO blocks on the top and bottom edges rather than on the left and right. I’ll look into this in a separate PR. @tpagarani FYI |
…rilog-to-routing into placement_search_range
|
@soheilshahrouz: I’ve addressed all your comments and implemented the changes you requested. Since the code has changed significantly since your last review, I’d appreciate it if you could take another look. Thanks! |
|
You should add QoR data on a couple of big benchmark suites. |
|
Also summarize the QoR-related regtest failrues (basic has two failing QoR, but it is because the wirelength improved a lot (45%) on two small circuits which is certainly fine! |
|
Results on 3 seeds show the cpd degradation isn't consistent (the other 2 seeds were fine). I think this is good to merge. Can you also add a link to the 3 seed data here for posterity @amin1377 ? |
|
It looks like some golden results need to be updated. I looked at the parmys basic failure and it is a single small design that improved too much, so it's not a problem (actually it's good news): |
|
That’s strange. If you only tried one seed I suggest trying a few to see if this is a consistent effect. I’d also print out a message to check the code is activating.VaughnOn Apr 28, 2025, at 9:23 AM, Amin Mohaghegh ***@***.***> wrote:
@amin1377 commented on this pull request.
In vpr/src/place/move_utils.cpp:
+ if (block_constrained) {
+ bool intersect = intersect_range_limit_with_floorplan_constraints(block_id,
+ search_range,
+ delta_cx,
+ to_layer_num);
+ if (!intersect) {
+ return false;
+ }
+ }
+
+ if (is_io_type(block_type) && !block_constrained) {
+ /* We empirically found that for the IO blocks,
+ * Given their sparsity, we expand the y-axis search range
+ * to include all blocks in the column
+ */
+ const t_compressed_block_grid& compressed_block_grid = g_vpr_ctx.placement().compressed_block_grids[block_type->index];
I actually tried that a while ago by not specifying the block type and simply expanding the search range if the number of blocks in the chosen column was less than 3. However, after making that change, in the example I mentioned earlier, we encountered the same issue where the IO blocks became scattered between the top and bottom edges.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because your review was requested.Message ID: ***@***.***>
|
|
@vaughnbetz Unfortunately, I lost the S10 results due to some issues on Wintermute. I’m rerunning the benchmark now and will post the results as soon as they’re available. |
|
Thanks @amin1377 . Intriguing ... I would have thought the algorithms would be equivalent in that case. |
Yes, I’m also a bit surprised that the results aren’t consistent across different seeds with the current change. I’ll need to dig into it a bit more, but I think this highlights that adding those weak links you mentioned earlier is likely a more reliable approach than relying on simulated annealing to place IO blocks close to each other. |
|
Results on S10: [Link] Timing is basically tied (I also included the number of swaps and heap operations). Packing time increased by 10%, which I believe is due to machine load variation, as the packing code hasn’t changed in this PR. In terms of QoR, CPD increased by 1% while WL decreased by 1%. Overall, the QoR is pretty much a tie, though I have to admit, I like the current code better since it’s cleaner... maybe I'm just a little biased :) |
…rilog-to-routing into placement_search_range
…ound is above search range min
…patible_compressed_loc_in_range
… initial placement to prevent search range to be adjusted
vpr/src/place/move_utils.cpp
Outdated
| @@ -19,6 +19,8 @@ | |||
| //Note: The flag is only effective if compiled with VTR_ENABLE_DEBUG_LOGGING | |||
| bool f_placer_breakpoint_reached = false; | |||
|
|
|||
| constexpr int MIN_NUMBER_OF_BLOCK_PER_COLUMN = 3; | |||
There was a problem hiding this comment.
This needs a comment. Better name would be MIN_BLK_PER_COLUMN_EXPAND (or something like that). Current name makes it sound like we can't have fewer blocks than that in a column.
I also think this should be pushed down to the routine that uses it, if it is only used in one place.
There was a problem hiding this comment.
Moved the variable to the routine and added the following comment:
// If the number of blocks in a column is less than this number, we
// will expand the search range to the whole column
|
@vaughnbetz: I made a few changes since there were some outliers in Titan results (CPD for two circuits increased by about 20%). I am gathering QoR now after these changes. |
This reverts commit af29e9d.
This reverts commit caaf456.
This reverts commit 4a6e333.
This reverts commit 96e9cc5.
…tions in initial placement to prevent search range to be adjusted" This reverts commit ade994b.
…in column" This reverts commit dae25cc.
…find_compatible_compressed_loc_in_range" This reverts commit f9e8517.
…cessed by initial placement
…rilog-to-routing into placement_search_range
This PR addresses Issue #2959. The solution to fix the problem is a bit different from the one stated there, though. To ensure moving sparse blocks (e.g., IO blocks), we expand the search range to include the whole column if the number of compatible blocks in the given column is less than a certain threshold (currently, this number is set to 3)
The above update changed the placement of the top picture to the placement of the bottom one (where there is only one IO block left on the top).
