Repository navigation
mpl: each rtl_macro_placer call starts from a fresh hierarchy - #11625
Merged
joaomai merged 4 commits intoOct 7, 2026
Merged
Conversation
A completed rtl_macro_placer run releases its PhysicalHierarchy in clear(). The tree is only created in the constructor, so a second call in the same session handed a null tree to the ClusteringEngine and crashed in setFloorplanShape() with signal 11. ORFS hits this when a MACRO_PLACEMENT_TCL script places some macros with rtl_macro_placer and the macro step then calls it again. A call that finds no unfixed macro sets skip_macro_placement_, and nothing reset it: every later call in the session skipped placement, printed "Skipping macro placement." and left its unfixed macros where they were. init() now creates the hierarchy and clears the flag for each call. Every setter that writes to the tree runs after init(). rerun1 calls rtl_macro_placer three times on fixed_macros1: with every macro fixed, with one macro to place, and with it to place again after that completed run. Before this change the last two calls skip the macro, which stays at ( 0 0 ); the crash is reached once the flag is reset. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
![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 updates HierRTLMP::init() to reinitialize the physical hierarchy tree and reset the skip_macro_placement_ flag on each call, ensuring that multiple runs of rtl_macro_placer in a single session behave correctly. A new integration test, rerun1, has been added to verify this behavior. No review comments were provided, so there is no additional feedback.
The test asserts what the fix is for: after a call that found every
macro fixed, and again after a completed run, the unfixed macro is
placed (LOCKED). Golden .ok/.defok files would have pinned the rest of
the output as well, including the soft blockages that accumulate over
reruns. rerun1 is a PASSFAIL test in Bazel and CMake.
On master the first check fails ("Expected LOCKED, got PLACED"); with
the first call left out, the second call crashes with signal 11.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
joaomai
reviewed
Oct 6, 2026
joaomai
left a comment
joaomai
left a comment
Contributor
There was a problem hiding this comment.
There's also a conflict that needs to be solved in a CMakeLists file.
| void HierRTLMP::init() | ||
| { | ||
| block_ = db_->getChip()->getBlock(); | ||
| // Each rtl_macro_placer call starts from a fresh hierarchy: a completed |
Contributor
There was a problem hiding this comment.
I don't think this comment is necessary. All added operations (initializing the tree and re-setting the flag) are pretty standard for a init method.
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
init() creates the PhysicalHierarchy for every rtl_macro_placer call, so the constructor no longer creates one that init() replaces. Every use of tree_ runs after init(). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Collaborator
Author
|
@vvbandeira @joaomai mac didn't fail in CI, just a hickup at github, internal server error. please retrigger. |
joaomai
approved these changes
Oct 7, 2026
Collaborator
Author
|
😌 |
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
2 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.
A completed
rtl_macro_placerrun releases itsPhysicalHierarchyinclear(), and the tree is only created in the constructor. A second call in the same session therefore hands a null tree to theClusteringEngineand crashes with signal 11 insetFloorplanShape().ORFS hits this when a
MACRO_PLACEMENT_TCLscript places some macros withrtl_macro_placerand the macro step then calls it again. Before #11504 the second call happened to stop earlier, on MPL-0050, so the crash was hidden.There is a second stale-state bug of the same kind. A call that finds no unfixed macro sets
skip_macro_placement_, and nothing reset it. Every later call in the session then printedSkipping macro placement.and left its unfixed macros unplaced.This PR makes
init()create the hierarchy and clear the flag for each call. Every setter that writes to the tree runs afterinit().Test
rerun1is a pass/fail test with no golden log or DEF. It callsrtl_macro_placerthree times onfixed_macros1and checks the intended outcome:MACRO_2unfixed. Check that it is placed (LOCKED).FAIL: placed after a call with nothing to place: Expected LOCKED, got PLACEDSummary 2 / 2 (100% pass)The 37 existing mpl regressions pass unchanged.
clang-format,tclfmt,tclintandbuildifierare clean.Seen while testing, not changed here
Each completed run adds a soft blockage per macro in
commitMacroPlacementToDb()and does not remove the ones from earlier runs. After a rerun that moves a macro, a blockage stays at its old location. It is left for a separate change.🤖 Generated with Claude Code