-
Notifications
You must be signed in to change notification settings - Fork 759
Update grt, drt, rsz utilities to support RC correlation #7254
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
Changes from 6 commits
405e4d2
6367f72
2ba7694
446cd1f
eb8cbad
3351fe4
5f78034
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,10 +49,10 @@ MakeWireParasitics::MakeWireParasitics(utl::Logger* logger, | |
| { | ||
| } | ||
|
|
||
| void MakeWireParasitics::estimateParasitcs(odb::dbNet* net, | ||
| std::vector<Pin>& pins, | ||
| GRoute& route, | ||
| sta::SpefWriter* spef_writer) | ||
| void MakeWireParasitics::estimateParasitics(odb::dbNet* net, | ||
| std::vector<Pin>& pins, | ||
| GRoute& route, | ||
| sta::SpefWriter* spef_writer) | ||
| { | ||
| debugPrint(logger_, GRT, "est_rc", 1, "net {}", net->getConstName()); | ||
| if (logger_->debugCheck(GRT, "est_rc", 2)) { | ||
|
|
@@ -93,7 +93,7 @@ void MakeWireParasitics::estimateParasitcs(odb::dbNet* net, | |
| parasitics_->deleteParasiticNetworks(sta_net); | ||
| } | ||
|
|
||
| void MakeWireParasitics::estimateParasitcs(odb::dbNet* net, GRoute& route) | ||
| void MakeWireParasitics::estimateParasitics(odb::dbNet* net, GRoute& route) | ||
| { | ||
| debugPrint(logger_, GRT, "est_rc", 1, "net {}", net->getConstName()); | ||
| if (logger_->debugCheck(GRT, "est_rc", 2)) { | ||
|
|
@@ -491,47 +491,31 @@ float MakeWireParasitics::getNetSlack(odb::dbNet* net) | |
| std::vector<int> MakeWireParasitics::routeLayerLengths(odb::dbNet* db_net) const | ||
| { | ||
| NetRouteMap& routes = grouter_->getRoutes(); | ||
| std::vector<int> layer_lengths(grouter_->getMaxRoutingLayer() + 1); | ||
|
|
||
| // dbu wirelength for wires, via count for vias | ||
| std::vector<int> layer_lengths(tech_->getLayerCount()); | ||
|
|
||
| if (!db_net->getSigType().isSupply()) { | ||
| GRoute& route = routes[db_net]; | ||
| std::set<RoutePt> route_pts; | ||
| for (GSegment& segment : route) { | ||
| if (segment.isVia()) { | ||
| route_pts.insert( | ||
| RoutePt(segment.init_x, segment.init_y, segment.init_layer)); | ||
| route_pts.insert( | ||
| RoutePt(segment.final_x, segment.final_y, segment.final_layer)); | ||
| auto& s = segment; | ||
| // Mimic makeRouteParasitics | ||
| int min_layer = min(s.init_layer, s.final_layer); | ||
| odb::dbTechLayer* cut_layer | ||
| = tech_->findRoutingLayer(min_layer)->getUpperLayer(); | ||
| layer_lengths[cut_layer->getNumber()] += 1; | ||
maliberty marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| route_pts.insert(RoutePt(s.init_x, s.init_y, s.init_layer)); | ||
| route_pts.insert(RoutePt(s.final_x, s.final_y, s.final_layer)); | ||
| } else { | ||
| int layer = segment.init_layer; | ||
| layer_lengths[layer] += segment.length(); | ||
| layer_lengths[tech_->findRoutingLayer(layer)->getNumber()] | ||
| += segment.length(); | ||
| route_pts.insert(RoutePt(segment.init_x, segment.init_y, layer)); | ||
| route_pts.insert(RoutePt(segment.final_x, segment.final_y, layer)); | ||
| } | ||
| } | ||
| // Mimic MakeWireParasitics::makeParasiticsToPin functionality. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is the following code no longer needed?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My understanding is the grt guides reach the center of the instance not the individual pins, and this code is adding a small patch of route on top of it to meet the pin. In the process it's adding a redundant via (so we overcount), and I'm not sure the wirelength it adds is beneficial either. If it's true that it doesn't help with the estimation (which can be confirmed empirically) then we should remove the same code from makeParasiticsPin to keep things in sync
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @povik A small clarification, but GRT guides don't reach the center of the instance. They reach the center of the GCell that encloses the pin shape. The distance from the center of the GCell to the pin location might be useful (more than 7 tracks, for example)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the redundant via that is being added?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually the redundant via exists only in
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm afraid I'm not following. My concern is that if estimate RC is creating a redundant via then it is incorrect and should be fixed. Perhaps you and @eder-matheus can review this issue.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We've discussed with @eder-matheus and agreed to put this code back.
There's redundant via in estimate RC but it's a dead end so if STA is considering topology of the parasitic network the extra resistive element is without effect. The length utility doesn't see topology. |
||
| Net* net = grouter_->getNet(db_net); | ||
| for (Pin& pin : net->getPins()) { | ||
| int layer = pin.getConnectionLayer() + 1; | ||
| odb::Point grid_pt = pin.getOnGridPosition(); | ||
| odb::Point pt = pin.getPosition(); | ||
|
|
||
| std::vector<std::pair<odb::Point, odb::Point>> ap_positions; | ||
| bool has_access_points | ||
| = grouter_->findPinAccessPointPositions(pin, ap_positions); | ||
| if (has_access_points) { | ||
| auto ap_position = ap_positions.front(); | ||
| pt = ap_position.first; | ||
| grid_pt = ap_position.second; | ||
| } | ||
|
|
||
| RoutePt grid_route(grid_pt.getX(), grid_pt.getY(), layer); | ||
| auto pt_itr = route_pts.find(grid_route); | ||
| if (pt_itr == route_pts.end()) | ||
| layer--; | ||
| int wire_length_dbu | ||
| = abs(pt.getX() - grid_pt.getX()) + abs(pt.getY() - grid_pt.getY()); | ||
| layer_lengths[layer] += wire_length_dbu; | ||
| } | ||
| } | ||
| return layer_lengths; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| tool net total_wl #pins #vias li1_wl met1_wl met2_wl met3_wl met4_wl met5_wl | ||
| grt: clk 201.6 2 2 0 2 0 2 0 2 144 2 57.6 2 0 | ||
| drt: clk 184.95 2 5 0 5 0.215 5 0.185 5 134.97 5 49.58 5 0 | ||
| grt: net60 14.4 2 2 0 2 0 2 14.4 2 0 2 0 2 0 | ||
| drt: net60 13.39 2 3 0 3 3.77 3 9.62 3 0 3 0 3 0 | ||
| grt: clk 201.6 2 2 0 0 0 144 57.6 0 | ||
| drt: clk 184.95 2 5 0 0.215 0.185 134.97 49.58 0 | ||
| grt: net60 14.4 2 2 0 0 14.4 0 0 0 | ||
| drt: net60 13.39 2 3 0 3.77 9.62 0 0 0 |
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 is this in drt as it appears only use odb APIs?
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.
It makes sense to have
drt::route_layer_lengthsas an analogue togrt::route_layer_lengths. The latter does use grt APIs so it can't be an odb utility. Should I move the drt one into the odb submodule, or just somewhere else insrc/drt/srcwhere it's more decoupled from the router itself?