Avoid testing AVX2 codepaths when AVX2 isn't supported (!!!!!!!!)

pitrou · Jul 13, 2023 · 0dd2736 · 0dd2736
1 parent c55eca4
commit 0dd2736
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 10 deletions.
diff --git a/cpp/src/arrow/acero/bloom_filter_test.cc b/cpp/src/arrow/acero/bloom_filter_test.cc
@@ -34,6 +34,7 @@ namespace arrow {
 
 using compute::Hashing32;
 using compute::Hashing64;
+using internal::CpuInfo;
 
 namespace acero {
 
@@ -171,8 +172,12 @@ void TestBloomSmallHashHelper(int64_t num_input_hashes, const T* input_hashes,
 // Output FPR and build and probe cost.
 //
 void TestBloomSmall(BloomFilterBuildStrategy strategy, int64_t num_build,
-                    int num_build_copies, bool use_simd, bool enable_prefetch) {
-  int64_t hardware_flags = use_simd ? ::arrow::internal::CpuInfo::AVX2 : 0;
+                    int num_build_copies, bool use_avx2, bool enable_prefetch) {
+  int64_t hardware_flags = use_avx2 ? CpuInfo::AVX2 : 0;
+  if (hardware_flags && !CpuInfo::GetInstance()->IsSupported(hardware_flags)) {
+    // What else?
+    return;
+  }
 
   // Generate input keys
   //
@@ -324,9 +329,9 @@ void TestBloomLargeHashHelper(int64_t hardware_flags, int64_t block,
 // Test with larger size Bloom filters (use large prime with arithmetic
 // sequence modulo 2^64).
 //
-void TestBloomLarge(BloomFilterBuildStrategy strategy, int64_t num_build, bool use_simd,
+void TestBloomLarge(BloomFilterBuildStrategy strategy, int64_t num_build, bool use_avx2,
                     bool enable_prefetch) {
-  int64_t hardware_flags = use_simd ? ::arrow::internal::CpuInfo::AVX2 : 0;
+  int64_t hardware_flags = use_avx2 ? ::arrow::internal::CpuInfo::AVX2 : 0;
 
   // Largest 63-bit prime
   constexpr uint64_t prime = 0x7FFFFFFFFFFFFFE7ULL;

diff --git a/cpp/src/arrow/compute/key_hash_test.cc b/cpp/src/arrow/compute/key_hash_test.cc
@@ -30,6 +30,7 @@
 namespace arrow {
 
 using internal::checked_pointer_cast;
+using internal::CpuInfo;
 
 namespace compute {
 
@@ -140,14 +141,19 @@ class TestVectorHash {
     hashes_simd32.resize(num_rows);
     hashes_simd64.resize(num_rows);
 
-    int64_t hardware_flags_scalar = 0LL;
-    int64_t hardware_flags_simd = ::arrow::internal::CpuInfo::AVX2;
+    const int64_t hardware_flags_scalar = 0LL;
+    const int64_t hardware_flags_simd = CpuInfo::AVX2;
+    const bool simd_supported = CpuInfo::GetInstance()->IsSupported(hardware_flags_simd);
 
     constexpr int mini_batch_size = 1024;
     std::vector<uint32_t> temp_buffer;
     temp_buffer.resize(mini_batch_size * 4);
 
     for (bool use_simd : {false, true}) {
+      if (use_simd && !simd_supported) {
+        // XXX what else?
+        continue;
+      }
       if (use_32bit_hash) {
         if (!use_varlen_input) {
           Hashing32::HashFixed(use_simd ? hardware_flags_simd : hardware_flags_scalar,
@@ -183,15 +189,19 @@ class TestVectorHash {
     if (use_32bit_hash) {
       for (int i = 0; i < num_rows; ++i) {
         hashes_scalar64[i] = hashes_scalar32[i];
-        hashes_simd64[i] = hashes_simd32[i];
+        if (simd_supported) {
+          hashes_simd64[i] = hashes_simd32[i];
+        }
       }
     }
 
     // Verify that both scalar and AVX2 implementations give the same hashes
     //
-    for (int i = 0; i < num_rows; ++i) {
-      ASSERT_EQ(hashes_scalar64[i], hashes_simd64[i])
-          << "scalar and simd approaches yielded different hashes";
+    if (simd_supported) {
+      for (int i = 0; i < num_rows; ++i) {
+        ASSERT_EQ(hashes_scalar64[i], hashes_simd64[i])
+            << "scalar and simd approaches yielded different hashes";
+      }
     }
 
     // Verify that the same key appearing multiple times generates the same hash

diff --git a/cpp/src/arrow/util/cpu_info.cc b/cpp/src/arrow/util/cpu_info.cc
@@ -285,6 +285,8 @@ void OsRetrieveCpuInfo(int64_t* hardware_flags, CpuInfo::Vendor* vendor,
   for (const auto& feature : features) {
     auto v = IntegerSysCtlByName(feature.name);
     if (v.value_or(0)) {
+      ARROW_LOG(WARN) << "** macOS: detected feature: " << feature.name
+                      << " (value: " << v.value_or(0) << ")";
       *hardware_flags |= feature.flag;
     }
   }