dft: fix crossplatform inconsistencies

* dft::ScanArchitect: Use a stable sort for scan cells within buckets. Also added a debugPrint to preview the buckets. * This is necessary as otherwise two scan elements with identical bit widths and clock domains's order is undefined and a place for inconsistency between platforms. * dft::ClockDomain::getClockDomainId: Uses the FNV1a hash algorithm which unlike std::hash is not implementation-dependent and will return an identical hash across all platforms. * This is necessary as C++ maps sort not by in the insertion order, but by the key order, e.g., the hash's absolute value. Yep. * dft::GetClockDomainHashFn: Now simply uses `getClockDomainId` for the NoMix mode. * To avoid code duplication. * Updated DFT tets to reflect new sort ordinals. --- DFT tests confirmed returning an identical result on both macOS and Linux on aarch64. Signed-off-by: Mohamed Gaber <[email protected]>
The-OpenROAD-Project · Jan 13, 2025 · 56edc63 · 56edc63
1 parent 2fb3cdf
commit 56edc63
Show file tree

Hide file tree

Showing 9 changed files with 199 additions and 163 deletions.
diff --git a/src/dft/src/architect/ScanArchitect.cpp b/src/dft/src/architect/ScanArchitect.cpp
@@ -63,7 +63,7 @@ bool CompareScanCells(const std::unique_ptr<ScanCell>& lhs,
 
 void SortScanCells(std::vector<std::unique_ptr<ScanCell>>& scan_cells)
 {
-  std::sort(scan_cells.begin(), scan_cells.end(), CompareScanCells);
+  std::stable_sort(scan_cells.begin(), scan_cells.end(), CompareScanCells);
 }
 
 }  // namespace
@@ -72,6 +72,21 @@ ScanCellsBucket::ScanCellsBucket(utl::Logger* logger) : logger_(logger)
 {
 }
 
+namespace {
+void showBuckets(
+    utl::Logger* logger,
+    const std::unordered_map<size_t, std::vector<std::unique_ptr<ScanCell>>>&
+        buckets)
+{
+  for (auto& bucket : buckets) {
+    debugPrint(logger, utl::DFT, "buckets", 2, "Bucket {}", bucket.first);
+    for (auto& scan_cell : bucket.second) {
+      debugPrint(logger, utl::DFT, "buckets", 2, "\t{}", scan_cell->getName());
+    }
+  }
+}
+};  // namespace
+
 void ScanCellsBucket::init(const ScanArchitectConfig& config,
                            std::vector<std::unique_ptr<ScanCell>>& scan_cells)
 {
@@ -85,6 +100,8 @@ void ScanCellsBucket::init(const ScanArchitectConfig& config,
   for (auto& [hash_domain, scan_cells] : buckets_) {
     SortScanCells(scan_cells);
   }
+
+  showBuckets(logger_, buckets_);
 }
 
 std::unordered_map<size_t, uint64_t>

diff --git a/src/dft/src/clock_domain/ClockDomain.cpp b/src/dft/src/clock_domain/ClockDomain.cpp
@@ -32,6 +32,9 @@
 
 #include "ClockDomain.hh"
 
+#include <boost/container_hash/hash.hpp>
+#include <cstdint>
+
 namespace dft {
 
 ClockDomain::ClockDomain(const std::string& clock_name, ClockEdge clock_edge)
@@ -63,10 +66,27 @@ std::string_view ClockDomain::getClockEdgeName() const
   return "Unknown clock edge";
 }
 
+namespace {
+size_t FNV1a(const uint8_t* data, size_t length)
+{
+  size_t hash = 0xcbf29ce484222325;
+  for (size_t i = 0; i < length; i += 1) {
+    hash ^= data[i];
+    hash *= 0x00000100000001b3;
+  }
+  return hash;
+}
+};  // namespace
+
 size_t ClockDomain::getClockDomainId() const
 {
-  return std::hash<std::string_view>{}(clock_name_)
-         ^ std::hash<ClockEdge>{}(clock_edge_);
+  size_t seed = 0;
+  seed = boost::hash_detail::hash_mix(
+      seed + 0x9e3779b9
+      + FNV1a((uint8_t*) clock_name_.data(), clock_name_.length()));
+  seed = boost::hash_detail::hash_mix(
+      seed + 0x9e3779b9 + FNV1a((uint8_t*) &clock_edge_, sizeof(ClockEdge)));
+  return seed;
 }
 
 }  // namespace dft
