3DIC timing analysis support in dbSta (Stages 0–8)#10417
3DIC timing analysis support in dbSta (Stages 0–8)#10417dsengupta0628 wants to merge 44 commits into
Conversation
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
There was a problem hiding this comment.
Welcome to OpenROAD! Thanks for opening your first PR.
Before we review:
- Contribution Guide: https://openroad.readthedocs.io/en/latest/contrib/contributing.html
- Build Instructions: https://openroad.readthedocs.io/en/latest/contrib/BuildWithCMake.html
Please ensure:
- CI passes
- Code is properly formatted
- Tests are included where applicable
A maintainer will review shortly!
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
There was a problem hiding this comment.
Code Review
This pull request enables 3DIC Static Timing Analysis (STA) by extending dbNetwork to support chiplet instances, chip-nets, and bump-inst pins, including ID disambiguation via block discriminators and synthesized LibertyCells for clock propagation. It also introduces 3DIC-aware iterators, diagnostic reporting, and comprehensive documentation, while ensuring proper object alignment for pointer tagging. Review feedback correctly identified an inconsistency in the documentation and warning messages regarding the .bmap file format, noting that bterm_name is the 5th column rather than the 4th.
| 1. **`.bmap` 4th column binds bump → chiplet port.** | ||
| Format per line: `<bump_inst_name> <BUMP_macro> <x> <y> <bterm_name> <signal>`. | ||
| `bterm_name = "-"` leaves the bump unbound — STA cannot cross that | ||
| boundary. Always set the 4th column to a real chiplet `dbBTerm` name | ||
| unless the test specifically exercises unmapped bumps. |
There was a problem hiding this comment.
The documentation regarding the .bmap file format is inconsistent. The format string on line 427 shows <bterm_name> as the 5th field, but the description on lines 426 and 429 refers to it as the "4th column". This is confusing for users trying to create these files.
To ensure clarity, please update the column number to be consistent with the format string. It should be the 5th column.
Additionally, the related warning message STA-3002 also mentions "col 4", which should probably be updated as well for consistency.
| logger_->warn(utl::STA, | ||
| 3002, | ||
| "3DIC chiplet '{}': {}/{} bump pads not mapped to " | ||
| "a chiplet port (missing name in .bmap col 4). " |
There was a problem hiding this comment.
The column number in this warning message appears to be incorrect. The .bmap format is <bump_inst_name> <BUMP_macro> <bterm_name> , which makes the bterm_name the 5th column, not the 4th. This inconsistency can be confusing for users.
Please update the column number in the message to 5 for accuracy.
| "a chiplet port (missing name in .bmap col 4). " | |
| "a chiplet port (missing name in .bmap col 5). " |
| // Per-block discriminator stamped into upper bits of getDbNwkObjectId | ||
| // so iterms/bterms/insts/nets from different chiplet blocks (each | ||
| // numbered from 1) don't collide in NetSet/PinSet keys. | ||
| std::map<odb::dbBlock*, uint32_t> block_disc_; |
There was a problem hiding this comment.
warning: no header providing "uint32_t" is directly included [misc-include-cleaner]
src/dbSta/include/db_sta/dbNetwork.hh:5:
- #include <map>
+ #include <cstdint>
+ #include <map>| LibertyBuilder builder(debug_, report_); | ||
| for (dbChipInst* chip_inst : chip->getChipInsts()) { | ||
| dbChip* master = chip_inst->getMasterChip(); | ||
| if (master == nullptr || chip_master_cells_.count(master)) { |
There was a problem hiding this comment.
warning: use 'contains' to check for membership [readability-container-contains]
| if (master == nullptr || chip_master_cells_.count(master)) { | |
| ;contains |
|
|
||
| Pin* pin = db_network_->dbToSta(fake_bump); | ||
| ASSERT_NE(pin, nullptr); | ||
| EXPECT_EQ(reinterpret_cast<std::uintptr_t>(pin) & 0b111U, 4U); |
There was a problem hiding this comment.
warning: no header providing "std::uintptr_t" is directly included [misc-include-cleaner]
src/dbSta/test/cpp/TestDbSta.cc:4:
- #include <cstdio>
+ #include <cstdint>
+ #include <cstdio>Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
|
clang-tidy review says "All clean, LGTM! 👍" |
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
| #include "dbChipBumpInst.h" | ||
|
|
||
| #include <cstdint> | ||
|
|
There was a problem hiding this comment.
warning: included header cstdint is not used directly [misc-include-cleaner]
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
…nc is used Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
|
clang-tidy review says "All clean, LGTM! 👍" |
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
|
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
|
clang-tidy review says "All clean, LGTM! 👍" |
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
|
clang-tidy review says "All clean, LGTM! 👍" |
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
| for (odb::dbChipInst* chip_inst : chip->getChipInsts()) { | ||
| odb::dbBlock* chiplet_block = db_network_->blockOf(chip_inst); | ||
| if (chiplet_block == nullptr) { | ||
| continue; | ||
| } | ||
| auto cbk = std::make_unique<dbStaCbk>(this); | ||
| cbk->setNetwork(db_network_); | ||
| cbk->addOwner(chiplet_block); | ||
| chiplet_cbks_.push_back(std::move(cbk)); |
There was a problem hiding this comment.
BTW, this is another area where using the unfolded model is better. assume the scenario of having a design that instantiates two hierarchical chiplets. With this implementation, you are missing hooking the callbacks to the leaf blocks that are used by chiplets under the hierarchical chiplets definitions.
This should not block the PR, but I prefer a clear warning message that if such a scenario exists to say that we currently support STA for flat 3DIC design only currently.
There was a problem hiding this comment.
Added STA-3001 warning mentioning this caveat about "flat" 3D being supported currently. Also added in the 3DIC_TODO.md regarding using unfolded model here.
…only Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
| cat = dbMarkerCategory::createOrReplace(top_cat, "Orphan Chip Nets"); | ||
| } | ||
| if (auto* marker = dbMarker::create(cat)) { | ||
| marker->addSource(net.chip_net); |
There was a problem hiding this comment.
dbChipNet as a marker source is not handled correctly in the dbMarker yet (check dbMarker::getName). Please remove for now and add a TODO comment.
There was a problem hiding this comment.
moot as this function is removed now
| for (dbChipInst* chip_inst : chip->getChipInsts()) { | ||
| dbChip* master = chip_inst->getMasterChip(); |
There was a problem hiding this comment.
| for (dbChipInst* chip_inst : chip->getChipInsts()) { | |
| dbChip* master = chip_inst->getMasterChip(); | |
| for (dbChip* chip : db_->getChips()) { |
| void checkOrphanChipNets(dbMarkerCategory* top_cat, | ||
| const UnfoldedModel* model); | ||
| void checkBumpPortBindings(dbMarkerCategory* top_cat, | ||
| const UnfoldedModel* model); |
There was a problem hiding this comment.
Those 2 checks are out of scope of this PR. could we handle in a separate PR.
I am not sure if those 2 should be considered violations in the blackbox stage.
| define_cmd_args "report_3dic_summary" {} | ||
|
|
||
| proc report_3dic_summary { args } { | ||
| set db [ord::get_db] | ||
| set chip [$db getChip] | ||
| if { $chip == "NULL" } { | ||
| utl::warn STA 3003 "No chip loaded; nothing to report." | ||
| return | ||
| } | ||
| set chip_insts [$chip getChipInsts] | ||
| set chip_nets [$chip getChipNets] | ||
| set chip_conns [$chip getChipConns] | ||
| set inst_count [llength $chip_insts] | ||
| set net_count [llength $chip_nets] | ||
| set conn_count [llength $chip_conns] | ||
| set bump_inst_count 0 | ||
| foreach n $chip_nets { | ||
| incr bump_inst_count [$n getNumBumpInsts] | ||
| } | ||
| utl::report "3DIC summary for chip [$chip getName]:" | ||
| utl::report " chiplets : $inst_count" | ||
| utl::report " top-level nets : $net_count" | ||
| utl::report " 3D bond regions : $conn_count" | ||
| utl::report " bump pads : $bump_inst_count" | ||
| if { $inst_count > 0 } { | ||
| utl::report " chiplet instances:" | ||
| foreach ci $chip_insts { | ||
| set ref [$ci getMasterChip] | ||
| set ref_name [expr { $ref == "NULL" ? "<unbound>" : [$ref getName] }] | ||
| utl::report " [$ci getName] (reference: $ref_name)" | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Not sure if this belongs here as well. It only uses db objects, so maybe it should be under odb?
There was a problem hiding this comment.
moved to odb.tcl
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
|
Relaunched secure ci with latest changes: http://secure-ci:8080/job/SB/job/secure-3dic_sta/ |
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d2dca66d7
ℹ️ 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".
| if (odb::dbChipBumpInst* bump = staToDbChipBumpInst(pin)) { | ||
| return getDbNwkObjectId(bump); |
There was a problem hiding this comment.
Preserve unique IDs for chiplet inner pins
In 3DIC mode this only routes chip-bump pins through the tagged encoder, but the traversal added in this change also exposes inner dbITerm/dbBTerm pins from multiple chiplet dbBlocks. When two chiplet blocks assign the same local term id, the later non-hierarchical branch still returns iterm->getId() << 1 / bterm->getId(), so PinSet users can treat distinct chiplet pins as identical and silently skip one side of a cross-chiplet path. The same has3DicChip() discriminator path used for nets needs to apply to these inner pins too.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
id(const Pin*) gated the tagged (block-discriminated) encoder on hasHierarchy() only. In v1 3DIC (has3DicChip && !hasHierarchy), inner chiplet iterm/bterm pins fell to the raw branch (iterm->getId() << 1 / (bterm->getId()<<1)|1). Since each chiplet dbBlock numbers terms from 1, two chiplets can hand out the same local id --> PinSet/visited_pins (sorted by id()) treat distinct cross-chiplet pins as identical --> one side of a cross-chiplet path silently dropped. Exactly the net-side problem that id(Net*)'s block_disc_ fix solved --> the pin side was missed.
Fix: one-line condition change at dbNetwork.cc:1871, mirroring id(Net*):
if (hasHierarchy() || has3DicChip()) { // was: hasHierarchy()
This routes inner pins through getDbNwkObjectId (which applies blockDiscBits), making pin ids block-unique -- consistent with nets and chip-bumps.
| if (network_->has3DicChip()) { | ||
| if (instance == network_->topInstance()) { | ||
| dbSet<odb::dbChipNet> nets = network_->topChip()->getChipNets(); | ||
| chip_walk_ = true; | ||
| chip_net_iter_ = nets.begin(); | ||
| chip_net_end_ = nets.end(); | ||
| } | ||
| return; |
There was a problem hiding this comment.
Return an empty net iterator for non-top 3DIC instances
For a 3DIC design, constructing DbInstanceNetIterator for any non-top instance (for example get_nets -of_objects [get_cells chipA], or an inner chiplet instance surfaced by the new flat walk) takes this early return with chip_walk_ still false and the legacy iter_/end_ members never initialized. The next hasNext() falls through to comparing those uninitialized iterators, which can crash or return garbage instead of an empty or chip-specific net list.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
In 3DIC mode a non-top instance: ctor early-returns (chip_walk_=false, iter_/end_ never set), then hasNext() falls through to iter_ != end_ (UB on default-constructed dbSet iterators), and next() does *iter_ (UB deref). Fix with a has3DicChip() short-circuit
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6fefa72b2
ℹ️ 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".
| if (Term* inner_term = term(bump_pin)) { | ||
| if (Net* inner_net = this->net(inner_term)) { | ||
| visitConnectedPins(inner_net, visitor, visited_nets); |
There was a problem hiding this comment.
Gate chip-net descent for shared chiplet masters
When a 3DIC design instantiates the same chiplet master more than once, those chip instances share the same master dbBlock and inner dbNet/dbInst pointers; setTopChip() even removes such blocks from block_to_chip_inst_ so they “stay treated as leaves.” This branch still unconditionally maps every bump to the master BTerm and descends into that shared inner net, so visited_nets collapses the two chiplet instances and STA can create/miss paths against the shared master net instead of per-instance connectivity. Please skip this descent for non-unique master blocks (or introduce path-aware identities) so repeated chiplets do not produce silently incorrect timing.
Useful? React with 👍 / 👎.
| kDbIterm = 1U, | ||
| kDbBterm = 2U, | ||
| kDbModIterm = 3U, | ||
| kDbChipBumpInst = 4U, |
There was a problem hiding this comment.
The BumpInst looks somewhat confusing. Bump is a kind of terminal. kDbChipBumpTerm looks more consistent to me.
There was a problem hiding this comment.
Yes the 3DICs have a bit weird terminology. An inst of bump object is called the bumpInst. There is no term actually- physically it is like a package bump- I think that is why the nomenclature is
There was a problem hiding this comment.
The tags are named after the odb TYPE they wrap, not the STA role
kDbIterm ← dbITerm
kDbBterm ← dbBTerm
kDbModIterm ← dbModITerm
kDbChipBumpInst ← dbChipBumpInst ← consistent
Every tag is kDb. The tag encodes a dbChipBumpInst*, so kDbChipBumpInst is the consistent name. dbChipBump vs dbChipBumpInst is the master/instance split this is the 3DIC nomenclature
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
Summary
Enable cross-chiplet STA on 3DBlox designs. read_3dbx now produces a single unified STA graph spanning multiple chiplets connected via bump pads / top-level nets / 3D bond regions. report_checks reports constrained flop-to-flop setup checks across chiplet boundaries with the natural create_clock [get_pins -of_objects [get_nets <top_net>]] SDC form. This version uses a zero-delay parasitic model on every chip-bump and chipnet — STA's fat-net wire-edge model traverses cross-chiplet connections directly. Per-dbChipConn RC binding (real bond/TSV delay) is deferred post-v1; the binding hook is Parasitics::makeParasiticNetwork(chipNet) when added.
Implemented in three commits:
Stages 0–6.5 — adapter foundation. Multi-block ownership in dbNetwork, pointer-tag encoding for dbChipBumpInst / dbChipInst / dbChipNet, chip-aware overrides in the four core iterators (childIterator, pinIterator, netIterator, NetPinIterator), term(Pin*) cross-boundary bridge, dbChipBumpInst 8-byte alignment fix, side-map for chip-bump VertexId, per-master synthetic Cell so cell(chip_inst) returns non-null, findInstance path-split for chipA/ff/CK-style queries.
Stage 7 — close cross-chiplet flop-to-flop path. direction(chip_bump) = BIDIRECT (Graph builds dual vertices + wire edges runs on every bump regardless of underlying bterm IoType). visitConnectedPins(Net*) chip-net branch descends each bump's inner net via term() so the fat-net wire-edge model reaches leaf loads across the boundary. Plus chip-bump clock-anchor caveat: synthetic LibertyCell per chiplet definition carries a zero-delay combinational self-arc per chip-bump port so Graph::makeInstanceEdges builds the internal load → bidir_drvr edge clock arrival needs; per-block 4-bit discriminator in getDbNwkObjectId so cross-block iterm/bterm/net ids don't collide in NetSet/PinSet.
Stage 8 — diagnostics. STA-3000 INFO summary on read_3dbx, STA-3001/3002 WARNs for orphan nets / unmapped bumps, Tcl helper report_3dic_summary. Fixture cleanup in 3dic_cross.tcl to satisfy upstream 3DBlox checker (Connection: + ground via bot: ~ + bmap XYs sized for BUMP macro center offset).
Tests: dbSta/test/3dic_cross.tcl (cross-chiplet flop-to-flop setup check, slack MET against 1 ns clock), dbSta/test/3dic_get_cells.tcl (structural iteration APIs). No src/sta/ edits.
Documentation: docs/contrib/3DIC.md.
Type of Change
Impact
Enable 3DIC timing analysis
Verification
./etc/Build.sh).Related Issues
[Link issues here]