grt: replace logger report with logger info and fix asserts in CUGR#9945
grt: replace logger report with logger info and fix asserts in CUGR#9945Divinesoumyadip wants to merge 2 commits intoThe-OpenROAD-Project:masterfrom
Conversation
|
@eder-matheus Check it out when u have time .Thanks again. |
![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 refactors the logging mechanism in CUGR.cpp by replacing logger_->report calls with logger_->info and introducing specific message IDs for various stages and statistics. Additionally, it replaces several assert statements with logger_->error calls to provide more robust error handling and diagnostic messages. A review comment suggests enhancing the error message for the null Steiner tree case by including the net ID for improved debugging.
|
@Divinesoumyadip you have some conflicts in this PR. I see that you're using a very old version of OpenROAD as a base for this PR. Please, make sure to always checkout to the latest master branch before creating new updates. |
Signed-off-by: Divinesoumyadip <soumyacode7@gmail.com>
Signed-off-by: Divinesoumyadip <soumyacode7@gmail.com>
Divinesoumyadip
force-pushed
the
grt/fix-cugr-logger-report-to-info
branch
from
cb8c03a to
1e82a15
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| logger_->info(utl::GRT, | ||
| 1258, | ||
| "{} / {} nets have overflow.", | ||
| netIndices.size(), |
There was a problem hiding this comment.
warning: use of undeclared identifier 'netIndices'; did you mean 'net_indices'? [clang-diagnostic-error]
| netIndices.size(), | |
| net_indices.size(), |
Additional context
src/grt/src/cugr/src/CUGR.cpp:130: 'net_indices' declared here
void CUGR::updateOverflowNets(std::vector<int>& net_indices)
^| maze_route.constructSparsifiedGraph(wire_cost_view, grid); | ||
| maze_route.run(); | ||
| std::shared_ptr<SteinerTreeNode> tree = maze_route.getSteinerTree(); | ||
| for (const int netIndex : netIndices) { |
There was a problem hiding this comment.
warning: use of undeclared identifier 'netIndices'; did you mean 'net_indices'? [clang-diagnostic-error]
| for (const int netIndex : netIndices) { | |
| for (const int netIndex : net_indices) { |
Additional context
src/grt/src/cugr/src/CUGR.cpp:209: 'net_indices' declared here
void CUGR::mazeRoute(std::vector<int>& net_indices)
^| for (const int netIndex : netIndices) { | ||
| GRNet* net = gr_nets_[netIndex].get(); | ||
| MazeRoute mazeRoute(net, grid_graph_.get(), logger_); | ||
| mazeRoute.constructSparsifiedGraph(wireCostView, grid); |
There was a problem hiding this comment.
warning: use of undeclared identifier 'wireCostView'; did you mean 'wire_cost_view'? [clang-diagnostic-error]
| mazeRoute.constructSparsifiedGraph(wireCostView, grid); | |
| mazeRoute.constructSparsifiedGraph(wire_cost_view, grid); |
Additional context
src/grt/src/cugr/src/CUGR.cpp:224: 'wire_cost_view' declared here
GridGraphView<CostT> wire_cost_view;
^| logger_->info(utl::GRT, | ||
| 1265, | ||
| "Wire length (metric): {}.", | ||
| wireLength / grid_graph_->getM2Pitch()); |
There was a problem hiding this comment.
warning: use of undeclared identifier 'wireLength'; did you mean 'wire_length'? [clang-diagnostic-error]
| wireLength / grid_graph_->getM2Pitch()); | |
| wire_length / grid_graph_->getM2Pitch()); |
Additional context
src/grt/src/cugr/src/CUGR.cpp:513: 'wire_length' declared here
uint64_t wire_length = 0;
^| 1265, | ||
| "Wire length (metric): {}.", | ||
| wireLength / grid_graph_->getM2Pitch()); | ||
| logger_->info(utl::GRT, 1266, "Total via count: {}.", viaCount); |
There was a problem hiding this comment.
warning: use of undeclared identifier 'viaCount'; did you mean 'via_count'? [clang-diagnostic-error]
| logger_->info(utl::GRT, 1266, "Total via count: {}.", viaCount); | |
| logger_->info(utl::GRT, 1266, "Total via count: {}.", via_count); |
Additional context
src/grt/src/cugr/src/CUGR.cpp:514: 'via_count' declared here
int via_count = 0;
^| logger_->info(utl::GRT, 1266, "Total via count: {}.", viaCount); | ||
| logger_->info(utl::GRT, 1267, "Total wire overflow: {}.", (int) overflow); | ||
|
|
||
| logger_->info(utl::GRT, 1268, "Min resource: {}.", minResource); |
There was a problem hiding this comment.
warning: use of undeclared identifier 'minResource'; did you mean 'min_resource'? [clang-diagnostic-error]
| logger_->info(utl::GRT, 1268, "Min resource: {}.", minResource); | |
| logger_->info(utl::GRT, 1268, "Min resource: {}.", min_resource); |
Additional context
src/grt/src/cugr/src/CUGR.cpp:546: 'min_resource' declared here
CapacityT min_resource = std::numeric_limits<CapacityT>::max();
^2 participants
CUGR.cpp used
logger_->report()for all routing stage announcements and statistics output, which bypasses the OpenROAD message ID system and prevents log filtering. Additionally, two rawassert()calls would crash silently in release builds with no diagnostic information.This PR replaces all 11
logger_->report()calls withlogger_->info()using GRT message IDs 1258-1269, replaces the 2 raw asserts withlogger_->error()calls (GRT 1270-1271) that include the actual values that caused the failure, and removes the now-unused#include <cassert>.Related Issues:
Follows pattern established in #9870 and #9938.