Repository navigation
grt: prefix incremental global route metrics with their step - #4828
Conversation
Each global_route -end_incremental reports the same global_route__* and dpl__* metrics as the initial route, so 5_1_grt.json held up to four copies of each key and only the last survived a JSON parse. Scope the incremental block after repair_design, repair_timing and recover_power with a nested metrics stage, giving keys such as globalroute__repair_timing__global_route__vias. The unprefixed globalroute__global_route__* keys now describe the initial route. Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds metrics stage tracking using utl::push_metrics_stage and utl::pop_metrics_stage around the repair design, repair timing, and recover power steps in flow/scripts/global_route.tcl. The feedback correctly identifies that since the parent metrics stage is already set to globalroute__{}, prefixing the new stages with globalroute__ results in redundant double-prefixing. It is recommended to simplify the stage names to repair_design__{}, repair_timing__{}, and recover_power__{} respectively.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c7c5a48a4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| report_metrics 5 "global route post repair timing" | ||
| } | ||
|
|
||
| utl::push_metrics_stage "globalroute__repair_timing__{}" |
There was a problem hiding this comment.
The new push/body/pop metrics-scoping pattern is repeated around all three incremental-routing blocks. The mandatory docs/contrib/TclContract.md explicitly requires that “An idiom a change needs in two places becomes a proc in util.tcl.” Introduce a shared scope-management helper there and reuse it for the three blocks instead of adding independently maintained push/pop pairs.
AGENTS.md reference: AGENTS.md:L5-L5
Useful? React with 👍 / 👎.
|
|
||
| # Running DPL to fix overlapped instances | ||
| # Run to get modified net by DPL | ||
| utl::push_metrics_stage "globalroute__repair_design__{}" |
There was a problem hiding this comment.
The mandatory docs/contrib/TclContract.md prohibits “quotes or braces it does not need.” Each newly added stage format is already a single literal Tcl word: the embedded {} remains literal without surrounding quotes. Verification with tclsh confirms that the quoted and bare forms produce identical arguments, so remove the unnecessary double quotes from all three additions to satisfy the stage-script contract.
AGENTS.md reference: AGENTS.md:L5-L5
Useful? React with 👍 / 👎.
158a667
into
The-OpenROAD-Project:master
🔍 QoR checkMetrics reflect the PR merge build — i.e. what will land on the target branch. Commit 62 design(s) checked — 0 with regression(s), 0 without a comparable baseline. |
Summary
Each
global_route -end_incrementalinglobal_route.tclreports the sameglobal_route__*anddpl__*metrics as the initial route, so5_1_grt.jsonheld up to four copies of each key, and only the last survived a JSON parse.Each incremental block (after
repair_design,repair_timingandrecover_power) now runs under its own nested metrics stage viautl::push_metrics_stage, ascts.tclalready does. That gives keys such as:globalroute__repair_design__global_route__vias,globalroute__repair_design__dpl__total__movesglobalroute__repair_timing__global_route__wirelengthglobalroute__recover_power__global_route__detour__ratioThe unprefixed
globalroute__global_route__*keys now describe the initial route only. No designrules/metrics_limitsfile orflow/utilscript reads these keys.Companion to The-OpenROAD-Project/OpenROAD#12263, which stops the per-reroute FastRoute metrics that otherwise repeat inside the
repair_timingblock.