rsz: exclude non-timing drivers from GPL slack ranking - #11611
precisionmoon merged 4 commits into
Conversation
Signed-off-by: Eder Monteiro <emrmonteiro@precisioninno.com>
Signed-off-by: Eder Monteiro <emrmonteiro@precisioninno.com>
Signed-off-by: Eder Monteiro <emrmonteiro@precisioninno.com>
Signed-off-by: Eder Monteiro <emrmonteiro@precisioninno.com>
![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 updates the resizer to skip loadless drivers and exclude unconstrained nets from slack ranking and weighting by checking for fuzzy infinity. It also adds a C++ unit test to verify this behavior and updates various test expectations accordingly. The review feedback suggests explicitly including sta/Fuzzy.hh in src/rsz/src/Resizer.cc to ensure compilation robustness, as sta::fuzzyInf is now used in that file.
|
@jhkim-pii @gudeh @precisionmoon Here are some results from public CI: QoR on 61 public ORFS designs (ORFS master 5105b0a, compared against latest master):
11 designs fail at least one CI check. Every design whose finish TNS degrades by more than 5% is in that table. Values are in ns, except asap7 (ps). Δ finish TNS is relative to master; positive is better. Degradations (designs failing a CI check)
Improvements (finish TNS 10% or more better)
|
| place_pins \ | ||
| -hor_layers met3 \ | ||
| -ver_layers met2 | ||
| global_placement -skip_initial_place -density uniform -routability_driven -timing_driven |
There was a problem hiding this comment.
This design has no SDC, so no clock and no timing constraints, and TD-GPL was running on fake slacks.
Before removing TD from this design, it diverged at iteration 3220. Not having the SDC turned TD off after the first iteration, and routability couldn't converge later.
|
I see some good results on private designs as well, with only one degradation on TNS. I think this is good to merge |
|
@dsengupta0628 is it expected that "Loadless drivers (no fanout) have no required time, so their slack can be |
This is a self contradictory line. No required time means it is unconstrained path and so “slack” at such a point has no meaning. It shouldn’t show up in any timing reports and optimizations should not worry about fixing such endpoints (except for drc vios) as there is literally no slack. But if a slack exists on such a driver it means it must have had required time (slack=required-arrival), and on loadless driver it may have come from a set_output_delay annotation (intentional or accidental) on the endpoint- in which case that constraint must be honored. And it becomes a virtual endpoint. so loadless driver with no required time but valid slack is incorrect. Either they are both valid or both infinite (no required and no slack) |
|
If it is a truly loadless driver, I think it should be removed. |
6 participants
Summary
Timing-driven global placement weights nets by the slack of their driver pins,
collected in
Resizer::findResizeSlacks1(). Two kinds of drivers producedmisleading slacks there:
worse than any real endpoint. In GPL: Strange behavior when enabling repair_timing during timing-driven #10751, a loadless input reported -0.270 ns
while the design WNS was -0.028 ns, so
GPL-0106printed -0.270 ns and the netweighting was anchored to that value.
still counted toward
-timing_driven_nets_percentage.This PR skips loadless drivers and excludes nets with infinite slack from the
ranking.
GPL-0106now reports the worst slack of a real timing path, and thenets percentage applies to constrained nets only.
Type of Change
Impact
loadless or unconstrained drivers. Other designs are bit-identical; many
public designs in CI show exactly zero change.
the percentage is now taken over constrained nets only.
Verification
bazel build //:openroad).Related Issues
Fixes #10751