From ffa7277deb07715c16b2fa54c1603145c6a0c707 Mon Sep 17 00:00:00 2001 From: "Werner, Stefan" Date: Wed, 19 Aug 2026 13:50:51 +0200 Subject: [PATCH] Fix ICX segfault caused by unconditional ARM64 SIMD wrapper types The __m128_wrapper/__m128i_wrapper structs in common/simd/arm/simd_wrapper_types.h were introduced to work around an MSVC/ARM64-specific overload ambiguity (__m128 and __m128i both alias __n128 on that platform). However, the wrapper was applied unconditionally in vboolf4_sse2.h, vfloat4_sse2.h, vint4_sse2.h, and vuint4_sse2.h, replacing the plain __m128/__m128i union members (and their implicit conversions) with a wrapper struct on ALL platforms, including x86. This introduced non-trivial-constructor types placed inside unions alongside raw int/float arrays (type punning that is technically UB) plus a class exposing both a value-returning const conversion operator and a non-const reference conversion operator simultaneously - a known overload-resolution footgun. This is suspected to have caused miscompilation under ICX's aggressive optimizer, leading to SegFaults in embree_verify during the nightly-linux-DEBUG-ICX-AVX/AVX2/AVX512 CI jobs shortly after the new ARM64 support merged (all three ISA widths failed identically, pointing to a compiler/ABI issue rather than an ISA-specific bug). Fix: scope the wrapper-based union member and conversion operators to MSVC+ARM64 only, and restore the original plain __m128/__m128i union member with implicit conversions for all other builds (x86 and non-MSVC ARM64, e.g. Linux/macOS aarch64 via clang/gcc, which already worked without the wrapper before this change). The explicit m128i()/m128d() accessor methods are kept available on both paths so call sites elsewhere in these files that already use them continue to compile unchanged. Validated: compiles cleanly and all embree_verify tests pass with Intel ICX/ICPX 2026.0.0 (RelWithDebInfo, AVX, TBB tasking, testing intensity 4 - matching CI settings as closely as possible locally: 6346 passed / 0 failed), Clang 21.1.8 (Debug, AVX2: 8476 passed / 0 failed), and GCC (header compile smoke test). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- common/simd/vboolf4_sse2.h | 21 +++++++++++++++++++++ common/simd/vfloat4_sse2.h | 10 ++++++++++ common/simd/vint4_sse2.h | 10 ++++++++++ common/simd/vuint4_sse2.h | 10 ++++++++++ 4 files changed, 51 insertions(+) diff --git a/common/simd/vboolf4_sse2.h b/common/simd/vboolf4_sse2.h index bfb72889b8..9b85adb70a 100644 --- a/common/simd/vboolf4_sse2.h +++ b/common/simd/vboolf4_sse2.h @@ -26,7 +26,17 @@ namespace embree typedef vfloat4 Float; enum { size = 4 }; // number of SIMD elements + // The __m128_wrapper indirection is only required to work around an + // MSVC/ARM64 overload-ambiguity issue (__m128 and __m128i both alias + // __n128 there). On all other platforms we keep the plain __m128 union + // member, which is what x86 and non-MSVC ARM64 builds have always used + // (see simd_wrapper_types.h); wrapping it unconditionally previously + // caused miscompiles/segfaults with some optimizing compilers (e.g. ICX). +#if defined(_MSC_VER) && defined(_M_ARM64) union { __m128_wrapper v; int i[4]; }; // data +#else + union { __m128 v; int i[4]; }; // data +#endif //////////////////////////////////////////////////////////////////////////////// /// Constructors, Assignment & Cast Operators @@ -37,11 +47,22 @@ namespace embree __forceinline vboolf4& operator =(const vboolf4& other) { v = other.v; return *this; } __forceinline vboolf(__m128 input) : v(input) {} +#if defined(_MSC_VER) && defined(_M_ARM64) __forceinline operator const __m128&() const { return v.data; } #if !defined(__EMSCRIPTEN__) __forceinline const __m128i m128i() const { return _mm_castps_si128(v.data); } __forceinline const __m128d m128d() const { return _mm_castps_pd(v.data); } #endif +#else + __forceinline operator const __m128&() const { return v; } + #if !defined(__EMSCRIPTEN__) + __forceinline operator const __m128i() const { return _mm_castps_si128(v); } + __forceinline operator const __m128d() const { return _mm_castps_pd(v); } + /* kept for source compatibility with code that calls .m128i()/.m128d() explicitly */ + __forceinline const __m128i m128i() const { return _mm_castps_si128(v); } + __forceinline const __m128d m128d() const { return _mm_castps_pd(v); } + #endif +#endif __forceinline vboolf(bool a) : v(mm_lookupmask_ps[(size_t(a) << 3) | (size_t(a) << 2) | (size_t(a) << 1) | size_t(a)]) {} diff --git a/common/simd/vfloat4_sse2.h b/common/simd/vfloat4_sse2.h index 13a048d73c..593ac1ff8b 100644 --- a/common/simd/vfloat4_sse2.h +++ b/common/simd/vfloat4_sse2.h @@ -26,7 +26,12 @@ namespace embree typedef vfloat4 Float; enum { size = 4 }; // number of SIMD elements + // See vboolf4_sse2.h for why the wrapper is restricted to MSVC/ARM64. +#if defined(_MSC_VER) && defined(_M_ARM64) union { __m128_wrapper v; float f[4]; int i[4]; }; // data +#else + union { __m128 v; float f[4]; int i[4]; }; // data +#endif //////////////////////////////////////////////////////////////////////////////// /// Constructors, Assignment & Cast Operators @@ -39,8 +44,13 @@ namespace embree __forceinline vfloat4& operator =(const vfloat4& other) { v = other.v; return *this; } __forceinline vfloat(__m128 a) : v(a) {} +#if defined(_MSC_VER) && defined(_M_ARM64) __forceinline operator const __m128&() const { return v.data; } __forceinline operator __m128&() { return v.data; } +#else + __forceinline operator const __m128&() const { return v; } + __forceinline operator __m128&() { return v; } +#endif __forceinline vfloat(float a) : v(_mm_set1_ps(a)) {} __forceinline vfloat(float a, float b, float c, float d) : v(_mm_set_ps(d, c, b, a)) {} diff --git a/common/simd/vint4_sse2.h b/common/simd/vint4_sse2.h index a64232a0d7..324c57f923 100644 --- a/common/simd/vint4_sse2.h +++ b/common/simd/vint4_sse2.h @@ -27,7 +27,12 @@ namespace embree typedef vfloat4 Float; enum { size = 4 }; // number of SIMD elements + // See vboolf4_sse2.h for why the wrapper is restricted to MSVC/ARM64. +#if defined(_MSC_VER) && defined(_M_ARM64) union { __m128i_wrapper v; int i[4]; }; // data +#else + union { __m128i v; int i[4]; }; // data +#endif //////////////////////////////////////////////////////////////////////////////// /// Constructors, Assignment & Cast Operators @@ -38,8 +43,13 @@ namespace embree __forceinline vint4& operator =(const vint4& a) { v = a.v; return *this; } __forceinline vint(__m128i a) : v(a) {} +#if defined(_MSC_VER) && defined(_M_ARM64) __forceinline operator const __m128i&() const { return v.data; } __forceinline operator __m128i&() { return v.data; } +#else + __forceinline operator const __m128i&() const { return v; } + __forceinline operator __m128i&() { return v; } +#endif __forceinline vint(int a) : v(_mm_set1_epi32(a)) {} __forceinline vint(int a, int b, int c, int d) : v(_mm_set_epi32(d, c, b, a)) {} diff --git a/common/simd/vuint4_sse2.h b/common/simd/vuint4_sse2.h index 4f1443c119..e2ac7ec8b2 100644 --- a/common/simd/vuint4_sse2.h +++ b/common/simd/vuint4_sse2.h @@ -27,7 +27,12 @@ namespace embree typedef vfloat4 Float; enum { size = 4 }; // number of SIMD elements + // See vboolf4_sse2.h for why the wrapper is restricted to MSVC/ARM64. +#if defined(_MSC_VER) && defined(_M_ARM64) union { __m128i_wrapper v; unsigned int i[4]; }; // data +#else + union { __m128i v; unsigned int i[4]; }; // data +#endif //////////////////////////////////////////////////////////////////////////////// /// Constructors, Assignment & Cast Operators @@ -38,8 +43,13 @@ namespace embree __forceinline vuint4& operator =(const vuint4& a) { v = a.v; return *this; } __forceinline vuint(const __m128i a) : v(a) {} +#if defined(_MSC_VER) && defined(_M_ARM64) __forceinline operator const __m128i&() const { return v.data; } __forceinline operator __m128i&() { return v.data; } +#else + __forceinline operator const __m128i&() const { return v; } + __forceinline operator __m128i&() { return v; } +#endif __forceinline vuint(unsigned int a) : v(_mm_set1_epi32(a)) {}