Skip to content

rcx: fix wire resistance subtraction during neighboring context adjustment pass#10439

Open
AcKoucher wants to merge 3 commits into
The-OpenROAD-Project:masterfrom
AcKoucher:rcx-fix-neighbor-resistance
Open

rcx: fix wire resistance subtraction during neighboring context adjustment pass#10439
AcKoucher wants to merge 3 commits into
The-OpenROAD-Project:masterfrom
AcKoucher:rcx-fix-neighbor-resistance

Conversation

@AcKoucher
Copy link
Copy Markdown
Contributor

@AcKoucher AcKoucher commented May 15, 2026

Context

While working on updating the layers' resistance on the ihp platform using the new segment-based regression, I got non-linear fit i.e., Rsq != 1.0 for all metal layers. Some investigation showed that a few metal shapes had a much smaller resistance than expected based on the sheet value. This unexpected resistance was tracked down to the bug being fixed in this PR. With the changes here, we get a linear fit for resistance on ihp as we should.

Summary

During extraction, RCX first sets each wire shape's resistance from its geometry and sheet-res, then a coupling-extraction pass adjusts it. The adjustment is a total - baseline subtraction that should cancel to zero when the PDK table has uniform per-distance resistance.

During the adjustment pass, when iterating the sections of a segment and looking at each neighborhood context, there are two physical components being stored at the same time and that should match: the span of the current neighbor (i.e., the length of the neighbor's "overlap") and the resistance contribution of that span.

The problem is that we always register the span, but for a very specific case we fail to register the overlap's resistance contribution (see extMeasure::computeRes). The mismatch causes the total - baseline expression to go negative, and that negative delta gets added to the odb::RSeg resistance.

So what the changes here do is: ensure that the context resistance is correctly registered and the final resistance of the metal segment is left untouched for a case in which:

  1. We're measuring the contribution of a neighbor whose distance to the source is beyond the RESOVER table's last entry.
  2. There is at least one other neighbor overlap for this segment covered by the table (so that we enter the total_resistance > 0 scope in extMeasure::measureRC) .

Also ensure that the last table entry is correctly used when the neighbor is at the maximum distance.

Observations

W.r.t. the regression test, most of the gcd.def routing is in metal1 and, in sky130hs/sky130hs.rcx_rules, the RESOVER table goes beyond the 4 tracks distance for this layer. We may have the bug triggered for some of the remaining layers but we'd rely on quite some luck as not only there is less routing, but the gap between the RESOVER limit and 4 * pitch distance is narrow. Based on that I added a new .rules files that should force the gap to be wide enough in every layer so that basically every wire with multiple-track neighbors end up triggering the bug.

Type of Change

  • Bug fix

Impact

This may have small QoR impact at least on ihp.

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

The-OpenROAD-Project/OpenROAD-flow-scripts#3969

…tment pass

Signed-off-by: Arthur Koucher <arthurkoucher@precisioninno.com>
@AcKoucher AcKoucher requested a review from a team as a code owner May 15, 2026 17:38
@AcKoucher AcKoucher requested a review from maliberty May 15, 2026 17:38
Copy link
Copy Markdown
Contributor

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

Choose a reason for hiding this comment

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

Code Review

This pull request modifies the resistance calculation logic in extmeasure_res.cpp to return the baseline resistance instead of nullptr when distances exceed the RESOVER table bounds. It also introduces a new regression test suite, short_resover, including its rules and execution script. Review feedback identifies a missing golden SPEF file necessary for the new test and suggests adjusting a boundary condition from >= to > to improve calculation accuracy at the table limits.

Comment thread src/rcx/test/short_resover.tcl Outdated
Comment thread src/rcx/src/extmeasure_res.cpp Outdated
@github-actions
Copy link
Copy Markdown
Contributor

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

@AcKoucher
Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@maliberty
Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not use sky130hs/sky130hs.rcx_rules ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Most of the gcd.def routing is in metal1 and, in sky130hs/sky130hs.rcx_rules, the RESOVER table goes beyond the 4 tracks distance for this layer.

We may have the bug triggered for some of the remaining layers but we'd rely on quite some luck as not only there is less routing, but the gap between the RESOVER limit and 4 * pitch distance is narrow.

The idea of the new rules is to force the gap to be wide enough in every layer so that basically every wire with multiple-track neighbors end up triggering the bug.

AcKoucher added 2 commits May 15, 2026 15:50
Signed-off-by: Arthur Koucher <arthurkoucher@precisioninno.com>
Signed-off-by: Arthur Koucher <arthurkoucher@precisioninno.com>
@github-actions
Copy link
Copy Markdown
Contributor

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

@AcKoucher
Copy link
Copy Markdown
Contributor Author

Running Secure-CI.

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.

2 participants