Skip to content

dbSta: use public LevelizeObserver API from OpenSTA#10556

Merged
jhkim-pii merged 2 commits into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:or_enhance_drvr_level_to_dbsta
Jun 3, 2026
Merged

dbSta: use public LevelizeObserver API from OpenSTA#10556
jhkim-pii merged 2 commits into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:or_enhance_drvr_level_to_dbsta

Conversation

@dsengupta0628
Copy link
Copy Markdown
Contributor

Summary

Switch dbSta's level-change observer over to the newly-exposed OpenSTA public API (sta/LevelizeObserver.hh + Sta::setLevelizeObserver), removing three workarounds that were needed when the observer machinery was internal to OpenSTA.

Cleanup code from #10352 post merge of upstream PR parallaxsw/OpenSTA#433

This is a cleanup PR only; behavior of dbSta::levelizedDrvrVertices() and its cache-invalidation path is identical.

Background

dbSta installs a LevelizeObserver so that its levelized_drvr_vertices_ cache is invalidated whenever the timing graph's vertex levels change. Levelize has a single observer slot, so dbSta's observer replaces the default StaLevelizeObserver that Sta::makeObservers installs. The default observer forwards level-change events to Search and GraphDelayCalc to keep their incremental-update iterators consistent — so dbSta's replacement must preserve that forwarding.

Before this change, doing that required:

  1. Adding ${OPENSTA_HOME} and ${OPENSTA_HOME}/include/sta as PRIVATE includes on dbSta_lib to reach search/Levelize.hh (not part of OpenSTA's public include set).
  2. Calling levelize_->setObserver(...) directly — levelize_ is protected on StaState, a private contract.
  3. Hand-replicating StaLevelizeObserver's Search + GraphDelayCalc forwarding inside DbStaLevelizeObserver. If upstream added new work to StaLevelizeObserver, the dbSta copy would silently drift.

OpenSTA now exposes:

  • A public header include/sta/LevelizeObserver.hh containing both LevelizeObserver (interface) and StaLevelizeObserver (default implementation with Search + GraphDelayCalc forwarding).
  • A public setter Sta::setLevelizeObserver(LevelizeObserver*) that owns the observer and deletes any prior one.

OpenROAD changes in this PR

src/dbSta/src/dbSta.cc

  • Replace #include "search/Levelize.hh" with #include "sta/LevelizeObserver.hh".
  • Change DbStaLevelizeObserver to inherit from StaLevelizeObserver instead of the bare LevelizeObserver interface, and chain to the base class methods instead of re-implementing the Search + GraphDelayCalc forwarding by hand
  • Use setLevelizeObserver(...) instead of levelize_->setObserver(...).

src/dbSta/src/CMakeLists.txt

  • Drop the PRIVATE include hack — no longer needed since the observer header is now in OpenSTA's public include set

Type of Change

  • Refactoring

Impact

None. Better maintainability

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 signed my commits (DCO).

Related Issues

[Link issues here]

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
@dsengupta0628 dsengupta0628 self-assigned this May 29, 2026
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 refactors DbStaLevelizeObserver to inherit from StaLevelizeObserver instead of LevelizeObserver, delegating the forwarding of level change events to the base class and removing redundant calls. Additionally, it cleans up the build configuration by removing unnecessary private OpenSTA include paths from CMakeLists.txt and updates the includes in dbSta.cc. There are no review comments, and I have no feedback to provide.

@dsengupta0628 dsengupta0628 marked this pull request as ready for review May 29, 2026 17:06
@dsengupta0628 dsengupta0628 requested a review from a team as a code owner May 29, 2026 17:06
@dsengupta0628 dsengupta0628 requested a review from jhkim-pii May 29, 2026 17:06
@dsengupta0628
Copy link
Copy Markdown
Contributor Author

dsengupta0628 commented May 29, 2026

@dsengupta0628
Copy link
Copy Markdown
Contributor Author

Even though the secure-ci shows some secure designs drift QoR, they are unrelated to my change and a separate metric update is already being done with a different PR from @maliberty to update those metrics. So this is okay to merge now

@dsengupta0628
Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

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

@jhkim-pii jhkim-pii merged commit 9b75dd8 into The-OpenROAD-Project:master Jun 3, 2026
16 checks passed
@jhkim-pii jhkim-pii deleted the or_enhance_drvr_level_to_dbsta branch June 3, 2026 04:03
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