rsz: Revamp rebuffering #6637

openroad-ci · 2025-02-04T18:09:46Z

Improve the rebuffering code:

replace "slack penalization" with separate timing-oriented and area-oriented passes over the buffer tree; this should allow better area recovery on non-critical sections of the buffer tree, especially if the tree is large
make slight adjustments to the delay model
restructure some of the code

github-actions

clang-tidy made some suggestions

github-actions · 2025-02-04T18:14:22Z

src/rsz/src/Rebuffer.cc

+       });
+
+  if (Z.empty())
+    return;


warning: statement should be inside braces [google-readability-braces-around-statements]

Suggested change

return;

if (Z.empty()) {

return;

}

github-actions · 2025-02-04T18:14:22Z

src/rsz/src/Rebuffer.cc

+       });
+
+  if (Z.empty())
+    return;


warning: statement should be inside braces [google-readability-braces-around-statements]

Suggested change

return;

if (Z.empty()) {

return;

}

github-actions · 2025-02-04T18:14:22Z

src/rsz/src/Rebuffer.cc

+}
+
+// Find initial timing-optimized rebuffering choice
+BufferedNetPtr RepairSetup::rebufferForTiming(BufferedNetPtr bnet)


warning: the parameter 'bnet' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]

Suggested change

BufferedNetPtr RepairSetup::rebufferForTiming(BufferedNetPtr bnet)

BufferedNetPtr RepairSetup::rebufferForTiming(const BufferedNetPtr& bnet)

src/rsz/src/RepairSetup.hh:210:

- BufferedNetPtr rebufferForTiming(BufferedNetPtr bnet); + BufferedNetPtr rebufferForTiming(const BufferedNetPtr& bnet);

github-actions · 2025-02-04T18:14:22Z

src/rsz/src/Rebuffer.cc

+  using BnetPtr = BufferedNetPtr;
+
+  const BnetSeq Z = visitTree(
+      [this](auto& recurse, int level, BnetPtr bnet) -> BnetSeq {


warning: the parameter 'bnet' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]

[this](auto& recurse, int level, BnetPtr bnet) -> BnetSeq { ^

github-actions · 2025-02-04T18:14:22Z

src/rsz/src/Rebuffer.cc

+          case BnetType::wire: {
+            BnetSeq Z = recurse(bnet->ref());
+            for (BnetPtr& z : Z)
+              z = addWire(z, bnet->location(), bnet->layer(), level);


warning: statement should be inside braces [google-readability-braces-around-statements]

Suggested change

z = addWire(z, bnet->location(), bnet->layer(), level);

for (BnetPtr& z : Z) {

z = addWire(z, bnet->location(), bnet->layer(), level);

}

github-actions · 2025-02-04T18:14:22Z

src/rsz/src/Rebuffer.cc

+
+  // spread down arrival delay
+  visitTree(
+      [](auto& recurse, int level, BufferedNetPtr bnet, Delay arrival) -> int {


warning: the parameter 'bnet' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]

[](auto& recurse, int level, BufferedNetPtr bnet, Delay arrival) -> int { ^

github-actions · 2025-02-04T18:14:22Z

src/rsz/src/Rebuffer.cc

+      drvr_delay);
+
+  BnetSeq Z = visitTree(
+      [&](auto& recurse, int level, BufferedNetPtr bnet) -> BufferedNetSeq {


warning: the parameter 'bnet' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]

[&](auto& recurse, int level, BufferedNetPtr bnet) -> BufferedNetSeq { ^

github-actions · 2025-02-04T18:14:23Z

src/rsz/src/Rebuffer.cc

+            assert(bnet->ref()->type == BnetType::wire);
+            BnetSeq Z = recurse(bnet->ref()->ref());
+            for (BnetPtr& z : Z)
+              z = addWire(z, bnet->location(), bnet->ref()->layer(), level);


warning: statement should be inside braces [google-readability-braces-around-statements]

Suggested change

z = addWire(z, bnet->location(), bnet->ref()->layer(), level);

for (BnetPtr& z : Z) {

z = addWire(z, bnet->location(), bnet->ref()->layer(), level);

}

github-actions · 2025-02-04T18:14:23Z

src/rsz/src/Rebuffer.cc

+          case BnetType::wire: {
+            BnetSeq Z = recurse(bnet->ref());
+            for (BnetPtr& z : Z)
+              z = addWire(z, bnet->location(), bnet->layer(), level);


warning: statement should be inside braces [google-readability-braces-around-statements]

Suggested change

z = addWire(z, bnet->location(), bnet->layer(), level);

for (BnetPtr& z : Z) {

z = addWire(z, bnet->location(), bnet->layer(), level);

}

github-actions · 2025-02-04T18:14:23Z

src/rsz/src/Rebuffer.cc

+  } else {
+    debugPrint(logger_, RSZ, "rebuffer", 2, "no available option");
+    return nullptr;
+  }


warning: do not use 'else' after 'return' [readability-else-after-return]

Suggested change

}

} if (best_slack_option) {

debugPrint(logger_,

RSZ,

"rebuffer",

2,

"best option {} (closest to meeting timing)",

best_slack_index);

return best_slack_option;

} else {

debugPrint(logger_, RSZ, "rebuffer", 2, "no available option");

return nullptr;

}

Buffered net constructor already sets the appropriate capacitance value for each bnet type. Signed-off-by: Martin Povišer <[email protected]>

Signed-off-by: Martin Povišer <[email protected]>

github-actions

clang-tidy made some suggestions

github-actions · 2025-02-05T14:04:40Z

src/rsz/src/Resizer.cc

+    while (pin_iter->hasNext()) {
+      const Pin *pin = pin_iter->next();
+      const LibertyPort *port;
+      if ((port = network_->libertyPort(pin)) &&


warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]

if ((port = network_->libertyPort(pin)) && ^

Additional context

src/rsz/src/Resizer.cc:303: if it should be an assignment, move it out of the 'if' condition

if ((port = network_->libertyPort(pin)) && ^

src/rsz/src/Resizer.cc:303: if it is meant to be an equality check, change '=' to '=='

if ((port = network_->libertyPort(pin)) && ^

github-actions · 2025-02-05T14:04:40Z

src/rsz/src/Resizer.cc

+          insts.push_back(inst);
+          const Pin *out_pin = network_->findPin(inst, out);
+          if (out_pin)
+            queue.push_back(network_->net(out_pin));


warning: statement should be inside braces [google-readability-braces-around-statements]

Suggested change

queue.push_back(network_->net(out_pin));

if (out_pin) {

queue.push_back(network_->net(out_pin));

}

Signed-off-by: Martin Povišer <[email protected]>

github-actions · 2025-02-06T22:43:54Z

clang-tidy review says "All clean, LGTM! 👍"

Signed-off-by: Martin Povišer <[email protected]>

github-actions · 2025-02-06T23:01:57Z

clang-tidy review says "All clean, LGTM! 👍"

Signed-off-by: Martin Povišer <[email protected]>

github-actions · 2025-02-07T10:24:43Z

clang-tidy review says "All clean, LGTM! 👍"

povik · 2025-02-07T10:30:18Z

Overall the effect of this change is a slight improvement in timing QoR: https://dashboard.precisioninno.com/compare?sourceAType=Commit&sourceBType=Commit&sourceBName=80fedddbbc3e57d381ef2685597027e220f23586&sourceBID=2&sourceAName=ae2e846418d75a113415ef9bf2d88999226a8846&sourceAID=5099

On public designs, the metric failures are:

gf180 ibex (base)
 Flow reached last stage.
 Found 2 metrics failures.
     [ERROR] globalroute__antenna_diodes_count fail test: 2.0 <= 0.0
     [ERROR] detailedroute__antenna_diodes_count fail test: 8.0 <= 5.0

gf180 riscv32i (base)
 Flow reached last stage.
 Found 1 metrics failures.
     [ERROR] globalroute__antenna_diodes_count fail test: 1.0 <= 0.0

sky130hd riscv32i (base)
 Flow reached last stage.
 Found 2 metrics failures.
     [ERROR] globalroute__antenna_diodes_count fail test: 5.0 <= 4.0
     [ERROR] finish__timing__drv__setup_violation_count fail test: 438.0 <= 376.0

I've started secure CI under secure-rsz-rebuffer-revamp

rovinski · 2025-02-09T19:11:58Z

src/rsz/test/clone_flat.ok

-worst slack -0.15
-No differences found.
+worst slack -0.13
+Differences found at line 312.


No differences

maliberty · 2025-02-12T17:20:40Z

@povik you have some conflicts to resolve
@precisionmoon please review

openroad-ci assigned povik Feb 4, 2025

github-actions bot reviewed Feb 4, 2025

View reviewed changes

povik added 9 commits February 5, 2025 14:44

rsz: Remove redundant bnet cap setting

ead47fd

Buffered net constructor already sets the appropriate capacitance value for each bnet type. Signed-off-by: Martin Povišer <[email protected]>

rsz: Split addWireAndBuffer -> addWire, addBuffers

2800a4a

Signed-off-by: Martin Povišer <[email protected]>

rsz: Factor out cap vs slack pruning

2a0d612

Signed-off-by: Martin Povišer <[email protected]>

rsz: Use accurate slack at driver pin

ffe424e

Signed-off-by: Martin Povišer <[email protected]>

rsz: Add area recovery on buffering options

46cb60e

Signed-off-by: Martin Povišer <[email protected]>

rsz: Add unbuffer net

4bcef56

Signed-off-by: Martin Povišer <[email protected]>

rsz: Adjust relaxation

fec891e

Signed-off-by: Martin Povišer <[email protected]>

rsz: Optimize for # of buffers, not area

79320a8

Signed-off-by: Martin Povišer <[email protected]>

rsz: Appease tidy

b175f20

Signed-off-by: Martin Povišer <[email protected]>

openroad-ci force-pushed the rsz-rebuffer-revamp branch from 175660a to b175f20 Compare February 5, 2025 13:57

github-actions bot reviewed Feb 5, 2025

View reviewed changes

povik added 4 commits February 5, 2025 16:27

rsz: Tidy unbufferNet

6397789

Signed-off-by: Martin Povišer <[email protected]>

rsz: Fix for timing-unconstrained paths

c776991

Signed-off-by: Martin Povišer <[email protected]>

rsz: Fix rebuffering on top-level port

af7e13a

Signed-off-by: Martin Povišer <[email protected]>

rsz: Update tests

a5da1d5

Signed-off-by: Martin Povišer <[email protected]>

rsz: Format unbufferNet

66c82e7

Signed-off-by: Martin Povišer <[email protected]>

rsz: Fix warning

2384f48

Signed-off-by: Martin Povišer <[email protected]>

povik requested a review from maliberty February 7, 2025 10:30

rovinski reviewed Feb 9, 2025

View reviewed changes

maliberty requested review from precisionmoon and removed request for maliberty February 12, 2025 17:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rsz: Revamp rebuffering #6637

rsz: Revamp rebuffering #6637

openroad-ci commented Feb 4, 2025

github-actions bot left a comment

github-actions bot Feb 4, 2025

github-actions bot Feb 4, 2025

github-actions bot Feb 4, 2025

github-actions bot Feb 4, 2025

github-actions bot Feb 4, 2025

github-actions bot Feb 4, 2025

github-actions bot Feb 4, 2025

github-actions bot Feb 4, 2025

github-actions bot Feb 4, 2025

github-actions bot Feb 4, 2025

github-actions bot left a comment

github-actions bot Feb 5, 2025

github-actions bot Feb 5, 2025

github-actions bot commented Feb 6, 2025

github-actions bot commented Feb 6, 2025

github-actions bot commented Feb 7, 2025

povik commented Feb 7, 2025

rovinski Feb 9, 2025

maliberty commented Feb 12, 2025

-    return;
+  if (Z.empty()) {
+    return;
+}

	BufferedNetPtr RepairSetup::rebufferForTiming(BufferedNetPtr bnet)
	BufferedNetPtr RepairSetup::rebufferForTiming(const BufferedNetPtr& bnet)

rsz: Revamp rebuffering #6637

Are you sure you want to change the base?

rsz: Revamp rebuffering #6637

Conversation

openroad-ci commented Feb 4, 2025

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Feb 4, 2025

Choose a reason for hiding this comment

github-actions bot Feb 4, 2025

Choose a reason for hiding this comment

github-actions bot Feb 4, 2025

Choose a reason for hiding this comment

github-actions bot Feb 4, 2025

Choose a reason for hiding this comment

github-actions bot Feb 4, 2025

Choose a reason for hiding this comment

github-actions bot Feb 4, 2025

Choose a reason for hiding this comment

github-actions bot Feb 4, 2025

Choose a reason for hiding this comment

github-actions bot Feb 4, 2025

Choose a reason for hiding this comment

github-actions bot Feb 4, 2025

Choose a reason for hiding this comment

github-actions bot Feb 4, 2025

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Feb 5, 2025

Choose a reason for hiding this comment

github-actions bot Feb 5, 2025

Choose a reason for hiding this comment

github-actions bot commented Feb 6, 2025

github-actions bot commented Feb 6, 2025

github-actions bot commented Feb 7, 2025

povik commented Feb 7, 2025

rovinski Feb 9, 2025

Choose a reason for hiding this comment

maliberty commented Feb 12, 2025