[Support/BLAKE3] Enable the SIMD implementations for macOS universal builds

[akyrtzi]

Summary

To accomodate macOS universal configuration include the assembly files
and blake3_neon.c without a CMake check but instead guard their source
with architecture "#ifdef" checks.

[akyrtzi]

Make sure to disable blake3 SIMD for i386 architecture.

[mstorsjo]

This LGTM, it seems to build correctly for me in all my test environments!

[akyrtzi]

In D128132#3596682, @mstorsjo wrote:

This LGTM, it seems to build correctly for me in all my test environments!

Thank you for testing! 🙇🏻‍♂️

[thevinster]

We've just recently updated our version of LLVM (I know we're slow...), and this patch breaks our builds when building LLVM for macOS Universal and with LTO. Specifically, this is the error I get when building for such configuration.

error: archive member blake3_sse2_x86-64_unix.S.o is a MachO, while previous archive member blake3.c.o was an IR LLVM object

Is there anyway to support LTO configurations with this patch? I've added a temporary internal patch on top of our mirror of LLVM that disables the assembly files, but it would be nice if there's a way to have that be supported.

llvm/lib/Support/BLAKE3/CMakeLists.txt

 set(LLVM_BLAKE3_FILES
   blake3.c
   blake3_dispatch.c
   blake3_portable.c
+  blake3_neon.c
 )
 
 if (LLVM_DISABLE_ASSEMBLY_FILES)
   set(CAN_USE_ASSEMBLER FALSE)
 else()
   set(CAN_USE_ASSEMBLER TRUE)
 endif()
 
+macro(disable_blake3_x86_simd)
+  add_definitions(-DBLAKE3_NO_AVX512 -DBLAKE3_NO_AVX2 -DBLAKE3_NO_SSE41 -DBLAKE3_NO_SSE2)
+endmacro()
+
 # The BLAKE3 team recommends using the assembly versions, from the README:
 #
 # "For each of the x86 SIMD instruction sets, four versions are available:
 # three flavors of assembly (Unix, Windows MSVC, and Windows GNU) and one
 # version using C intrinsics. The assembly versions are generally
 # preferred. They perform better, they perform more consistently across
 # different compilers, and they build more quickly."
 
+if (CAN_USE_ASSEMBLER)
   if (MSVC)
     check_symbol_exists(_M_X64 "" IS_X64)
-  check_symbol_exists(_M_ARM64 "" IS_ARM64)
-else()
-  check_symbol_exists(__x86_64__ "" IS_X64)
-  check_symbol_exists(__aarch64__ "" IS_ARM64)
-
     if (IS_X64)
-    # Clang-6 needs this flag.
-    set_source_files_properties(blake3_avx512_x86-64_windows_gnu.S
-      PROPERTIES COMPILE_OPTIONS "-mavx512vl")
-    set_source_files_properties(blake3_avx512_x86-64_unix.S
-      PROPERTIES COMPILE_OPTIONS "-mavx512vl")
-  endif()
-endif()
-
-if (IS_X64 AND CAN_USE_ASSEMBLER)
-  if (MSVC)
       enable_language(ASM_MASM)
       list(APPEND LLVM_BLAKE3_FILES
         blake3_sse2_x86-64_windows_msvc.asm
         blake3_sse41_x86-64_windows_msvc.asm
         blake3_avx2_x86-64_windows_msvc.asm
         blake3_avx512_x86-64_windows_msvc.asm
       )
+    else()
+      disable_blake3_x86_simd()
+    endif()
   elseif(WIN32 OR CYGWIN)
+    check_symbol_exists(__x86_64__ "" IS_X64)
+    if (IS_X64)
       list(APPEND LLVM_BLAKE3_FILES
         blake3_sse2_x86-64_windows_gnu.S
         blake3_sse41_x86-64_windows_gnu.S
         blake3_avx2_x86-64_windows_gnu.S
         blake3_avx512_x86-64_windows_gnu.S
       )
+      # Clang-6 needs this flag.
+      set_source_files_properties(blake3_avx512_x86-64_windows_gnu.S
+        PROPERTIES COMPILE_OPTIONS "-mavx512vl")
     else()
+      disable_blake3_x86_simd()
+    endif()
+  else()
+    check_symbol_exists(__i386__ "" IS_X32)
+    if (IS_X32)
+      # blake3 C code autoenables SIMD for i386, but those implementations are
+      # only available via the intrinsics sources which we don't enable here.
+      disable_blake3_x86_simd()
+    else()
+      # In a macOS Universal build (setting CMAKE_OSX_ARCHITECTURES to multiple
+      # values), compilation of the source files will target multiple architectures
+      # (each source file is internally compiled once for each architecture).
+      # To accomodate this configuration we include these assembly files without a
+      # CMake check but their source is guarded with architecture "#ifdef" checks.
       list(APPEND LLVM_BLAKE3_FILES
         blake3_sse2_x86-64_unix.S
         blake3_sse41_x86-64_unix.S
         blake3_avx2_x86-64_unix.S
         blake3_avx512_x86-64_unix.S
       )
