Skip to content

dbSta: check LEF pins against Liberty#10821

Closed
naveenvenk17 wants to merge 1 commit into
The-OpenROAD-Project:masterfrom
naveenvenk17:nv-ip-checker-liberty-pins-4872
Closed

dbSta: check LEF pins against Liberty#10821
naveenvenk17 wants to merge 1 commit into
The-OpenROAD-Project:masterfrom
naveenvenk17:nv-ip-checker-liberty-pins-4872

Conversation

@naveenvenk17

Copy link
Copy Markdown
Contributor

Summary

  • add LEF/LIB-CHK-012 to warn when a LEF non-supply pin is missing from the matching Liberty cell
  • keep the check conditional on a matching Liberty cell so LEF-only IP checks do not become noisy
  • register a focused pass/fail regression fixture in both Bazel and CMake

Related to #4872.

Tests

  • PASS: git diff --check
  • PASS: bazelisk build --jobs=4 --local_cpu_resources=4 //src/dbSta:IpChecker
  • BLOCKED: bazelisk test --jobs=4 --local_cpu_resources=4 //src/dbSta/test:check_ip_liberty_pins-tcl_test on the Ubuntu 22.04 GCP VM fails before test execution while linking openroad with unresolved __isoc23_* glibc symbols from the Bazel LLVM/sysroot toolchain.

@naveenvenk17 naveenvenk17 requested a review from a team as a code owner July 5, 2026 15:38
@naveenvenk17 naveenvenk17 requested a review from jhkim-pii July 5, 2026 15:38
@github-actions github-actions Bot added the size/M label Jul 5, 2026

@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 introduces a new check, LEF/LIB-CHK-012, to verify that LEF signal pins exist in the corresponding Liberty cell. It implements the checkLibertyPinPresence method in IpChecker and adds a new pass/fail regression test check_ip_liberty_pins to validate this functionality. There are no review comments, so I have no feedback to provide.

Comment thread src/dbSta/src/IpChecker.cc Outdated
Comment on lines +707 to +711
LibertyCell* liberty_cell = sta_->getDbNetwork()->findLibertyCell(
std::string_view(master->getName()));
if (liberty_cell == nullptr) {
return;
}

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.

Are you somewhere else checking that the liberty cell isn't missing when expected?

@naveenvenk17

Copy link
Copy Markdown
Contributor Author

Good catch. I updated the checker so check_ip now reports a missing Liberty cell for a LEF BLOCK macro when Liberty data has been loaded, before doing the per-pin checks. I also kept the no-Liberty-loaded case as a skip so existing LEF-only check_ip use does not start failing.

Verified on the VM only:

  • $HOME/.local/bin/clang-format --dry-run --Werror src/dbSta/src/IpChecker.cc src/dbSta/include/db_sta/IpChecker.hh
  • git diff --check
  • Ubuntu 24 Bazel container: bazelisk build //src/dbSta:IpChecker
  • Ubuntu 24 Bazel container: bazelisk test //src/dbSta/test:check_ip_liberty_pins-tcl_test
  • Ubuntu 24 Bazel container: manual no-Liberty smoke test for check_ip -master lef_lib_cell_missing
  • Ubuntu 24 Bazel container: bazelisk test //src/dbSta/test:all

All 75 dbSta test targets passed.

@naveenvenk17 naveenvenk17 force-pushed the nv-ip-checker-liberty-pins-4872 branch from d34074a to 2af5c83 Compare July 7, 2026 06:06
Signed-off-by: Naveen Venkat <archgen.guest@nyayanidhi.in>
@naveenvenk17 naveenvenk17 force-pushed the nv-ip-checker-liberty-pins-4872 branch from 2af5c83 to 602bee9 Compare July 7, 2026 06:45
@naveenvenk17

Copy link
Copy Markdown
Contributor Author

Fixed the Clang-Tidy-Bazel include-cleaner failure in 602bee9ad7 by directly including sta/NetworkClass.hh for sta::LibertyLibraryIterator.

VM verification:

  • clang-format --dry-run -Werror src/dbSta/src/IpChecker.cc
  • bazelisk build --config=lint -- //src/dbSta:IpChecker
  • Ubuntu24 Docker: bazelisk build //src/dbSta:IpChecker
  • Ubuntu24 Docker: bazelisk test //src/dbSta/test:check_ip_liberty_pins-tcl_test
  • Ubuntu24 Docker: bazelisk test //src/dbSta/test:all (75 targets passed)
  • git diff --check

@naveenvenk17

Copy link
Copy Markdown
Contributor Author

Folded this work into #10820 after the review feedback to keep Liberty presence and direction validation in one traversal. Closing this duplicate so the remaining review stays in one PR.

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