From 0dd273656d282b0db2ff35a69f6a4d113925424c Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Thu, 13 Jul 2023 17:54:01 +0200 Subject: [PATCH] Avoid testing AVX2 codepaths when AVX2 isn't supported (!!!!!!!!) --- cpp/src/arrow/acero/bloom_filter_test.cc | 13 +++++++++---- cpp/src/arrow/compute/key_hash_test.cc | 22 ++++++++++++++++------ cpp/src/arrow/util/cpu_info.cc | 2 ++ 3 files changed, 27 insertions(+), 10 deletions(-) diff --git a/cpp/src/arrow/acero/bloom_filter_test.cc b/cpp/src/arrow/acero/bloom_filter_test.cc index de433ac68c11a..0304b559a6045 100644 --- 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 index d10645391b413..f7c1834030c55 100644 --- 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 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 index 7c2e9fa921246..51b2b128b3d88 100644 --- 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; } }