Odb bazel split#10130
Conversation
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>
There was a problem hiding this comment.
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.
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>
|
clang-tidy review says "All clean, LGTM! 👍" |
|
@codex review |
|
clang-tidy review says "All clean, LGTM! 👍" |
There was a problem hiding this comment.
💡 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".
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>
|
clang-tidy review says "All clean, LGTM! 👍" |
|
@hzeller FYI |
Summary
Break up the monolithic odb bazel target
Type of Change
Impact
Create better scope and build parallelism
Verification
./etc/Build.sh).