-
Notifications
You must be signed in to change notification settings - Fork 503
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
ant: return CAR and CSR violations to repair_antennas #5242
ant: return CAR and CSR violations to repair_antennas #5242
Conversation
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
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.
Minor cleanup before merging
src/ant/src/AntennaChecker.cc
Outdated
if (layer->getRoutingLevel() != 0) { | ||
auto psr_violation | ||
= checkPSR(layer, node_info, verbose, report, report_file); | ||
bool csr_violation | ||
= checkCSR(layer, node_info, verbose, report, report_file); | ||
node_has_violation | ||
= node_has_violation || psr_violation.first || csr_violation; | ||
} |
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.
Does check* have a side-effect or should we skip this if node_has_violation was already set by PAR/CAR?
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.
We should not skip because a net can have both PAR/CAR and PSR/CSR. Skipping it will give us the wrong number of violations and make the report incorrect.
src/ant/src/AntennaChecker.cc
Outdated
bool node_has_violation = false; | ||
auto par_violation = checkPAR(layer, node_info, verbose, report, report_file); | ||
bool car_violation = checkCAR(layer, node_info, verbose, report, report_file); | ||
node_has_violation = par_violation.first || car_violation; |
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.
There's no need to define node_has_violation and then set it afterwards, just combine 749 & 752
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
clang-tidy review says "All clean, LGTM! 👍" |
Fixes The-OpenROAD-Project/OpenROAD-flow-scripts#1981
This is an initial implementation, as mentioned in a comment in the code. Using this naive approach, we can reduce the number of nets with violations from 81 to 55 in ihp-sg13g2/riscv32i.