Skip to content

mpl: add -random_seed to rtl_macro_placer (and fix re-run segfault) - #11274

Closed
oharboe wants to merge 2 commits into
The-OpenROAD-Project:masterfrom
oharboe:mpl-random-seed
Closed

mpl: add -random_seed to rtl_macro_placer (and fix re-run segfault)#11274
oharboe wants to merge 2 commits into
The-OpenROAD-Project:masterfrom
oharboe:mpl-random-seed

Conversation

@oharboe

@oharboe oharboe commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

rtl_macro_placer seeds its simulated annealing RNGs with a hard-coded
constant, so producing a population of macro placements today requires
perturbing unrelated inputs (CORE_AREA nudges, cost-weight jitter), which
confounds any comparison between candidates.

This PR adds -random_seed (default 0, the previous hard-coded value, so
default behavior is unchanged): the same seed reproduces the same
placement exactly, and different seeds explore different annealing
trajectories. That turns the placer into a deterministic candidate
generator: sweep seeds to generate candidates, re-run the winning seed to
materialize it.

The first commit fixes a latent bug the new test exposed: a second
rtl_macro_placer invocation in the same process segfaults, because run()
ends by destroying the physical hierarchy tree and it was only ever
created in the constructor.

Test: random_seed1 runs the placer three times in one process and checks
same-seed reproducibility and different-seed distinctness.

Measured context (asap7 swerv_wrapper, 28 SRAM macros): adjacent seeds
produce placements ~40% apart on a post-placement path metric, so the
seed distribution is wide enough for best-of-k selection to be useful.

🤖 Generated with Claude Code

HierRTLMP::run() ends by calling clear(), which destroys the physical
hierarchy tree. The tree was only ever created in the constructor, so a
second rtl_macro_placer invocation in the same process handed a null
tree to the clustering engine and crashed. Recreate the tree in init()
and reset the stale skip_macro_placement_ flag so every place() call is
self-contained.

Covered by the new random_seed1 regression test, which runs the placer
three times in one process.

Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Seed the simulated annealing random number generators from a new
-random_seed argument (default 0, the previous hard-coded value, so
existing behavior is unchanged). Runs with the same seed reproduce the
same macro placement; different seeds explore different annealing
trajectories.

This turns rtl_macro_placer into a deterministic candidate generator:
a population of macro placements can be produced by sweeping seeds and
the winner rematerialized exactly by re-running its seed, instead of
perturbing CORE_AREA or the cost weights to obtain distinct placements.

The new random_seed1 regression test checks that the same seed is
reproducible and that a different seed yields a distinct placement.

Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
@oharboe
oharboe requested a review from a team as a code owner August 31, 2026 01:00
@oharboe
oharboe requested a review from joaomai August 31, 2026 01:00

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a -random_seed option to the rtl_macro_placer command to enable deterministic macro placement runs. It updates the TCL interface, SWIG bindings, and the underlying C++ classes (MacroPlacer and HierRTLMP), and adds a new integration test to verify reproducibility. The review feedback highlights two important issues: the need to clear placement_blockages_ and io_blockages_ in HierRTLMP::init() to prevent state accumulation across multiple runs, and the need to guard setNumThreads against non-positive values to avoid an infinite loop.

Comment on lines 314 to +321
void HierRTLMP::init()
{
// run() ends by destroying the tree, so a new one is needed to
// make a subsequent run possible.
if (!tree_) {
tree_ = std::make_unique<PhysicalHierarchy>();
}
skip_macro_placement_ = false;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The placement_blockages_ and io_blockages_ vectors are populated during the placement run but are never cleared. When running the macro placer multiple times in the same process (as demonstrated in the new random_seed1 test), these vectors will accumulate blockages from previous runs. This leads to duplicate blockages, increased memory consumption, and degraded performance in subsequent runs. Clearing them in init() ensures a clean state for every run.

Suggested change
void HierRTLMP::init()
{
// run() ends by destroying the tree, so a new one is needed to
// make a subsequent run possible.
if (!tree_) {
tree_ = std::make_unique<PhysicalHierarchy>();
}
skip_macro_placement_ = false;
void HierRTLMP::init()
{
// run() ends by destroying the tree, so a new one is needed to
// make a subsequent run possible.
if (!tree_) {
tree_ = std::make_unique<PhysicalHierarchy>();
}
skip_macro_placement_ = false;
placement_blockages_.clear();
io_blockages_.clear();

Comment thread src/mpl/src/hier_rtlmp.h
@@ -118,6 +118,7 @@ class HierRTLMP
void setDebugTargetClusterId(int target_cluster_id);

void setNumThreads(int threads) { num_threads_ = threads; }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

If threads is set to 0 or a negative value, number_of_attempts and run_thread in the SA loops will be 0. This will cause remaining_runs to never decrement, resulting in an infinite loop that hangs the application. Adding a guard to ensure num_threads_ is at least 1 prevents this critical failure.

Suggested change
void setNumThreads(int threads) { num_threads_ = threads; }
void setNumThreads(int threads) { num_threads_ = threads > 0 ? threads : 1; }

@oharboe

oharboe commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator Author

Closing per our downstream plan: mpl is churning right now, so we'll carry this as a patch in bazel-orfs for the macro-placement selection campaign and re-upstream once things settle. The segfault half of this PR is now tracked with a minimal reproducer in #11277; the -random_seed feature will come back as a fresh PR later.

@oharboe oharboe closed this Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant