Skip to content

dcalc: Restore TwoPole parasitic reduction - #406

Closed
jhkim-pii wants to merge 1 commit into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:secure-fix-two-pole-dcalc
Closed

dcalc: Restore TwoPole parasitic reduction#406
jhkim-pii wants to merge 1 commit into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:secure-fix-two-pole-dcalc

Conversation

@jhkim-pii

Copy link
Copy Markdown
Contributor

Summary

  • Restore calculator-specific PiPoleResidue2 reduction for dmp_ceff_two_pole.

Problem

  • The parasitics API migration removed the TwoPole reduced-type selector without adding the equivalent reduceParasitic override.
  • Eager reduction therefore inherited LumpedCapDelayCalc and produced PiElmore, leaving TwoPole without pole/residue data after the detailed network was deleted.

Solution

  • Override DmpCeffTwoPoleDelayCalc::reduceParasitic to call reduceToPiPoleResidue2.
  • Keep the inherited net-level reduction overload visible for generic callers.

Impact

  • TwoPole consumers receive the parasitic representation required by their load-delay model.
  • DMP Ceff Elmore reduction remains unchanged.

Testing

  • Passed the OpenROAD dmp_ceff_dcalc_load_propagation Bazel regression with fresh placement estimation for Elmore, TwoPole, and Lambert-W.
  • Passed OpenROAD build, Tcl lint/format, dbSta lint, clang-tidy, and direct CLI QA.

Related

Override the TwoPole reduction hook so eager parasitic reduction creates PiPoleResidue2 instead of inheriting the PiElmore implementation. This restores calculator-specific dispatch for placement and reduced SPEF paths.

Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>

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

Copy link
Copy Markdown

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 overrides the reduceParasitic method in DmpCeffTwoPoleDelayCalc to reduce the parasitic network using reduceToPiPoleResidue2. Feedback was provided to add a defensive null check for the parasitics pointer returned by scene->parasitics(min_max) to prevent a potential null pointer dereference crash.

Comment thread dcalc/DmpDelayCalc.cc
Comment on lines +274 to +276
Parasitics *parasitics = scene->parasitics(min_max);
return parasitics->reduceToPiPoleResidue2(
parasitic_network, drvr_pin, rf, scene, min_max);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The scene->parasitics(min_max) call can return nullptr if parasitics are not loaded or available. Dereferencing parasitics without a null check will cause a crash. Please add a defensive null check before calling reduceToPiPoleResidue2.

  Parasitics *parasitics = scene->parasitics(min_max);
  if (parasitics == nullptr)
    return nullptr;
  return parasitics->reduceToPiPoleResidue2(
      parasitic_network, drvr_pin, rf, scene, min_max);

@jhkim-pii

Copy link
Copy Markdown
Contributor Author

Opened in the wrong repository; closing this PR.

@jhkim-pii jhkim-pii closed this Aug 26, 2026
@jhkim-pii
jhkim-pii deleted the secure-fix-two-pole-dcalc branch August 26, 2026 00:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant