Build error on osx #229
Comments
|
Not specific to arm64: https://ci.nodejs.org/job/node-test-commit-osx/45583/nodes=osx11-x64/console |
|
Created a V8 issue: https://bugs.chromium.org/p/v8/issues/detail?id=12982 |
|
/cc @nodejs/v8 |
|
The compiler is Technically Right: V8 is using the wrong type check. Untested (as in: I didn't even try to compile it) but a fix would look like this: diff --git a/src/objects/simd.cc b/src/objects/simd.cc
index be6b72d1574..25be2f3b0ec 100644
--- a/src/objects/simd.cc
+++ b/src/objects/simd.cc
@@ -148,9 +148,14 @@ inline int32_t reinterpret_vmaxvq_u64(uint64x2_t v) {
template <typename T>
inline uintptr_t fast_search_noavx(T* array, uintptr_t array_len,
uintptr_t index, T search_element) {
- static_assert(std::is_same<T, uint32_t>::value ||
- std::is_same<T, uint64_t>::value ||
- std::is_same<T, double>::value);
+ static constexpr bool is_uint32 =
+ sizeof(T) == sizeof(uint32_t) && std::is_integral<T>::value;
+ static constexpr bool is_uint64 =
+ sizeof(T) == sizeof(uint64_t) && std::is_integral<T>::value;
+ static constexpr bool is_double =
+ sizeof(T) == sizeof(double) && std::is_floating_point<T>::value;
+
+ static_assert(is_uint32 || is_uint64 || is_double);
#if !(defined(__SSE3__) || defined(NEON64))
// No SIMD available.
@@ -178,14 +183,14 @@ inline uintptr_t fast_search_noavx(T* array, uintptr_t array_len,
// Inserting one of the vectorized loop
#ifdef __SSE3__
- if constexpr (std::is_same<T, uint32_t>::value) {
+ if constexpr (is_uint32) {
#define MOVEMASK(x) _mm_movemask_ps(_mm_castsi128_ps(x))
#define EXTRACT(x) base::bits::CountTrailingZeros32(x)
VECTORIZED_LOOP_x86(__m128i, __m128i, _mm_set1_epi32, _mm_cmpeq_epi32,
MOVEMASK, EXTRACT)
#undef MOVEMASK
#undef EXTRACT
- } else if constexpr (std::is_same<T, uint64_t>::value) {
+ } else if constexpr (is_uint64) {
#define SET1(x) _mm_castsi128_ps(_mm_set1_epi64x(x))
#define CMP(a, b) _mm_cmpeq_pd(_mm_castps_pd(a), _mm_castps_pd(b))
#define EXTRACT(x) base::bits::CountTrailingZeros32(x)
@@ -193,20 +198,20 @@ inline uintptr_t fast_search_noavx(T* array, uintptr_t array_len,
#undef SET1
#undef CMP
#undef EXTRACT
- } else if constexpr (std::is_same<T, double>::value) {
+ } else if constexpr (is_double) {
#define EXTRACT(x) base::bits::CountTrailingZeros32(x)
VECTORIZED_LOOP_x86(__m128d, __m128d, _mm_set1_pd, _mm_cmpeq_pd,
_mm_movemask_pd, EXTRACT)
#undef EXTRACT
}
#elif defined(NEON64)
- if constexpr (std::is_same<T, uint32_t>::value) {
+ if constexpr (is_uint32) {
VECTORIZED_LOOP_Neon(uint32x4_t, uint32x4_t, vdupq_n_u32, vceqq_u32,
vmaxvq_u32)
- } else if constexpr (std::is_same<T, uint64_t>::value) {
+ } else if constexpr (is_uint64) {
VECTORIZED_LOOP_Neon(uint64x2_t, uint64x2_t, vdupq_n_u64, vceqq_u64,
reinterpret_vmaxvq_u64)
- } else if constexpr (std::is_same<T, double>::value) {
+ } else if constexpr (is_double) {
VECTORIZED_LOOP_Neon(float64x2_t, uint64x2_t, vdupq_n_f64, vceqq_f64,
reinterpret_vmaxvq_u64)
}
@@ -240,9 +245,15 @@ template <typename T>
TARGET_AVX2 inline uintptr_t fast_search_avx(T* array, uintptr_t array_len,
uintptr_t index,
T search_element) {
- static_assert(std::is_same<T, uint32_t>::value ||
- std::is_same<T, uint64_t>::value ||
- std::is_same<T, double>::value);
+ static constexpr bool is_uint32 =
+ sizeof(T) == sizeof(uint32_t) && std::is_integral<T>::value;
+ static constexpr bool is_uint64 =
+ sizeof(T) == sizeof(uint64_t) && std::is_integral<T>::value;
+ static constexpr bool is_double =
+ sizeof(T) == sizeof(double) && std::is_floating_point<T>::value;
+
+ static_assert(is_uint32 || is_uint64 || is_double);
+
const int target_align = 32;
// Scalar loop to reach desired alignment
@@ -256,21 +267,21 @@ TARGET_AVX2 inline uintptr_t fast_search_avx(T* array, uintptr_t array_len,
}
// Generating vectorized loop
- if constexpr (std::is_same<T, uint32_t>::value) {
+ if constexpr (is_uint32) {
#define MOVEMASK(x) _mm256_movemask_ps(_mm256_castsi256_ps(x))
#define EXTRACT(x) base::bits::CountTrailingZeros32(x)
VECTORIZED_LOOP_x86(__m256i, __m256i, _mm256_set1_epi32, _mm256_cmpeq_epi32,
MOVEMASK, EXTRACT)
#undef MOVEMASK
#undef EXTRACT
- } else if constexpr (std::is_same<T, uint64_t>::value) {
+ } else if constexpr (is_uint64) {
#define MOVEMASK(x) _mm256_movemask_pd(_mm256_castsi256_pd(x))
#define EXTRACT(x) base::bits::CountTrailingZeros32(x)
VECTORIZED_LOOP_x86(__m256i, __m256i, _mm256_set1_epi64x,
_mm256_cmpeq_epi64, MOVEMASK, EXTRACT)
#undef MOVEMASK
#undef EXTRACT
- } else if constexpr (std::is_same<T, double>::value) {
+ } else if constexpr (is_double) {
#define CMP(a, b) _mm256_cmp_pd(a, b, _CMP_EQ_OQ)
#define EXTRACT(x) base::bits::CountTrailingZeros32(x)
VECTORIZED_LOOP_x86(__m256d, __m256d, _mm256_set1_pd, CMP, |
|
@bnoordhuis thanks, it seems to work on my computer. Could you please upstream it? |
|
@bnoordhuis Thanks for the suggested fix. Could you please upload it to Gerrit and add me (dmercadier@chromium.org) as reviewer? (or, alternatively, I can do it myself, if you don't care about having your name on the commit) |
|
@DadaIsCrazy it's on my todo list but there's a bit of a corona outbreak going on in my household right now so I have little hands-on time this week. |
|
@bnoordhuis I'm sorry to hear that. I hope you and your family get well soon! |
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
See https://ci.nodejs.org/job/node-test-commit-osx-arm/6086/nodes=osx11/console
The text was updated successfully, but these errors were encountered: