rsz: clamp buffer locations to core area during insertion

[openroad-ci]

Summary

RSZ-inserted buffers could land outside the core area, in the die-core gap where no placement rows exist, causing downstream DPL legalization failures. Clamp buffer coordinates to the core bounding box at the point of insertion.

Root Cause

The buffers in question are wire repeaters produced by repair_design (and similarly by repair_setup / repair_hold rebuffering). The insertion mechanism:

1. RSZ builds a Steiner tree over the driver pin + all load pins of a net (SteinerTree::findSteinerPoints).
2. isTopLevelPort(load_pin) returns true for BTerms, so top-level ports become Steiner terminals. Their coordinates come from dbBTerm::getFirstPinLocation() — which returns the die-boundary pin location, not a core-side coordinate.
3. When a wire segment from a core-side point to a die-edge BTerm exceeds max_wire_length, RepairDesign::repairNet inserts a repeater by interpolating a point along the segment:

   buf_x = to_x + d * dx    // d ∈ (0, 1]
   buf_y = to_y + d * dy
   

   If the segment endpoint is at the die edge, the interpolated point lands inside the die-core gap — the band between die boundary and core row area.
4. setLocation(buf_inst, {buf_x, buf_y}) was called without clamping — buffer placed outside core.
5. DPL cannot legalize: no placement row exists at those coordinates.

Verification

Reproduced on a customer design — see The-OpenROAD-Project-private/OpenROAD-flow-scripts#1622 for details. Wire repeaters inserted during the place-resize stage were observed outside the core, all traced to die-edge BTerms feeding the Steiner-point interpolation.

Fix

clampLocToCore() (Resizer.cc, Resizer.hh):

• Returns (x, y) clamped into [core.xMin, core.xMax - cell.width] × [core.yMin, core.yMax - cell.height]
• std::max guard for the upper bound handles the edge case where the core is smaller than the cell (so std::clamp is safe — requires hi >= lo)

setLocation() applies the clamp and emits [INFO RSZ-0077] {instance_name} clamped to core ({orig_x}, {orig_y}) -> ({new_x}, {new_y}) whenever a buffer is moved — per-buffer detail, always on, instance name lets you grep the DEF directly.

Two insertion paths are both covered:

1. setLocation() path (repair_hold, repair_setup, make_buffer) — clamp inside setLocation()
2. ODB insertBuffer* path (repair_design wire repeaters) — insertBufferPostProcess(buffer_inst, loc) calls setLocation after ODB places the buffer, re-placing with the clamped coordinate if needed

Why RSZ-layer (not ODB-layer) clamping

Tempting alternative: clamp inside dbInsertBuffer::placeBufferAtLocation() (single ODB site). Rejected because:

• "Core area" is a placement/floorplan concept; ODB is the geometry DB layer. Adding core-awareness to ODB is a layering violation.
• dbInsertBuffer is used by non-RSZ callers (e.g., swap_pins_dont_touch) that don't necessarily want core-clamping.
• RSZ-layer clamping keeps the policy co-located with the decision-maker and still hits every path through one helper.

Testing

New test: repair_design_outside_core

• 200×200 μm die with tight core (10, 10)–(90, 90) μm
• Driver at core center, 10 loads deliberately placed outside core at x=195000 DBU
• set_max_fanout 5 + repair_design -max_wire_length 100 triggers wire-repeater insertion
• Asserts all TIMING-source instances remain inside the core post-repair
• Log confirms per-buffer clamp actions

Updated golden files (11 tests) — all exercise paths that now emit per-buffer RSZ-0077 info messages:

Test	Why it hits the clamp
repair_hold2 / repair_hold8 / repair_hold11 / repair_hold12	Loads at die-bottom pins (y=0) → hold-buffer insertion lands below core
repair_hold14	Load placement outside core → hold buffer clamped
repair_slew4 / repair_slew12	Driver cell larger than core on one axis → clamp fires
repair_wire11	Long wires to off-core loads → repeaters clamped
repair_tie5	Tie-cell insertion near die edge
upf_aes	Timing-driven global_placement inserts a repair_design buffer whose top edge extends past core yMax
repair_design_outside_core	New test (see above)

Full rsz regression: 195/195 pass.

Type of Change

• Bug fix

Impact

• All RSZ-inserted buffers now land inside the core. DPL legalization no longer fails on wire repeaters created from die-edge BTerms.
• Designs that previously produced out-of-core buffers will see slightly different timing results (wire lengths / parasitics reflect the new, legal locations).
• New per-buffer RSZ-0077 info messages replace the prior summary warning — instance name + coordinates, more useful for diagnosis, no debug flag needed.

Verification

• Local build passes (./etc/Build.sh).
• Relevant rsz regression (195/195) passes.
• clang-format, tclfmt, tclint clean.
• Commits signed (DCO).

