flow: add RTLMP_KEEP_CLUSTERING option - #4412
Open
jorge-ferreira-pii wants to merge 1 commit into
Open
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request adds the -keep_clustering_data flag to additional_rtlmp_args in flow/scripts/macro_place_util.tcl. The reviewer noted that if the RTLMP_ARGS environment variable is defined, it will overwrite all_args and bypass this new flag. They suggested appending -keep_clustering_data after the RTLMP_ARGS check to ensure the flag is always applied.
| append_env_var additional_rtlmp_args RTLMP_NOTCH_WT -notch_weight 1 | ||
| append_env_var additional_rtlmp_args RTLMP_RPT_DIR -report_directory 1 | ||
|
|
||
| append additional_rtlmp_args " -keep_clustering_data" |
Contributor
There was a problem hiding this comment.
If the environment variable RTLMP_ARGS is defined, all_args is completely overwritten by $::env(RTLMP_ARGS) (lines 68-70), which will bypass the -keep_clustering_data flag added here. To ensure that the clustering tree is always stored in the ODB as intended, consider appending -keep_clustering_data to all_args after the RTLMP_ARGS check rather than appending it to additional_rtlmp_args here.
Member
🔍 QoR checkMetrics reflect the PR merge build — i.e. what will land on the target branch. Advisory — results are log-only and do not affect build status. The authoritative QoR gate remains the local rules-file check. Commit 61 design(s) checked — 0 with regression(s), 0 without a comparable baseline.
|
jorge-ferreira-pii
requested review from
AcKoucher,
gadfort,
joaomai and
maliberty
August 11, 2026 14:19
Contributor
|
I don't think |
jorge-ferreira-pii
force-pushed
the
feature-MPL-cluster
branch
from
August 11, 2026 18:19
7bba822 to
a3500c8
Compare
Contributor
Author
I wasn't aware it could interfere with other group types. I've updated the default value of -keep_clustering_data to false. |
AcKoucher
suggested changes
Aug 11, 2026
AcKoucher
left a comment
AcKoucher
left a comment
Contributor
There was a problem hiding this comment.
The name of this PR doesn't tell what it is actually doing. Please, adjust it.
| append_env_var additional_rtlmp_args RTLMP_BOUNDARY_WT -boundary_weight 1 | ||
| append_env_var additional_rtlmp_args RTLMP_NOTCH_WT -notch_weight 1 | ||
| append_env_var additional_rtlmp_args RTLMP_RPT_DIR -report_directory 1 | ||
| # Off by default: an inspection aid that most runs do not need. |
Comment on lines
+1466
to
+1467
| stages: | ||
| - floorplan |
Comment on lines
+1453
to
+1465
| Set to 1 to keep the physical hierarchy that the RTL macro placer's | ||
| clustering engine builds, stored in the ODB as nested dbGroups of type | ||
| VISUAL_DEBUG. An inspection aid, off by default: the web viewer's Clusters | ||
| panel then lists the clusters and can color the layout by cluster, and | ||
| `save_image -web -display_option {cluster_view true}` plots it headless. | ||
| The clustering itself runs either way, so the only cost is storing it - | ||
| around 1% of the macro placement runtime on the largest ORFS cluster tree | ||
| (nangate45/bp_quad, 220 macros, 98 clusters) and a few KB of ODB. On a | ||
| design that uses power domains or region groups, the instances already | ||
| owned by one of those stay in it and are left out of the clustering data | ||
| (reported once as MPL-0078), since an instance belongs to a single dbGroup - | ||
| so those cells show up uncolored in the Clusters view. Ignored when | ||
| RTLMP_ARGS is set, which replaces the whole argument list. |
Contributor
There was a problem hiding this comment.
Don't cite other things apart from MPL behavior here, it ends up polluting the docs.
Set it to 1 and the macro placer runs with -keep_clustering_data, which
keeps the physical hierarchy its clustering engine builds in the ODB as
nested dbGroups. That is what makes the partitioning inspectable after
the run: the web viewer's Clusters panel lists it, and `save_image -web
-display_option {cluster_view true}` plots it headless.
Off by default, for two reasons. It is an inspection aid that most runs
do not need, and the clusters take ownership of the instances they hold --
an instance belongs to a single dbGroup -- so on a design with power
domains or region groups it would take them away from those. OpenROAD
leaves such instances alone and reports them (MPL-0078), but there is no
reason to spend the ODB on a tree nobody asked for.
The clustering itself runs either way, so the only cost of turning it on
is storing the result: measured on nangate45/bp_quad (220 macros, 1.18M
instances, 98 clusters over 3 levels -- the largest cluster tree among the
ORFS designs) the macro placer goes from 137.8s to 139.9s, inside the
annealer's own run-to-run spread, and the ODB grows ~22KB on 90MB.
Note that RTLMP_ARGS still replaces the whole argument list, so flows
setting it must pass -keep_clustering_data themselves.
Part of The-OpenROAD-Project/OpenROAD#7959
Signed-off-by: Jorge Ferreira <jorge.ferreira@precisioninno.com>
openroad-ci
force-pushed
the
feature-MPL-cluster
branch
from
August 12, 2026 14:49
a3500c8 to
6582e79
Compare
AcKoucher
approved these changes
Aug 12, 2026
Contributor
There was a problem hiding this comment.
@jorge-ferreira-pii Please, fix the PR title, it's still misleading as you're only adding the option here.
Two more things:
- Avoid adding information that is not related to the changes themselves in the commit message. It's very long and it's not very helpful.
- Please, don't force-push after a review - we lose the history.
jorge-ferreira-pii
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
rtl_macro_placer can run with the -keep_clustering_data flag (which is off by default), so the physical hierarchy that its clustering engine builds is kept in the database as dbGroups and stays inspectable after the run: The Web GUI Hierarchy Browser panel lists the clusters, and save_image -web -display_option {cluster_view true} plots it headless.
Part of issue #7959