Skip to content

Invert use of macro_can_be_placed argument in check_all_legality #2998

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 4 commits into from
Apr 30, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions vpr/src/place/analytic_placer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ void AnalyticPlacer::setup_solve_blks(t_logical_block_type_ptr blkTypes) {
void AnalyticPlacer::update_macros() {
for (auto& macro : place_macros_.macros()) {
ClusterBlockId head_id = macro.members[0].blk_index;
bool mac_can_be_placed = macro_can_be_placed(macro, blk_locs[head_id].loc, true, blk_loc_registry_ref_);
bool mac_can_be_placed = macro_can_be_placed(macro, blk_locs[head_id].loc, false, blk_loc_registry_ref_);

//if macro can not be placed in this head pos, change the head pos
if (!mac_can_be_placed) {
Expand All @@ -420,7 +420,7 @@ void AnalyticPlacer::update_macros() {
}

//macro should be placed successfully after changing the head position
VTR_ASSERT(macro_can_be_placed(macro, blk_locs[head_id].loc, true, blk_loc_registry_ref_));
VTR_ASSERT(macro_can_be_placed(macro, blk_locs[head_id].loc, false, blk_loc_registry_ref_));

//update other member's location based on head pos
for (auto member = ++macro.members.begin(); member != macro.members.end(); ++member) {
Expand Down
9 changes: 2 additions & 7 deletions vpr/src/place/initial_placement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -780,12 +780,10 @@ static inline t_pl_loc find_nearest_compatible_loc(const t_flat_pl_loc& src_flat
// floorplanning constraints and compatibility for all
// members of the macro. This prevents some macros being
// placed where they obviously cannot be implemented.
// Note: The check_all_legality flag is poorly named. false means
// that it WILL check all legality...
t_pl_loc new_loc = t_pl_loc(grid_loc.x, grid_loc.y, new_sub_tile, grid_loc.layer_num);
bool site_legal_for_macro = macro_can_be_placed(pl_macro,
new_loc,
false /*check_all_legality*/,
true /*check_all_legality*/,
blk_loc_registry);
if (site_legal_for_macro) {
// Update the best solition.
Expand Down Expand Up @@ -1248,10 +1246,7 @@ bool try_place_macro(const t_pl_macro& pl_macro,
return macro_placed;
}

bool mac_can_be_placed = macro_can_be_placed(pl_macro, head_pos, /*check_all_legality=*/false, blk_loc_registry);

if (mac_can_be_placed) {
// Place down the macro
if (macro_can_be_placed(pl_macro, head_pos, /*check_all_legality=*/true, blk_loc_registry)) {
macro_placed = true;
VTR_LOGV_DEBUG(f_placer_debug, "\t\t\t\tMacro is placed at the given location\n");
for (const t_pl_macro_member& pl_macro_member : pl_macro.members) {
Expand Down
2 changes: 1 addition & 1 deletion vpr/src/place/place_util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ bool macro_can_be_placed(const t_pl_macro& pl_macro,
* floorplan constraint is not supported by analytical placement yet,
* hence, if macro_can_be_placed is called from analytical placer, no further actions are required.
*/
if (check_all_legality) {
if (!check_all_legality) {
continue;
}

Expand Down
2 changes: 1 addition & 1 deletion vpr/src/place/place_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ inline bool is_loc_on_chip(t_physical_tile_loc loc) {
* determines whether the routine should check all legality constraint
* Analytic placer does not require to check block's capacity or
* floorplanning constraints. However, initial placement or SA-based approach
* require to check for all legality constraints.
* require checking all legality constraints.
* @param blk_loc_registry Placement block location information.
*
*/
Expand Down