odb: impl 3dblox parser path asserstions#10055
odb: impl 3dblox parser path asserstions#10055openroad-ci wants to merge 1 commit intoThe-OpenROAD-Project:masterfrom
Conversation
Signed-off-by: Ahmed R. Mohamed <ahmed@precisioninno.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements path assertion validation within the 3DBlox checker. It introduces a 'blackbox' attribute for chips to handle components without detailed routing and adds a CSR-based RoutingGraph to model design connectivity. The checker now utilizes a BFS-based algorithm to verify reachability constraints defined in path assertions. Feedback was provided regarding the use of magic numbers for buffer capacities in the BFS state.
| queue.reserve(std::min<uint32_t>(n, 4096u)); | ||
| dirty_blocked.reserve(64); | ||
| dirty_target.reserve(64); |
There was a problem hiding this comment.
The values 4096u and 64 are magic numbers. To improve readability and maintainability, they should be defined as named constants. This also aligns with the general rule to avoid hardcoded magic numbers for tunable parameters.
For example, you could add them to the BfsState struct:
struct BfsState
{
static constexpr uint32_t kInitialBfsQueueCapacity = 4096u;
static constexpr uint32_t kInitialDirtyListCapacity = 64u;
std::vector<uint8_t> is_blocked;
// ...
explicit BfsState(uint32_t n)
: is_blocked(n, 0), is_target(n, 0), visited(n, 0)
{
queue.reserve(std::min<uint32_t>(n, kInitialBfsQueueCapacity));
dirty_blocked.reserve(kInitialDirtyListCapacity);
dirty_target.reserve(kInitialDirtyListCapacity);
}
// ...
};References
- Define tunable parameters as named constants instead of using hardcoded magic numbers.
|
clang-tidy review says "All clean, LGTM! 👍" |
| static std::vector<std::string> splitPath(const std::string& path) | ||
| { | ||
| std::vector<std::string> parts; | ||
| std::istringstream stream(path); | ||
| std::string part; | ||
|
|
||
| while (std::getline(stream, part, '/')) { | ||
| if (!part.empty()) { | ||
| parts.push_back(part); | ||
| } | ||
| } | ||
|
|
||
| return parts; | ||
| } |
| @@ -0,0 +1,33 @@ | |||
| // SPDX-License-Identifier: BSD-3-Clause | |||
| // Copyright (c) 2023-2026, The OpenROAD Authors | |||
| @@ -0,0 +1,133 @@ | |||
| // SPDX-License-Identifier: BSD-3-Clause | |||
| // Copyright (c) 2023-2026, The OpenROAD Authors | |||
| if (!conn.top_region || !conn.bottom_region) { | ||
| continue; // Virtual (ground) connection — no region-to-region edge. | ||
| } |
There was a problem hiding this comment.
I don't think we should check if top_region is null because that looks like a fail safe. At this point, top_region should never be null, right?
| if (it_top == region_to_idx.end() || it_bot == region_to_idx.end()) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Another fail safe that shouldn't happen. A connection should never hold a region that is not defined already in the unfolded model.
| const auto it_b = region_to_idx.find(®ions[b]); | ||
| if (it_b == region_to_idx.end()) { | ||
| continue; | ||
| } |
| edges.push_back({u, v}); | ||
| g.offsets[u + 1]++; | ||
| g.offsets[v + 1]++; |
There was a problem hiding this comment.
Actually, even in blackbox, connecting all regions is not correct according to the standard.
| void setBlackbox(bool blackbox); | ||
|
|
||
| bool isBlackbox() const; | ||
|
|
There was a problem hiding this comment.
I don't think that the blackbox state should be user defined. It should be inferred from the current state of the chiplet. From looking at the code, I can see that you set it once and never change it again even if the netlist is added later with routing.
For now let's remove it and assume that the path assertions are in the scope of the blockbox state only.
| #include "objects.h" | ||
| #include "odb/db.h" | ||
| #include "utl/Logger.h" | ||
| #include "yaml-cpp/yaml.h" |
| struct RoutingGraph | ||
| { | ||
| std::vector<uint32_t> offsets; // size num_nodes+1; CSR row pointers | ||
| std::vector<uint32_t> neighbors; // all adjacency lists concatenated | ||
| uint32_t num_nodes = 0; | ||
| }; |
There was a problem hiding this comment.
use boost::compressed_sparse_row_graph
3 participants
Summary
Add path assertion checking to the 3DBlox checker. Path assertions (parsed from
.3dbxfiles) specify must-touch and do-not-touch region constraints. The checker validates these by building a CSR routing graph from the unfolded model and running BFS connectivity queries to detect violations.Key changes:
PathAssertionEntry/PathAssertionstructs in the public header with aregion_instfield resolved at parse timegetPathAssertions()/setPathAssertions()API onThreeDBloxcheckPathAssertionsin the checker using a CSR routing graph + BFSdbChip::setIsBlackbox()/isBlackbox()so blackbox chips get intra-chip clique edges in the routing graphroutingGraph.cpp/hfor O(V+E) CSR graph constructionType of Change
Impact
ThreeDBlox::check()now validates path assertions in addition to existing checks (logical connectivity, floating chips, overlaps, etc.)blackbox_field ondbChip; backward-compatible with schema 128 databasesVerification
./etc/Build.sh).Related Issues
Closes: #9300