diff --git a/src/dft/src/clock_domain/ClockDomainHash.cpp b/src/dft/src/clock_domain/ClockDomainHash.cpp
@@ -42,8 +42,7 @@ std::function<size_t(const ClockDomain&)> GetClockDomainHashFn(
     // For NoMix, every clock domain is different
     case ScanArchitectConfig::ClockMixing::NoMix:
       return [](const ClockDomain& clock_domain) {
-        return std::hash<std::string_view>{}(clock_domain.getClockName())
-               ^ std::hash<ClockEdge>{}(clock_domain.getClockEdge());
+        return clock_domain.getClockDomainId();
       };
     case ScanArchitectConfig::ClockMixing::ClockMix:
       return [](const ClockDomain& clock_domain) { return 1; };

diff --git a/src/dft/test/scan_architect_clock_mix_sky130.ok b/src/dft/test/scan_architect_clock_mix_sky130.ok
@@ -9,43 +9,43 @@ Clock domain: Clock Mix
 Scan chain 'chain_0' has 3 cells (3 bits)
 
   ff8_clk2_falling (clock2, falling)
-  ff2_clk2_falling
   ff6_clk2_falling
+  ff4_clk2_falling
 
 Scan chain 'chain_1' has 3 cells (3 bits)
 
-  ff10_clk2_falling (clock2, falling)
-  ff4_clk2_falling
-  ff10_clk2_rising (clock2, rising)
+  ff2_clk2_falling (clock2, falling)
+  ff10_clk2_falling
+  ff8_clk2_rising (clock2, rising)
 
 Scan chain 'chain_2' has 3 cells (3 bits)
 
-  ff8_clk2_rising (clock2, rising)
+  ff6_clk2_rising (clock2, rising)
+  ff4_clk2_rising
   ff2_clk2_rising
-  ff6_clk2_rising
 
 Scan chain 'chain_3' has 3 cells (3 bits)
 
-  ff1_clk1_falling (clock1, falling)
-  ff3_clk1_falling
-  ff4_clk2_rising (clock2, rising)
+  ff9_clk1_falling (clock1, falling)
+  ff7_clk1_falling
+  ff10_clk2_rising (clock2, rising)
 
 Scan chain 'chain_4' has 3 cells (3 bits)
 
-  ff7_clk1_falling (clock1, falling)
-  ff9_clk1_falling
-  ff5_clk1_falling
+  ff5_clk1_falling (clock1, falling)
+  ff3_clk1_falling
+  ff1_clk1_falling
 
 Scan chain 'chain_5' has 3 cells (3 bits)
 
-  ff3_clk1_rising (clock1, rising)
-  ff5_clk1_rising
+  ff9_clk1_rising (clock1, rising)
   ff7_clk1_rising
+  ff5_clk1_rising
 
 Scan chain 'chain_6' has 2 cells (2 bits)
 
-  ff1_clk1_rising (clock1, rising)
-  ff9_clk1_rising
+  ff3_clk1_rising (clock1, rising)
+  ff1_clk1_rising
 
 
 No differences found.

diff --git a/src/dft/test/scan_architect_clock_mix_sky130.vok b/src/dft/test/scan_architect_clock_mix_sky130.vok
@@ -66,91 +66,91 @@ module scan_architect (clock1,
 
  sky130_fd_sc_hd__sdfsbp_1 ff10_clk2_rising (.D(port1),
     .Q(output10),
-    .SCD(output14),
+    .SCD(output17),
     .SCE(scan_enable_0),
     .SET_B(set_b),
     .CLK(clock2));
  sky130_fd_sc_hd__sdfbbn_1 ff1_clk1_falling (.D(port1),
     .Q(output11),
-    .SCD(scan_in_3),
+    .SCD(output13),
     .SCE(scan_enable_0),
     .SET_B(set_b),
     .CLK_N(clock1));
  sky130_fd_sc_hd__sdfsbp_1 ff1_clk1_rising (.D(port1),
     .Q(output1),
-    .SCD(scan_in_6),
+    .SCD(output3),
     .SCE(scan_enable_0),
     .SET_B(set_b),
     .CLK(clock1));
  sky130_fd_sc_hd__sdfbbn_1 ff2_clk2_falling (.D(port1),
     .Q(output12),
-    .SCD(output18),
+    .SCD(scan_in_1),
     .SCE(scan_enable_0),
     .SET_B(set_b),
     .CLK_N(clock2));
  sky130_fd_sc_hd__sdfsbp_1 ff2_clk2_rising (.D(port1),
     .Q(output2),
-    .SCD(output8),
+    .SCD(output4),
     .SCE(scan_enable_0),
     .SET_B(set_b),
     .CLK(clock2));
  sky130_fd_sc_hd__sdfbbn_1 ff3_clk1_falling (.D(port1),
     .Q(output13),
-    .SCD(output11),
+    .SCD(output15),
     .SCE(scan_enable_0),
     .SET_B(set_b),
     .CLK_N(clock1));
  sky130_fd_sc_hd__sdfsbp_1 ff3_clk1_rising (.D(port1),
     .Q(output3),
-    .SCD(scan_in_5),
+    .SCD(scan_in_6),
     .SCE(scan_enable_0),
     .SET_B(set_b),
     .CLK(clock1));
  sky130_fd_sc_hd__sdfbbn_1 ff4_clk2_falling (.D(port1),
     .Q(output14),
-    .SCD(output20),
+    .SCD(output16),
     .SCE(scan_enable_0),
     .SET_B(set_b),
     .CLK_N(clock2));
  sky130_fd_sc_hd__sdfsbp_1 ff4_clk2_rising (.D(port1),
     .Q(output4),
-    .SCD(output13),
+    .SCD(output6),
     .SCE(scan_enable_0),
     .SET_B(set_b),
     .CLK(clock2));
  sky130_fd_sc_hd__sdfbbn_1 ff5_clk1_falling (.D(port1),
     .Q(output15),
-    .SCD(output19),
+    .SCD(scan_in_4),
     .SCE(scan_enable_0),
     .SET_B(set_b),
     .CLK_N(clock1));
  sky130_fd_sc_hd__sdfsbp_1 ff5_clk1_rising (.D(port1),
     .Q(output5),
-    .SCD(output3),
+    .SCD(output7),
     .SCE(scan_enable_0),
     .SET_B(set_b),
     .CLK(clock1));
  sky130_fd_sc_hd__sdfbbn_1 ff6_clk2_falling (.D(port1),
     .Q(output16),
-    .SCD(output12),
+    .SCD(output18),
     .SCE(scan_enable_0),
     .SET_B(set_b),
     .CLK_N(clock2));
  sky130_fd_sc_hd__sdfsbp_1 ff6_clk2_rising (.D(port1),
     .Q(output6),
-    .SCD(output2),
+    .SCD(scan_in_2),
     .SCE(scan_enable_0),
     .SET_B(set_b),
     .CLK(clock2));
  sky130_fd_sc_hd__sdfbbn_1 ff7_clk1_falling (.D(port1),
     .Q(output17),
-    .SCD(scan_in_4),
+    .SCD(output19),
     .SCE(scan_enable_0),
     .SET_B(set_b),
     .CLK_N(clock1));
  sky130_fd_sc_hd__sdfsbp_1 ff7_clk1_rising (.D(port1),
     .Q(output7),
-    .SCD(output5),
+    .SCD(output9),
     .SCE(scan_enable_0),
     .SET_B(set_b),
     .CLK(clock1));
@@ -162,25 +162,25 @@ module scan_architect (clock1,
     .CLK_N(clock2));
  sky130_fd_sc_hd__sdfsbp_1 ff8_clk2_rising (.D(port1),
     .Q(output8),
-    .SCD(scan_in_2),
+    .SCD(output20),
     .SCE(scan_enable_0),
     .SET_B(set_b),
     .CLK(clock2));
  sky130_fd_sc_hd__sdfbbn_1 ff9_clk1_falling (.D(port1),
     .Q(output19),
-    .SCD(output17),
+    .SCD(scan_in_3),
     .SCE(scan_enable_0),
     .SET_B(set_b),
     .CLK_N(clock1));
  sky130_fd_sc_hd__sdfsbp_1 ff9_clk1_rising (.D(port1),
     .Q(output9),
-    .SCD(output1),
+    .SCD(scan_in_5),
     .SCE(scan_enable_0),
     .SET_B(set_b),
     .CLK(clock1));
  sky130_fd_sc_hd__sdfbbn_1 ff10_clk2_falling (.D(port1),
     .Q(output20),
-    .SCD(scan_in_1),
+    .SCD(output12),
     .SCE(scan_enable_0),
     .SET_B(set_b),
     .CLK_N(clock2));

diff --git a/src/dft/test/scan_architect_no_mix_sky130.ok b/src/dft/test/scan_architect_no_mix_sky130.ok
@@ -16,28 +16,28 @@ Scan chain 'chain_0' has 5 cells (5 bits)
 
 Scan chain 'chain_1' has 5 cells (5 bits)
 
+  ff8_clk2_rising (clock2, rising)
+  ff6_clk2_rising
+  ff4_clk2_rising
+  ff2_clk2_rising
+  ff10_clk2_rising
+
+Scan chain 'chain_2' has 5 cells (5 bits)
+
   ff9_clk1_rising (clock1, rising)
   ff7_clk1_rising
   ff5_clk1_rising
   ff3_clk1_rising
   ff1_clk1_rising
 
-Scan chain 'chain_2' has 5 cells (5 bits)
+Scan chain 'chain_3' has 5 cells (5 bits)
 
   ff8_clk2_falling (clock2, falling)
   ff6_clk2_falling
   ff4_clk2_falling
   ff2_clk2_falling
   ff10_clk2_falling
 
-Scan chain 'chain_3' has 5 cells (5 bits)
-
-  ff8_clk2_rising (clock2, rising)
-  ff6_clk2_rising
-  ff4_clk2_rising
-  ff2_clk2_rising
-  ff10_clk2_rising
-
 
 No differences found.
 No differences found.
diff --git a/src/dft/test/scan_architect_no_mix_sky130.vok b/src/dft/test/scan_architect_no_mix_sky130.vok
@@ -150,13 +150,13 @@ module scan_architect (clock1,
     .CLK(clock1));
  sky130_fd_sc_hd__sdfbbn_1 ff8_clk2_falling (.D(port1),
     .Q(output18),
-    .SCD(scan_in_2),
+    .SCD(scan_in_3),
     .SCE(scan_enable_0),
     .SET_B(set_b),
     .CLK_N(clock2));
  sky130_fd_sc_hd__sdfsbp_1 ff8_clk2_rising (.D(port1),
     .Q(output8),
-    .SCD(scan_in_3),
+    .SCD(scan_in_1),
     .SCE(scan_enable_0),
     .SET_B(set_b),
     .CLK(clock2));
@@ -168,7 +168,7 @@ module scan_architect (clock1,
     .CLK_N(clock1));
  sky130_fd_sc_hd__sdfsbp_1 ff9_clk1_rising (.D(port1),
     .Q(output9),
-    .SCD(scan_in_1),
+    .SCD(scan_in_2),
     .SCE(scan_enable_0),
     .SET_B(set_b),
     .CLK(clock1));