Repository navigation
Conversation
clang ships ppc_wrappers/{x,e,p,t,s,n}mmintrin.h that implement the
SSE..SSE4.2 intrinsics on top of VSX (POWER8 and later), which is the
ISA level the SSE4.2 kernels need. Treat powerpc64 hosts like the
AArch64 path: define the x86 ISA macros by hand in clang.cmake, mark the
platform as 64-bit, report SSE4.2 features at runtime, and opt into the
wrapper headers from platform.h so that every translation unit including
embree headers gets them. The few intrinsics the wrappers lack
(_mm_popcnt_u32/u64, _mm_dp_ps, _mm_insert_ps, _mm_stream_load_si128)
and the MXCSR control family (no POWER equivalent; FTZ/DAZ become
no-ops) get small definitions in intrinsics.h.
Tested with clang 19 on FreeBSD powerpc64le and powerpc64 (big-endian):
the ANARI SDK helide device renders all 13 of its test scenes
pixel-identical to an x86-64 build.
Follow-up after building and testing embree standalone (the first commit was only exercised through the ANARI SDK's bundled copy): - Take a -mcpu= from CMAKE_CXX_FLAGS into the ISA flags instead of always forcing -mcpu=power8, so that e.g. -mcpu=power9 is not downgraded. power8 remains the default; the x86 intrinsic wrappers need it. - The AVX/AVX2/AVX-512/APX compiler probes pass trivially without any x86 flags, which made the build try to compile the AVX kernels. Switch them off explicitly. - The tutorials need _MM_SET_FLUSH_ZERO_MODE, which clang's wrappers do not provide. - ISPC 1.31.0 added ppc64le with VSX targets: use vsx-i32x4 (ISPC only allows one VSX variant per object) and --arch=ppc64le for the ISPC tutorials. Tested as a backport to embree 4.4.1 with clang 19 on FreeBSD powerpc64le and powerpc64 (big-endian): embree_verify passes (112 test groups, 0 failures) on both, and the tutorials render identically through the C++ and the ISPC 1.31.0 paths on powerpc64le.
Collaborator
|
Thanks for this contribution. If we were to merge this PR, we would have to consider this an unsupported/experimental feature, since we don't have the CI or testing infrastructure to guarantee correctness. |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
PowerPC configuration paths still allow incompatible compiler, kernel-target, and ISPC selections.
Review effort: Balanced
Findings: 3
Open (3)
What changed in this PR
Adds PowerPC64 support to Embree’s SSE kernels through Clang’s VSX-backed intrinsic wrappers.
Changes:
- Detects PowerPC64 and configures POWER8 compiler flags.
- Adds intrinsic compatibility helpers and runtime feature reporting.
- Adds PowerPC ISPC target selection.
| File | Description |
|---|---|
| common/sys/sysinfo.cpp | Reports emulated SSE4.2 capabilities on PowerPC64. |
| common/sys/platform.h | Enables wrapper headers and 64-bit platform detection. |
| common/sys/intrinsics.h | Supplies missing intrinsics and no-op MXCSR controls. |
| common/cmake/ispc.cmake | Selects the little-endian PowerPC ISPC architecture. |
| common/cmake/clang.cmake | Configures PowerPC CPU flags and SSE macros. |
| CMakeLists.txt | Detects PowerPC64 and configures ISA defaults and VSX targets. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+231
to
+233
| ELSEIF(CMAKE_SYSTEM_PROCESSOR MATCHES "^(powerpc|ppc)64") | ||
| MESSAGE(STATUS "Building for PowerPC64") | ||
| SET(EMBREE_PPC64 ON) |
Comment on lines
+576
to
+578
| IF (EMBREE_PPC64) | ||
| LIST(APPEND ISPC_TARGETS "vsx-i32x4") | ||
| ENDIF() |
Comment on lines
+78
to
+79
| ELSEIF (${CMAKE_SYSTEM_PROCESSOR} MATCHES "^(powerpc|ppc)64le") | ||
| SET(ISPC_ARCHITECTURE "ppc64le") |
Author
That's still better than the current state. I suppose the community will be able to provide feedback and patches when something breaks (just like me here). Also, I don't expect many actual breakages. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
clang ships ppc_wrappers/{x,e,p,t,s,n}mmintrin.h that implement the
SSE..SSE4.2 intrinsics on top of VSX (POWER8 and later), which is the
ISA level the SSE4.2 kernels need. Treat powerpc64 hosts like the
AArch64 path: define the x86 ISA macros by hand in clang.cmake, mark the
platform as 64-bit, report SSE4.2 features at runtime, and opt into the
wrapper headers from platform.h so that every translation unit including
embree headers gets them. The few intrinsics the wrappers lack
(_mm_popcnt_u32/u64, _mm_dp_ps, _mm_insert_ps, _mm_stream_load_si128)
and the MXCSR control family (no POWER equivalent; FTZ/DAZ become
no-ops) get small definitions in intrinsics.h.
Tested with clang 19 on FreeBSD powerpc64le and powerpc64 (big-endian):
the ANARI SDK helide device renders all 13 of its test scenes
pixel-identical to an x86-64 build.