Skip to content

place: DETAIL_PLACEMENT_ARGS reaches every detailed_placement call - #4546

Open
oharboe wants to merge 1 commit into
The-OpenROAD-Project:masterfrom
oharboe:detail-placement-args-everywhere
Open

oharboe wants to merge 1 commit into
The-OpenROAD-Project:masterfrom
oharboe:detail-placement-args-everywhere

Conversation

@oharboe

@oharboe oharboe commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

detail_place.tcl passed DETAIL_PLACEMENT_ARGS to detailed_placement; the four other calls, after clock tree synthesis and after repair_timing in cts.tcl, after repair_design and after repair_timing in global_route.tcl, were bare. A design that picks a legalizer for the place stage, say -use_diamond_legalizer -max_displacement {450 450} because the default negotiation legalizer could not place its wire buffers, was legalized by the default again as soon as CTS inserted its clock buffers.

Measured on a 1.5 M-instance hierarchical asap7 design with 44 macros: detailed_placement in the place stage took 149 s with the diamond search; the re-legalization after clock_tree_synthesis in the same flow ran the negotiation legalizer over 212,700 violations for 400 iterations, 17 minutes, on the same floorplan.

All five calls now go through detailed_placement_helper in util.tcl, beside repair_timing_helper and repair_design_helper, so the argument handling lives in one place and a sixth call cannot miss it. variables.yaml scopes the variable to place, cts and grt, so a build system that filters variables by stage passes it to all three. The default is empty, so a flow that does not set it is unchanged; the variable's description already says it configures detailed placement, not one stage of it. FlowVariables.md and variables.json are regenerated with the repository's scripts.

🤖 Generated with Claude Code

@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 extends the application of the "DETAIL_PLACEMENT_ARGS" environment variable to all stages of the flow that perform detailed placement, specifically including Clock Tree Synthesis (CTS) and Global Routing (GRT). The documentation in "FlowVariables.md", "variables.json", and "variables.yaml" has been updated to reflect this change, and the TCL scripts "cts.tcl" and "global_route.tcl" have been modified to pass these arguments to their respective "detailed_placement" calls. There are no review comments, and I have no feedback to provide.

detail_place.tcl passed DETAIL_PLACEMENT_ARGS to detailed_placement; the
four other calls -- after clock tree synthesis and after repair_timing in
cts.tcl, after repair_design and after repair_timing in global_route.tcl
-- were bare. A design that picks a legalizer for the place stage, say
-use_diamond_legalizer -max_displacement {450 450} because the default
negotiation legalizer could not place its wire buffers, was legalized by
the default again as soon as CTS inserted its clock buffers.

Measured on a 1.5 M-instance hierarchical design (asap7, 44 macros):
detailed_placement in the place stage took 149 s with the diamond
search; the re-legalization after clock_tree_synthesis in the same flow
ran the negotiation legalizer over 212,700 violations for 400 iterations,
17 minutes, on the same floorplan.

All five calls now go through detailed_placement_helper in util.tcl,
beside repair_timing_helper and repair_design_helper, so the argument
handling lives in one place and a sixth call cannot miss it. The variable
is scoped to place, cts and grt in variables.yaml, so a build system that
filters variables by stage passes it to all three. The default is empty,
so a flow that does not set it is unchanged. FlowVariables.md and
variables.json are regenerated.

Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
@oharboe
oharboe force-pushed the detail-placement-args-everywhere branch from f7eb965 to a0bb826 Compare September 18, 2026 13:31
@maliberty
maliberty enabled auto-merge September 18, 2026 14:34
@openroad-ci

openroad-ci commented Sep 18, 2026

Copy link
Copy Markdown
Member

🔍 QoR check

Metrics reflect the PR merge build — i.e. what will land on the target branch.

Commit a23b622 · Jenkins build #3 · Baseline: build · View build on dashboard

62 design(s) checked — 0 with regression(s), 1 without a comparable baseline.

@oharboe

oharboe commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator Author

@LucasYuki @maliberty ORFS origin/master is broken, I'm sure you didn't hear it here first. This PR is approved and will go in when pr-merge goes green after origin/master is fixed.

No further action required on this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants