est: Filter ideal clock net invalidation by jhkim-pii · Pull Request #10571 · The-OpenROAD-Project/OpenROAD

jhkim-pii · 2026-05-31T15:27:03Z

Summary

Filter ideal clock nets out of estimated-parasitic refresh and STA fanin delay invalidation in EstimateParasitics::updateParasitics().
Fix multi-mode ideal-clock classification so scan clocks that exist only in test mode are still recognized as ideal clocks.
Add C++ regressions that fail on unfixed master and pass with this PR.

Problem

Problem 1 (MAJOR): Ideal clock-net invalidation can increase incremental STA runtime

During repair_design, DFF resizing can mark the DFF CK net as parasitics-invalid.
In pre-CTS placement flows, the clock network can still be ideal, so clock arrivals and slews are fixed by STA clock modeling and do not depend on estimated RC.
The old updateParasitics() path treated every invalid net the same, so an ideal clock net could trigger RC re-estimation and sta_->delaysInvalidFromFanin(clock_net).
On a high-fanout ideal clock net, delaysInvalidFromFanin(clock_net) invalidates the top clock port and every CK load vertex, which can make the next incremental STA update much larger than the actual ECO requires.

flowchart TD
  A["repair_design ECO"]
  B["Resize DFF"]
  C["CK net is marked<br/>parasitics-invalid"]
  D["updateParasitics()<br/>visits clock_net"]
  E["estimateWireParasitic<br/>(clock_net)"]
  F["sta_->delaysInvalidFromFanin<br/>(clock_net)"]
  G["Invalidate clock port"]
  H["Invalidate every<br/>CK load vertex"]
  I["Large incremental STA<br/>fanout update"]

  A --> B --> C --> D
  D --> E --> F
  D --> F
  F --> G --> I
  F --> H --> I

  classDef problem fill:#ffe3e3,stroke:#c92a2a,color:#5c0000;
  class D,E,F,G,H,I problem;

Problem 2 (minor): Scan clocks can be ideal only in test mode

The old ideal-clock check asked isIdealClock(pin, mode) for every mode.
If scan_clk is created only in test mode, the scan clock pin is not a clock in function mode.
In that case isIdealClock(scan_clk_pin, function_mode) is false because the pin is not a function-mode clock, not because it is a propagated or RC-dependent clock.
The old all-mode check therefore classified the scan clock net as non-ideal and allowed unnecessary parasitic and STA invalidation.

flowchart TD
  P["scan_clk pin"] --> F["Function mode:<br/>isClock = false"]
  P --> T["Test mode:<br/>isClock = true<br/>isIdealClock = true"]
  F --> O["Old check:<br/>isIdealClock = false"]
  O --> R["Reject ideal-clock<br/>filtering"]
  T --> N["New check:<br/>require ideal only when<br/>isClock = true"]
  F --> S["New check:<br/>ignore non-clock mode"]
  N --> K["Classify scan_clk<br/>as ideal"]
  S --> K

  classDef problem fill:#ffe3e3,stroke:#c92a2a,color:#5c0000;
  class O,R problem;

Solution

Problem 1 Fix: Skip ideal clock nets in updateParasitics()

Classify each invalid net before refreshing RC or invalidating STA; ideal clock nets are skipped because their arrivals/slews are not RC-dependent.

 for (const sta::Net* net : parasitics_invalid_) {
+  if (isIdealClockNet(net)) {
+    continue;
+  }
   estimateWireParasitic(net);
 }
 
 for (const sta::Net* net : parasitics_invalid_) {
+  if (isIdealClockNet(net)) {
+    continue;
+  }
   sta_->delaysInvalidFromFanin(net);
 }

Problem 2 Fix: Ignore modes where the pin is not a clock

Replace the all-modes ideal-clock requirement with a clock-aware check: non-clock modes are ignored, and every mode where the pin is a clock must still be ideal.

-bool all_modes_ideal_clock = true;
+bool is_clock = false;
 for (sta::Mode* mode : sta_->modes()) {
-  if (!sta_->isIdealClock(drvr_pin, mode)) {
-    all_modes_ideal_clock = false;
-    break;
+  if (!sta_->isClock(pin, mode)) {
+    continue;
+  }
+  is_clock = true;
+  if (!sta_->isIdealClock(pin, mode)) {
+    return false;
   }
 }
+return is_clock;

Add EstimateParasitics::isIdealClockPin() to classify a pin as ideal only if it is a clock in at least one mode and ideal in every mode where it is a clock.
Ignore modes where sta_->isClock(pin, mode) is false, which handles scan clocks that are only defined in test mode.
Add EstimateParasitics::isIdealClockNet() to classify invalid nets by looking up the net driver and applying the ideal-clock pin check.
Keep parasiticsInvalid() behavior simple by still recording the flat invalid net, so callers and callbacks do not need special ideal-clock handling.
In updateParasitics(), skip ideal clock nets in the placement RC refresh loop, the routed RC refresh loop, and the bulk sta_->delaysInvalidFromFanin() loop.
Preserve normal behavior for signal nets, propagated clock nets, non-clock nets, and clock nets whose driver is not ideal in any clock mode.
Add TestEstimateParasitics.IdealClockNetSkipsStaInvalidation to prove an ideal clock net in parasitics_invalid_ does not invalidate STA delay or arrival state.
Add TestEstimateParasitics.ScanClockIdealOnlyInTestMode to prove a scan clock absent from function mode but ideal in test mode is still filtered correctly.

flowchart TD
  A["parasitics_invalid_<br/>contains net"] --> B{"isIdealClockNet<br/>(net)?"}
  B -->|"yes"| C["Skip RC<br/>refresh"]
  C --> D["Skip delaysInvalidFromFanin<br/>(net)"]
  D --> E["No STA fanout<br/>invalidation"]
  B -->|"no"| F["Refresh<br/>estimated RC"]
  F --> G["Call delaysInvalidFromFanin<br/>(net)"]
  G --> H["Normal incremental STA<br/>invalidation"]

  classDef fix fill:#d9fbe5,stroke:#2f9e44,color:#0b3d20;
  class B,C,D,E fix;

Impact

Improves repair_design runtime by avoiding RC refresh and STA fanout invalidation for high-fanout ideal clock nets.
Keeps ideal-clock filtering inside EstimateParasitics, so callers need no special handling.
No QoR change in both public & secure CIs.
- Public CI: https://jenkins.openroad.tools/job/OpenROAD-flow-scripts-Private/job/secure-filter-ideal-clknet/1/stages/
- TEST secure CI: http://secure-ci:8080/job/SB/job/secure-filter-ideal-clknet/1/stages/
- BASE secure CI: http://secure-ci:8080/job/SB/job/master/1424/stages/?selected-node=1197
  - Both BASE (no fix) and TEST (fix) show exactly the same QoR.

Testing

New regression ctest -R TestEstimateParasitics --output-on-failure: Passed, 2/2.
No-fix master validation at 73ab73903351a102bf8b92c9ac5fda7ab5c483dd: Both new tests fail, with ideal clock CK vertices entering STA invalidation sets.
Local ORFS BASE(master e98fb4f4f7) vs TEST(1f7bc1d224) on 10 small designs: Final setup WNS/TNS, hold WNS/TNS, instance area/count, and total/internal/switching/leakage power matched exactly.

Skip adding ideal clock nets to the estimated-parasitic invalidation set because ideal clock arrivals do not depend on RC. This avoids repeatedly seeding large ideal clock networks during placement-stage ECO repair while preserving normal invalidation for propagated clock and signal nets. Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>

Update rsz golden logs for ideal clock net filtering. The repair_clk_nets1 output now keeps the CLOCK net type while preserving the reported inserted buffer wire lengths. The repair_setup_gcd output reflects the deterministic power value produced after skipping ideal clock parasitic invalidation. Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>

Move ideal clock net filtering out of the ODB invalidation callback and into the incremental parasitic update path. This avoids rebuilding the STA clock network while repair_clock_nets is in the middle of disconnecting and reconnecting clock path pins. Add a small helper to classify ideal clock driver pins across modes, and restore the affected regression outputs to the stable clock-network behavior. Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>

Replace the duplicate all-mode ideal clock loop in Steiner parasitic estimation with the shared ideal clock pin helper. The helper keeps the mode-aware behavior that ignores modes where the pin is not a clock. Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>

Add C++ regressions for EstimateParasitics incremental invalidation of ideal clock nets. The tests cover DFF resize invalidating an ideal clock net and a scan clock that is ideal only in test mode while absent from function mode. Register the new regression in both CMake and Bazel so the ideal clock filtering behavior is covered by unit test infrastructure. Tested: clang-tidy src/est/test/cpp/TestEstimateParasitics.cc -p build Tested: cmake --build build --target TestEstimateParasitics -j16 Tested: ctest -R TestEstimateParasitics --output-on-failure Tested: bazel test //src/est/test:TestEstimateParasitics Co-authored-by: OmX <omx@oh-my-codex.dev> Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>

gemini-code-assist

Code Review

This pull request optimizes the parasitic update process by skipping RC re-estimation and STA delay invalidation for ideal clock nets, particularly handling multi-mode designs like scan clocks correctly. It also introduces a C++ unit test suite to verify these changes. The review feedback correctly identifies a memory leak in the newly added isIdealClockNet function, where the dynamically allocated PinSet returned by network_->drivers is not freed, and provides a clean suggestion to use std::unique_ptr to manage its lifetime.

jhkim-pii · 2026-05-31T15:32:58Z

+#include "sta/GraphDelayCalc.hh"
+#include "sta/Search.hh"
+#undef protected
+#undef private


@maliberty Is this ok for test?

jhkim-pii · 2026-05-31T15:39:50Z

+    // modes. Ignore modes where the pin is not a clock at all.
+    // e.g., scan clock pin may not be defined as clock in function mode.
+    if (!sta_->isClock(pin, mode)) {
+      continue;


@eder-matheus isClock() check is newly added to handle the Problem 2 in the PR description.

jhkim-pii · 2026-05-31T15:41:37Z

  if (parasitics_src_ != ParasiticsSrc::kNone) {
    for (const sta::Net* net : parasitics_invalid_) {
+      if (isIdealClockNet(net)) {
+        continue;


This is the main performance fix.
It does not retrigger the redundant update of all DFFs connected to an ideal clock net.

jhkim-pii · 2026-05-31T15:42:21Z

+// DFF resizing may mark the clock net parasitics invalid. For an ideal clock,
+// those parasitics do not contribute to clock arrival/slew, so updateParasitics
+// should skip both RC re-estimation and delaysInvalidFromFanin() for that net.
+TEST_F(TestEstimateParasitics, IdealClockNetSkipsStaInvalidation)


New test case for Problem 1.

jhkim-pii · 2026-05-31T15:42:30Z

+// test mode: the scan pin is not a clock in function mode, so that mode must be
+// ignored. The fixed logic first checks isClock(pin, mode), then requires ideal
+// status only in modes where the pin is actually a clock.
+TEST_F(TestEstimateParasitics, ScanClockIdealOnlyInTestMode)


New test case for Problem 2.

jhkim-pii · 2026-05-31T15:43:38Z

@minjukim55 This is another repair_design performance fix.

jhkim-pii · 2026-05-31T15:50:31Z

@codex review

chatgpt-codex-connector · 2026-05-31T15:55:43Z

Codex Review: Didn't find any major issues. What shall we delve into next?

ℹ️ 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".

oharboe · 2026-05-31T16:22:21Z

🥳

oharboe · 2026-05-31T16:25:20Z

any examples of impact on run times?

jhkim-pii · 2026-06-01T00:04:51Z

@oharboe I tested the fix w/ the long-tail design that you shared originally.

Long-tail design `repair_design` runtime

BASE (no fix) : Completed in 8890s
TEST (fix) : Completed in 850s (10.5x speedup)

oharboe · 2026-06-01T03:22:44Z

🤯

@ovebryne FYI

dsengupta0628 · 2026-06-01T13:13:52Z

+    return false;
+  }
+
+  const Pin* drvr_pin = *drivers->begin();


Is there a check somewhere upfront that clock nets cannot be multidriven nets? Maybe just add a comment here about that. Even an assert might work!

I let Codex & Claude search the detector for multiple drivers in clock network, but they couldn't find it.

Typically, clock network does not have multiple drivers except for clock mesh design.
I am not sure if OpenROAD supports the clock mesh design, but it seems unlikely.

If there are multiple clock drivers, this API retrieves only the first driver and check if it is an ideal clock.
This logic works even if multiple drivers are present because the multiple drivers have the identical clock type (there will be no such case that one of the multiple drivers is an ideal clock while another clock driver is not).

There has been some discussion of adding clock mesh support but it hasn't gone anywhere so far.

eder-matheus

LGTM. Please address @dsengupta0628's comment before merging.

jhkim-pii and others added 5 commits May 31, 2026 19:30

jhkim-pii self-assigned this May 31, 2026

github-actions Bot added the size/M label May 31, 2026

gemini-code-assist Bot reviewed May 31, 2026

View reviewed changes

Comment thread src/est/src/EstimateParasitics.cpp

jhkim-pii commented May 31, 2026

View reviewed changes

jhkim-pii marked this pull request as ready for review May 31, 2026 15:36

jhkim-pii requested a review from a team as a code owner May 31, 2026 15:36

jhkim-pii requested a review from eder-matheus May 31, 2026 15:36

jhkim-pii mentioned this pull request May 31, 2026

[rsz] repair_design: lib+odb screen + defer in-loop parasitic flush #10326

Closed

4 tasks

jhkim-pii commented May 31, 2026

View reviewed changes

jhkim-pii requested a review from minjukim55 May 31, 2026 15:43

dsengupta0628 reviewed Jun 1, 2026

View reviewed changes

dsengupta0628 approved these changes Jun 1, 2026

View reviewed changes

eder-matheus approved these changes Jun 1, 2026

View reviewed changes

eder-matheus merged commit eff4a61 into The-OpenROAD-Project:master Jun 1, 2026
17 checks passed

eder-matheus deleted the secure-filter-ideal-clknet branch June 1, 2026 15:04

Conversation

jhkim-pii commented May 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Problem

Problem 1 (MAJOR): Ideal clock-net invalidation can increase incremental STA runtime

Problem 2 (minor): Scan clocks can be ideal only in test mode

Solution

Problem 1 Fix: Skip ideal clock nets in updateParasitics()

Problem 2 Fix: Ignore modes where the pin is not a clock

Impact

Testing

Related

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jhkim-pii commented May 31, 2026

Uh oh!

jhkim-pii commented May 31, 2026

Uh oh!

chatgpt-codex-connector Bot commented May 31, 2026

Uh oh!

oharboe commented May 31, 2026

Uh oh!

oharboe commented May 31, 2026

Uh oh!

jhkim-pii commented Jun 1, 2026

Long-tail design repair_design runtime

Uh oh!

oharboe commented Jun 1, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eder-matheus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jhkim-pii commented May 31, 2026 •

edited

Loading

Long-tail design `repair_design` runtime