Skip to content

add variable to enable repair_timing post global placement#4252

Open
eder-matheus wants to merge 4 commits into
The-OpenROAD-Project:masterfrom
eder-matheus:place_repair_timing
Open

add variable to enable repair_timing post global placement#4252
eder-matheus wants to merge 4 commits into
The-OpenROAD-Project:masterfrom
eder-matheus:place_repair_timing

Conversation

@eder-matheus
Copy link
Copy Markdown
Member

  • Add ENABLE_PLACE_REPAIR_TIMING as a new ORFS flow variable, defaulting to 0.
  • Added repair_timing call behind ENABLE_PLACE_REPAIR_TIMING, using placement parasitics when enabled.
  • Remove the standalone post-detail-placement place_repair_timing step and its Tcl script/hooks.
  • Apply set_dont_use $::env(DONT_USE_CELLS) in floorplan.tcl before floorplan-stage resizer operations so early repair/timing transforms use the same library filtering as later stages.

Signed-off-by: Eder Monteiro <emrmonteiro@precisioninno.com>
Signed-off-by: Eder Monteiro <emrmonteiro@precisioninno.com>
Signed-off-by: Eder Monteiro <emrmonteiro@precisioninno.com>
@eder-matheus eder-matheus requested a review from maliberty May 21, 2026 23:01
@eder-matheus
Copy link
Copy Markdown
Member Author

@rafaelmoresco FYI, this is the new var that might help you on your experiments.

Copy link
Copy Markdown

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

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 refactors the placement timing repair flow by introducing the ENABLE_PLACE_REPAIR_TIMING configuration variable, which allows running timing repair during the placement resize stage. Consequently, the standalone repair_timing_post_place.tcl script and its associated Makefile targets have been removed. The floorplan script now also incorporates set_dont_use for specified cells. Feedback indicates that the documentation and log messages should be updated to specify that only setup violations are repaired, as pre-CTS hold repair is generally discouraged and not explicitly enabled in the current implementation.

Comment thread flow/scripts/resize.tcl
Comment on lines +27 to +33
if { $::env(ENABLE_PLACE_REPAIR_TIMING) } {
# Repair timing using placement parasitics.
puts "Repair setup and hold violations..."
log_cmd estimate_parasitics -placement

repair_timing_helper
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The comment and the puts message indicate that both setup and hold violations are being repaired. However, the underlying repair_timing command (which repair_timing_helper wraps) defaults to fixing only setup violations unless the -hold flag is explicitly provided. Additionally, repairing hold violations pre-CTS (with an ideal clock) is generally discouraged as it can lead to excessive buffering based on inaccurate timing.

If only setup repair is intended, it is better to be explicit and update the messages for clarity. If hold repair was truly intended, the appropriate flags should be passed to the helper, though this should be done with caution at this stage of the flow.

if { $::env(ENABLE_PLACE_REPAIR_TIMING) } {
  # Repair timing using placement parasitics.
  puts "Repair setup violations..."
  log_cmd estimate_parasitics -placement

  repair_timing_helper -setup
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The first part of this review is incorrect: repair_timing_helper fixes hold violations even without the -hold flag.

However, the second point is interesting. @precisionmoon do you think we should skip hold fixes at this stage?

Comment thread flow/scripts/variables.yaml
Signed-off-by: Eder Monteiro <emrmonteiro@precisioninno.com>
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.

1 participant