-
Notifications
You must be signed in to change notification settings - Fork 766
odb: create check_3dblox tcl command #9171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
odb: create check_3dblox tcl command #9171
Conversation
Refactor out check_3dblox command out of read_3dbx so we can run it independently. Also, added tests to assert check_3dblox correctness. Closes: The-OpenROAD-Project#9161 Signed-off-by: Ahmed R. Mohamed <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
The PR successfully exposes the 3D-Blox checker as a new TCL command check_3dblox and refactors the existing read_3dbx command to use it. The changes are logical and well-structured. I've found a potential crash in the new C++ implementation and have suggested a fix. Additionally, I've provided several suggestions to improve the new test script's maintainability and correctness by refactoring repeated code and making assertions more specific.
|
clang-tidy review says "All clean, LGTM! 👍" |
consolidate the test logic into a helper function and tighten the test assertions `==` instead of `>` Signed-off-by: Ahmed R. Mohamed <[email protected]>
|
clang-tidy review says "All clean, LGTM! 👍" |
src/odb/test/check_3dblox.tcl
Outdated
| # Suppress noisy standard loading messages and checker warnings | ||
| suppress_message ODB 227 | ||
| suppress_message ODB 128 | ||
| suppress_message ODB 131 | ||
| suppress_message ODB 133 | ||
| suppress_message STA 1171 | ||
| suppress_message ODB 156 | ||
| suppress_message ODB 151 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason for this? usually if the logger messages are not environment/run dependent (like elapsed time or consumed memory), I prefer if they stay in the log.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No particular reason; just wanted to keep the .ok file simpler. I'll add them back if that is the convention, sure.
These messages are useful for debugging especially in CI runs. Signed-off-by: Ahmed R. Mohamed <[email protected]>
|
clang-tidy review says "All clean, LGTM! 👍" |
Description
This PR introduces the check_3dblox TCL command, exposing the 3D-Blox checker functionality that was previously internal to the
read_3dbx command. This allows users to run physical integrity checks (overlap and floating chip detection) on 3D designs
independently of the loading process.
Key Changes
(including vertical gaps).
Motivation and Context
Providing check_3dblox as a standalone command allows for better interactive debugging and enables verification steps in flows
where the 3D database might be modified after initial loading.
How Has This Been Tested?
Closes: #9161