Skip to content

Clean up the usage tracking in grid blocks. #3088

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 6 commits into from
May 28, 2025
Merged

Conversation

haydar-c
Copy link
Contributor

@haydar-c haydar-c commented May 26, 2025

This PR removes the unused 'usage' member from t_grid_blocks and all associated routines of set_usage, increment_usage, decrement_usage (get_usage changed as below).

Description

Changes:

  • Rewrote get_usage() to dynamically count valid blocks based on sub-tile occupancy.
  • Improves maintainability and eliminates risk of stale or inconsistent 'usage' data.

Related Issue

This PR resolves the issue #1607

@github-actions github-actions bot added VPR VPR FPGA Placement & Routing Tool lang-cpp C/C++ code labels May 26, 2025
@haydar-c haydar-c marked this pull request as ready for review May 27, 2025 02:02
Copy link
Contributor

@AlexandreSinger AlexandreSinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good @haydar-c ! It looks like this get_usage method was only used for drawing, logging, and verification which are not performance critical. I agree with removing the usage value and having it be dynamically calculated for simplicity.

I left a small comment on documentation, other than that it looks good to me!

@haydar-c
Copy link
Contributor Author

Thank you for reviewing @AlexandreSinger! I've just added documentation for get_usage.

@@ -31,7 +31,6 @@ void GridBlock::zero_initialize() {
for (int layer_num = 0; layer_num < (int)device_ctx.grid.get_num_layers(); layer_num++) {
for (int i = 0; i < (int)device_ctx.grid.width(); i++) {
for (int j = 0; j < (int)device_ctx.grid.height(); j++) {
set_usage({i, j, layer_num}, 0);
auto tile = device_ctx.grid.get_physical_type({i, j, layer_num});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you replace auto with explicit type names?

Copy link
Contributor

@AlexandreSinger AlexandreSinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but see Soheil's comment above. Should be a quick fix.

@haydar-c
Copy link
Contributor Author

Thank you @soheilshahrouz for reviewing! I have changed the auto types to explicit ones for grid blocks.

@AlexandreSinger AlexandreSinger merged commit 2dc22c5 into master May 28, 2025
32 checks passed
@AlexandreSinger AlexandreSinger deleted the cleanup_grid_usage branch May 28, 2025 00:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang-cpp C/C++ code VPR VPR FPGA Placement & Routing Tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants