-
Notifications
You must be signed in to change notification settings - Fork 551
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
dpl: convert to storing markers in odb #6024
dpl: convert to storing markers in odb #6024
Conversation
@gadfort dpl got missed in the conversion to odb markers. |
clang-tidy review says "All clean, LGTM! 👍" |
Signed-off-by: Matt Liberty <[email protected]>
25bc249
to
e3801c0
Compare
clang-tidy review says "All clean, LGTM! 👍" |
double yMin = (failure->y_ + core.yMin()).v / dbUnits; | ||
double xMax = (failure->x_ + failure->width_ + core.xMin()).v / dbUnits; | ||
double yMax = (failure->y_ + failure->height_ + core.yMin()).v / dbUnits; | ||
odb::dbMarker* marker = odb::dbMarker::create(category); |
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.
you need to check if this is a nullptr
If the category has reached it's limit, then this will return a nullptr
src/dpl/src/CheckPlacement.cpp
Outdated
|
||
auto* tool_category = odb::dbMarkerCategory::createOrGet(block_, "DPL"); | ||
try { |
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.
this try
should not be needed anymore
src/dpl/src/CheckPlacement.cpp
Outdated
|
||
auto* tool_category = odb::dbMarkerCategory::createOrGet(block_, "DPL"); |
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.
You might want to use createOrReplace
if I run the check placement command twice and have placement failures at one time, but later I only have in row failures it would appear that both will be reported.
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.
I used
odb::dbMarkerCategory* tool_category
= odb::dbMarkerCategory::createOrGet(getBlock(), "PSM");
as a template. Is this incorrect too?
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.
also in connect.cpp and TritonRoute.cpp
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.
in PSM the net markers are in different categories so that get handled there. It's possible the behavior is not correct in the other places. (in PDN I believe I destroy the old markers first)
src/dpl/src/CheckPlacement.cpp
Outdated
processViolationsPtree(entry, in_rows_failures); | ||
drcArray.push_back(std::make_pair("", entry)); | ||
auto category = odb::dbMarkerCategory::createOrReplace( | ||
tool_category, "In_rows_failures"); |
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.
why not In rows failures
? the placement failures didn't get an underscore.
src/dpl/src/CheckPlacement.cpp
Outdated
auto category = odb::dbMarkerCategory::createOrReplace( | ||
tool_category, "Placement_failures"); | ||
category->setDescription("Cells that DPL failed to place."); | ||
saveViolations(placement_failures, category); |
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.
how is this different from the previous placement failures?
src/dpl/src/CheckPlacement.cpp
Outdated
} catch (std::exception& ex) { | ||
logger_->error( | ||
DPL, 45, "Failed to write JSON report. Exception: {}", ex.what()); | ||
} |
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.
not needed.
Signed-off-by: Matt Liberty <[email protected]>
clang-tidy review says "All clean, LGTM! 👍" |
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.
LGTM, I still think the category names with underscores and no-underscores is a little odd, but that can always be fixed later
Agreed but I just moved this as is. |
No description provided.