+      # Clang-6 needs this flag. We also suppress '-Wunused-command-line-argument'
+      # in case the file is included with arm architecture.
+      set_source_files_properties(blake3_avx512_x86-64_unix.S
+        PROPERTIES COMPILE_OPTIONS "-mavx512vl;-Wno-unused-command-line-argument")
     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.
-  #
-  # Also, if targeting i386, then the blake3 source code automatically enables
-  # the SIMD implementations, but we don't provide those sources.
-  #
-  # FIXME: We could improve the CMAKE_OSX_ARCHITECTURES configuration by
-  # including all SIMD implementation files that might be relevant, and
-  # wrapping them in ifdefs like "#ifdef __x86_64__", to allow them to be
-  # included in a build for any architecture.
-  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
-  )
 else()
-  add_definitions(-DBLAKE3_USE_NEON=0)
+  # CAN_USE_ASSEMBLER == FALSE
+  disable_blake3_x86_simd()
 endif()
 
 add_library(LLVMSupportBlake3 OBJECT EXCLUDE_FROM_ALL ${LLVM_BLAKE3_FILES})
 llvm_update_compile_flags(LLVMSupportBlake3)

llvm/lib/Support/BLAKE3/blake3_avx2_x86-64_unix.S

+#if defined(__x86_64__)
+
 #if defined(__ELF__) && defined(__linux__)
 .section .note.GNU-stack,"",%progbits
 #endif
 
 #if defined(__ELF__) && defined(__CET__) && defined(__has_include)
 #if __has_include(<cet.h>)
 #include <cet.h>
 #endif

 ROT8:
         .byte  1, 2, 3, 0, 5, 6, 7, 4, 9, 10, 11, 8, 13, 14, 15, 12
 CMP_MSB_MASK:
         .long  0x80000000, 0x80000000, 0x80000000, 0x80000000
         .long  0x80000000, 0x80000000, 0x80000000, 0x80000000
 BLAKE3_IV:
         .long  0x6A09E667, 0xBB67AE85, 0x3C6EF372, 0xA54FF53A
 
+#endif

llvm/lib/Support/BLAKE3/blake3_avx512_x86-64_unix.S

+#if defined(__x86_64__)
+
 #if defined(__ELF__) && defined(__linux__)
 .section .note.GNU-stack,"",%progbits
 #endif
 
 #if defined(__ELF__) && defined(__CET__) && defined(__has_include)
 #if __has_include(<cet.h>)
 #include <cet.h>
 #endif

 BLAKE3_IV_0:
         .long   0x6A09E667
 BLAKE3_IV_1:
         .long   0xBB67AE85
 BLAKE3_IV_2:
         .long   0x3C6EF372
 BLAKE3_IV_3:
         .long   0xA54FF53A
+
+#endif

llvm/lib/Support/BLAKE3/blake3_neon.c

 #include "blake3_impl.h"
 
+#if BLAKE3_USE_NEON
+
 #include <arm_neon.h>
 
 #ifdef __ARM_BIG_ENDIAN
 #error "This implementation only supports little-endian ARM."
 // It might be that all we need for big-endian support here is to get the loads
 // and stores right, but step zero would be finding a way to test it in CI.
 #endif
 
while (num_inputs > 0) {
     if (increment_counter) {
       counter += 1;
     }
     inputs += 1;
     num_inputs -= 1;
     out = &out[BLAKE3_OUT_LEN];
   }
 }
+
+#endif // BLAKE3_USE_NEON

llvm/lib/Support/BLAKE3/blake3_sse2_x86-64_unix.S

+#if defined(__x86_64__)
+
 #if defined(__ELF__) && defined(__linux__)
 .section .note.GNU-stack,"",%progbits
 #endif
 
 #if defined(__ELF__) && defined(__CET__) && defined(__has_include)
 #if __has_include(<cet.h>)
 #include <cet.h>
 #endif

 PBLENDW_0x33_MASK:
   .long  0xFFFFFFFF, 0x00000000, 0xFFFFFFFF, 0x00000000
 PBLENDW_0xCC_MASK:
   .long  0x00000000, 0xFFFFFFFF, 0x00000000, 0xFFFFFFFF
 PBLENDW_0x3F_MASK:
   .long  0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0x00000000
 PBLENDW_0xC0_MASK:
   .long  0x00000000, 0x00000000, 0x00000000, 0xFFFFFFFF
+
+#endif

llvm/lib/Support/BLAKE3/blake3_sse41_x86-64_unix.S

+#if defined(__x86_64__)
+
 #if defined(__ELF__) && defined(__linux__)
 .section .note.GNU-stack,"",%progbits
 #endif
 
 #if defined(__ELF__) && defined(__CET__) && defined(__has_include)
 #if __has_include(<cet.h>)
 #include <cet.h>
 #endif

 BLAKE3_IV_2:
   .long  0x3C6EF372, 0x3C6EF372, 0x3C6EF372, 0x3C6EF372
 BLAKE3_IV_3:
   .long  0xA54FF53A, 0xA54FF53A, 0xA54FF53A, 0xA54FF53A
 BLAKE3_BLOCK_LEN:
   .long  64, 64, 64, 64
 CMP_MSB_MASK:
   .long  0x80000000, 0x80000000, 0x80000000, 0x80000000
+
+#endif