Skip to content

tap: error on invalid endcap_master type - #11268

Open
gadfort wants to merge 1 commit into
The-OpenROAD-Project:masterfrom
gadfort:error-on-endcap
Open

tap: error on invalid endcap_master type#11268
gadfort wants to merge 1 commit into
The-OpenROAD-Project:masterfrom
gadfort:error-on-endcap

Conversation

@gadfort

@gadfort gadfort commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Summary

  1. odb::cutRows could mark the whole core as a narrow region.

To find gaps too narrow to hold endcaps, cutRows scans every pair of blockages and flags the vertical space between them when it is shorter than min_row_height + site_height - 1. The core's top and bottom edges are appended to that list as zero-thickness sentinel bands so that slivers between a macro and the core boundary are caught by the same scan.

The scan paired every band with every other one — including the two sentinels with each other. That pair does not measure a gap between obstructions; it measures the core height itself. So whenever the core was shorter than min_region_height, the scan produced a single "narrow region" spanning the entire core, and every row was cut away.

A sentinel is now only ever paired with a real blockage.

This is not specific to a bad -endcap_master. min_row_height is 2 * endcap_height + max_core_cell_height (or an explicit -row_min_height), so a legitimate endcap master plus a tall multi-height cell in the library is enough to exceed a small core's height and silently delete every row.

  1. A CLASS BLOCK master was accepted as -endcap_master.

isCoreAutoPlaceable() returns true for BLOCK, so checkPlaceable let macros through. Besides being unplaceable in a row, an endcap master's dimensions set the minimum row width and height used to cut rows — the issue's test case passes the macro's own 419x484um master, which alone guarantees over-cutting. Block masters are now rejected with TAP-0036. The message dropped its "would be ignored by detailed placement" tail, which is not accurate for a macro; the golden was updated.

Type of Change

  • Bug fix

Impact

Should only impact users with incorrect input to the function

Verification

  • I have verified that the local build succeeds (./etc/Build.sh).
  • I have run the relevant tests and they pass.
  • My code follows the repository's formatting guidelines.
  • I have included tests to prevent regressions.
  • I have signed my commits (DCO).

Related Issues

Closes #11223

Signed-off-by: Peter Gadfort <gadfort@zeroasic.com>
@gadfort
gadfort requested review from a team as code owners August 30, 2026 13:58
@gadfort
gadfort requested a review from maliberty August 30, 2026 13:59
@gadfort
gadfort requested a review from eder-matheus August 30, 2026 13:59

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request prevents rows from being incorrectly cut away on short cores by ensuring sentinel obstruction bands are not paired with each other in cutRows, and by rejecting block masters (macros) in checkPlaceable. A new test case cut_rows_short_core has been added to verify these changes. The feedback suggests a performance optimization in src/odb/src/db/util.cpp to reserve capacity for the bands vector to avoid reallocation overhead.

Comment thread src/odb/src/db/util.cpp
Comment on lines +270 to +273
vector<Rect> bands = effective_blockages;
const size_t blockage_count = bands.size();
bands.emplace_back(core.xMin(), core.yMax(), core.xMax(), core.yMax() + 1);
bands.emplace_back(core.xMin(), core.yMin() - 1, core.xMax(), core.yMin());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

When copying effective_blockages to bands, the vector is initialized with the exact size of effective_blockages. Calling emplace_back immediately after will trigger a reallocation and copy of all elements because the vector's capacity is fully utilized. To avoid this unnecessary reallocation and copy overhead, we can reserve the required capacity (effective_blockages.size() + 2) beforehand.

    vector<Rect> bands;
    bands.reserve(effective_blockages.size() + 2);
    bands.assign(effective_blockages.begin(), effective_blockages.end());
    const size_t blockage_count = bands.size();
    bands.emplace_back(core.xMin(), core.yMax(), core.xMax(), core.yMax() + 1);
    bands.emplace_back(core.xMin(), core.yMin() - 1, core.xMax(), core.yMin());

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ifp: Regression, cut_rows removes all rows

1 participant