Skip to content

Odb bazel split#10130

Merged
maliberty merged 13 commits into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:odb-bazel-split
Apr 14, 2026
Merged

Odb bazel split#10130
maliberty merged 13 commits into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:odb-bazel-split

Conversation

@openroad-ci

Copy link
Copy Markdown
Member

Summary

Break up the monolithic odb bazel target

Type of Change

  • Refactoring

Impact

Create better scope and build parallelism

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

Break up the monolithic //src/odb:odb cc_library into per-subdirectory
libraries under //src/odb/src/{zutil,db,cdl,defout,lefout,gdsin,gdsout,
defin,lefin,3dblox}, plus a shared //src/odb:headers target for the
public include/odb/*.h headers.

//src/odb:odb becomes a thin umbrella that depends on every per-subdir
library and republishes the public headers so the 48 downstream BUILD
files depending on //src/odb continue to work unchanged. The umbrella
also retains swig_common.{cpp,h} since the SWIG targets reference those
files relative to the top-level package.

Each subdir now carries only the Boost/abseil dependencies it actually
uses, so e.g. lefin's Boost.Spirit/phoenix/fusion deps no longer
propagate into cdl, defout, gdsout, etc. Incremental rebuilds should
improve since touching one subdir no longer invalidates compile actions
in unrelated subdirs.

Also remove a stale #include "odb/defout.h" from dbBlock.cpp (DefOut was
never referenced); this eliminates a false db -> defout dependency edge.

Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
Move 11 module-specific public headers from the shared //src/odb:headers
target into the subdir cc_library that actually implements them:
  3dblox.h     -> //src/odb/src/3dblox
  cdl.h        -> //src/odb/src/cdl
  defin.h      -> //src/odb/src/defin
  defout.h     -> //src/odb/src/defout
  gdsin.h,
  gdsUtil.h    -> //src/odb/src/gdsin
  gdsout.h     -> //src/odb/src/gdsout
  lefin.h      -> //src/odb/src/lefin
  lefout.h     -> //src/odb/src/lefout
  poly_decomp.h,
  util.h       -> //src/odb/src/zutil

The files stay physically in src/odb/include/odb/ (preserving the CMake
layout); each subdir references its header via exports_files from the
parent package. The //src/odb:headers target now contains only core
shared odb types (db.h, geom.h, dbTypes.h, set/iterator templates, etc.)
that every subsystem genuinely needs.

Introduce //src/odb/src/lefout:lefout_headers as a headers-only split of
the lefout target so //src/odb/src/db can include odb/lefout.h (it
implements writeLef() overloads that call inline methods on lefout)
without forming a link cycle: db depends on :lefout_headers, while
:lefout (the implementation) depends on :db.

The //src/odb:odb umbrella keeps its existing public hdrs glob so the
48 downstream consumers depending on //src/odb continue to work
unchanged.

Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
The zutil library contained only two files — util.cpp (general odb
utilities operating on dbBlock/dbNet/dbTechLayer) and poly_decomp.cpp
(polygon decomposition used by db, defin, and lefin). Both are naturally
part of db: util.cpp already required the full db API, and poly_decomp
is called from db itself. The zutil layer existed as an artificial seam
purely because of how the build was organized, and it created a circular
constraint (db linking zutil while zutil's util.cpp needed db headers).

Move src/zutil/{util,poly_decomp}.cpp into src/db/ and delete the zutil
subdirectory. Their public headers (util.h, poly_decomp.h) are now owned
by //src/odb/src/db via exports_files from the parent package.

Update both Bazel and CMake:
  - Remove //src/odb/src/zutil target and src/zutil/CMakeLists.txt
  - Add util.cpp, poly_decomp.cpp to src/db/CMakeLists.txt and drop the
    zutil dep from db's target_link_libraries
  - Drop zutil from defin, lefin, and odb test CMakeLists (they all
    transitively get it through db)
  - Drop zutil from the //src/odb umbrella and all sibling Bazel deps

Downstream consumers that depend on //src/odb continue to work unchanged
(the umbrella still republishes all public headers). Anyone who was
directly depending on the zutil target would now depend on //src/odb/src/db,
but no such direct dependents exist outside the odb tree.

Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
The db classes (dbMTerm, dbTechLayerSpacingRule, dbTechMinCutRule,
dbTechMinEncRule, dbTechV55InfluenceEntry, dbTechLayerAntennaRule,
dbTechAntennaPinModel, dbTechLayer) had member methods that took
lefout& arguments and emitted LEF text. This forced db.h to forward-
declare class lefout, and src/db/ to compile against odb/lefout.h --
an inverted dependency that required a Bazel headers-only sub-target
on lefout to avoid forming a link cycle.

Lift these emitters into private member methods of lefout itself, so
they live next to the existing writeLayer/writeMTerm/writeVia code
and access the db API only through public getters.

This commit only adds the new lefout methods and switches the call
sites (in lefout::writeLayer and lefout::writeMTerm) to use them.
The old methods on the db classes are kept temporarily so the code
remains compilable; they are removed in a follow-up commit. The
db.h forward declaration of lefout is also kept until the follow-up.

Two preparatory changes are bundled here because the new lefout
methods depend on them:

  - dbMTerm gains three public partial-area accessors
    (getPartialMetalArea, getPartialMetalSideArea, getPartialCutArea)
    following the existing getDiffArea pattern, replacing the old
    direct access to private impl fields from the writer.

  - _dbTechAntennaPinModel::getAntennaValues now takes _dbTech*
    instead of _dbDatabase*, and its callers (in dbMTerm and
    dbTechAntennaPinModel) compute the right tech via the owning
    master/lib (dbLib::getTech) rather than dbDatabase::getTech.
    The old code path called dbDatabase::getTech which errors out
    in multi-tech databases; the public getDiffArea/getGateArea/
    etc. were silently broken in that case. The 3DBlox flow
    exercises this and the LEF write would have failed there
    once the new code path went live.

LEF write tests still pass (all 112 odb tests pass).

Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
Remove the 10 lefout-dependent methods that the previous commit replaced:
  - dbMTerm::writeAntennaLef
  - dbTechLayerSpacingRule::writeLef
  - dbTechMinCutRule::writeLef
  - dbTechMinEncRule::writeLef
  - dbTechV55InfluenceEntry::writeLef
  - dbTechLayerAntennaRule::writeLef
  - dbTechAntennaPinModel::writeLef
  - dbTechLayer::printV55SpacingRules
  - dbTechLayer::printTwoWidthsSpacingRules (dead code, never called)
  - dbTechLayer::writeAntennaRulesLef
  - _dbTechAntennaAreaElement::writeLef (internal class, dead code)

Also clean up:
  - Remove `class lefout;` forward declaration from db.h.
  - Remove `#include "odb/lefout.h"` from 6 db .cpp files.
  - Remove `class lefout;` and _dbTechAntennaAreaElement::writeLef
    declaration from dbTechLayerAntennaRule.h.
  - Drop the //src/odb/src/lefout:lefout_headers Bazel sub-target that
    existed solely to break the db -> lefout header cycle. The cycle no
    longer exists since db no longer includes odb/lefout.h.
  - Drop the :lefout_headers dep from //src/odb/src/db:db.

After this commit, src/odb/src/db/ has no compile-time or link-time
dependency on lefout. The lefout -> db dependency is the only direction.

Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
The //src/odb:headers cc_library existed as a shared home for the core
odb public headers (db.h, geom.h, dbTypes.h, etc.) that every subsystem
needed. Now that the db -> lefout cycle is gone and zutil has been
folded into db, there is no remaining reason for a separate headers
target — every subdir already depends on //src/odb/src/db.

Move the 30 core public headers from :headers into :db's hdrs (via
cross-package exports_files references). Add includes = ["../../include"]
to :db so the -I path resolves correctly for all consumers. Absorb the
two deps that :headers carried but :db did not (@abseil-cpp//absl/base:
core_headers, @boost.multi_array).

Replace the per-module exports_files entries with a single broad
exports_files(glob(...)) — all non-MakeOdb.h public headers are now
visible to all subpackages.

Drop //src/odb:headers from every subdir's deps (they get the headers
transitively through //src/odb/src/db).

Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
Create a new Bazel package at src/odb/src/swig/ containing:
  - swig_common (cc_library): swig_common.cpp/.h with explicit deps on
    defin, defout, lefin, lefout instead of the entire umbrella
  - swig (tcl_wrap_cc): TCL SWIG wrapper
  - tcl (tcl_encode): TCL encoding
  - ui (cc_library): MakeOdb + SWIG/TCL glue
  - swig_imports_py (filegroup): Python .i files for the parent

The Python targets (swig-py, _odb.so, odb_py) stay in the parent
//src/odb package because the generated odb.py import path encodes
the Bazel package path (import src.odb.odb) — moving them would break
all downstream Python consumers.

Alias targets (swig, ui, tcl) are added in the parent BUILD so the
44 downstream references to //src/odb:swig etc. continue to work.

The messages_txt glob patterns are narrowed to include/odb/*.h since
all source subdirs now have their own BUILD files (making recursive
src/** globs unreachable from the parent). The messages.checked file
is empty, so no validation is lost.

Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
The per-target features = ["-use_header_modules"] was disabling a
feature that is not enabled by the LLVM toolchain in the first place.
Verified by building all 225 odb targets and running all 112 tests
without it. Only the package-level features = ["layering_check"]
remains.

Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
gdsout.h included odb/gdsin.h but never referenced GDSReader or
anything else from it. The shared GDS types (record_t, RecordType, etc.)
come from odb/gdsUtil.h which both gdsin.h and gdsout.h include
independently.

Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
Signed-off-by: Matt Liberty <mliberty@precisioninno.com>

@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 refactors the odb library by modularizing the build system into per-subdirectory targets and merging zutil utilities into the core db library. It also transitions LEF output generation logic from database objects to the lefout class to improve decoupling. Review feedback highlights critical bugs in the new lefout implementation where antenna data vectors are not cleared between category retrievals, leading to accumulated output. Additionally, a potential null pointer dereference was identified in dbTechAntennaPinModel when resolving the parent technology.

Comment thread src/odb/src/lefout/lefout.cpp
Comment thread src/odb/src/lefout/lefout.cpp
Comment thread src/odb/src/db/dbTechLayerAntennaRule.cpp
The get*() antenna accessors (getGateArea, getMaxAreaCAR, getDiffArea,
getPartialMetalArea, etc.) all delegate to getAntennaValues which
appends via emplace_back without clearing the output vector first. If
a caller reuses the same vector across multiple calls, entries from
earlier calls would be duplicated in the output.

Fix by clearing the result vector at the top of getAntennaValues so
callers always get a clean result regardless of the vector's prior
state.

Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
@github-actions

Copy link
Copy Markdown
Contributor

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

@maliberty

Copy link
Copy Markdown
Member

@codex review

@github-actions

Copy link
Copy Markdown
Contributor

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

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 22b435b5e2

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

Comment thread src/odb/BUILD
After the odb Bazel split, messages_txt only globbed include/odb/*.h
(8 messages) and missed ~576 logger messages in sub-package source
files. Add message_srcs filegroups to each sub-package, an extra_srcs
parameter to the messages_txt macro, and positional file-path support
in find_messages.py so that sources spanning multiple Bazel packages
are scanned correctly.

Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
@github-actions

Copy link
Copy Markdown
Contributor

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

@maliberty

Copy link
Copy Markdown
Member

@hzeller FYI

@maliberty maliberty enabled auto-merge April 14, 2026 18:43
@maliberty maliberty merged commit 61932e8 into The-OpenROAD-Project:master Apr 14, 2026
16 checks passed
@maliberty maliberty deleted the odb-bazel-split branch April 14, 2026 18:49
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