This is an automated email from the ASF dual-hosted git repository.
apitrou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new 60f29e951a GH-44249: [C++] Unify simd header includings (#44250)
60f29e951a is described below
commit 60f29e951aa4280a8cda22c536395a19796262c3
Author: Rossi Sun <[email protected]>
AuthorDate: Wed Oct 2 16:51:33 2024 +0800
GH-44249: [C++] Unify simd header includings (#44250)
### Rationale for this change
#44249
### What changes are included in this PR?
Change all the system header inclusions to `simd.h`.
Also removed some duplicated defines.
### Are these changes tested?
Existing tests should be good.
### Are there any user-facing changes?
None.
* GitHub Issue: #44249
Authored-by: Ruoxi Sun <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
---
cpp/build-support/cpplint.py | 6 ++++-
cpp/src/arrow/acero/bloom_filter.cc | 6 +++--
cpp/src/arrow/acero/bloom_filter.h | 5 +---
cpp/src/arrow/acero/bloom_filter_avx2.cc | 3 +--
cpp/src/arrow/compute/key_hash_internal.h | 5 +---
cpp/src/arrow/compute/key_hash_internal_avx2.cc | 3 +--
cpp/src/arrow/compute/key_map_internal_avx2.cc | 3 +--
cpp/src/arrow/compute/row/encode_internal_avx2.cc | 3 +--
cpp/src/arrow/compute/util.h | 9 +------
cpp/src/arrow/compute/util_avx2.cc | 2 +-
cpp/src/arrow/util/bit_util.h | 1 -
cpp/src/arrow/util/macros.h | 3 ---
cpp/src/arrow/util/prefetch.h | 31 +++++++++++++++++++++++
13 files changed, 48 insertions(+), 32 deletions(-)
diff --git a/cpp/build-support/cpplint.py b/cpp/build-support/cpplint.py
index 1bceed9a67..dc3d47ba8b 100755
--- a/cpp/build-support/cpplint.py
+++ b/cpp/build-support/cpplint.py
@@ -699,7 +699,11 @@ _C_HEADERS = frozenset([
# Hardware specific headers
'arm_neon.h',
'emmintrin.h',
- 'xmmintin.h',
+ 'immintrin.h',
+ 'intrin.h',
+ 'nmmintrin.h',
+ 'x86intrin.h',
+ 'xmmintrin.h',
])
# Folders of C libraries so commonly used in C++,
diff --git a/cpp/src/arrow/acero/bloom_filter.cc
b/cpp/src/arrow/acero/bloom_filter.cc
index db39ad1a83..47263387e5 100644
--- a/cpp/src/arrow/acero/bloom_filter.cc
+++ b/cpp/src/arrow/acero/bloom_filter.cc
@@ -16,11 +16,13 @@
// under the License.
#include "arrow/acero/bloom_filter.h"
+
#include <random>
-#include "arrow/acero/util.h" // PREFETCH
+
#include "arrow/util/bit_util.h" // Log2
#include "arrow/util/bitmap_ops.h" // CountSetBits
#include "arrow/util/config.h"
+#include "arrow/util/prefetch.h" // PREFETCH
namespace arrow {
namespace acero {
@@ -152,7 +154,7 @@ void BlockedBloomFilter::FindImp(int64_t num_rows, const T*
hashes,
if (enable_prefetch && UsePrefetch()) {
constexpr int kPrefetchIterations = 16;
for (int64_t i = 0; i < num_rows - kPrefetchIterations; ++i) {
- PREFETCH(blocks_ + block_id(hashes[i + kPrefetchIterations]));
+ ARROW_PREFETCH(blocks_ + block_id(hashes[i + kPrefetchIterations]));
uint64_t result = Find(hashes[i]) ? 1ULL : 0ULL;
bits |= result << (i & 63);
if ((i & 63) == 63) {
diff --git a/cpp/src/arrow/acero/bloom_filter.h
b/cpp/src/arrow/acero/bloom_filter.h
index 530beaea64..8f9fe171ba 100644
--- a/cpp/src/arrow/acero/bloom_filter.h
+++ b/cpp/src/arrow/acero/bloom_filter.h
@@ -17,10 +17,6 @@
#pragma once
-#if defined(ARROW_HAVE_RUNTIME_AVX2)
-# include <immintrin.h>
-#endif
-
#include <atomic>
#include <cstdint>
#include <memory>
@@ -30,6 +26,7 @@
#include "arrow/memory_pool.h"
#include "arrow/result.h"
#include "arrow/status.h"
+#include "arrow/util/simd.h"
namespace arrow {
namespace acero {
diff --git a/cpp/src/arrow/acero/bloom_filter_avx2.cc
b/cpp/src/arrow/acero/bloom_filter_avx2.cc
index 5816bb4fc0..448b80fdb6 100644
--- a/cpp/src/arrow/acero/bloom_filter_avx2.cc
+++ b/cpp/src/arrow/acero/bloom_filter_avx2.cc
@@ -15,10 +15,9 @@
// specific language governing permissions and limitations
// under the License.
-#include <immintrin.h>
-
#include "arrow/acero/bloom_filter.h"
#include "arrow/util/bit_util.h"
+#include "arrow/util/simd.h"
namespace arrow {
namespace acero {
diff --git a/cpp/src/arrow/compute/key_hash_internal.h
b/cpp/src/arrow/compute/key_hash_internal.h
index 582cf28732..769f3b2145 100644
--- a/cpp/src/arrow/compute/key_hash_internal.h
+++ b/cpp/src/arrow/compute/key_hash_internal.h
@@ -17,14 +17,11 @@
#pragma once
-#if defined(ARROW_HAVE_RUNTIME_AVX2)
-# include <immintrin.h>
-#endif
-
#include <cstdint>
#include "arrow/compute/light_array_internal.h"
#include "arrow/compute/util.h"
+#include "arrow/util/simd.h"
namespace arrow {
namespace compute {
diff --git a/cpp/src/arrow/compute/key_hash_internal_avx2.cc
b/cpp/src/arrow/compute/key_hash_internal_avx2.cc
index 4def87bd7a..5a4366d6b2 100644
--- a/cpp/src/arrow/compute/key_hash_internal_avx2.cc
+++ b/cpp/src/arrow/compute/key_hash_internal_avx2.cc
@@ -15,10 +15,9 @@
// specific language governing permissions and limitations
// under the License.
-#include <immintrin.h>
-
#include "arrow/compute/key_hash_internal.h"
#include "arrow/util/bit_util.h"
+#include "arrow/util/simd.h"
namespace arrow {
namespace compute {
diff --git a/cpp/src/arrow/compute/key_map_internal_avx2.cc
b/cpp/src/arrow/compute/key_map_internal_avx2.cc
index 8c98166f49..1a16603a0f 100644
--- a/cpp/src/arrow/compute/key_map_internal_avx2.cc
+++ b/cpp/src/arrow/compute/key_map_internal_avx2.cc
@@ -15,10 +15,9 @@
// specific language governing permissions and limitations
// under the License.
-#include <immintrin.h>
-
#include "arrow/compute/key_map_internal.h"
#include "arrow/util/logging.h"
+#include "arrow/util/simd.h"
namespace arrow {
namespace compute {
diff --git a/cpp/src/arrow/compute/row/encode_internal_avx2.cc
b/cpp/src/arrow/compute/row/encode_internal_avx2.cc
index 26f8e3a63d..d2e317deb8 100644
--- a/cpp/src/arrow/compute/row/encode_internal_avx2.cc
+++ b/cpp/src/arrow/compute/row/encode_internal_avx2.cc
@@ -15,9 +15,8 @@
// specific language governing permissions and limitations
// under the License.
-#include <immintrin.h>
-
#include "arrow/compute/row/encode_internal.h"
+#include "arrow/util/simd.h"
namespace arrow {
namespace compute {
diff --git a/cpp/src/arrow/compute/util.h b/cpp/src/arrow/compute/util.h
index 9034849bbc..1aaff43e10 100644
--- a/cpp/src/arrow/compute/util.h
+++ b/cpp/src/arrow/compute/util.h
@@ -28,24 +28,17 @@
#include "arrow/compute/type_fwd.h"
#include "arrow/result.h"
#include "arrow/util/cpu_info.h"
+#include "arrow/util/simd.h"
#if defined(__clang__) || defined(__GNUC__)
# define BYTESWAP(x) __builtin_bswap64(x)
# define ROTL(x, n) (((x) << (n)) | ((x) >> ((-n) & 31)))
# define ROTL64(x, n) (((x) << (n)) | ((x) >> ((-n) & 63)))
-# define PREFETCH(ptr) __builtin_prefetch((ptr), 0 /* rw==read */, 3 /*
locality */)
#elif defined(_MSC_VER)
# include <intrin.h>
# define BYTESWAP(x) _byteswap_uint64(x)
# define ROTL(x, n) _rotl((x), (n))
# define ROTL64(x, n) _rotl64((x), (n))
-# if defined(_M_X64) || defined(_M_I86)
-// https://msdn.microsoft.com/fr-fr/library/84szxsww(v=vs.90).aspx
-# include <mmintrin.h>
-# define PREFETCH(ptr) _mm_prefetch((const char*)(ptr), _MM_HINT_T0)
-# else
-# define PREFETCH(ptr) (void)(ptr) /* disabled */
-# endif
#endif
namespace arrow {
diff --git a/cpp/src/arrow/compute/util_avx2.cc
b/cpp/src/arrow/compute/util_avx2.cc
index 0191ab06f9..f0ff4575bb 100644
--- a/cpp/src/arrow/compute/util_avx2.cc
+++ b/cpp/src/arrow/compute/util_avx2.cc
@@ -15,11 +15,11 @@
// specific language governing permissions and limitations
// under the License.
-#include <immintrin.h>
#include <cstring>
#include "arrow/util/bit_util.h"
#include "arrow/util/logging.h"
+#include "arrow/util/simd.h"
namespace arrow::util::bit_util::avx2 {
diff --git a/cpp/src/arrow/util/bit_util.h b/cpp/src/arrow/util/bit_util.h
index 17d1de406d..e7eb3f833e 100644
--- a/cpp/src/arrow/util/bit_util.h
+++ b/cpp/src/arrow/util/bit_util.h
@@ -20,7 +20,6 @@
#if defined(_MSC_VER)
# if defined(_M_AMD64) || defined(_M_X64)
# include <intrin.h> // IWYU pragma: keep
-# include <nmmintrin.h>
# endif
# pragma intrinsic(_BitScanReverse)
diff --git a/cpp/src/arrow/util/macros.h b/cpp/src/arrow/util/macros.h
index 5658874b42..af29fd636b 100644
--- a/cpp/src/arrow/util/macros.h
+++ b/cpp/src/arrow/util/macros.h
@@ -78,7 +78,6 @@
# define ARROW_FORCE_INLINE __attribute__((always_inline))
# define ARROW_PREDICT_FALSE(x) (__builtin_expect(!!(x), 0))
# define ARROW_PREDICT_TRUE(x) (__builtin_expect(!!(x), 1))
-# define ARROW_PREFETCH(addr) __builtin_prefetch(addr)
# define ARROW_RESTRICT __restrict
# if defined(__clang__) // clang-specific
# define ARROW_COMPILER_ASSUME(expr) __builtin_assume(expr)
@@ -105,7 +104,6 @@
# define ARROW_FORCE_INLINE __forceinline
# define ARROW_PREDICT_FALSE(x) (x)
# define ARROW_PREDICT_TRUE(x) (x)
-# define ARROW_PREFETCH(addr)
# define ARROW_RESTRICT __restrict
# define ARROW_COMPILER_ASSUME(expr) __assume(expr)
#else
@@ -114,7 +112,6 @@
# define ARROW_FORCE_INLINE
# define ARROW_PREDICT_FALSE(x) (x)
# define ARROW_PREDICT_TRUE(x) (x)
-# define ARROW_PREFETCH(addr)
# define ARROW_RESTRICT
# define ARROW_COMPILER_ASSUME(expr)
#endif
diff --git a/cpp/src/arrow/util/prefetch.h b/cpp/src/arrow/util/prefetch.h
new file mode 100644
index 0000000000..1e9b5ae670
--- /dev/null
+++ b/cpp/src/arrow/util/prefetch.h
@@ -0,0 +1,31 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#if defined(__GNUC__) // GCC and compatible compilers (clang, Intel ICC)
+# define ARROW_PREFETCH(addr) __builtin_prefetch(addr)
+#elif defined(_MSC_VER) // MSVC
+# if defined(ARROW_HAVE_SSE4_2) || defined(ARROW_HAVE_RUNTIME_SSE4_2)
+# include <nmmintrin.h>
+# define ARROW_PREFETCH(addr) _mm_prefetch((const char*)(addr), _MM_HINT_T0)
+# else
+# define ARROW_PREFETCH(addr)
+# endif
+#else
+# define ARROW_PREFETCH(addr)
+#endif
