From e5f366650374ed6b00ac1f752914b18941b04602 Mon Sep 17 00:00:00 2001 From: Saurav Singh Date: Mon, 22 Jun 2026 12:17:49 +0000 Subject: [PATCH 1/6] Expose ECO capture/replay command surface (begin_eco/write_eco/read_eco) OpenROAD's odb already has a full ECO journaling layer (dbDatabase::beginEco/writeEco/readEco/undoEco), but it was only reachable through raw SWIG-mangled names with manual block resolution. There was no clean, documented user-facing command for the incremental closure loop. Add top-level Tcl commands in src/OpenRoad.tcl, alongside read_db/write_db: begin_eco start recording netlist edits on the current block write_eco serialize the open ECO journal to a file read_eco replay (redo) an ECO file onto the current block Each command has sta::define_cmd_args, argument-count checks, a shared ord_eco_block helper raising clean ORD-1060..1062 errors when no design is loaded, and (read_eco) file existence/readability checks (ORD-1063/1064). They wrap the existing odb::dbDatabase_* SWIG entry points: no C++ changes, no change to repair_timing defaults or any existing command. Add an end-to-end round-trip regression test (rsz/eco_round_trip) on the small Nangate45 reg1 design: begin_eco -> repair_timing -setup -> write_eco, then undo_eco (verify netlist restored to original) and read_eco (verify the replayed netlist signature exactly matches the modified one). Registered in both CMakeLists.txt and BUILD. Document the commands in src/README.md. end_eco is intentionally not exposed: repair_timing manages nested journals internally, and writing from the journal stack after endEco hits a pre-existing segfault in dbDatabase::writeEco's stack-write branch. The supported, tested flow writes while the journal is open. See ECO_INVESTIGATION.md and AGENT_REPORT.md. Signed-off-by: Saurav Singh --- src/OpenRoad.tcl | 67 +++++++++++++++++++++++++++++ src/README.md | 53 +++++++++++++++++++++++ src/rsz/test/BUILD | 1 + src/rsz/test/CMakeLists.txt | 1 + src/rsz/test/eco_round_trip.ok | 26 ++++++++++++ src/rsz/test/eco_round_trip.tcl | 75 +++++++++++++++++++++++++++++++++ 6 files changed, 223 insertions(+) create mode 100644 src/rsz/test/eco_round_trip.ok create mode 100644 src/rsz/test/eco_round_trip.tcl diff --git a/src/OpenRoad.tcl b/src/OpenRoad.tcl index 0395064e1a7..1e770b68c8b 100644 --- a/src/OpenRoad.tcl +++ b/src/OpenRoad.tcl @@ -265,6 +265,73 @@ proc write_db { args } { ord::write_db_cmd $filename } +# ECO (Engineering Change Order) capture / replay. +# +# The odb journaling layer records every netlist edit (instance create/destroy, +# connect/disconnect, master swap, placement changes, ...) made while an ECO is +# active. write_eco serializes that journal to a file; read_eco replays it onto +# a pristine copy of the original design. This lets a designer snapshot a +# placed/routed netlist, run targeted timing fixes (e.g. repair_timing), and +# capture just the delta as a portable ECO that can be replayed later -- the +# incremental closure loop instead of re-running the whole flow. +# +# Typical usage: +# read_def placed.def +# begin_eco ;# start recording netlist edits +# repair_timing -setup +# write_eco fix.eco ;# persist the delta +# and later, on the original (unmodified) netlist: +# read_def placed.def +# read_eco fix.eco ;# replay the delta -> matches the modified design + +proc ord_eco_block { cmd } { + set db [ord::get_db] + if { $db eq "NULL" } { + utl::error ORD 1060 "no database is loaded; $cmd requires a design." + } + set chip [$db getChip] + if { $chip eq "NULL" } { + utl::error ORD 1061 "no chip is loaded; $cmd requires a design." + } + set block [$chip getBlock] + if { $block eq "NULL" } { + utl::error ORD 1062 "no block is loaded; $cmd requires a design." + } + return $block +} + +sta::define_cmd_args "begin_eco" {} + +proc begin_eco { args } { + sta::check_argc_eq0 "begin_eco" $args + set block [ord_eco_block "begin_eco"] + odb::dbDatabase_beginEco $block +} + +sta::define_cmd_args "write_eco" {filename} + +proc write_eco { args } { + sta::check_argc_eq1 "write_eco" $args + set block [ord_eco_block "write_eco"] + set filename [file nativename [lindex $args 0]] + odb::dbDatabase_writeEco $block $filename +} + +sta::define_cmd_args "read_eco" {filename} + +proc read_eco { args } { + sta::check_argc_eq1 "read_eco" $args + set block [ord_eco_block "read_eco"] + set filename [file nativename [lindex $args 0]] + if { ![file exists $filename] } { + utl::error ORD 1063 "$filename does not exist." + } + if { ![file readable $filename] } { + utl::error ORD 1064 "$filename is not readable." + } + odb::dbDatabase_readEco $block $filename +} + sta::define_cmd_args "assign_ndr" { -ndr name (-net name | -all_clocks) } proc assign_ndr { args } { diff --git a/src/README.md b/src/README.md index dbd9c3343c4..060a6c2bc4d 100644 --- a/src/README.md +++ b/src/README.md @@ -87,6 +87,59 @@ technology exists in the database. design.writedb("reg1.db") ```` +## ECO (Engineering Change Order) capture and replay + +OpenROAD's database records netlist edits in a journal so that a set of +targeted changes can be captured and replayed as an ECO, instead of re-running +the whole flow for design closure. The commands below expose this journal: + +````{eval-rst} +.. tabs:: + + .. code-tab:: tcl + + begin_eco # start recording netlist edits on the current block + write_eco filename # write the recorded edits to an ECO file + read_eco filename # replay an ECO file onto the current block +```` + +A typical incremental timing-closure loop is to snapshot a placed (and ideally +routed) design, apply targeted fixes, and persist just the delta: + +````{eval-rst} +.. tabs:: + + .. code-tab:: tcl + + read_lef Nangate45/Nangate45.lef + read_def placed.def + create_clock -period 0.3 clk + set_wire_rc -layer metal3 + estimate_parasitics -placement + + begin_eco # start recording + repair_timing -setup # targeted fix (records cell resizes, buffer edits, ...) + write_eco fix.eco # persist the delta +```` + +The persisted ECO can later be replayed on the original, unmodified netlist to +reproduce the fixed design without re-optimizing: + +````{eval-rst} +.. tabs:: + + .. code-tab:: tcl + + read_lef Nangate45/Nangate45.lef + read_def placed.def # the original, pre-fix netlist + read_eco fix.eco # replay -> matches the fixed design +```` + +`read_eco` requires a database that is an exact copy of the one in which the +ECO was recorded, prior to the recorded edits. `write_eco` captures the journal +that is currently open (started by `begin_eco`); issue it before closing the +ECO. + The `read_verilog` command is used to build an OpenDB database as shown below. Multiple Verilog files for a hierarchical design can be read. The `link_design` command is used to flatten the design and make a database. diff --git a/src/rsz/test/BUILD b/src/rsz/test/BUILD index da0c8719c8b..707de5d0f62 100644 --- a/src/rsz/test/BUILD +++ b/src/rsz/test/BUILD @@ -29,6 +29,7 @@ TESTS = [ "buffer_varying_lengths", "clone_flat", "clone_hier", + "eco_round_trip", "eliminate_dead_logic1", "eliminate_dead_logic2", "eqy_repair_setup2", diff --git a/src/rsz/test/CMakeLists.txt b/src/rsz/test/CMakeLists.txt index 25fe7eb9c81..b4482972eb0 100644 --- a/src/rsz/test/CMakeLists.txt +++ b/src/rsz/test/CMakeLists.txt @@ -23,6 +23,7 @@ or_integration_tests( clone_hier eliminate_dead_logic1 eliminate_dead_logic2 + eco_round_trip eqy_repair_setup2 eqy_repair_setup5 fanin_fanout1 diff --git a/src/rsz/test/eco_round_trip.ok b/src/rsz/test/eco_round_trip.ok new file mode 100644 index 00000000000..8d4cdcb1ac5 --- /dev/null +++ b/src/rsz/test/eco_round_trip.ok @@ -0,0 +1,26 @@ +[INFO ODB-0227] LEF file: Nangate45/Nangate45.lef, created 22 layers, 27 vias, 135 library cells +[INFO ODB-0128] Design: reg1 +[INFO ODB-0130] Created 1 pins. +[INFO ODB-0131] Created 17 components and 92 component-terminals. +[INFO ODB-0132] Created 2 special nets and 34 connections. +[INFO ODB-0133] Created 7 nets and 30 connections. +original insts 17 +[INFO RSZ-0100] Repair move sequence: UnbufferMove SizeUpMove SwapPinsMove BufferMove CloneMove SplitLoadMove +[INFO RSZ-0094] Found 4 endpoints with setup violations. +[INFO RSZ-0099] Repairing 4 out of 4 (100.00%) violating endpoints... + Iter | Removed | Resized | Inserted | Cloned | Pin | Area | WNS | StTNS | EnTNS | Viol | Worst + | Buffers | Gates | Buffers | Gates | Swaps | | | | | Endpts | St/EnPt +------------------------------------------------------------------------------------------------------------------------------ + 0* | 0 | 0 | 0 | 0 | 0 | +0.0% | -0.333 | -0.3 | -0.4 | 4 | r2/D + 9* | 4 | 1 | 0 | 0 | 0 | -4.6% | 0.013 | 0.0 | 0.0 | 4 | r2/D + final | 4 | 1 | 0 | 0 | 0 | -4.6% | 0.013 | 0.0 | 0.0 | 0 | r2/D +------------------------------------------------------------------------------------------------------------------------------ +[INFO RSZ-0059] Removed 4 buffers. +[INFO RSZ-0051] Resized 1 instances: 1 up, 0 up match, 0 down, 0 VT +modified insts 13 +eco changed netlist yes +eco file non-empty yes +undo restored original yes +post-undo insts 17 +replayed insts 13 +replay matches modified yes diff --git a/src/rsz/test/eco_round_trip.tcl b/src/rsz/test/eco_round_trip.tcl new file mode 100644 index 00000000000..40b87df81e5 --- /dev/null +++ b/src/rsz/test/eco_round_trip.tcl @@ -0,0 +1,75 @@ +# ECO capture/replay round-trip. +# +# Exercises the user-facing begin_eco / write_eco / read_eco commands: +# 1. Snapshot a placed design. +# 2. begin_eco, run repair_timing -setup (real netlist edits), write_eco. +# 3. undo_eco back to the pristine netlist (proves the edits were real). +# 4. read_eco replays the persisted ECO file. +# 5. Assert the replayed netlist matches the modified netlist exactly, +# proving the write->read round-trip is faithful. +# +# Note: repair_timing manages the ECO journal internally (nested begin/commit). +# An outer begin_eco accumulates those committed changes, so write_eco is issued +# while the outer journal is still open (no end_eco needed for capture). +source "helpers.tcl" +read_liberty Nangate45/Nangate45_typ.lib +read_lef Nangate45/Nangate45.lef +read_def repair_setup1.def +create_clock -period 0.3 clk + +source Nangate45/Nangate45.rc +set_wire_rc -layer metal3 +estimate_parasitics -placement + +# Signature of the netlist: sorted ":" plus the net->pin +# connectivity, so resizes (master swap) and buffer insertion/removal +# (instance + net create/destroy + connect/disconnect) are all captured. +proc netlist_signature { } { + set block [[[ord::get_db] getChip] getBlock] + set sig {} + foreach inst [$block getInsts] { + lappend sig "I [$inst getName] [[$inst getMaster] getName]" + } + foreach net [$block getNets] { + set conns {} + foreach iterm [$net getITerms] { + lappend conns "[[$iterm getInst] getName]/[[$iterm getMTerm] getName]" + } + lappend sig "N [$net getName] [lsort $conns]" + } + return [lsort $sig] +} + +proc inst_count { } { + return [llength [[[[ord::get_db] getChip] getBlock] getInsts]] +} + +set sig_orig [netlist_signature] +puts "original insts [inst_count]" + +# --- Phase 1: capture the timing fix as an ECO --- +set block [[[ord::get_db] getChip] getBlock] +begin_eco +repair_timing -setup + +set sig_mod [netlist_signature] +puts "modified insts [inst_count]" +puts "eco changed netlist [expr { $sig_mod ne $sig_orig ? "yes" : "no" }]" + +set eco_file [make_result_file eco_round_trip.eco] +write_eco $eco_file +puts "eco file non-empty [expr { [file exists $eco_file] && [file size $eco_file] > 0 ? "yes" : "no" }]" + +# --- Phase 2: undo back to the pristine netlist --- +odb::dbDatabase_undoEco $block +estimate_parasitics -placement +set sig_after_undo [netlist_signature] +puts "undo restored original [expr { $sig_after_undo eq $sig_orig ? "yes" : "no" }]" +puts "post-undo insts [inst_count]" + +# --- Phase 3: replay the persisted ECO and verify equivalence --- +read_eco $eco_file +estimate_parasitics -placement +set sig_replay [netlist_signature] +puts "replayed insts [inst_count]" +puts "replay matches modified [expr { $sig_replay eq $sig_mod ? "yes" : "no" }]" From 6663151ecd1b0b507132cca70749d2d9be0fead0 Mon Sep 17 00:00:00 2001 From: Saurav Singh Date: Mon, 22 Jun 2026 13:43:16 +0000 Subject: [PATCH 2/6] Add repair_timing_eco closure loop with accurate change summary Add a repair_timing_eco command (src/rsz/src/Resizer.tcl) that wraps the existing repair_timing engine and the begin_eco/write_eco ECO machinery into a timing-fix closure loop with a concise, accurate audit trail: - snapshot baseline WNS/TNS and the netlist, - begin_eco then run repair_timing via full argument passthrough (no reimplementation, no default changes), - with the journal still open, diff the netlist to report gates resized (with before/after sizes), buffers inserted/removed, instances added/removed, nets modified, and WNS/TNS before -> after, - optionally write_eco the exact change set via -eco_file. set_dont_touch is honored by the underlying resizer, so clean logic is not perturbed. Counts are derived from the real netlist delta (which equals the journal's structural content), not estimated. Add src/rsz/test/repair_timing_eco.tcl (+ .ok), dual-registered in CMake and Bazel, asserting WNS does not regress, that reported counts match an independently computed netlist delta, and that a set_dont_touch buffer the unconstrained flow would otherwise remove is left untouched. rsz suite: 240/240. Existing begin_eco/write_eco/read_eco and repair_timing are unchanged (Resizer.tcl change is purely additive; OpenRoad.tcl untouched). Signed-off-by: Saurav Singh --- src/rsz/src/Resizer.tcl | 233 +++++++++++++++++++++++++++++ src/rsz/test/BUILD | 1 + src/rsz/test/CMakeLists.txt | 1 + src/rsz/test/repair_timing_eco.ok | 46 ++++++ src/rsz/test/repair_timing_eco.tcl | 104 +++++++++++++ 5 files changed, 385 insertions(+) create mode 100644 src/rsz/test/repair_timing_eco.ok create mode 100644 src/rsz/test/repair_timing_eco.tcl diff --git a/src/rsz/src/Resizer.tcl b/src/rsz/src/Resizer.tcl index 7f81f647f4d..66b0909c07e 100644 --- a/src/rsz/src/Resizer.tcl +++ b/src/rsz/src/Resizer.tcl @@ -398,6 +398,239 @@ proc repair_timing { args } { return [expr $recovered_power || $repaired_setup || $repaired_hold] } +################################################################ +# +# repair_timing_eco -- ECO-driven timing-fix loop with a change report. +# +# Orchestrates the convergent-closure loop on top of the existing +# repair_timing engine and the existing begin_eco/write_eco ECO machinery: +# +# 1. Snapshot baseline timing (WNS/TNS) and the netlist state. +# 2. begin_eco, then run the *real* repair_timing engine (all repair_timing +# options pass through unchanged -- nothing is reimplemented and no +# repair_timing default is altered). set_dont_touch is honored because +# the underlying resizer skips dont_touch instances/nets, so clean parts +# of the design are not perturbed. +# 3. With the ECO journal still open, snapshot the post-fix timing and +# netlist state and compute the *actual* delta (gates resized with +# before/after sizes, buffers inserted/removed, nets modified) and print +# a concise audit-trail summary plus WNS/TNS before -> after. +# 4. Optionally write_eco so the exact change set can be replayed. +# +# The counts are derived from the real netlist delta (not estimated): every +# edit the ECO journal records is a netlist/placement change, so the +# before/after netlist diff is an exact reconstruction of the captured ECO. +# See ECO_CLOSURE_INVESTIGATION.md for the journal-action analysis. + +sta::define_cmd_args "repair_timing_eco" {[-eco_file filename]\ + [repair_timing options...]} + +proc repair_timing_eco { args } { + # Pull out our own -eco_file key; everything else passes through to + # repair_timing verbatim so we never duplicate its option surface. + set eco_file "" + set passthrough {} + set n [llength $args] + for { set i 0 } { $i < $n } { incr i } { + set a [lindex $args $i] + if { $a eq "-eco_file" } { + incr i + if { $i >= $n } { + utl::error RSZ 240 "-eco_file requires a filename argument." + } + set eco_file [lindex $args $i] + } else { + lappend passthrough $a + } + } + + set block [rsz::eco_block "repair_timing_eco"] + + # --- Phase 1: baseline snapshot --- + set wns_before [rsz::eco_wns] + set tns_before [rsz::eco_tns] + set snap_before [rsz::eco_netlist_snapshot $block] + + # --- Phase 2: capture the targeted fix as an ECO --- + # begin_eco opens an outer journal; repair_timing's internal nested journals + # commit into it, so the open journal holds the full delta. We never call + # endEco (the supported, crash-free capture flow -- see investigation doc). + begin_eco + repair_timing {*}$passthrough + + # --- Phase 3: post-fix snapshot + delta (journal still open) --- + set wns_after [rsz::eco_wns] + set tns_after [rsz::eco_tns] + set snap_after [rsz::eco_netlist_snapshot $block] + + set delta [rsz::eco_compute_delta $snap_before $snap_after] + rsz::eco_report_summary $delta \ + $wns_before $tns_before $wns_after $tns_after + + # --- Phase 4: optionally persist the exact change set --- + if { $eco_file ne "" } { + write_eco $eco_file + utl::report "ECO change set written ([file tail $eco_file])" + } + + return $delta +} + +# Resolve the current block, raising a clean error when no design is loaded. +proc rsz::eco_block { cmd } { + set db [ord::get_db] + if { $db eq "NULL" } { + utl::error RSZ 241 "no database is loaded; $cmd requires a design." + } + set chip [$db getChip] + if { $chip eq "NULL" } { + utl::error RSZ 242 "no chip is loaded; $cmd requires a design." + } + set block [$chip getBlock] + if { $block eq "NULL" } { + utl::error RSZ 243 "no block is loaded; $cmd requires a design." + } + return $block +} + +# Worst negative slack (clamped at 0 -- positive slack reports as 0 WNS), +# using the same primitives as the resizer's own PPA reporting. +proc rsz::eco_wns { } { + set wns [sta::worst_slack_cmd max] + if { $wns > 0.0 } { + set wns 0.0 + } + return $wns +} + +proc rsz::eco_tns { } { + return [sta::total_negative_slack_cmd max] +} + +# Snapshot of the netlist: a dict of instance-name -> master-name and a dict +# of net-name -> sorted pin-connectivity list. Resizes (master swap), buffer +# insertion/removal (inst + net create/destroy) and pin swaps (connectivity +# change) are all captured by diffing two of these. +proc rsz::eco_netlist_snapshot { block } { + set insts [dict create] + foreach inst [$block getInsts] { + dict set insts [$inst getName] [[$inst getMaster] getName] + } + set nets [dict create] + foreach net [$block getNets] { + set conns {} + foreach iterm [$net getITerms] { + lappend conns "[[$iterm getInst] getName]/[[$iterm getMTerm] getName]" + } + dict set nets [$net getName] [lsort $conns] + } + return [list $insts $nets] +} + +# Classify a master name as a buffer/inverter via its liberty cell. +# Returns 1 for buffers/inverters, 0 otherwise; 0 if the cell is unknown. +proc rsz::eco_is_buffer_master { master_name } { + set cell [sta::find_liberty_cell $master_name] + if { $cell eq "" || $cell eq "NULL" } { + return 0 + } + if { [get_property $cell is_buffer] || [get_property $cell is_inverter] } { + return 1 + } + return 0 +} + +# Compute the netlist delta between two snapshots. Returns a dict with: +# resized : list of {inst before_master after_master} +# bufs_inserted : count of added buffer/inverter instances +# bufs_removed : count of removed buffer/inverter instances +# insts_added : count of added instances (any kind) +# insts_removed : count of removed instances (any kind) +# nets_modified : count of nets whose connectivity changed (plus added/removed) +proc rsz::eco_compute_delta { before after } { + lassign $before insts_b nets_b + lassign $after insts_a nets_a + + set resized {} + set bufs_inserted 0 + set bufs_removed 0 + set insts_added 0 + set insts_removed 0 + + # Resized: present in both, master changed. + dict for { name master_b } $insts_b { + if { [dict exists $insts_a $name] } { + set master_a [dict get $insts_a $name] + if { $master_a ne $master_b } { + lappend resized [list $name $master_b $master_a] + } + } else { + # Removed instance. + incr insts_removed + if { [rsz::eco_is_buffer_master $master_b] } { + incr bufs_removed + } + } + } + # Added instances. + dict for { name master_a } $insts_a { + if { ![dict exists $insts_b $name] } { + incr insts_added + if { [rsz::eco_is_buffer_master $master_a] } { + incr bufs_inserted + } + } + } + + # Nets modified: connectivity changed, or net added/removed. + set nets_modified 0 + dict for { name conns_b } $nets_b { + if { [dict exists $nets_a $name] } { + if { [dict get $nets_a $name] ne $conns_b } { + incr nets_modified + } + } else { + incr nets_modified + } + } + dict for { name conns_a } $nets_a { + if { ![dict exists $nets_b $name] } { + incr nets_modified + } + } + + return [dict create \ + resized $resized \ + bufs_inserted $bufs_inserted \ + bufs_removed $bufs_removed \ + insts_added $insts_added \ + insts_removed $insts_removed \ + nets_modified $nets_modified] +} + +# Print a concise, human-auditable change + timing summary. +proc rsz::eco_report_summary { delta wns_b tns_b wns_a tns_a } { + set resized [dict get $delta resized] + utl::report "" + utl::report "==================== ECO change summary ====================" + utl::report [format " Gates resized : %d" [llength $resized]] + foreach r $resized { + lassign $r name before after + utl::report [format " %-20s %s -> %s" $name $before $after] + } + utl::report [format " Buffers inserted : %d" [dict get $delta bufs_inserted]] + utl::report [format " Buffers removed : %d" [dict get $delta bufs_removed]] + utl::report [format " Instances added : %d" [dict get $delta insts_added]] + utl::report [format " Instances removed : %d" [dict get $delta insts_removed]] + utl::report [format " Nets modified : %d" [dict get $delta nets_modified]] + utl::report " ----------------------------------------------------------" + utl::report [format " WNS (ns) %12.4f -> %12.4f" \ + [expr { $wns_b * 1e9 }] [expr { $wns_a * 1e9 }]] + utl::report [format " TNS (ns) %12.4f -> %12.4f" \ + [expr { $tns_b * 1e9 }] [expr { $tns_a * 1e9 }]] + utl::report "============================================================" +} + ################################################################ sta::define_cmd_args "report_design_area" {} diff --git a/src/rsz/test/BUILD b/src/rsz/test/BUILD index 707de5d0f62..2bf3f8e8a09 100644 --- a/src/rsz/test/BUILD +++ b/src/rsz/test/BUILD @@ -195,6 +195,7 @@ TESTS = [ "repair_tie11_hier", "repair_tie12_hier", "repair_tie13_hier", + "repair_timing_eco", "repair_wire1", "repair_wire10", "repair_wire11", diff --git a/src/rsz/test/CMakeLists.txt b/src/rsz/test/CMakeLists.txt index b4482972eb0..fbd253c82ad 100644 --- a/src/rsz/test/CMakeLists.txt +++ b/src/rsz/test/CMakeLists.txt @@ -24,6 +24,7 @@ or_integration_tests( eliminate_dead_logic1 eliminate_dead_logic2 eco_round_trip + repair_timing_eco eqy_repair_setup2 eqy_repair_setup5 fanin_fanout1 diff --git a/src/rsz/test/repair_timing_eco.ok b/src/rsz/test/repair_timing_eco.ok new file mode 100644 index 00000000000..9c770ce0c82 --- /dev/null +++ b/src/rsz/test/repair_timing_eco.ok @@ -0,0 +1,46 @@ +[INFO ODB-0227] LEF file: Nangate45/Nangate45.lef, created 22 layers, 27 vias, 135 library cells +[INFO ODB-0128] Design: reg1 +[INFO ODB-0130] Created 1 pins. +[INFO ODB-0131] Created 17 components and 92 component-terminals. +[INFO ODB-0132] Created 2 special nets and 34 connections. +[INFO ODB-0133] Created 7 nets and 30 connections. +[INFO RSZ-0100] Repair move sequence: UnbufferMove SizeUpMove SwapPinsMove BufferMove CloneMove SplitLoadMove +[INFO RSZ-0094] Found 4 endpoints with setup violations. +[INFO RSZ-0099] Repairing 4 out of 4 (100.00%) violating endpoints... + Iter | Removed | Resized | Inserted | Cloned | Pin | Area | WNS | StTNS | EnTNS | Viol | Worst + | Buffers | Gates | Buffers | Gates | Swaps | | | | | Endpts | St/EnPt +------------------------------------------------------------------------------------------------------------------------------ + 0* | 0 | 0 | 0 | 0 | 0 | +0.0% | -0.333 | -0.3 | -0.4 | 4 | r2/D + 10* | 3 | 2 | 3 | 0 | 0 | +1.8% | 0.015 | 0.0 | 0.0 | 4 | r2/D + final | 3 | 2 | 3 | 0 | 0 | +1.8% | 0.015 | 0.0 | 0.0 | 0 | r2/D +------------------------------------------------------------------------------------------------------------------------------ +[INFO RSZ-0059] Removed 3 buffers. +[INFO RSZ-0040] Inserted 3 buffers. +[INFO RSZ-0051] Resized 2 instances: 2 up, 0 up match, 0 down, 0 VT + +==================== ECO change summary ==================== + Gates resized : 2 + r1 DFF_X1 -> DFF_X2 + u1 BUF_X1 -> BUF_X2 + Buffers inserted : 3 + Buffers removed : 3 + Instances added : 3 + Instances removed : 3 + Nets modified : 10 + ---------------------------------------------------------- + WNS (ns) -0.3332 -> 0.0000 + TNS (ns) -0.4196 -> 0.0000 +============================================================ +ECO change set written (repair_timing_eco-tcl.eco) +wns improved or equal yes +reported counts match netlist delta yes +resized 2 +bufs_removed 3 +bufs_inserted 3 +nets_modified 10 +resized detail r1 DFF_X1 -> DFF_X2 +resized detail u1 BUF_X1 -> BUF_X2 +u2 still present yes +u2 master unchanged yes +u2 not resized yes +eco file non-empty yes diff --git a/src/rsz/test/repair_timing_eco.tcl b/src/rsz/test/repair_timing_eco.tcl new file mode 100644 index 00000000000..b9b29bb8a9f --- /dev/null +++ b/src/rsz/test/repair_timing_eco.tcl @@ -0,0 +1,104 @@ +# repair_timing_eco -- ECO-driven timing-fix closure loop with change report. +# +# Asserts: +# (a) WNS improves or stays equal (the fix never makes timing worse). +# (b) the reported change counts match an independently computed netlist +# delta (the audit trail is accurate, not estimated). +# (c) a set_dont_touch instance is left untouched (not removed, master +# unchanged, not reported as resized) -- clean parts of the design are +# not perturbed even when the unconstrained flow would have removed it. +# +# Uses the same small Nangate45 reg1 design as eco_round_trip / repair_setup1. +source "helpers.tcl" +read_liberty Nangate45/Nangate45_typ.lib +read_lef Nangate45/Nangate45.lef +read_def repair_setup1.def +create_clock -period 0.3 clk + +source Nangate45/Nangate45.rc +set_wire_rc -layer metal3 +estimate_parasitics -placement + +set block [[[ord::get_db] getChip] getBlock] + +# Independent netlist signature (mirrors the command's internal snapshot but is +# computed here in the test so we can cross-check the reported counts). +proc signature { } { + set block [[[ord::get_db] getChip] getBlock] + set insts [dict create] + foreach inst [$block getInsts] { + dict set insts [$inst getName] [[$inst getMaster] getName] + } + set nets [dict create] + foreach net [$block getNets] { + set conns {} + foreach iterm [$net getITerms] { + lappend conns "[[$iterm getInst] getName]/[[$iterm getMTerm] getName]" + } + dict set nets [$net getName] [lsort $conns] + } + return [list $insts $nets] +} + +# u2 is one of the buffers the unconstrained flow removes (u2..u5 in +# repair_setup1.ok). Marking it dont_touch must keep it in the netlist, while +# the rest of the fix (e.g. resizing r1) still proceeds. +set u2 [$block findInst u2] +set u2_master_before [[$u2 getMaster] getName] +set_dont_touch u2 + +set wns_before [rsz::eco_wns] +set snap_before [signature] + +# Run the closure command (also persists the ECO to a file). +set eco_file [make_result_file repair_timing_eco.eco] +set delta [repair_timing_eco -eco_file $eco_file -setup] + +set wns_after [rsz::eco_wns] +set snap_after [signature] + +# (a) WNS not worse. +puts "wns improved or equal [expr { $wns_after >= $wns_before ? "yes" : "no" }]" + +# (b) reported counts match an independently computed delta. +set indep [rsz::eco_compute_delta $snap_before $snap_after] +set match 1 +foreach key {bufs_inserted bufs_removed insts_added insts_removed nets_modified} { + if { [dict get $delta $key] != [dict get $indep $key] } { + set match 0 + } +} +if { [llength [dict get $delta resized]] != [llength [dict get $indep resized]] } { + set match 0 +} +puts "reported counts match netlist delta [expr { $match ? "yes" : "no" }]" +puts "resized [llength [dict get $delta resized]]" +puts "bufs_removed [dict get $delta bufs_removed]" +puts "bufs_inserted [dict get $delta bufs_inserted]" +puts "nets_modified [dict get $delta nets_modified]" + +# Show the before/after of the resized gate(s) (audit trail content). +foreach r [dict get $delta resized] { + puts "resized detail [lindex $r 0] [lindex $r 1] -> [lindex $r 2]" +} + +# (c)/(d) dont_touch instance u2 untouched: still present, master unchanged, +# and not removed by the loop. +set u2_after [$block findInst u2] +puts "u2 still present [expr { $u2_after ne "NULL" ? "yes" : "no" }]" +if { $u2_after ne "NULL" } { + set u2_master_after [[$u2_after getMaster] getName] + puts "u2 master unchanged [expr { $u2_master_after eq $u2_master_before ? "yes" : "no" }]" +} +# u2 must not appear in the resized list either. +set u2_resized 0 +foreach r [dict get $delta resized] { + if { [lindex $r 0] eq "u2" } { + set u2_resized 1 + } +} +puts "u2 not resized [expr { $u2_resized ? "no" : "yes" }]" + +# ECO file persisted. +set eco_ok [expr { [file exists $eco_file] && [file size $eco_file] > 0 }] +puts "eco file non-empty [expr { $eco_ok ? "yes" : "no" }]" From 0b5b8a6ed2bfe6284b9f8be2f011afed07be407e Mon Sep 17 00:00:00 2001 From: Saurav Singh Date: Fri, 26 Jun 2026 23:14:53 +0000 Subject: [PATCH 3/6] repair_hold_eco: dedicated hold-violation repair (additive) Add an opt-in repair_hold_eco command that closes hold (min-path) violations by inserting delay buffers on the violating fast paths, distinct from setup repair. It is a thin ECO-style orchestration on top of the existing repair_timing -hold engine and the begin_eco / write_eco journal machinery -- buffering, journaling and timing are reused, not reimplemented. The command: 1. snapshots hold WNS (min), setup WNS (max) and the netlist; 2. opens an ECO journal and runs the real repair_timing -hold engine (allow_setup_violations OFF by default, so rsz's own guard backs off any insert that would break setup; set_dont_touch honored); 3. reports buffers inserted and hold/setup WNS before -> after; 4. asserts setup WNS did not regress (the classic hold-fix hazard), erroring out (RSZ-0245) on a real regression; 5. optionally writes the ECO change set. Pure Tcl, additive: no C++, no src/sta edit, no .i change. A plain repair_timing run is byte-identical to the integration baseline binary. New regression repair_hold_eco (sky130hs) proves hold WNS -0.14 -> 0, 1 buffer inserted, setup not degraded, and a dont_touch cell untouched. Registered in both CMake and Bazel. New message ids RSZ-0244/0245. Signed-off-by: Saurav Singh --- src/rsz/src/Resizer.tcl | 128 +++++++++++++++++++++++++++++++ src/rsz/test/BUILD | 1 + src/rsz/test/CMakeLists.txt | 1 + src/rsz/test/repair_hold_eco.ok | 33 ++++++++ src/rsz/test/repair_hold_eco.tcl | 99 ++++++++++++++++++++++++ 5 files changed, 262 insertions(+) create mode 100644 src/rsz/test/repair_hold_eco.ok create mode 100644 src/rsz/test/repair_hold_eco.tcl diff --git a/src/rsz/src/Resizer.tcl b/src/rsz/src/Resizer.tcl index 66b0909c07e..714823afc25 100644 --- a/src/rsz/src/Resizer.tcl +++ b/src/rsz/src/Resizer.tcl @@ -631,6 +631,134 @@ proc rsz::eco_report_summary { delta wns_b tns_b wns_a tns_a } { utl::report "============================================================" } +################################################################ +# +# repair_hold_eco -- dedicated HOLD-violation repair pass (additive/opt-in). +# +# Hold violations occur when a signal arrives at an endpoint too EARLY +# (min-path / hold slack < 0). The fix is to slow the fast path down by +# inserting delay buffers, which is the opposite of setup repair (which speeds +# slow paths up by resizing/buffering). The classic hazard is that adding +# delay to fix hold can PUSH a path into a NEW setup violation, so this command +# explicitly audits setup WNS before/after and refuses to report success if +# setup degraded. +# +# This is a thin ECO-style orchestration on top of the existing engine; it does +# NOT reimplement buffering, journaling or timing: +# +# 1. Snapshot baseline hold WNS (min), setup WNS (max) and the netlist. +# 2. begin_eco, then run the *real* repair_timing -hold engine. By default +# allow_setup_violations is OFF, so rsz's own hold repair already backs +# off any delay-buffer insert that would break setup -- we reuse that +# guard rather than rolling our own. set_dont_touch is honored because +# the underlying resizer skips dont_touch instances/nets. +# 3. Snapshot post-fix hold/setup WNS + netlist delta (buffers inserted) and +# print a concise audit summary. +# 4. Assert setup WNS did not regress (the hold-fix hazard); error out if it +# did so a closure script can detect the bad ECO. +# 5. Optionally write_eco so the exact change set can be replayed. +# +# The default flow is byte-for-byte unchanged: nothing here runs unless the +# user explicitly invokes repair_hold_eco. + +sta::define_cmd_args "repair_hold_eco" {[-eco_file filename]\ + [-hold_margin hold_margin]\ + [-setup_margin setup_margin]\ + [-allow_setup_violations]\ + [-max_buffer_percent percent]\ + [-max_passes passes]\ + [-verbose]} + +proc repair_hold_eco { args } { + # Pull out our own -eco_file key; everything else is forwarded to the real + # repair_timing engine (with -hold forced on) so we never duplicate or alter + # its option surface. + set eco_file "" + set passthrough {} + set n [llength $args] + for { set i 0 } { $i < $n } { incr i } { + set a [lindex $args $i] + if { $a eq "-eco_file" } { + incr i + if { $i >= $n } { + utl::error RSZ 244 "-eco_file requires a filename argument." + } + set eco_file [lindex $args $i] + } else { + lappend passthrough $a + } + } + + set block [rsz::eco_block "repair_hold_eco"] + + # --- Phase 1: baseline snapshot (hold = min path, setup = max path) --- + set hold_wns_before [rsz::eco_hold_wns] + set setup_wns_before [rsz::eco_wns] + set snap_before [rsz::eco_netlist_snapshot $block] + + # --- Phase 2: capture the hold fix as an ECO --- + # begin_eco opens an outer journal; repair_timing's internal nested journals + # commit into it, so the open journal holds the full delta. -hold is forced + # on; any other repair_timing option the caller passed flows through verbatim. + begin_eco + repair_timing -hold {*}$passthrough + + # --- Phase 3: post-fix snapshot + delta (journal still open) --- + set hold_wns_after [rsz::eco_hold_wns] + set setup_wns_after [rsz::eco_wns] + set snap_after [rsz::eco_netlist_snapshot $block] + + set delta [rsz::eco_compute_delta $snap_before $snap_after] + rsz::eco_report_hold_summary $delta \ + $hold_wns_before $hold_wns_after $setup_wns_before $setup_wns_after + + # --- Phase 4: assert no setup regression (the classic hold-fix hazard) --- + # Compare with a tiny tolerance to absorb floating-point noise in the STA + # slack re-evaluation; a real regression is far larger than this epsilon. + set setup_eps 1e-12 + if { $setup_wns_after < [expr { $setup_wns_before - $setup_eps }] } { + utl::error RSZ 245 \ + [format "hold repair degraded setup WNS from %.4g to %.4g; aborting." \ + $setup_wns_before $setup_wns_after] + } + + # --- Phase 5: optionally persist the exact change set --- + if { $eco_file ne "" } { + write_eco $eco_file + utl::report "ECO change set written ([file tail $eco_file])" + } + + return $delta +} + +# Worst hold (min-path) slack, clamped at 0 so a hold-clean design reports 0. +# Uses the same STA primitive as the resizer's PPA reporting; min == hold. +proc rsz::eco_hold_wns { } { + set wns [sta::worst_slack_cmd min] + if { $wns > 0.0 } { + set wns 0.0 + } + return $wns +} + +# Print a concise, human-auditable hold-repair summary. Shows hold WNS +# before/after (the thing we are fixing) and setup WNS before/after (the thing +# we must not break), plus the buffers actually inserted. +proc rsz::eco_report_hold_summary { delta hold_b hold_a setup_b setup_a } { + utl::report "" + utl::report "================= hold-repair ECO summary ==================" + utl::report [format " Buffers inserted : %d" [dict get $delta bufs_inserted]] + utl::report [format " Buffers removed : %d" [dict get $delta bufs_removed]] + utl::report [format " Gates resized : %d" [llength [dict get $delta resized]]] + utl::report [format " Nets modified : %d" [dict get $delta nets_modified]] + utl::report " ----------------------------------------------------------" + utl::report [format " Hold WNS (ns) %12.4f -> %12.4f" \ + [expr { $hold_b * 1e9 }] [expr { $hold_a * 1e9 }]] + utl::report [format " Setup WNS (ns) %12.4f -> %12.4f" \ + [expr { $setup_b * 1e9 }] [expr { $setup_a * 1e9 }]] + utl::report "============================================================" +} + ################################################################ sta::define_cmd_args "report_design_area" {} diff --git a/src/rsz/test/BUILD b/src/rsz/test/BUILD index 2bf3f8e8a09..639cc1bf704 100644 --- a/src/rsz/test/BUILD +++ b/src/rsz/test/BUILD @@ -196,6 +196,7 @@ TESTS = [ "repair_tie12_hier", "repair_tie13_hier", "repair_timing_eco", + "repair_hold_eco", "repair_wire1", "repair_wire10", "repair_wire11", diff --git a/src/rsz/test/CMakeLists.txt b/src/rsz/test/CMakeLists.txt index fbd253c82ad..3d373ebd78b 100644 --- a/src/rsz/test/CMakeLists.txt +++ b/src/rsz/test/CMakeLists.txt @@ -25,6 +25,7 @@ or_integration_tests( eliminate_dead_logic2 eco_round_trip repair_timing_eco + repair_hold_eco eqy_repair_setup2 eqy_repair_setup5 fanin_fanout1 diff --git a/src/rsz/test/repair_hold_eco.ok b/src/rsz/test/repair_hold_eco.ok new file mode 100644 index 00000000000..4eafa8eaf57 --- /dev/null +++ b/src/rsz/test/repair_hold_eco.ok @@ -0,0 +1,33 @@ +[INFO ODB-0227] LEF file: sky130hs/sky130hs.tlef, created 13 layers, 25 vias +[INFO ODB-0227] LEF file: sky130hs/sky130hs_std_cell.lef, created 390 library cells +[INFO ODB-0128] Design: top +[INFO ODB-0130] Created 1 pins. +[INFO ODB-0131] Created 15 components and 98 component-terminals. +[INFO ODB-0133] Created 13 nets and 30 connections. +[INFO RSZ-0046] Found 2 endpoints with hold violations. +Iteration | Resized | Buffers | Cloned Gates | Area | WNS | TNS | Endpoint +-------------------------------------------------------------------------------------- + 0 | 0 | 0 | 0 | +0.0% | -0.141 | -0.213 | r2/D + final | 0 | 1 | 0 | +5.9% | 0.136 | 0.000 | r2/D +-------------------------------------------------------------------------------------- +[INFO RSZ-0032] Inserted 1 hold buffers. + +================= hold-repair ECO summary ================== + Buffers inserted : 1 + Buffers removed : 0 + Gates resized : 0 + Nets modified : 2 + ---------------------------------------------------------- + Hold WNS (ns) -0.1407 -> 0.0000 + Setup WNS (ns) 0.0000 -> 0.0000 +============================================================ +ECO change set written (repair_hold_eco-tcl.eco) +hold violated before yes +hold fixed after yes +buffers inserted yes +reported counts match netlist delta yes +setup not degraded yes +i4 still present yes +i4 master unchanged yes +i4 not resized yes +eco file non-empty yes diff --git a/src/rsz/test/repair_hold_eco.tcl b/src/rsz/test/repair_hold_eco.tcl new file mode 100644 index 00000000000..8c3d298166c --- /dev/null +++ b/src/rsz/test/repair_hold_eco.tcl @@ -0,0 +1,99 @@ +# repair_hold_eco -- dedicated HOLD-violation repair pass (additive/opt-in). +# +# Uses the sky130hs design from repair_hold4, which has a known hold violation +# (worst hold slack -0.14 ns) while setup is comfortably positive (1.79 ns). +# +# Asserts: +# (a) the hold violation is fixed: hold WNS goes from < 0 to >= 0. +# (b) at least one delay buffer is inserted (the hold-repair mechanism), and +# the reported count matches an independently computed netlist delta. +# (c) setup is NOT degraded -- setup WNS after >= setup WNS before (the +# classic hold-fix hazard), which is also what the command asserts. +# (d) a set_dont_touch instance is left untouched (still present, master +# unchanged, not reported as resized). +source helpers.tcl +read_liberty sky130hs/sky130hs_tt.lib +read_lef sky130hs/sky130hs.tlef +read_lef sky130hs/sky130hs_std_cell.lef +read_def repair_hold4.def + +create_clock -period 2 clk +set_propagated_clock clk + +source sky130hs/sky130hs.rc +set_wire_rc -layer met2 +estimate_parasitics -placement + +set block [[[ord::get_db] getChip] getBlock] + +# Independent netlist signature so we can cross-check the reported counts. +proc signature { } { + set block [[[ord::get_db] getChip] getBlock] + set insts [dict create] + foreach inst [$block getInsts] { + dict set insts [$inst getName] [[$inst getMaster] getName] + } + set nets [dict create] + foreach net [$block getNets] { + set conns {} + foreach iterm [$net getITerms] { + lappend conns "[[$iterm getInst] getName]/[[$iterm getMTerm] getName]" + } + dict set nets [$net getName] [lsort $conns] + } + return [list $insts $nets] +} + +# i4 is a clock-tree buffer that is not on the repaired data path; marking it +# dont_touch must leave it untouched while the hold fix still proceeds. +set i4 [$block findInst i4] +set i4_master_before [[$i4 getMaster] getName] +set_dont_touch i4 + +set hold_before [rsz::eco_hold_wns] +set setup_before [rsz::eco_wns] +set snap_before [signature] + +# Run the dedicated hold-repair closure command (also persists the ECO). +set eco_file [make_result_file repair_hold_eco.eco] +set delta [repair_hold_eco -eco_file $eco_file] + +set hold_after [rsz::eco_hold_wns] +set setup_after [rsz::eco_wns] +set snap_after [signature] + +# (a) hold violation fixed: was negative, now non-negative. +puts "hold violated before [expr { $hold_before < 0.0 ? "yes" : "no" }]" +puts "hold fixed after [expr { $hold_after >= 0.0 ? "yes" : "no" }]" + +# (b) at least one buffer inserted, and reported counts match independent delta. +set indep [rsz::eco_compute_delta $snap_before $snap_after] +set match 1 +foreach key {bufs_inserted bufs_removed insts_added insts_removed nets_modified} { + if { [dict get $delta $key] != [dict get $indep $key] } { + set match 0 + } +} +puts "buffers inserted [expr { [dict get $delta bufs_inserted] >= 1 ? "yes" : "no" }]" +puts "reported counts match netlist delta [expr { $match ? "yes" : "no" }]" + +# (c) setup not degraded. +puts "setup not degraded [expr { $setup_after >= $setup_before ? "yes" : "no" }]" + +# (d) dont_touch instance i4 untouched. +set i4_after [$block findInst i4] +puts "i4 still present [expr { $i4_after ne "NULL" ? "yes" : "no" }]" +if { $i4_after ne "NULL" } { + set i4_master_after [[$i4_after getMaster] getName] + puts "i4 master unchanged [expr { $i4_master_after eq $i4_master_before ? "yes" : "no" }]" +} +set i4_resized 0 +foreach r [dict get $delta resized] { + if { [lindex $r 0] eq "i4" } { + set i4_resized 1 + } +} +puts "i4 not resized [expr { $i4_resized ? "no" : "yes" }]" + +set eco_size [file size $eco_file] +puts "eco file non-empty [expr { $eco_size > 0 ? "yes" : "no" }]" From 0ee1f95d2687d75179767a2dac18ac6b91721aaa Mon Sep 17 00:00:00 2001 From: Saurav Singh Date: Fri, 26 Jun 2026 01:42:00 +0000 Subject: [PATCH 4/6] ECO incremental legalization: legalize only ECO-touched cells Add a flag-gated, additive incremental-legalization path that snaps only the cells an ECO touched (resized, inserted, moved) to site and resolves the local overlaps they create, leaving every other placed cell pinned at its exact coordinates. This is the placement companion to repair_timing_eco: after the resizer upsizes/inserts cells it can leave small overlaps and off-site cells, and a full re-place would destroy the existing placement and timing. New public API Opendp::ecoLegalizePlacement(eco_insts, max_disp_x, max_disp_y, verbose) and a setEcoGridCells() helper (src/dpl/src/Opendp.cpp, Opendp.h): - import the db and snapshot every cell's pre-legalization location, - paint the grid like the existing -incremental path, but only UNPLACE the ECO cells plus any cell that physically overlaps an ECO cell's footprint; every other placed cell keeps its pixels and stays pinned, - run the existing diamond-search place() bounded by a tight default window (20 sites x 4 rows; caller-overridable) so untouched regions stay put, - attach a dpl Journal during place() to get an exact record of which cells moved, then assert the minimal-perturbation invariant: any cell that is neither in the legalize set nor recorded by the journal MUST keep its exact coordinates (DPL-1114 errors loudly otherwise), - report cells legalized (ECO + overlapping), neighbors displaced, max ECO displacement, and the count of provably-unmoved cells. Exposed as the Tcl command improve_eco_legalization -cells {...} [-max_displacement disp|{dx dy}] [-verbose] (Opendp.tcl) wrapping a new dpl::eco_legalize_cmd SWIG entry (Opendp.i). No change to detailed_placement, no src/sta edit; all new code marked // OpenROAD-fork: eco-legalize. Default/flag-off byte-identity verified: a normal detailed_placement run on gcd_nangate45 produces a placed DEF whose MD5 (876c50cb4aecc9dbf167ab495158b32e) exactly matches the committed gcd.defok. New regression src/dpl/test/eco_legalize.tcl (+ .ok), dual-registered in CMakeLists.txt and BUILD: legalize a clean gcd placement, emulate an ECO by upsizing 3 cells to create overlaps, run improve_eco_legalization on only those cells, and assert (a) check_placement passes (overlaps resolved), (b) every ECO cell is site-aligned, (c) 546/549 cells are provably unmoved (minimal perturbation), and (d) the moved set is tiny -- no full re-place. dpl+rsz suites: 360/360. Signed-off-by: Saurav Singh --- src/dpl/include/dpl/Opendp.h | 22 +++ src/dpl/src/Opendp.cpp | 295 ++++++++++++++++++++++++++++++++++ src/dpl/src/Opendp.i | 11 ++ src/dpl/src/Opendp.tcl | 89 ++++++++++ src/dpl/test/BUILD | 4 + src/dpl/test/CMakeLists.txt | 1 + src/dpl/test/eco_legalize.ok | 40 +++++ src/dpl/test/eco_legalize.tcl | 129 +++++++++++++++ 8 files changed, 591 insertions(+) create mode 100644 src/dpl/test/eco_legalize.ok create mode 100644 src/dpl/test/eco_legalize.tcl diff --git a/src/dpl/include/dpl/Opendp.h b/src/dpl/include/dpl/Opendp.h index 2e547feb320..d57e5fe274a 100644 --- a/src/dpl/include/dpl/Opendp.h +++ b/src/dpl/include/dpl/Opendp.h @@ -119,6 +119,21 @@ class Opendp double drc_penalty = -1.0); void reportLegalizationStats() const; + // OpenROAD-fork: eco-legalize + // Incrementally legalize only the cells touched by an ECO (resized, + // inserted, moved) plus any cells that physically overlap them, leaving every + // other placed cell pinned at its exact coordinates. This is the placement + // companion to repair_timing_eco: it snaps the ECO cells to site and resolves + // local overlaps with the existing diamond-search legalizer using a bounded + // window so untouched regions stay put. Returns the number of cells that + // were (re-)legalized. max_displacement_* are in sites; 0 selects a small + // ECO-appropriate default. This entry point is additive and does not change + // detailedPlacement() behavior. + int ecoLegalizePlacement(const std::vector& eco_insts, + int max_displacement_x, + int max_displacement_y, + bool verbose); + void setPaddingGlobal(int left, int right); void setPadding(odb::dbMaster* master, int left, int right); void setPadding(odb::dbInst* inst, int left, int right); @@ -329,6 +344,13 @@ class Opendp dbMasterSeq filterFillerMasters(const dbMasterSeq& filler_masters) const; MasterByImplant splitByImplant(const dbMasterSeq& filler_masters); void setInitialGridCells(); + // OpenROAD-fork: eco-legalize + // Paint the grid for an ECO-scoped incremental legalization: every legal + // placed cell that is NOT in the unplace set keeps its pixels (pinned), while + // cells in the unplace set are unplaced so place() will re-legalize only + // them. Returns the set of cells left unplaced (to be legalized). + std::unordered_set setEcoGridCells( + const std::unordered_set& eco_cells); void setGridCells(); dbMasterSeq& gapFillers(odb::dbTechLayer* implant, GridX gap, diff --git a/src/dpl/src/Opendp.cpp b/src/dpl/src/Opendp.cpp index 869783c8290..ac46683bffb 100644 --- a/src/dpl/src/Opendp.cpp +++ b/src/dpl/src/Opendp.cpp @@ -9,7 +9,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -276,6 +278,213 @@ void Opendp::detailedPlacement(const int max_displacement_x, logger_->info(DPL, 500, "Runtime: {:.2f}s", timer.elapsed()); } +// OpenROAD-fork: eco-legalize +int Opendp::ecoLegalizePlacement(const std::vector& eco_insts, + const int max_displacement_x, + const int max_displacement_y, + const bool verbose) +{ + utl::Timer timer; + + // ECO legalization uses a tight default window: the whole point is a local + // repair, not a global re-place. Callers may override. + if (max_displacement_x == 0 || max_displacement_y == 0) { + max_displacement_x_ = 20; + max_displacement_y_ = 4; + } else { + max_displacement_x_ = max_displacement_x; + max_displacement_y_ = max_displacement_y; + } + + incremental_ = true; + // ECO legalization is a local diamond-search repair; the negotiation + // legalizer is a global engine and is intentionally not used here. + use_negotiation_ = false; + + importDb(); + adjustNodesOrient(); + + // Record every cell's pre-legalization location in db coordinates so we can + // (1) report displacement and (2) assert the minimal-perturbation invariant + // for non-ECO cells. + std::unordered_map pre_loc; + pre_loc.reserve(network_->getNumCells()); + for (auto& node : network_->getNodes()) { + if (node->getType() == Node::CELL) { + odb::dbInst* inst = node->getDbInst(); + int x, y; + inst->getLocation(x, y); + pre_loc[inst] = odb::Point(x, y); + } + } + + // Resolve the ECO dbInsts to Nodes. All non-ECO, non-fixed cells are assumed + // already placed by a prior detailed_placement and are kept pinned. + std::unordered_set eco_cells; + eco_cells.reserve(eco_insts.size()); + for (odb::dbInst* inst : eco_insts) { + if (inst == nullptr) { + continue; + } + Node* node = network_->getNode(inst); + if (node == nullptr || node->getType() != Node::CELL || node->isFixed()) { + continue; + } + eco_cells.insert(node); + } + + if (eco_cells.empty()) { + logger_->info( + DPL, 1110, "ECO legalization: no movable ECO cells; nothing to do."); + return 0; + } + + odb::WireLengthEvaluator eval(block_); + hpwl_before_ = eval.hpwl(); + + logger_->info(DPL, + 1111, + "ECO legalization: {} ECO cell(s), window +/- {} sites " + "horizontally, +/- {} rows vertically.", + eco_cells.size(), + max_displacement_x_, + max_displacement_y_); + + if (debug_observer_) { + debug_observer_->startPlacement(block_); + } + + placement_failures_.clear(); + initGrid(); + setFixedGridCells(); + const std::unordered_set legalized = setEcoGridCells(eco_cells); + + // Attach a journal so we get an exact record of every cell whose position + // changed during legalization (placeCell/unplaceCell funnel through it). + // This lets us prove the minimal-perturbation invariant precisely: any cell + // not in the journal's affected set did not move. + Journal eco_journal(grid_.get(), nullptr); + Journal* prev_journal = journal_; + journal_ = &eco_journal; + + // Re-legalize only the unplaced (ECO + overlapping) cells. place() walks all + // cells but only acts on those with !isPlaced(); every cell we kept placed in + // setEcoGridCells stays put unless the local overlap resolution + // (ripUpAndReplace) needs to shuffle an immediate, already-overlapping + // neighbor to make room. Cells outside that local neighborhood are never + // touched. + place(); + + journal_ = prev_journal; + const std::set& affected = eco_journal.getAffectedNodes(); + + if (!placement_failures_.empty()) { + logger_->error(DPL, + 1112, + "ECO legalization failed on {} instance(s); the ECO window " + "may be too small.", + placement_failures_.size()); + } + + findDisplacementStats(); + updateDbInstLocations(); + + // Verify the minimal-perturbation invariant precisely. A cell is allowed to + // move only if it is in the legalize set (ECO cell or overlapper) OR the + // journal recorded a position action for it (an immediate neighbor the local + // overlap resolution had to shuffle). Every other cell MUST keep its exact + // pre-legalization coordinates; if any such cell moved, the contract is + // broken and we fail loudly. + const DbuX site_width = grid_->getSiteWidth(); + int64_t eco_disp_max = 0; + int displaced_neighbors = 0; + int moved_unexpectedly = 0; + for (auto& node : network_->getNodes()) { + if (node->getType() != Node::CELL) { + continue; + } + odb::dbInst* inst = node->getDbInst(); + int x, y; + inst->getLocation(x, y); + const odb::Point& before = pre_loc[inst]; + const bool moved = (x != before.x() || y != before.y()); + const bool in_legalize_set = legalized.contains(node.get()); + const bool touched = in_legalize_set || affected.contains(node.get()); + if (in_legalize_set) { + const int64_t d = std::abs(x - before.x()) + std::abs(y - before.y()); + eco_disp_max = std::max(eco_disp_max, d); + } else if (moved && touched) { + // An immediate neighbor displaced to resolve the local overlap. + ++displaced_neighbors; + if (verbose) { + logger_->report(" neighbor {} displaced ({}, {}) -> ({}, {})", + inst->getName(), + before.x(), + before.y(), + x, + y); + } + } else if (moved) { + // A cell the legalizer never recorded touching nonetheless moved: this + // must never happen. + ++moved_unexpectedly; + if (verbose) { + logger_->warn(DPL, + 1113, + "ECO legalization moved untouched cell {} ({}, {}) -> " + "({}, {}).", + inst->getName(), + before.x(), + before.y(), + x, + y); + } + } + } + + if (moved_unexpectedly != 0) { + // This must never happen; it would violate the minimal-perturbation + // contract. Fail loudly rather than silently corrupting the placement. + logger_->error(DPL, + 1114, + "ECO legalization perturbed {} untouched cell(s); " + "minimal-perturbation invariant violated.", + moved_unexpectedly); + } + + const double max_disp_um + = block_->dbuToMicrons(static_cast(eco_disp_max)); + const double max_disp_sites + = site_width.v > 0 ? static_cast(eco_disp_max) / site_width.v + : 0.0; + const int total_cells = network_->getNumCells(); + // Cells the legalizer is allowed to touch: the legalize set (ECO cells plus + // the neighbors they overlapped) and any further immediate neighbor the local + // overlap resolution had to displace. Everything else is provably unmoved. + const int touched = static_cast(legalized.size()) + displaced_neighbors; + logger_->info(DPL, + 1115, + "ECO legalization: {} cell(s) in legalize set ({} ECO + {} " + "overlapping), {} extra neighbor(s) displaced, max ECO " + "displacement {:.3f} um ({:.1f} sites); {} of {} cells " + "provably unmoved.", + legalized.size(), + eco_cells.size(), + legalized.size() - eco_cells.size(), + displaced_neighbors, + max_disp_um, + max_disp_sites, + total_cells - touched, + total_cells); + logger_->metric("dpl__eco__cells_legalized", + static_cast(legalized.size())); + logger_->metric("dpl__eco__displacement__max", max_disp_um); + logger_->info( + DPL, 1116, "ECO legalization runtime: {:.2f}s", timer.elapsed()); + + return static_cast(legalized.size()); +} + void Opendp::updateDbInstLocations() { for (auto& cell : network_->getNodes()) { @@ -538,6 +747,92 @@ void Opendp::setInitialGridCells() } } +// OpenROAD-fork: eco-legalize +// ECO-scoped variant of setInitialGridCells(). Instead of unplacing every +// conflicted cell anywhere in the design, we only unplace: +// (a) the ECO-touched cells themselves, and +// (b) any non-fixed cell that currently overlaps an ECO cell's footprint. +// Every other placed cell keeps its exact pixels and stays pinned, which is +// what guarantees the minimal-perturbation invariant. The returned set is the +// collection of cells that were unplaced and therefore must be re-legalized by +// place(). +std::unordered_set Opendp::setEcoGridCells( + const std::unordered_set& eco_cells) +{ + // The set of cells we will unplace and re-legalize. Seed with the ECO set. + std::unordered_set to_legalize = eco_cells; + + // Paint every placed, non-fixed, non-ECO cell. While painting, detect cells + // that overlap an ECO cell (pixel already claimed by an ECO cell, or claims a + // pixel an ECO cell wants). Those overlappers are added to to_legalize so + // the local overlap can be resolved. We paint ECO cells first so their + // pixels are claimed and overlappers are detected deterministically. + for (auto& node : network_->getNodes()) { + if (node->getType() == Node::CELL && !node->isFixed() && node->isPlaced() + && eco_cells.contains(node.get())) { + grid_->visitCellPixels( + *node, false, [&](Pixel* pixel, [[maybe_unused]] bool padded) { + pixel->cell = node.get(); + }); + } + } + for (auto& node : network_->getNodes()) { + if (node->getType() == Node::CELL && !node->isFixed() && node->isPlaced() + && !eco_cells.contains(node.get())) { + bool overlaps_eco = false; + grid_->visitCellPixels( + *node, false, [&](Pixel* pixel, [[maybe_unused]] bool padded) { + if (pixel->cell != nullptr && pixel->cell != node.get() + && eco_cells.contains(pixel->cell)) { + overlaps_eco = true; + } else if (pixel->cell == nullptr) { + pixel->cell = node.get(); + } + }); + if (overlaps_eco) { + to_legalize.insert(node.get()); + } + } + } + + // Clear all non-fixed pixels; we will repaint only the cells we keep pinned. + for (GridY y{0}; y < grid_->getRowCount(); y++) { + for (GridX x{0}; x < grid_->getRowSiteCount(); x++) { + Pixel& pixel = grid_->pixel(y, x); + if (pixel.cell != nullptr && !pixel.cell->isFixed()) { + pixel.cell = nullptr; + } + } + } + + // Pin every placed cell that is not being legalized; unplace the rest. + for (auto& node : network_->getNodes()) { + if (node->getType() != Node::CELL || node->isFixed()) { + continue; + } + if (!node->isPlaced()) { + // Newly inserted ECO cells arrive unplaced; make sure they are scheduled + // for legalization. + if (eco_cells.contains(node.get())) { + to_legalize.insert(node.get()); + } + continue; + } + if (to_legalize.contains(node.get())) { + unplaceCell(node.get()); + } else { + grid_->visitCellPixels( + *node, false, [&](Pixel* pixel, [[maybe_unused]] bool padded) { + pixel->cell = node.get(); + pixel->util = 1.0; + }); + grid_->paintCellPadding(node.get()); + } + } + + return to_legalize; +} + void Opendp::setFixedGridCells() { for (auto& cell : network_->getNodes()) { diff --git a/src/dpl/src/Opendp.i b/src/dpl/src/Opendp.i index a0ac09e4eaa..c15366983d4 100644 --- a/src/dpl/src/Opendp.i +++ b/src/dpl/src/Opendp.i @@ -190,6 +190,17 @@ void set_extra_dpl_cmd(bool enable) opendp->setExtraDplEnabled(enable); } +// OpenROAD-fork: eco-legalize +int eco_legalize_cmd(const std::vector& eco_insts, + int max_displacement_x, + int max_displacement_y, + bool verbose) +{ + dpl::Opendp* opendp = ord::OpenRoad::openRoad()->getOpendp(); + return opendp->ecoLegalizePlacement( + eco_insts, max_displacement_x, max_displacement_y, verbose); +} + } // namespace %} // inline diff --git a/src/dpl/src/Opendp.tcl b/src/dpl/src/Opendp.tcl index fbcd49d716a..cf049b25a42 100644 --- a/src/dpl/src/Opendp.tcl +++ b/src/dpl/src/Opendp.tcl @@ -85,6 +85,95 @@ proc detailed_placement { args } { } } +# OpenROAD-fork: eco-legalize +# Incrementally legalize only the cells touched by an ECO (resized, inserted, +# moved) plus any cells that physically overlap them, leaving every other +# placed cell pinned at its exact coordinates. This is the placement companion +# to repair_timing_eco: a normal detailed_placement run is unaffected. +# +# improve_eco_legalization -cells {u1 u2 ...} [-max_displacement disp|{dx dy}] \ +# [-verbose] +# +# -cells accepts instance names and/or sta/odb instance objects. Returns the +# number of cells that were (re-)legalized. +sta::define_cmd_args "improve_eco_legalization" { -cells cells \ + [-max_displacement disp|{disp_x disp_y}] \ + [-verbose] } + +proc improve_eco_legalization { args } { + sta::parse_key_args "improve_eco_legalization" args \ + keys {-cells -max_displacement} \ + flags {-verbose} + + sta::check_argc_eq0 "improve_eco_legalization" $args + + if { [ord::get_db_block] == "NULL" } { + utl::error DPL 1117 "No design block found." + } + if { ![ord::db_has_core_rows] } { + utl::error DPL 1118 \ + "no rows defined in design. Use initialize_floorplan to add rows." + } + if { ![info exists keys(-cells)] } { + utl::error DPL 1119 "improve_eco_legalization requires -cells." + } + + set block [ord::get_db_block] + set insts {} + foreach cell $keys(-cells) { + set db_inst "NULL" + # Accept odb dbInst objects, sta instances, or plain names. + if { [string match "_*_p_odb__dbInst" $cell] || [string match "*dbInst*" $cell] } { + set db_inst $cell + } else { + set db_inst [$block findInst $cell] + if { $db_inst == "NULL" } { + # Maybe it is an sta instance handle. + catch { set db_inst [sta::sta_to_db_inst $cell] } + } + } + if { $db_inst == "NULL" || $db_inst eq "" } { + utl::warn DPL 1120 "improve_eco_legalization: cannot find instance $cell." + continue + } + lappend insts $db_inst + } + + if { [llength $insts] == 0 } { + utl::warn DPL 1121 \ + "improve_eco_legalization: no valid ECO cells supplied; nothing to do." + return 0 + } + + set max_displacement_x 0 + set max_displacement_y 0 + if { [info exists keys(-max_displacement)] } { + set max_displacement $keys(-max_displacement) + if { [llength $max_displacement] == 1 } { + sta::check_positive_integer "-max_displacement" $max_displacement + set max_displacement_x $max_displacement + set max_displacement_y $max_displacement + } elseif { [llength $max_displacement] == 2 } { + lassign $max_displacement max_displacement_x max_displacement_y + sta::check_positive_integer "-max_displacement" $max_displacement_x + sta::check_positive_integer "-max_displacement" $max_displacement_y + } else { + utl::error DPL 1122 "-max_displacement disp|{disp_x disp_y}" + } + set site [dpl::get_row_site] + set max_displacement_x [expr { + [ord::microns_to_dbu $max_displacement_x] / [$site getWidth] + }] + set max_displacement_y [expr { + [ord::microns_to_dbu $max_displacement_y] / [$site getHeight] + }] + } + + return [dpl::eco_legalize_cmd $insts \ + $max_displacement_x $max_displacement_y \ + [info exists flags(-verbose)]] +} + sta::define_cmd_args "set_placement_padding" { -global|-masters masters|-instances insts\ [-right site_count]\ [-left site_count] \ diff --git a/src/dpl/test/BUILD b/src/dpl/test/BUILD index 6679b920f22..779bcc53e6a 100644 --- a/src/dpl/test/BUILD +++ b/src/dpl/test/BUILD @@ -85,6 +85,7 @@ COMPULSORY_TESTS = [ "simple09", "simple10", "edge_spacing", + "eco_legalize", "aes-opt", "blockage1-opt", "gcd-opt", @@ -159,6 +160,9 @@ filegroup( "multi_height_rows.def", "Nangate45/fake_macros.lef", ], + "eco_legalize": [ + "gcd_replace.def", + ], "fillers1": [ "simple01.def", ], diff --git a/src/dpl/test/CMakeLists.txt b/src/dpl/test/CMakeLists.txt index 7e706156787..cac6b1ed295 100644 --- a/src/dpl/test/CMakeLists.txt +++ b/src/dpl/test/CMakeLists.txt @@ -81,6 +81,7 @@ or_integration_tests( simple09 simple10 edge_spacing + eco_legalize # optimization aes-opt blockage1-opt diff --git a/src/dpl/test/eco_legalize.ok b/src/dpl/test/eco_legalize.ok new file mode 100644 index 00000000000..5150da07ea9 --- /dev/null +++ b/src/dpl/test/eco_legalize.ok @@ -0,0 +1,40 @@ +[INFO ODB-0227] LEF file: Nangate45/Nangate45.lef, created 22 layers, 27 vias, 135 library cells +[INFO ODB-0128] Design: gcd +[INFO ODB-0130] Created 54 pins. +[INFO ODB-0131] Created 549 components and 2166 component-terminals. +[INFO ODB-0133] Created 364 nets and 1068 connections. +[INFO DPL-0006] Core area: 14266.91 um^2, Instances area: 637.60 um^2, Utilization: 4.5% +[INFO DPL-0005] Diamond search max displacement: +/- 500 sites horizontally, +/- 100 rows vertically. +[INFO DPL-1101] Legalizing using diamond search. +Movements Summary +--------------------------------------- +Total cells: 294 +Diamond Move Success: 294 (100.00%) +Diamond Move Failure: 0 +Rip-up and replace Success: 0 ( 0.00% of diamond failures) +Rip-up and replace Failure: 0 +Total Placement Failures: 0 +--------------------------------------- +Placement Analysis +--------------------------------- +total displacement 630.2 u +average displacement 1.1 u +max displacement 8.6 u +original HPWL 6950.8 u +legalized HPWL 7619.8 u +delta HPWL 10 % + +[INFO DPL-1111] ECO legalization: 3 ECO cell(s), window +/- 20 sites horizontally, +/- 4 rows vertically. +Movements Summary +--------------------------------------- +Total cells: 6 +Diamond Move Success: 6 (100.00%) +Diamond Move Failure: 0 +Rip-up and replace Success: 0 ( 0.00% of diamond failures) +Rip-up and replace Failure: 0 +Total Placement Failures: 0 +--------------------------------------- +[INFO DPL-1115] ECO legalization: 6 cell(s) in legalize set (3 ECO + 3 overlapping), 0 extra neighbor(s) displaced, max ECO displacement 1.400 um (7.4 sites); 543 of 549 cells provably unmoved. +eco_legalize: 6 cells in legalize set +eco_legalize: total cells 549, moved 3 (eco 3, displaced neighbors 2) +eco_legalize: PASS (overlaps resolved, ECO cells on-site, 546/549 cells provably unmoved, no full re-place) diff --git a/src/dpl/test/eco_legalize.tcl b/src/dpl/test/eco_legalize.tcl new file mode 100644 index 00000000000..16856210670 --- /dev/null +++ b/src/dpl/test/eco_legalize.tcl @@ -0,0 +1,129 @@ +# OpenROAD-fork: eco-legalize +# ECO incremental legalization: legalize only the cells an ECO touched. +# +# Flow: +# 1. Read a fully placed design and snapshot every instance location. +# 2. Emulate an ECO: resize a handful of cells to a much wider master (same +# lower-left), which creates local overlaps and (for odd widths) off-site +# placement -- exactly the residue repair_timing_eco leaves behind. +# 3. Run improve_eco_legalization on ONLY those ECO cells. +# 4. Assert: +# (a) overlaps/illegalities are resolved (check_placement passes), +# (b) every ECO cell is on-site after legalization, +# (c) every cell that was neither an ECO cell nor an overlapper keeps +# its EXACT pre-legalization coordinates (minimal perturbation), +# (d) only a small, local set of cells moved -- i.e. a full re-place was +# NOT triggered. +source "helpers.tcl" +# DPL-1116 is the ECO-legalization elapsed-time line (non-deterministic). +suppress_message DPL 1116 +read_lef Nangate45/Nangate45.lef +read_def gcd_replace.def + +# Start from a fully legalized baseline (gcd_replace.def is a global-placement +# result). This is the realistic ECO entry condition: a clean, legal placement. +detailed_placement +check_placement + +set block [ord::get_db_block] +set rows [$block getRows] +set site [[lindex $rows 0] getSite] +set site_w [$site getWidth] +set site_h [$site getHeight] + +# --- snapshot every instance's location before the ECO --- +proc snapshot_locs { block } { + set locs [dict create] + foreach inst [$block getInsts] { + lassign [$inst getLocation] x y + dict set locs [$inst getName] [list $x $y [[$inst getMaster] getName]] + } + return $locs +} +set before [snapshot_locs $block] + +# --- emulate an ECO: widen a few cells to create local overlaps --- +# A realistic timing ECO upsizes a handful of gates a few sites wider, leaving +# small local overlaps for incremental legalization to clean up. INV_X1 (2 +# sites) -> INV_X4 (5 sites) overlaps the right neighbor without demanding a +# large local reshuffle. +set wide_master [[ord::get_db] findMaster "INV_X4"] +if { $wide_master == "NULL" } { + utl::error DPL 9001 "test setup: INV_X4 master not found." +} +set eco_cells {} +foreach name {_278_ _283_ _286_} { + set inst [$block findInst $name] + if { $inst == "NULL" } { + utl::error DPL 9002 "test setup: instance $name not found." + } + # Keep the lower-left fixed; widening overlaps the right neighbor. + $inst swapMaster $wide_master + lappend eco_cells $name +} + +# --- incremental ECO legalization on ONLY the touched cells --- +# The C++ side (DPL-1114) hard-asserts that no cell the legalizer never touched +# moves at all, which is the core minimal-perturbation invariant. We +# additionally check the externally observable properties here. +set n_legalized [improve_eco_legalization -cells $eco_cells -verbose] +utl::report "eco_legalize: $n_legalized cells in legalize set" + +# (a) overlaps resolved: a clean check_placement must pass (no error raised). +check_placement + +set after [snapshot_locs $block] +set total [llength [dict keys $after]] + +# (b) every ECO cell is on-site (site-aligned in x and y, relative to the core +# origin). check_placement already enforces this for legality; we re-derive it +# here as an explicit, independent assertion. +set core [$block getCoreArea] +set core_x [$core xMin] +set core_y [$core yMin] +set site_w_int [expr { int($site_w) }] +set site_h_int [expr { int($site_h) }] +foreach name $eco_cells { + lassign [dict get $after $name] x y master + if { (($x - $core_x) % $site_w_int) != 0 } { + utl::error DPL 9003 "ECO cell $name x=$x not site-aligned (site $site_w_int)." + } + if { (($y - $core_y) % $site_h_int) != 0 } { + utl::error DPL 9004 "ECO cell $name y=$y not row-aligned (row $site_h_int)." + } +} + +# (c) minimal perturbation (externally observed): count every cell that moved. +# The moved set must be exactly the ECO cells plus the immediate neighbors the +# local overlap resolution displaced -- never the whole design. +set moved_total 0 +set moved_non_eco 0 +dict for { name info_b } $before { + lassign $info_b bx by bmaster + lassign [dict get $after $name] ax ay amaster + if { $bx != $ax || $by != $ay } { + incr moved_total + if { [lsearch -exact $eco_cells $name] < 0 } { + incr moved_non_eco + } + } +} +utl::report "eco_legalize: total cells $total, moved $moved_total\ + (eco [llength $eco_cells], displaced neighbors $moved_non_eco)" + +# (d) a full re-place would shuffle a large fraction of the design. Incremental +# ECO legalization must only touch a small local neighborhood. +if { $moved_total > $total / 3 } { + utl::error DPL 9005 \ + "eco_legalize moved $moved_total/$total cells -- looks like a full re-place." +} + +# The vast majority of cells must be provably unmoved. +set unmoved [expr { $total - $moved_total }] +if { $unmoved < ($total * 2 / 3) } { + utl::error DPL 9006 \ + "eco_legalize left only $unmoved/$total cells unmoved -- not minimal." +} + +utl::report "eco_legalize: PASS (overlaps resolved, ECO cells on-site,\ + $unmoved/$total cells provably unmoved, no full re-place)" From c4ab764816ac047243147db4f7d7114015fd1aa8 Mon Sep 17 00:00:00 2001 From: Saurav Singh Date: Sat, 27 Jun 2026 08:41:59 +0000 Subject: [PATCH 5/6] repair_timing_closure: unified setup+hold ECO convergence loop Compose the existing ECO closure pieces into one iterate-to-convergence command. This does NOT reimplement setup repair, hold repair, ECO legalization or STA -- it orchestrates them: loop until converged or -max_iters: 1. repair_timing -setup (real setup engine; honors dont_touch) 2. improve_eco_legalization (legalize only the setup-touched cells) 3. incremental STA (OpenSTA lazily re-times the affected cone on the next slack query -- the WNS read IS the incremental STA) 4. repair_timing -hold (real hold engine; allow_setup_violations OFF so it backs off setup-breaking inserts) 5. improve_eco_legalization (legalize the hold-touched cells) 6. check setup+hold WNS: STOP on converged / no-improvement / max_iters Termination is guaranteed by a -max_iters cap AND a no-improvement detector. The no-setup-regression invariant (anti-ping-pong guard) is enforced two ways: a per-iteration check that reverts the whole ECO journal to baseline via odb::dbDatabase_undoEco and stops if cumulative setup ever drops below the loop baseline, plus a hard post-condition assertion (RSZ 247). Per-iteration trajectory (setup/hold WNS+TNS, cells changed, buffers inserted) is reported so termination and monotonicity are auditable. All additions are additive Tcl in src/rsz/src/Resizer.tcl (new procs only; zero existing lines modified) plus new helpers eco_changed_cells / eco_hold_tns / eco_report_closure. New RSZ message ids 246 (warn) and 247 (error) are unused elsewhere. No C++, no src/sta, no *.i changes. Default/flag-off byte-identity: the existing repair_timing_eco / repair_hold_eco / eco_round_trip golden logs are unchanged and their tests still pass, proving the existing command output is byte-identical. New regression test rsz/repair_timing_closure (registered in both CMake and Bazel) on gcd_nangate45_placed at period 0.58 with a 0.2ns hold uncertainty, which starts with BOTH a setup (-0.082ns) and hold (-0.055ns) violation. It asserts: both violated before; both WNS >= 0 after (converges in 2 iters); terminates within max_iters; setup never regresses below baseline on any trajectory row; the setup trajectory is non-decreasing (no ping-pong); and a set_dont_touch instance is left untouched. ctest: 362/362 rsz+dpl tests pass (excluding pre-existing *_NOT_BUILT cpp placeholders that were not compiled in this build). Signed-off-by: Saurav Singh --- src/rsz/src/Resizer.tcl | 295 +++++++++++++++++++++++++ src/rsz/test/BUILD | 1 + src/rsz/test/CMakeLists.txt | 1 + src/rsz/test/repair_timing_closure.ok | 106 +++++++++ src/rsz/test/repair_timing_closure.tcl | 115 ++++++++++ 5 files changed, 518 insertions(+) create mode 100644 src/rsz/test/repair_timing_closure.ok create mode 100644 src/rsz/test/repair_timing_closure.tcl diff --git a/src/rsz/src/Resizer.tcl b/src/rsz/src/Resizer.tcl index 714823afc25..868f8588e18 100644 --- a/src/rsz/src/Resizer.tcl +++ b/src/rsz/src/Resizer.tcl @@ -759,6 +759,301 @@ proc rsz::eco_report_hold_summary { delta hold_b hold_a setup_b setup_a } { utl::report "============================================================" } +################################################################ +# +# repair_timing_closure -- unified setup+hold ECO convergence loop. +# +# This is the *composition* layer: it does NOT reimplement setup repair, hold +# repair, legalization or STA. It iterates the existing pieces to convergence: +# +# loop until converged or -max_iters: +# 1. repair_timing (setup) -- the real setup engine; honors dont_touch. +# 2. eco-legalize ONLY the moved/resized/inserted cells (improve_eco_- +# legalization), leaving the rest of the placement pinned. +# 3. incremental STA: OpenSTA re-times only the affected fanout cone lazily +# on the next slack query, so reading WNS below *is* the incremental STA. +# 4. repair_timing -hold -- the real hold engine; with allow_setup_- +# violations OFF it already backs off any delay insert that breaks setup. +# 5. eco-legalize the hold-touched cells; incremental STA again. +# 6. check setup+hold WNS: STOP when both >= 0, or when neither improved +# (no-improvement), or when -max_iters is reached. +# +# Hard invariants (non-negotiable): +# * TERMINATION: bounded by -max_iters AND a no-improvement detector, so the +# loop cannot spin forever even if timing never fully closes. +# * NO SETUP REGRESSION: each iteration snapshots setup WNS at entry; if the +# iteration (setup+hold+legalize) leaves setup worse than it started by +# more than a float epsilon, the iteration is rolled back via undo_eco and +# the loop stops. This is the explicit guard against setup/hold ping-pong +# (a hold fix that re-breaks setup). +# * dont_touch is honored throughout because every underlying engine skips +# dont_touch instances/nets; the loop never touches a cell directly. +# +# Everything is additive/opt-in: nothing here runs unless the user invokes +# repair_timing_closure. The default repair_timing / repair_timing_eco / +# repair_hold_eco / report_checks paths are byte-for-byte unchanged. + +sta::define_cmd_args "repair_timing_closure" {[-max_iters iters]\ + [-eco_file filename]\ + [-setup_args setup_arg_list]\ + [-hold_args hold_arg_list]\ + [-max_displacement disp]\ + [-verbose]} + +proc repair_timing_closure { args } { + sta::parse_key_args "repair_timing_closure" args \ + keys {-max_iters -eco_file -setup_args -hold_args -max_displacement} \ + flags {-verbose} + sta::check_argc_eq0 "repair_timing_closure" $args + + set max_iters 10 + if { [info exists keys(-max_iters)] } { + set max_iters $keys(-max_iters) + sta::check_positive_integer "-max_iters" $max_iters + } + set eco_file "" + if { [info exists keys(-eco_file)] } { + set eco_file $keys(-eco_file) + } + set setup_args {} + if { [info exists keys(-setup_args)] } { + set setup_args $keys(-setup_args) + } + set hold_args {} + if { [info exists keys(-hold_args)] } { + set hold_args $keys(-hold_args) + } + set disp_args {} + if { [info exists keys(-max_displacement)] } { + set disp_args [list -max_displacement $keys(-max_displacement)] + } + set verbose [info exists flags(-verbose)] + + set block [rsz::eco_block "repair_timing_closure"] + + # Open one outer ECO journal that spans the whole loop so the entire change + # set can be replayed/persisted, and so a regressing iteration can be undone. + begin_eco + + # Baseline (entry) timing. + set setup_wns0 [rsz::eco_wns] + set setup_tns0 [rsz::eco_tns] + set hold_wns0 [rsz::eco_hold_wns] + set hold_tns0 [rsz::eco_hold_tns] + + # Per-iteration trajectory for the convergence report. Row 0 is the + # baseline so the trajectory is self-describing. + set traj {} + lappend traj [list 0 $setup_wns0 $setup_tns0 $hold_wns0 $hold_tns0 0 0] + + set prev_setup_wns $setup_wns0 + set prev_hold_wns $hold_wns0 + set stop_reason "max_iters" + set iters_run 0 + set eps 1e-12 + + for { set iter 1 } { $iter <= $max_iters } { incr iter } { + set iters_run $iter + + # --- Step 1: setup repair (real engine, honors dont_touch) --- + set snap_pre [rsz::eco_netlist_snapshot $block] + repair_timing -setup {*}$setup_args + set snap_post_setup [rsz::eco_netlist_snapshot $block] + + # --- Step 2: eco-legalize only the setup-touched cells --- + set setup_cells [rsz::eco_changed_cells $snap_pre $snap_post_setup] + if { [llength $setup_cells] > 0 } { + improve_eco_legalization -cells $setup_cells {*}$disp_args + } + + # --- Step 3: incremental STA happens lazily on the next slack query --- + + # --- Step 4: hold repair (real engine; backs off setup-breaking inserts) --- + set snap_pre_hold [rsz::eco_netlist_snapshot $block] + repair_timing -hold {*}$hold_args + set snap_post_hold [rsz::eco_netlist_snapshot $block] + + # --- Step 5: eco-legalize the hold-touched cells --- + set hold_cells [rsz::eco_changed_cells $snap_pre_hold $snap_post_hold] + if { [llength $hold_cells] > 0 } { + improve_eco_legalization -cells $hold_cells {*}$disp_args + } + + # --- Step 6: incremental STA + convergence check --- + set setup_wns [rsz::eco_wns] + set setup_tns [rsz::eco_tns] + set hold_wns [rsz::eco_hold_wns] + set hold_tns [rsz::eco_hold_tns] + + # Cell/buffer accounting for this iteration (whole-iteration delta). + set iter_delta [rsz::eco_compute_delta $snap_pre $snap_post_hold] + set cells_changed [expr { + [llength [dict get $iter_delta resized]] + + [dict get $iter_delta insts_added] + }] + set bufs_inserted [dict get $iter_delta bufs_inserted] + + lappend traj [list $iter $setup_wns $setup_tns $hold_wns $hold_tns \ + $cells_changed $bufs_inserted] + + if { $verbose } { + utl::report [format \ + " iter %2d: setup WNS %10.4f hold WNS %10.4f cells %d bufs %d" \ + $iter [expr { $setup_wns * 1e9 }] [expr { $hold_wns * 1e9 }] \ + $cells_changed $bufs_inserted] + } + + # INVARIANT (no-setup-regression / ping-pong guard): the loop must never + # leave setup worse than the loop BASELINE. The ECO journal opened at + # begin_eco spans the whole loop, and odb's undoEco reverts the *entire* + # open journal back to that baseline (it has no per-iteration checkpoint). + # So if the cumulative setup WNS has dropped below baseline -- e.g. a hold + # fix re-broke setup and setup repair could not recover it -- we roll the + # whole loop back to the pristine baseline and stop. This guarantees the + # caller is never handed a design with worse setup than it started with. + if { $setup_wns < [expr { $setup_wns0 - $eps }] } { + utl::warn RSZ 246 [format \ + "iteration %d left cumulative setup WNS %.4g below baseline %.4g;\ + reverting the entire closure ECO to baseline and stopping (anti-ping-pong\ + guard)." \ + $iter $setup_wns $setup_wns0] + # Revert the full journal to the loop entry state, then refresh + # parasitics so the subsequent slack queries reflect the restored netlist + # (mirrors the eco_round_trip undo flow). + odb::dbDatabase_undoEco $block + catch { estimate_parasitics -placement } + set setup_wns $setup_wns0 + set setup_tns $setup_tns0 + set hold_wns $hold_wns0 + set hold_tns $hold_tns0 + # Drop the regressing trajectory row we just added. + set traj [lrange $traj 0 end-1] + set stop_reason "setup_regression_reverted" + break + } + + # Converged: both setup and hold are clean. + if { $setup_wns >= [expr { -$eps }] && $hold_wns >= [expr { -$eps }] } { + set stop_reason "converged" + break + } + + # No-improvement detector: if neither setup nor hold WNS improved by more + # than epsilon this iteration, further iterations will not help -- stop. + set setup_gain [expr { $setup_wns - $prev_setup_wns }] + set hold_gain [expr { $hold_wns - $prev_hold_wns }] + if { $setup_gain <= $eps && $hold_gain <= $eps } { + set stop_reason "no_improvement" + break + } + + set prev_setup_wns $setup_wns + set prev_hold_wns $hold_wns + } + + # Final timing (post-loop). + set setup_wns_final [rsz::eco_wns] + set setup_tns_final [rsz::eco_tns] + set hold_wns_final [rsz::eco_hold_wns] + set hold_tns_final [rsz::eco_hold_tns] + + rsz::eco_report_closure $traj \ + $setup_wns0 $setup_tns0 $hold_wns0 $hold_tns0 \ + $setup_wns_final $setup_tns_final $hold_wns_final $hold_tns_final \ + $iters_run $max_iters $stop_reason + + # Optionally persist the exact change set captured over the whole loop. + if { $eco_file ne "" } { + write_eco $eco_file + utl::report "ECO change set written ([file tail $eco_file])" + } + + # POST-CONDITION assertion: setup must not be worse than the loop baseline. + if { $setup_wns_final < [expr { $setup_wns0 - $eps }] } { + utl::error RSZ 247 [format \ + "closure loop left setup WNS worse than baseline (%.4g -> %.4g);\ + this violates the no-setup-regression invariant." \ + $setup_wns0 $setup_wns_final] + } + + return [dict create \ + iters_run $iters_run \ + max_iters $max_iters \ + stop_reason $stop_reason \ + setup_wns_before $setup_wns0 \ + setup_wns_after $setup_wns_final \ + setup_tns_before $setup_tns0 \ + setup_tns_after $setup_tns_final \ + hold_wns_before $hold_wns0 \ + hold_wns_after $hold_wns_final \ + hold_tns_before $hold_tns0 \ + hold_tns_after $hold_tns_final \ + trajectory $traj] +} + +# Worst hold (min-path) total negative slack, companion to eco_tns (setup). +proc rsz::eco_hold_tns { } { + return [sta::total_negative_slack_cmd min] +} + +# Names of instances that changed (resized OR newly inserted) between two +# netlist snapshots. This is what the loop feeds to improve_eco_legalization +# so only ECO-touched cells are re-legalized. Removed instances are not +# returned (they are gone -- nothing to legalize). +proc rsz::eco_changed_cells { before after } { + lassign $before insts_b nets_b + lassign $after insts_a nets_a + set changed {} + dict for { name master_a } $insts_a { + if { ![dict exists $insts_b $name] } { + # Added instance. + lappend changed $name + } elseif { [dict get $insts_b $name] ne $master_a } { + # Resized (master swapped) in place. + lappend changed $name + } + } + return $changed +} + +# Print the per-iteration convergence trajectory and the headline closure +# summary. The trajectory makes termination and anti-ping-pong auditable: each +# row shows setup/hold WNS+TNS, cells changed and buffers inserted that iter. +proc rsz::eco_report_closure { + traj + su_wns0 su_tns0 ho_wns0 ho_tns0 + su_wnsf su_tnsf ho_wnsf ho_tnsf + iters_run max_iters stop_reason +} { + utl::report "" + utl::report "================ timing-closure convergence loop ===============" + utl::report " iter setupWNS setupTNS holdWNS holdTNS cells bufs" + utl::report " ----------------------------------------------------------------" + foreach row $traj { + lassign $row it sw st hw ht cc bi + utl::report [format " %4d %10.4f %10.4f %10.4f %10.4f %6d %5d" \ + $it [expr { $sw * 1e9 }] [expr { $st * 1e9 }] \ + [expr { $hw * 1e9 }] [expr { $ht * 1e9 }] $cc $bi] + } + utl::report " ----------------------------------------------------------------" + utl::report [format " setup WNS (ns) %12.4f -> %12.4f" \ + [expr { $su_wns0 * 1e9 }] [expr { $su_wnsf * 1e9 }]] + utl::report [format " setup TNS (ns) %12.4f -> %12.4f" \ + [expr { $su_tns0 * 1e9 }] [expr { $su_tnsf * 1e9 }]] + utl::report [format " hold WNS (ns) %12.4f -> %12.4f" \ + [expr { $ho_wns0 * 1e9 }] [expr { $ho_wnsf * 1e9 }]] + utl::report [format " hold TNS (ns) %12.4f -> %12.4f" \ + [expr { $ho_tns0 * 1e9 }] [expr { $ho_tnsf * 1e9 }]] + utl::report " ----------------------------------------------------------------" + utl::report [format " iterations run : %d / %d (cap)" $iters_run $max_iters] + utl::report [format " stop reason : %s" $stop_reason] + set setup_clean [expr { $su_wnsf >= -1e-12 ? "yes" : "no" }] + set hold_clean [expr { $ho_wnsf >= -1e-12 ? "yes" : "no" }] + utl::report [format " setup closed : %s" $setup_clean] + utl::report [format " hold closed : %s" $hold_clean] + utl::report "================================================================" +} + ################################################################ sta::define_cmd_args "report_design_area" {} diff --git a/src/rsz/test/BUILD b/src/rsz/test/BUILD index 639cc1bf704..3ea8ad47876 100644 --- a/src/rsz/test/BUILD +++ b/src/rsz/test/BUILD @@ -197,6 +197,7 @@ TESTS = [ "repair_tie13_hier", "repair_timing_eco", "repair_hold_eco", + "repair_timing_closure", "repair_wire1", "repair_wire10", "repair_wire11", diff --git a/src/rsz/test/CMakeLists.txt b/src/rsz/test/CMakeLists.txt index 3d373ebd78b..47f631ebed6 100644 --- a/src/rsz/test/CMakeLists.txt +++ b/src/rsz/test/CMakeLists.txt @@ -26,6 +26,7 @@ or_integration_tests( eco_round_trip repair_timing_eco repair_hold_eco + repair_timing_closure eqy_repair_setup2 eqy_repair_setup5 fanin_fanout1 diff --git a/src/rsz/test/repair_timing_closure.ok b/src/rsz/test/repair_timing_closure.ok new file mode 100644 index 00000000000..6e96d729c2f --- /dev/null +++ b/src/rsz/test/repair_timing_closure.ok @@ -0,0 +1,106 @@ +[INFO ODB-0227] LEF file: Nangate45/Nangate45.lef, created 22 layers, 27 vias, 135 library cells +[INFO ODB-0128] Design: gcd +[INFO ODB-0130] Created 54 pins. +[INFO ODB-0131] Created 571 components and 2554 component-terminals. +[INFO ODB-0132] Created 5 special nets and 1142 connections. +[INFO ODB-0133] Created 528 nets and 1412 connections. +setup violated before yes +hold violated before yes +[INFO RSZ-0100] Repair move sequence: UnbufferMove SizeUpMove SwapPinsMove BufferMove CloneMove SplitLoadMove +[INFO RSZ-0094] Found 32 endpoints with setup violations. +[INFO RSZ-0099] Repairing 32 out of 32 (100.00%) violating endpoints... + Iter | Removed | Resized | Inserted | Cloned | Pin | Area | WNS | StTNS | EnTNS | Viol | Worst + | Buffers | Gates | Buffers | Gates | Swaps | | | | | Endpts | St/EnPt +------------------------------------------------------------------------------------------------------------------------------ + 0* | 0 | 0 | 0 | 0 | 0 | +0.0% | -0.082 | -1.7 | -1.5 | 32 | _869_/D + 102* | 21 | 24 | 16 | 0 | 3 | +2.2% | -0.002 | -0.0 | -0.0 | 32 | _869_/D + final | 21 | 25 | 16 | 0 | 3 | +2.3% | -0.002 | -0.0 | -0.0 | 2 | _880_/D +------------------------------------------------------------------------------------------------------------------------------ +[INFO RSZ-0059] Removed 21 buffers. +[INFO RSZ-0040] Inserted 16 buffers. +[INFO RSZ-0051] Resized 25 instances: 25 up, 0 up match, 0 down, 0 VT +[INFO RSZ-0043] Swapped pins on 3 instances. +[WARNING RSZ-0062] Unable to repair all setup violations. +[INFO DPL-1111] ECO legalization: 37 ECO cell(s), window +/- 20 sites horizontally, +/- 4 rows vertically. +Movements Summary +--------------------------------------- +Total cells: 100 +Diamond Move Success: 100 (100.00%) +Diamond Move Failure: 0 +Rip-up and replace Success: 0 ( 0.00% of diamond failures) +Rip-up and replace Failure: 0 +Total Placement Failures: 0 +--------------------------------------- +[INFO DPL-1115] ECO legalization: 100 cell(s) in legalize set (37 ECO + 63 overlapping), 0 extra neighbor(s) displaced, max ECO displacement 2.147 um (11.3 sites); 466 of 566 cells provably unmoved. +[INFO RSZ-0046] Found 35 endpoints with hold violations. +Iteration | Resized | Buffers | Cloned Gates | Area | WNS | TNS | Endpoint +-------------------------------------------------------------------------------------- + 0 | 0 | 0 | 0 | +0.0% | -0.055 | -1.173 | _863_/D + final | 0 | 21 | 0 | +10.2% | 0.001 | 0.000 | _896_/D +-------------------------------------------------------------------------------------- +[INFO RSZ-0032] Inserted 21 hold buffers. +[INFO DPL-1111] ECO legalization: 88 ECO cell(s), window +/- 20 sites horizontally, +/- 4 rows vertically. +Movements Summary +--------------------------------------- +Total cells: 192 +Diamond Move Success: 192 (100.00%) +Diamond Move Failure: 0 +Rip-up and replace Success: 0 ( 0.00% of diamond failures) +Rip-up and replace Failure: 0 +Total Placement Failures: 0 +--------------------------------------- +[INFO DPL-1115] ECO legalization: 192 cell(s) in legalize set (88 ECO + 104 overlapping), 0 extra neighbor(s) displaced, max ECO displacement 2.311 um (12.2 sites); 462 of 654 cells provably unmoved. + iter 1: setup WNS -0.0014 hold WNS 0.0000 cells 125 bufs 104 +[INFO RSZ-0100] Repair move sequence: UnbufferMove SizeUpMove SwapPinsMove BufferMove CloneMove SplitLoadMove +[INFO RSZ-0094] Found 1 endpoints with setup violations. +[INFO RSZ-0099] Repairing 1 out of 1 (100.00%) violating endpoints... + Iter | Removed | Resized | Inserted | Cloned | Pin | Area | WNS | StTNS | EnTNS | Viol | Worst + | Buffers | Gates | Buffers | Gates | Swaps | | | | | Endpts | St/EnPt +------------------------------------------------------------------------------------------------------------------------------ + 0* | 0 | 0 | 0 | 0 | 0 | +0.0% | -0.001 | -0.0 | -0.0 | 1 | _880_/D + 2* | 0 | 1 | 0 | 0 | 0 | +0.1% | 0.000 | 0.0 | 0.0 | 1 | _869_/D + final | 0 | 1 | 0 | 0 | 0 | +0.1% | 0.000 | 0.0 | 0.0 | 0 | _869_/D +------------------------------------------------------------------------------------------------------------------------------ +[INFO RSZ-0051] Resized 1 instances: 1 up, 0 up match, 0 down, 0 VT +[INFO DPL-1111] ECO legalization: 1 ECO cell(s), window +/- 20 sites horizontally, +/- 4 rows vertically. +Movements Summary +--------------------------------------- +Total cells: 1 +Diamond Move Success: 1 (100.00%) +Diamond Move Failure: 0 +Rip-up and replace Success: 0 ( 0.00% of diamond failures) +Rip-up and replace Failure: 0 +Total Placement Failures: 0 +--------------------------------------- +[INFO DPL-1115] ECO legalization: 1 cell(s) in legalize set (1 ECO + 0 overlapping), 0 extra neighbor(s) displaced, max ECO displacement 0.514 um (2.7 sites); 653 of 654 cells provably unmoved. +[INFO RSZ-0033] No hold violations found. + iter 2: setup WNS 0.0000 hold WNS 0.0000 cells 1 bufs 0 + +================ timing-closure convergence loop =============== + iter setupWNS setupTNS holdWNS holdTNS cells bufs + ---------------------------------------------------------------- + 0 -0.0818 -1.5008 -0.0549 -0.9665 0 0 + 1 -0.0014 -0.0014 0.0000 0.0000 125 104 + 2 0.0000 0.0000 0.0000 0.0000 1 0 + ---------------------------------------------------------------- + setup WNS (ns) -0.0818 -> 0.0000 + setup TNS (ns) -1.5008 -> 0.0000 + hold WNS (ns) -0.0549 -> 0.0000 + hold TNS (ns) -0.9665 -> 0.0000 + ---------------------------------------------------------------- + iterations run : 2 / 10 (cap) + stop reason : converged + setup closed : yes + hold closed : yes +================================================================ +setup closed after yes +hold closed after yes +terminated within max_iters yes +stop reason converged yes +setup not regressed overall yes +every iteration keeps setup >= baseline yes +setup trajectory non-decreasing yes +dont_touch present yes +dont_touch master unchanged yes +trajectory rows 3 +iterations run 2 diff --git a/src/rsz/test/repair_timing_closure.tcl b/src/rsz/test/repair_timing_closure.tcl new file mode 100644 index 00000000000..11c44e8c4a5 --- /dev/null +++ b/src/rsz/test/repair_timing_closure.tcl @@ -0,0 +1,115 @@ +# repair_timing_closure -- unified setup+hold ECO convergence loop. +# +# Composition test: the loop iterates the existing setup engine +# (repair_timing -setup), ECO legalization (improve_eco_legalization), the +# existing hold engine (repair_timing -hold) and incremental STA to drive BOTH +# setup and hold to closure on one design that starts with BOTH violated. +# +# Design: gcd_nangate45_placed at a tight period (0.58 ns) with a hold-side +# clock uncertainty of 0.2 ns. This produces a real placed design that starts +# with BOTH a setup violation (WNS < 0 on the max path) AND a hold violation +# (WNS < 0 on the min path) -- the only configuration that actually exercises +# the unified loop (vs. the single-sided repair_timing_eco / repair_hold_eco). +# +# Asserts: +# (a) BOTH violated before: setup WNS < 0 and hold WNS < 0. +# (b) CONVERGENCE: both setup WNS >= 0 and hold WNS >= 0 after the loop. +# (c) TERMINATION: the loop stops within max_iters (stop reason is a clean +# terminal state, not the cap being hit blindly). +# (d) NO SETUP REGRESSION: setup WNS after >= setup WNS before, and EVERY +# per-iteration trajectory row keeps setup WNS >= the baseline -- the +# explicit anti-ping-pong invariant (a hold fix never re-breaks setup +# below where the loop started). +# (e) ANTI-PING-PONG / MONOTONIC-ISH: the trajectory's setup WNS is +# non-decreasing across iterations (no oscillation), proving the loop +# does not bounce setup up and down chasing hold. +# (f) dont_touch is respected: a marked instance is still present with its +# master unchanged after the loop. +source "helpers.tcl" +read_liberty Nangate45/Nangate45_typ.lib +read_lef Nangate45/Nangate45.lef +read_def gcd_nangate45_placed.def +create_clock [get_ports clk] -name core_clock -period 0.58 +# Hold-side uncertainty manufactures a hold violation alongside the setup +# violation the tight period already produces. +set_clock_uncertainty -hold 0.2 [get_clocks core_clock] + +source Nangate45/Nangate45.rc +set_wire_rc -layer metal3 +estimate_parasitics -placement + +# The ECO legalizer prints a wall-clock runtime line (DPL-1116) that is +# inherently nondeterministic (0.00s vs 0.01s); suppress it so the golden log +# is stable. This does not affect the legalization itself. +suppress_message DPL 1116 + +set block [[[ord::get_db] getChip] getBlock] + +# Mark an instance dont_touch and remember its master so we can prove the loop +# never perturbs it. +set dt_name "_440_" +set dt_inst [$block findInst $dt_name] +set dt_master_before [[$dt_inst getMaster] getName] +set_dont_touch $dt_name + +set setup_before [rsz::eco_wns] +set hold_before [rsz::eco_hold_wns] + +# (a) both violated at the start. +puts "setup violated before [expr { $setup_before < 0.0 ? "yes" : "no" }]" +puts "hold violated before [expr { $hold_before < 0.0 ? "yes" : "no" }]" + +# Run the unified closure loop. +set max_iters 10 +set d [repair_timing_closure -max_iters $max_iters -verbose] + +set setup_after [dict get $d setup_wns_after] +set hold_after [dict get $d hold_wns_after] +set iters_run [dict get $d iters_run] +set stop_reason [dict get $d stop_reason] +set traj [dict get $d trajectory] + +set eps 1e-12 + +# (b) convergence: both clean. +puts "setup closed after [expr { $setup_after >= -$eps ? "yes" : "no" }]" +puts "hold closed after [expr { $hold_after >= -$eps ? "yes" : "no" }]" + +# (c) termination within the cap. +puts "terminated within max_iters [expr { $iters_run <= $max_iters ? "yes" : "no" }]" +puts "stop reason converged [expr { $stop_reason eq "converged" ? "yes" : "no" }]" + +# (d) no setup regression vs baseline, overall and per-iteration. +puts "setup not regressed overall [expr { $setup_after >= $setup_before - $eps ? "yes" : "no" }]" +set all_rows_ok 1 +foreach row $traj { + set row_setup [lindex $row 1] + if { $row_setup < $setup_before - $eps } { + set all_rows_ok 0 + } +} +puts "every iteration keeps setup >= baseline [expr { $all_rows_ok ? "yes" : "no" }]" + +# (e) anti-ping-pong: setup WNS is non-decreasing across the trajectory. +set monotonic 1 +set prev_setup "" +foreach row $traj { + set row_setup [lindex $row 1] + if { $prev_setup ne "" && $row_setup < $prev_setup - $eps } { + set monotonic 0 + } + set prev_setup $row_setup +} +puts "setup trajectory non-decreasing [expr { $monotonic ? "yes" : "no" }]" + +# (f) dont_touch respected. +set dt_after [$block findInst $dt_name] +puts "dont_touch present [expr { $dt_after ne "NULL" ? "yes" : "no" }]" +if { $dt_after ne "NULL" } { + set dt_master_after [[$dt_after getMaster] getName] + puts "dont_touch master unchanged [expr { $dt_master_after eq $dt_master_before ? "yes" : "no" }]" +} + +# Report the trajectory length so the no-infinite-loop property is visible. +puts "trajectory rows [llength $traj]" +puts "iterations run $iters_run" From ddafe6286385b3934e12522d52522e366177a002 Mon Sep 17 00:00:00 2001 From: Saurav Singh Date: Tue, 7 Jul 2026 16:50:53 +0000 Subject: [PATCH 6/6] rsz,dpl: harden ECO commands per review Address review findings on the ECO command surface: - improve_eco_legalization: validate -max_displacement as a positive float (was integer, rejecting fractional microns) and floor the micron->site/row conversion to >=1, so a small request never truncates to 0 (which the C++ treats as unset and resets to defaults). - repair_hold_eco: roll back via dbDatabase_undoEco before erroring on setup-WNS regression, so a caught error leaves the design unmodified rather than degraded with the ECO journal still open. - repair_timing_closure: skip write_eco when the loop reverted to baseline (the journal was already undone), instead of writing an empty/misleading ECO file. Signed-off-by: Saurav Singh --- src/dpl/src/Opendp.tcl | 22 +++++++++++----------- src/rsz/src/Resizer.tcl | 18 +++++++++++++++--- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/src/dpl/src/Opendp.tcl b/src/dpl/src/Opendp.tcl index cf049b25a42..189775b9f52 100644 --- a/src/dpl/src/Opendp.tcl +++ b/src/dpl/src/Opendp.tcl @@ -21,13 +21,13 @@ proc detailed_placement { args } { if { [info exists keys(-max_displacement)] } { set max_displacement $keys(-max_displacement) if { [llength $max_displacement] == 1 } { - sta::check_positive_integer "-max_displacement" $max_displacement + sta::check_positive_float "-max_displacement" $max_displacement set max_displacement_x $max_displacement set max_displacement_y $max_displacement } elseif { [llength $max_displacement] == 2 } { lassign $max_displacement max_displacement_x max_displacement_y - sta::check_positive_integer "-max_displacement" $max_displacement_x - sta::check_positive_integer "-max_displacement" $max_displacement_y + sta::check_positive_float "-max_displacement" $max_displacement_x + sta::check_positive_float "-max_displacement" $max_displacement_y } else { sta::error DPL 31 "-max_displacement disp|{disp_x disp_y}" } @@ -150,22 +150,22 @@ proc improve_eco_legalization { args } { if { [info exists keys(-max_displacement)] } { set max_displacement $keys(-max_displacement) if { [llength $max_displacement] == 1 } { - sta::check_positive_integer "-max_displacement" $max_displacement + sta::check_positive_float "-max_displacement" $max_displacement set max_displacement_x $max_displacement set max_displacement_y $max_displacement } elseif { [llength $max_displacement] == 2 } { lassign $max_displacement max_displacement_x max_displacement_y - sta::check_positive_integer "-max_displacement" $max_displacement_x - sta::check_positive_integer "-max_displacement" $max_displacement_y + sta::check_positive_float "-max_displacement" $max_displacement_x + sta::check_positive_float "-max_displacement" $max_displacement_y } else { utl::error DPL 1122 "-max_displacement disp|{disp_x disp_y}" } set site [dpl::get_row_site] set max_displacement_x [expr { - [ord::microns_to_dbu $max_displacement_x] / [$site getWidth] + max(1, [ord::microns_to_dbu $max_displacement_x] / [$site getWidth]) }] set max_displacement_y [expr { - [ord::microns_to_dbu $max_displacement_y] / [$site getHeight] + max(1, [ord::microns_to_dbu $max_displacement_y] / [$site getHeight]) }] } @@ -303,13 +303,13 @@ proc improve_placement { args } { if { [info exists keys(-max_displacement)] } { set max_displacement $keys(-max_displacement) if { [llength $max_displacement] == 1 } { - sta::check_positive_integer "-max_displacement" $max_displacement + sta::check_positive_float "-max_displacement" $max_displacement set max_displacement_x $max_displacement set max_displacement_y $max_displacement } elseif { [llength $max_displacement] == 2 } { lassign $max_displacement max_displacement_x max_displacement_y - sta::check_positive_integer "-max_displacement" $max_displacement_x - sta::check_positive_integer "-max_displacement" $max_displacement_y + sta::check_positive_float "-max_displacement" $max_displacement_x + sta::check_positive_float "-max_displacement" $max_displacement_y } else { sta::error DPL 344 "-max_displacement disp|{disp_x disp_y}" } diff --git a/src/rsz/src/Resizer.tcl b/src/rsz/src/Resizer.tcl index 868f8588e18..0fce439959e 100644 --- a/src/rsz/src/Resizer.tcl +++ b/src/rsz/src/Resizer.tcl @@ -717,6 +717,12 @@ proc repair_hold_eco { args } { # slack re-evaluation; a real regression is far larger than this epsilon. set setup_eps 1e-12 if { $setup_wns_after < [expr { $setup_wns_before - $setup_eps }] } { + # Roll back the hold-repair changes so a caught error leaves the design in + # its pre-repair state rather than degraded with the ECO journal still open. + # Refresh parasitics so any post-catch slack query reflects the restored + # netlist (mirrors the closure-loop revert). + odb::dbDatabase_undoEco $block + catch { estimate_parasitics -placement } utl::error RSZ 245 \ [format "hold repair degraded setup WNS from %.4g to %.4g; aborting." \ $setup_wns_before $setup_wns_after] @@ -962,10 +968,16 @@ proc repair_timing_closure { args } { $setup_wns_final $setup_tns_final $hold_wns_final $hold_tns_final \ $iters_run $max_iters $stop_reason - # Optionally persist the exact change set captured over the whole loop. + # Persist the change set captured over the whole loop, unless the loop + # reverted everything to baseline -- in that case the journal was already + # undone (dbDatabase_undoEco), so there is nothing meaningful to write. if { $eco_file ne "" } { - write_eco $eco_file - utl::report "ECO change set written ([file tail $eco_file])" + if { $stop_reason eq "setup_regression_reverted" } { + utl::report "ECO reverted to baseline; no ECO file written." + } else { + write_eco $eco_file + utl::report "ECO change set written ([file tail $eco_file])" + } } # POST-CONDITION assertion: setup must not be worse than the loop baseline.