[PATCH] D122884: [Support] [BLAKE3] Fix compilation with CMAKE_OSX_ARCHITECTURES

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo created this revision.
mstorsjo added a reviewer: akyrtzi.
Herald added subscribers: dexonsmith, pengfei, hiraditya, kristof.beyls, mgorny.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: LLVM.

With CMake, one can build for multiple macOS architectures
at the same time by setting CMAKE_OSX_ARCHITECTURES to multiple
architectures (avoiding needing to do two separate builds and
gluing the binaries together after the build).

In this case, while targeting x86_64 and arm64, neither IS_X64
nor IS_ARM64 is set, while compilation of the individual source
files will hit those cases (in either architecture mode).

Therefore, if we on the CMake level decide not to include the
architecture specific SIMD implementation files, also tell the
source this explicitly by passing the defines indicating that we
don't expect to use them.

Such a build clearly is less ideal than explicitly targeting one
architecture at a time if it won't include all the SIMD optimizations,
but that's a tradeoff that is up to the one deciding to do such an
universal build.

An alternative would be to include all potentially relevant source
files in the build, and wrap them in ifdefs, like `#if BLAKE3_USE_NEON`
within `blake3_neon.c` after including `blake3_impl.h`, or
`#ifdef __x86_64__` in the .S assembly files.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122884

Files:
  llvm/lib/Support/BLAKE3/CMakeLists.txt


Index: llvm/lib/Support/BLAKE3/CMakeLists.txt
===================================================================
--- llvm/lib/Support/BLAKE3/CMakeLists.txt
+++ llvm/lib/Support/BLAKE3/CMakeLists.txt
@@ -74,16 +74,21 @@
       blake3_avx512_x86-64_unix.S
     )
   endif()
+else()
+  # In a macOS Universal build (setting CMAKE_OSX_ARCHITECTURES to multiple values),
+  # IS_X64 and IS_ARM64 won't be set, but compilation of the source files will consider
+  # targeting either of them (each source file is internally compiled once for each
+  # architecture). Thus, if we on the CMake level decide not to include the assembly
+  # files, tell the source to not expect it to be present either.
+  add_definitions(-DBLAKE3_NO_AVX512 -DBLAKE3_NO_AVX2 -DBLAKE3_NO_SSE41 -DBLAKE3_NO_SSE2)
 endif()
 
 if (IS_ARM64)
   list(APPEND LLVM_BLAKE3_FILES
     blake3_neon.c
   )
-endif()
-
-if (IS_X64 AND NOT CAN_USE_ASSEMBLER)
-  add_definitions(-DBLAKE3_NO_AVX512 -DBLAKE3_NO_AVX2 -DBLAKE3_NO_SSE41 -DBLAKE3_NO_SSE2)
+else()
+  add_definitions(-DBLAKE3_USE_NEON=0)
 endif()
 
 add_library(LLVMSupportBlake3 OBJECT EXCLUDE_FROM_ALL ${LLVM_BLAKE3_FILES})


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D122884.419677.patch
Type: text/x-patch
Size: 1145 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220401/2221330b/attachment.bin>