Related Issues

Fixes The-OpenROAD-Project-private/OpenROAD-flow-scripts#1622

[gemini-code-assist]

Code Review

This pull request replaces the manual buffer-moved-into-core warning with an automatic clamping mechanism using a new clampLocToCore method, and adds a corresponding test case. Feedback highlights that the modified signature of insertBufferPostProcess causes compilation errors at several call sites where references are passed instead of pointers, and misses other sites like makeBuffer. To resolve these errors and improve API usability, it is recommended to revert the signature change and have the method retrieve the buffer's location internally, ensuring consistent clamping for all inserted buffers.

[gemini-code-assist]

The change to the signature of insertBufferPostProcess to take a pointer const odb::Point* loc introduces several issues:

1. Compilation Errors: The call sites updated in Resizer.cc (e.g., line 4910) pass a reference loc to this pointer parameter, which will not compile in C++.
2. Incomplete Updates: Other call sites not included in this diff (such as makeBuffer) will also fail to compile as they now lack the required second argument.
3. Clamping Policy: Making the location optional via a pointer risks missing clamping for some buffers if nullptr is passed, which contradicts the goal of ensuring all RSZ-inserted buffers land inside the core.

Instead of changing the signature, it is recommended to keep the original void insertBufferPostProcess(odb::dbInst* buffer_inst) and internally call setLocation(buffer_inst, buffer_inst->getLocation()). This ensures all inserted buffers are clamped using their current location without needing to modify any call sites, prioritizing API usability.

Suggested change

	void insertBufferPostProcess(odb::dbInst* buffer_inst, const odb::Point* loc);
	void insertBufferPostProcess(odb::dbInst* buffer_inst);

References

1. API usability can be prioritized over strict const-correctness.

[gemini-code-assist]

This call will fail to compile because loc is a reference (const odb::Point&) but the new signature of insertBufferPostProcess expects a pointer. Reverting the signature change as suggested in the header file would resolve this and simplify the call sites.

  insertBufferPostProcess(buffer_inst);

[gemini-code-assist]

This call will fail to compile because loc is a reference (const odb::Point&) but the new signature of insertBufferPostProcess expects a pointer.

  insertBufferPostProcess(buffer_inst);

[gemini-code-assist]

This call will fail to compile because loc is a reference (const odb::Point&) but the new signature of insertBufferPostProcess expects a pointer.

  insertBufferPostProcess(buffer_inst);

[gemini-code-assist]

By reverting the signature change and querying the instance's current location, you ensure that clamping is always applied to any buffer undergoing post-processing, regardless of how it was instantiated. This also fixes the compilation errors at the call sites and ensures that methods like makeBuffer also benefit from the clamping logic. This approach prioritizes API usability over the proposed signature change.

void Resizer::insertBufferPostProcess(dbInst* buffer_inst)
{
  // Clamp to core. ODB's insertBuffer* already placed the instance;
  // setLocation will re-place it if clamping is required.
  setLocation(buffer_inst, buffer_inst->getLocation());
}

References

1. API usability can be prioritized over strict const-correctness.

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[minjukim55]

Scope clarification — the branch now includes commit afb03b614a adding a code comment in insertBufferPostProcess to document the port buffer exception. The PR description below should also be read with this clarification:

Port buffer exception

bufferInputs / bufferOutputs pass loc=nullptr into insertBufferAfterDriver / insertBufferBeforeLoad; insertBufferPostProcess skips the clamp when loc == nullptr. Port buffers are placed by ODB near their BTerm location, which can sit at the die edge outside the core area, and that is by design — a port buffer should stay adjacent to the port it buffers. This matches the pre-existing behavior.

The three paths through insertBufferPostProcess:

1. setLocation() path (repair_hold, repair_setup, make_buffer) — clamp inside setLocation()
2. ODB insertBuffer* path with explicit loc (repair_design wire repeaters) — clamp via insertBufferPostProcess(buffer_inst, loc)
3. Port buffer path (bufferInputs / bufferOutputs, loc=nullptr) — not clamped (by design, see above)

The "Impact" section of the description should be read as: wire repeaters and other loc-aware inserts now land inside the core; port buffers remain at BTerm location (unchanged from prior behavior).

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[jhkim-pii]

How about replacing the setLocation() call in the following API?

void dbInsertBuffer::placeBufferAtLocation(dbInst* buffer_inst,
                                           const Point& loc,
                                           const char* reason)
{
  buffer_inst->setLocation(loc.getX(), loc.getY());
  buffer_inst->setPlacementStatus(dbPlacementStatus::PLACED);
  dlogPlacedBuffer(buffer_inst, loc, reason);
}

[maliberty]

I have no further comments beyond @jhkim-pii