Merge pull request #8968 from calewis/resizer_determinism

RSZ: Remove non-determinism and platform-specific behavior

Author: maliberty

src/dbSta/src/dbNetwork.cc

@@ -415,7 +415,7 @@ DbInstanceNetIterator::DbInstanceNetIterator(const Instance* instance,
 
       // Keep track of flat nets that are already represented by a mod_net
       // to avoid returning both.
-      std::set<dbNet*> handled_flat_nets;
+      std::unordered_set<dbNet*> handled_flat_nets;
       for (dbModNet* mod_net : mod_nets) {
         dbNet* flat_net = network_->findRelatedDbNet(mod_net);
         if (flat_net) {
@@ -425,7 +425,7 @@ DbInstanceNetIterator::DbInstanceNetIterator(const Instance* instance,
 
       // Collect dbNets from children dbInsts' pins that are not already
       // handled.
-      std::set<dbNet*> flat_nets_set;
+      std::unordered_set<dbNet*> flat_nets_set;
       for (dbInst* child_inst : module->getInsts()) {
         for (dbITerm* iterm : child_inst->getITerms()) {
           dbNet* flat_net = iterm->getNet();
@@ -447,6 +447,9 @@ DbInstanceNetIterator::DbInstanceNetIterator(const Instance* instance,
         }
       }
       flat_nets_vec_.assign(flat_nets_set.begin(), flat_nets_set.end());
+      // Sort these nets so their order is determinisitc
+      std::ranges::sort(
+          flat_nets_vec_, {}, [](auto const* net1) { return net1->getId(); });
     }
   } else {
     //

src/rsz/include/rsz/Resizer.hh

@@ -6,6 +6,7 @@
 #include <array>
 #include <optional>
 #include <string>
+#include <unordered_set>
 #include <vector>
 
 #include "db_sta/dbNetwork.hh"
@@ -738,7 +739,7 @@ class Resizer : public dbStaState, public dbNetworkObserver
   const MinMax* max_ = MinMax::max();
   LibertyCellSeq buffer_cells_;
   LibertyCell* buffer_lowest_drive_ = nullptr;
-  std::set<LibertyCell*> buffer_fast_sizes_;
+  std::unordered_set<LibertyCell*> buffer_fast_sizes_;
   // Buffer list created by CTS kept here so that we use the
   // exact same buffers when reparing clock nets.
   LibertyCellSeq clk_buffers_;

src/rsz/src/BaseMove.cc

@@ -982,14 +982,26 @@ float BaseMove::computeElmoreSlewFactor(const Pin* output_pin,
 
 LibertyCellSeq BaseMove::getSwappableCells(LibertyCell* base)
 {
-  LibertyCellSeq buffer_sizes;
   if (base->isBuffer()) {
-    for (LibertyCell* buffer : resizer_->buffer_fast_sizes_) {
-      buffer_sizes.push_back(buffer);
-    }
     if (resizer_->buffer_fast_sizes_.count(base) == 0) {
       return LibertyCellSeq();
     }
+
+    LibertyCellSeq buffer_sizes;
+    buffer_sizes.reserve(resizer_->buffer_fast_sizes_.size());
+    for (LibertyCell* buffer : resizer_->buffer_fast_sizes_) {
+      buffer_sizes.push_back(buffer);
+    }
+    // Sort output to ensure deterministic order
+    std::ranges::sort(buffer_sizes,
+                      [](LibertyCell const* c1, LibertyCell const* c2) {
+                        auto area1 = c1->area();
+                        auto area2 = c2->area();
+                        if (area1 != area2) {
+                          return area1 < area2;
+                        }
+                        return c1->id() < c2->id();
+                      });
     return buffer_sizes;
   }
   return resizer_->getSwappableCells(base);

src/rsz/src/Resizer.cc

@@ -2719,8 +2719,19 @@ NetSeq Resizer::resizeWorstSlackNets()
   for (auto pair : net_slack_map_) {
     nets.push_back(pair.first);
   }
-  sort(nets, [this](const Net* net1, const Net* net2) {
-    return resizeNetSlack(net1) < resizeNetSlack(net2);
+  // We are manually breaking ties to enforce stability, so use std::sort since
+  // there is no need to pay the cost for stable sorting here
+  std::ranges::sort(nets, [this](const Net* net1, const Net* net2) {
+    auto slack1 = resizeNetSlack(net1);
+    auto slack2 = resizeNetSlack(net2);
+    if (slack1 != slack2) {
+      return slack1 < slack2;
+    }
+
+    // In the event of a tie use ID to keep the sort stable.
+    auto dbnet1 = db_network_->staToDb(net1);
+    auto dbnet2 = db_network_->staToDb(net2);
+    return dbnet1->getId() < dbnet2->getId();
   });
 
   int nworst_nets = std::ceil(nets.size() * worst_slack_nets_percent_ / 100.0);

src/rsz/src/SwapPinsMove.cc

@@ -11,12 +11,14 @@
 #include <optional>
 #include <sstream>
 #include <string>
+#include <unordered_set>
 #include <utility>
 
 #include "BaseMove.hh"
 #include "odb/db.h"
 #include "sta/ArcDelayCalc.hh"
 #include "sta/Delay.hh"
+#include "sta/FuncExpr.hh"
 #include "sta/Graph.hh"
 #include "sta/Liberty.hh"
 #include "sta/MinMax.hh"
@@ -85,7 +87,7 @@ bool SwapPinsMove::doMove(const Path* drvr_path,
     // We get the driver port and the cell for that port.
     LibertyPort* input_port = network_->libertyPort(in_pin);
     LibertyPort* swap_port = input_port;
-    sta::LibertyPortSet ports;
+    LibertyPortVec ports;
 
     // Skip output to output paths
     if (input_port->direction()->isOutput()) {
@@ -204,7 +206,7 @@ void SwapPinsMove::swapPins(Instance* inst,
 // (depending on agreement).
 void SwapPinsMove::equivCellPins(const LibertyCell* cell,
                                  LibertyPort* input_port,
-                                 sta::LibertyPortSet& ports)
+                                 LibertyPortVec& ports)
 {
   if (cell->hasSequentials() || cell->isIsolationCell()) {
     ports.clear();
@@ -230,51 +232,59 @@ void SwapPinsMove::equivCellPins(const LibertyCell* cell,
     }
   }
 
-  if (outputs >= 1 && inputs >= 2) {
-    sta::LibertyCellPortIterator port_iter2(cell);
-    while (port_iter2.hasNext()) {
-      LibertyPort* candidate_port = port_iter2.next();
-      if (!candidate_port->direction()->isInput()) {
-        continue;
-      }
+  if (outputs < 1 || inputs < 2) {
+    return;
+  }
 
-      sta::LibertyCellPortIterator output_port_iter(cell);
-      std::optional<bool> is_equivalent;
-      // Loop through all the output ports and make sure they are equivalent
-      // under swaps of candidate_port and input_port. For multi-ouput gates
-      // like full adders.
-      while (output_port_iter.hasNext()) {
-        LibertyPort* output_candidate_port = output_port_iter.next();
-        sta::FuncExpr* output_expr = output_candidate_port->function();
-        if (!output_candidate_port->direction()->isOutput()) {
-          continue;
-        }
+  sta::LibertyCellPortIterator port_iter2(cell);
+  std::unordered_set<LibertyPort*> seen_ports;
+  while (port_iter2.hasNext()) {
+    LibertyPort* candidate_port = port_iter2.next();
+    if (!candidate_port->direction()->isInput()) {
+      continue;
+    }
 
-        if (output_expr == nullptr) {
-          continue;
-        }
+    sta::LibertyCellPortIterator output_port_iter(cell);
+    std::optional<bool> is_equivalent;
+    // Loop through all the output ports and make sure they are equivalent
+    // under swaps of candidate_port and input_port. For multi-ouput gates
+    // like full adders.
+    while (output_port_iter.hasNext()) {
+      LibertyPort* output_candidate_port = output_port_iter.next();
+      sta::FuncExpr* output_expr = output_candidate_port->function();
+      if (!output_candidate_port->direction()->isOutput()) {
+        continue;
+      }
 
-        if (input_port == candidate_port) {
-          continue;
-        }
+      if (output_expr == nullptr) {
+        continue;
+      }
 
-        bool is_equivalent_result
-            = isPortEqiv(output_expr, cell, input_port, candidate_port);
+      if (input_port == candidate_port) {
+        continue;
+      }
 
-        if (!is_equivalent.has_value()) {
-          is_equivalent = is_equivalent_result;
-          continue;
-        }
+      bool is_equivalent_result
+          = isPortEqiv(output_expr, cell, input_port, candidate_port);
 
-        is_equivalent = is_equivalent.value() && is_equivalent_result;
+      if (!is_equivalent.has_value()) {
+        is_equivalent = is_equivalent_result;
+        continue;
       }
 
-      // candidate_port is equivalent to input_port under all output ports
-      // of this cell.
-      if (is_equivalent.has_value() && is_equivalent.value()) {
-        ports.insert(candidate_port);
-      }
+      is_equivalent = is_equivalent.value() && is_equivalent_result;
     }
+
+    // candidate_port is equivalent to input_port under all output ports
+    // of this cell.
+    if (is_equivalent.has_value() && is_equivalent.value()
+        && !seen_ports.contains(candidate_port)) {
+      seen_ports.insert(candidate_port);
+      ports.push_back(candidate_port);
+    }
+  }
+  if (!seen_ports.empty()) {  // If we added any ports sort them.
+    std::ranges::sort(ports, {}, [](auto* p1) { return p1->id(); });
   }
 }
 
@@ -293,7 +303,7 @@ void SwapPinsMove::reportSwappablePins()
         if (!port->direction()->isInput()) {
           continue;
         }
-        sta::LibertyPortSet ports;
+        LibertyPortVec ports;
         equivCellPins(cell, port, ports);
         std::ostringstream ostr;
         for (auto port : ports) {
@@ -310,7 +320,7 @@ void SwapPinsMove::reportSwappablePins()
 // where 2 paths go through the same gate (we could end up swapping pins twice)
 void SwapPinsMove::findSwapPinCandidate(LibertyPort* input_port,
                                         LibertyPort* drvr_port,
-                                        const sta::LibertyPortSet& equiv_ports,
+                                        const LibertyPortVec& equiv_ports,
                                         float load_cap,
                                         const DcalcAnalysisPt* dcalc_ap,
                                         LibertyPort** swap_port)
@@ -359,18 +369,21 @@ void SwapPinsMove::findSwapPinCandidate(LibertyPort* input_port,
   }
 
   for (LibertyPort* port : equiv_ports) {
-    if (port_delays.find(port) == port_delays.end()) {
-      // It's possible than an equivalent pin doesn't have
-      // a path to the driver.
+    // Guard Clause:
+    // 1. Check if port delay exists
+    // 2. Check if port is NOT an input
+    // 3. Check if port is equivalent to input_port
+    // 4. Check if port is equivalent to drvr_port
+    if (!port_delays.contains(port) || !port->direction()->isInput()
+        || sta::LibertyPort::equiv(input_port, port)
+        || sta::LibertyPort::equiv(drvr_port, port)) {
       continue;
     }
 
-    if (port->direction()->isInput()
-        && !sta::LibertyPort::equiv(input_port, port)
-        && !sta::LibertyPort::equiv(drvr_port, port)
-        && port_delays[port] < base_delay) {
+    auto port_delay = port_delays[port];
+    if (port_delay < base_delay) {
       *swap_port = port;
-      base_delay = port_delays[port];
+      base_delay = port_delay;
     }
   }
 }

src/rsz/src/SwapPinsMove.hh

@@ -1,7 +1,11 @@
 // SPDX-License-Identifier: BSD-3-Clause
 // Copyright (c) 2025-2025, The OpenROAD Authors
 
+#include <vector>
+
 #include "BaseMove.hh"
+#include "sta/Liberty.hh"
+#include "sta/UnorderedMap.hh"
 
 namespace rsz {
 
@@ -34,23 +38,21 @@ class SwapPinsMove : public BaseMove
   void reportSwappablePins();
 
  private:
+  using LibertyPortVec = std::vector<LibertyPort*>;
   void swapPins(Instance* inst, LibertyPort* port1, LibertyPort* port2);
   void equivCellPins(const LibertyCell* cell,
                      LibertyPort* input_port,
-                     sta::LibertyPortSet& ports);
+                     LibertyPortVec& ports);
   void annotateInputSlews(Instance* inst, const DcalcAnalysisPt* dcalc_ap);
   void findSwapPinCandidate(LibertyPort* input_port,
                             LibertyPort* drvr_port,
-                            const sta::LibertyPortSet& equiv_ports,
+                            const LibertyPortVec& equiv_ports,
                             float load_cap,
                             const DcalcAnalysisPt* dcalc_ap,
                             LibertyPort** swap_port);
   void resetInputSlews();
 
-  Map<Instance*, LibertyPortTuple> swapped_pins_;
-  InstanceSet all_swapped_pin_inst_set_;
-
-  sta::UnorderedMap<LibertyPort*, sta::LibertyPortSet> equiv_pin_map_;
+  sta::UnorderedMap<LibertyPort*, LibertyPortVec> equiv_pin_map_;
 };
 
 }  // namespace rsz

src/rsz/test/BUILD

@@ -36,6 +36,7 @@ TESTS = [
     "liberty_dont_use",
     "pinswap_flat",
     "pinswap_hier",
+    "pin_swap1",
     "rebuffer1",
     "rebuffer1_hier",
     "recover_power_verbose",
@@ -181,6 +182,7 @@ TESTS = [
     "resize5",
     "resize6",
     "resize_slack1",
+    "resize_slack2",
     "resize_slack3",
     "set_dont_touch1",
     "set_dont_use1",
@@ -230,8 +232,6 @@ BIG_TESTS = [
 MANUAL_FOR_BAZEL_TESTS = [
     "repair_tie11_hier",
     "repair_tie12_hier",
-    "resize_slack2",
-    "pin_swap1",
     "liberty_dont_use",
     "pinswap_flat",
     "pinswap_hier",