Resize to target slew removal 1

[openroad-robot]

Decouple resize to target slew from max slew repair to prepare its removal. Rewrite the top-level logic of repairNet(Net*,Pin*,Vertex*) to make it clearer what happens in response to what.

[github-actions]

clang-tidy made some suggestions

[github-actions]

warning: function 'rsz::Resizer::areCellsSwappable' has a definition with different parameter names [readability-inconsistent-declaration-parameter-name]

  bool areCellsSwappable(LibertyCell* existing, LibertyCell* replacement);
       ^

Additional context

src/rsz/src/Resizer.cc:1171: the definition seen here

bool Resizer::areCellsSwappable(LibertyCell* existing, LibertyCell* candidate)
              ^

src/rsz/include/rsz/Resizer.hh:481: differing parameters are named here: ('replacement'), in definition: ('candidate')

  bool areCellsSwappable(LibertyCell* existing, LibertyCell* replacement);
       ^

[github-actions]

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

Suggested change

	} else {
	return a.first < b.first;
	}
	} return a.first < b.first;

[github-actions]

warning: The expression is an uninitialized value. The computed value will also be garbage [clang-analyzer-core.uninitialized.Assign]

        slew_violations++;
        ^

Additional context

src/rsz/src/RepairDesign.cc:363: Assuming 'drivers' is non-null

  if (drivers && !drivers->empty()) {
      ^

src/rsz/src/RepairDesign.cc:363: Left side of '&&' is true

  if (drivers && !drivers->empty()) {
      ^

src/rsz/src/RepairDesign.cc:363: Assuming the condition is true

  if (drivers && !drivers->empty()) {
                 ^

src/rsz/src/RepairDesign.cc:363: Taking true branch

  if (drivers && !drivers->empty()) {
  ^

src/rsz/src/RepairDesign.cc:367: Calling 'RepairDesign::repairNet'

    repairNet(net,
    ^

src/rsz/src/RepairDesign.cc:767: Assuming the condition is true

  if (!db_network_->isSpecial(net)) {
      ^

src/rsz/src/RepairDesign.cc:767: Taking true branch

  if (!db_network_->isSpecial(net)) {
  ^

src/rsz/src/RepairDesign.cc:768: Taking false branch

    debugPrint(logger_,
    ^

src/utl/include/utl/Logger.h:370: expanded from macro 'debugPrint'

  if (logger->debugCheck(tool, group, level)) {     \
  ^

src/rsz/src/RepairDesign.cc:777: Assuming the condition is false

    if (buffer_gain_ != 0.0) {
        ^

src/rsz/src/RepairDesign.cc:777: Taking false branch

    if (buffer_gain_ != 0.0) {
    ^

src/rsz/src/RepairDesign.cc:794: 'check_fanout' is true

    if (check_fanout) {
        ^

src/rsz/src/RepairDesign.cc:794: Taking true branch

    if (check_fanout) {
    ^

src/rsz/src/RepairDesign.cc:797: Assuming the condition is true

      if (max_fanout > 0.0 && fanout_slack < 0.0) {
          ^

src/rsz/src/RepairDesign.cc:797: Left side of '&&' is true

      if (max_fanout > 0.0 && fanout_slack < 0.0) {
          ^

src/rsz/src/RepairDesign.cc:797: Assuming the condition is true

      if (max_fanout > 0.0 && fanout_slack < 0.0) {
                              ^

src/rsz/src/RepairDesign.cc:797: Taking true branch

      if (max_fanout > 0.0 && fanout_slack < 0.0) {
      ^

src/rsz/src/RepairDesign.cc:801: Taking false branch

        debugPrint(logger_, RSZ, "repair_net", 3, "fanout violation");
        ^

src/utl/include/utl/Logger.h:370: expanded from macro 'debugPrint'

  if (logger->debugCheck(tool, group, level)) {     \
  ^

src/rsz/src/RepairDesign.cc:804: Calling 'RepairDesign::makeRegionRepeaters'

        makeRegionRepeaters(region,
        ^

src/rsz/src/RepairDesign.cc:1586: Assuming the condition is true

  if (!region.regions_.empty()) {
      ^

src/rsz/src/RepairDesign.cc:1586: Taking true branch

  if (!region.regions_.empty()) {
  ^

src/rsz/src/RepairDesign.cc:1589: Calling 'RepairDesign::makeRegionRepeaters'

      makeRegionRepeaters(sub,
      ^

src/rsz/src/RepairDesign.cc:1586: Assuming the condition is true

  if (!region.regions_.empty()) {
      ^

src/rsz/src/RepairDesign.cc:1586: Taking true branch

  if (!region.regions_.empty()) {
  ^

src/rsz/src/RepairDesign.cc:1618: Assuming the condition is false

    while (!region.pins_.empty()) {
           ^

src/rsz/src/RepairDesign.cc:1618: Loop condition is false. Execution continues on line 1633

    while (!region.pins_.empty()) {
    ^

src/rsz/src/RepairDesign.cc:1632: Assuming the condition is true

    if (!repeater_loads.empty() && repeater_loads.size() >= max_fanout / 2) {
        ^

src/rsz/src/RepairDesign.cc:1632: Left side of '&&' is true

    if (!repeater_loads.empty() && repeater_loads.size() >= max_fanout / 2) {
        ^

src/rsz/src/RepairDesign.cc:1632: Assuming the condition is true

    if (!repeater_loads.empty() && repeater_loads.size() >= max_fanout / 2) {
                                   ^

src/rsz/src/RepairDesign.cc:1632: Taking true branch

    if (!repeater_loads.empty() && repeater_loads.size() >= max_fanout / 2) {
    ^

src/rsz/src/RepairDesign.cc:1633: Calling 'RepairDesign::makeFanoutRepeater'

      makeFanoutRepeater(repeater_loads,
      ^

src/rsz/src/RepairDesign.cc:1677: 'slew_violations' declared without an initial value

  int repaired_net_count, slew_violations, cap_violations = 0;
                          ^

src/rsz/src/RepairDesign.cc:1688: Passing value via 10th parameter 'slew_violations'

            slew_violations,
            ^

src/rsz/src/RepairDesign.cc:1679: Calling 'RepairDesign::repairNet'

  repairNet(out_net,
  ^

src/rsz/src/RepairDesign.cc:767: Assuming the condition is true

  if (!db_network_->isSpecial(net)) {
      ^

src/rsz/src/RepairDesign.cc:767: Taking true branch

  if (!db_network_->isSpecial(net)) {
  ^

src/rsz/src/RepairDesign.cc:768: Assuming the condition is false

    debugPrint(logger_,
               ^

src/utl/include/utl/Logger.h:370: expanded from macro 'debugPrint'

  if (logger->debugCheck(tool, group, level)) {     \
      ^

src/rsz/src/RepairDesign.cc:768: Taking false branch

    debugPrint(logger_,
    ^

src/utl/include/utl/Logger.h:370: expanded from macro 'debugPrint'

  if (logger->debugCheck(tool, group, level)) {     \
  ^

src/rsz/src/RepairDesign.cc:777: Assuming the condition is false

    if (buffer_gain_ != 0.0) {
        ^

src/rsz/src/RepairDesign.cc:777: Taking false branch

    if (buffer_gain_ != 0.0) {
    ^

src/rsz/src/RepairDesign.cc:794: 'check_fanout' is false

    if (check_fanout) {
        ^

src/rsz/src/RepairDesign.cc:794: Taking false branch

    if (check_fanout) {
    ^

src/rsz/src/RepairDesign.cc:817: Assuming field 'parasitics_src_' is not equal to placement

    if (parasitics_src_ == ParasiticsSrc::placement && resize_drvr) {
        ^

src/rsz/src/RepairDesign.cc:817: Left side of '&&' is false

    if (parasitics_src_ == ParasiticsSrc::placement && resize_drvr) {
                                                    ^

src/rsz/src/RepairDesign.cc:827: 'check_slew' is true

    if (check_slew) {
        ^

src/rsz/src/RepairDesign.cc:827: Taking true branch

    if (check_slew) {
    ^

src/rsz/src/RepairDesign.cc:835: Assuming the condition is false

      if (slew_slack1 < 0.0f) {
          ^

src/rsz/src/RepairDesign.cc:835: Taking false branch

      if (slew_slack1 < 0.0f) {
      ^

src/rsz/src/RepairDesign.cc:863: Assuming the condition is true

      if (!resizer_->isTristateDriver(drvr_pin)) {
          ^

src/rsz/src/RepairDesign.cc:863: Taking true branch

      if (!resizer_->isTristateDriver(drvr_pin)) {
      ^

src/rsz/src/RepairDesign.cc:868: Assuming the condition is true

        if (slew_slack1 < 0.0f) {
            ^

src/rsz/src/RepairDesign.cc:868: Taking true branch

        if (slew_slack1 < 0.0f) {
        ^

src/rsz/src/RepairDesign.cc:869: Assuming the condition is false

          debugPrint(logger_,
                     ^

src/utl/include/utl/Logger.h:370: expanded from macro 'debugPrint'

  if (logger->debugCheck(tool, group, level)) {     \
      ^

src/rsz/src/RepairDesign.cc:869: Taking false branch

          debugPrint(logger_,
          ^

src/utl/include/utl/Logger.h:370: expanded from macro 'debugPrint'

  if (logger->debugCheck(tool, group, level)) {     \
  ^

src/rsz/src/RepairDesign.cc:882: 'repair_cap' is false

          if (!repair_cap) {
               ^

src/rsz/src/RepairDesign.cc:882: Taking true branch

          if (!repair_cap) {
          ^

src/rsz/src/RepairDesign.cc:888: 'slew_violation' is true

      if (slew_violation) {
          ^

src/rsz/src/RepairDesign.cc:888: Taking true branch

      if (slew_violation) {
      ^

src/rsz/src/RepairDesign.cc:889: The expression is an uninitialized value. The computed value will also be garbage

        slew_violations++;
        ^

[github-actions]

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

[github-actions]

clang-tidy made some suggestions

[github-actions]

warning: unused variable 'limit_w_margin' [clang-diagnostic-unused-variable]

          float limit, limit_w_margin, violation = 0;
                       ^

[github-actions]

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

[precisionmoon]

I understand that this is an intermediate change. But how do we ensure that users don't experience QoR degradation until the API change is complete? Would it be better to make a combined PR which actually delivers better QoR?

[povik]

Even if it's an intermediate change it shouldn't be degrading QoR. I have run ORFS on this, these were the failures:

------------------------------
      Failing designs
------------------------------
nangate45 bp_fe (base)
 Flow reached last stage.
 Found 1 metrics failures.
     [ERROR] finish__timing__drv__hold_violation_count fail test: 1433.0 <= 1154.0

nangate45 swerv_wrapper (base)
 Last log file 3_3_place_gp.log
 Found 1 errors in the logs.
     [ERROR GPL-0307] RePlAce divergence detected. Re-run with a smaller max_phi_cof value.

sky130hd aes (base)
 Flow reached last stage.
 Found 1 metrics failures.
     [ERROR] detailedroute__antenna_diodes_count fail test: 16.0 <= 15.0

sky130hd jpeg (base)
 Flow reached last stage.
 Found 1 metrics failures.
     [ERROR] detailedroute__antenna_diodes_count fail test: 71.0 <= 56.0

sky130hd microwatt (base)
 Last log file 5_1_grt.log
 Found 1 errors in the logs.
     [ERROR GRT-0232] Routing congestion too high. Check the congestion heatmap in the GUI.

sky130hs ibex (base)
 Flow reached last stage.
 Found 1 metrics failures.
     [ERROR] detailedroute__antenna_diodes_count fail test: 15.0 <= 14.0

Augusto is looking into the placer divergence. The sky130hd/microwatt congestion doesn't worry me too much as I have seen that design is sensitive to flow changes and prone to congestion.