Fix OPT_POST_GRT_WNS and disable for some PDKs#4343
Conversation
…ng estimate_parasitics Signed-off-by: Jonas Gava <jfgava@precisioninno.com>
Signed-off-by: Jonas Gava <jfgava@precisioninno.com>
Signed-off-by: Jonas Gava <jfgava@precisioninno.com>
![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 refactors the global routing helper script to execute incremental global routing, detailed placement, and parasitics estimation unconditionally, while restricting the post-repair timing optimization to run conditionally based on the OPT_POST_GRT_WNS environment variable. Additionally, it defines OPT_POST_GRT_WNS as 0 across multiple platform configuration files. The reviewer feedback suggests using the conditional assignment operator ?= instead of = for OPT_POST_GRT_WNS in the platform configuration files to allow design-specific overrides.
| export MAX_ROUTING_LAYER ?= Metal5 | ||
| export DISABLE_VIA_GEN ?= 1 | ||
|
|
||
| export OPT_POST_GRT_WNS = 0 |
There was a problem hiding this comment.
Using ?= instead of = allows design-specific configurations to override this platform default. In OpenROAD-flow-scripts, design-specific config.mk files are typically included before the platform config.mk, so a hard assignment with = will overwrite any design-specific settings. Changing this to ?= aligns with the other variables in this file and preserves user configurability.
export OPT_POST_GRT_WNS ?= 0
| #export VIA_IN_PIN_MAX_LAYER ?= Metal1 | ||
| #export DISABLE_VIA_GEN ?= 1 | ||
|
|
||
| export OPT_POST_GRT_WNS = 0 |
There was a problem hiding this comment.
Using ?= instead of = allows design-specific configurations to override this platform default. In OpenROAD-flow-scripts, design-specific config.mk files are typically included before the platform config.mk, so a hard assignment with = will overwrite any design-specific settings. Changing this to ?= aligns with the other variables in this file and preserves user configurability.
export OPT_POST_GRT_WNS ?= 0
| export MIN_CLK_ROUTING_LAYER = metal4 | ||
| export MAX_ROUTING_LAYER = metal10 | ||
|
|
||
| export OPT_POST_GRT_WNS = 0 |
There was a problem hiding this comment.
Using ?= instead of = allows design-specific configurations to override this platform default. In OpenROAD-flow-scripts, design-specific config.mk files are typically included before the platform config.mk, so a hard assignment with = will overwrite any design-specific settings. Changing this to ?= aligns with the other variables in this file and preserves user configurability.
export OPT_POST_GRT_WNS ?= 0
| export MIN_CLK_ROUTING_LAYER ?= met3 | ||
| export MAX_ROUTING_LAYER ?= met5 | ||
|
|
||
| export OPT_POST_GRT_WNS = 0 |
There was a problem hiding this comment.
Using ?= instead of = allows design-specific configurations to override this platform default. In OpenROAD-flow-scripts, design-specific config.mk files are typically included before the platform config.mk, so a hard assignment with = will overwrite any design-specific settings. Changing this to ?= aligns with the other variables in this file and preserves user configurability.
export OPT_POST_GRT_WNS ?= 0
| export MIN_CLK_ROUTING_LAYER = met3 | ||
| export MAX_ROUTING_LAYER = met5 | ||
|
|
||
| export OPT_POST_GRT_WNS = 0 |
There was a problem hiding this comment.
Using ?= instead of = allows design-specific configurations to override this platform default. In OpenROAD-flow-scripts, design-specific config.mk files are typically included before the platform config.mk, so a hard assignment with = will overwrite any design-specific settings. Changing this to ?= aligns with the other variables in this file and preserves user configurability.
export OPT_POST_GRT_WNS ?= 0
| # Running DPL to fix overlapped instances | ||
| # Run to get modified net by DPL | ||
| log_cmd global_route -start_incremental | ||
| log_cmd detailed_placement {*}$dpl_args | ||
| log_cmd check_placement -verbose | ||
| # Route only the modified net by DPL | ||
| log_cmd global_route -end_incremental {*}$res_aware \ | ||
| -congestion_report_file $::env(REPORTS_DIR)/congestion_post_repair_timing.rpt |
There was a problem hiding this comment.
Why is this no longer required?
There was a problem hiding this comment.
I'm moving this part before the additional repair_timing.
VtSwap and Reroute do not change placement. So we don't need to legalize and run GRT incremental after it.
…ROAD-flow-scripts into secure-fix-wns-opt
Signed-off-by: Jonas Gava <jfgava@precisioninno.com>
2 participants
There is no need to run vt_swap and reroute for PDKs that don't benefit from it.
In this PR I disabled this optimization for Sky130HD/HS, GF180, Nangate45, IHP-SG13G2.
The reroute move does not account for capacitance, which is more relevant to timing closure on these PDKs.
It is possible to reroute a net to a lower-resistance path, but at significantly higher capacitance, which degrades timing.
Currently, we are missing an estimate_parasitics call before running the additional optimizations.
And as we are not changing placement with Reroute and VtSwap, there is no need for a legalization + incremental GRT post-repair_setup.
This prevents issues with another PR with Reroute move enhancements
designs/sky130hd/microwatt/rules-base.json updates: