[libc] Fix builtin definition for memory functions

[michaelrj]

Summary

The memory functions are highly performance sensitive and use builtins
where possible, but also need to define those functions names when they
don't exist to avoid compilation errors. Previously all those
redefinitions were behind the SSE2 flag for x86, which caused errors on
CPUs that supported SSE2 but not AVX512. This patch splits the various
CPU extensions out to avoid errors on such CPUs.

[lntue]

libc/src/string/memory_utils/op_x86.h

36

Can you move __AVX2__ switch to between __AVX512__ and __SSE2__?

[gchatelet]

libc/src/string/memory_utils/op_x86.h

31

This requires __AVX512BW__
https://www.intel.com/content/www/us/en/docs/intrinsics-guide/index.html#text=_mm512_cmpneq_epi8_mask&ig_expand=1249

[michaelrj]

add AVX512BW to the list of features to be checked and move to simpler redefinitions for the builtins.

[michaelrj]

libc/src/string/memory_utils/op_x86.h

31

ah, yes. Fixed

[lntue]

libc/src/string/memory_utils/op_x86.h

30

How will this work if _mm512_cmpneq_epi8_mask is defined as a function or compiler builtin instead of macro? Will defined(_mm512_cmpneq_epi8_mask) also return false, and the macro will overwrite the real function calls?

[gchatelet]

libc/cmake/modules/LLVMLibCCheckCpuFeatures.cmake

9

Thx! We'd also need to have it added to the CMakefile.txt for the bcmp and memcmp functions

• https://github.com/llvm/llvm-project/blob/a4ae029b087070c43d8eb25c9240de3eb345ed63/libc/src/string/CMakeLists.txt#L358
• https://github.com/llvm/llvm-project/blob/a4ae029b087070c43d8eb25c9240de3eb345ed63/libc/src/string/CMakeLists.txt#L412

libc/src/string/memory_utils/op_x86.h

30

So technically the right way to do it is to rely on the __has_builtin intrinsic.
It is available on clang forever but is quite recent on gcc, it requires gcc 10 (another bummer to have to support both gcc and clang)
https://gcc.gnu.org/onlinedocs/gcc-10.1.0/cpp/_005f_005fhas_005fbuiltin.html

[michaelrj]

move from checking if macros are defined back to which extensions are defined.

libc/cmake/modules/LLVMLibCCheckCpuFeatures.cmake

9

I switched the memory functions over to AVX512BW instead of AVX512F since it seems like only BW matters for this case.

libc/src/string/memory_utils/op_x86.h

30

__has_builtin doesn't work because this isn't the name of the builtin. From avx512bwintrin.h:

#define _mm512_cmp_epi8_mask(a, b, p) \
  ((__mmask64)__builtin_ia32_cmpb512_mask((__v64qi)(__m512i)(a), \
                                          (__v64qi)(__m512i)(b), (int)(p), \
                                          (__mmask64)-1))

And #ifndef doesn't work either because sometimes these are defined as functions, from avx2intrin.h (on the same system):

static __inline__ int __DEFAULT_FN_ATTRS256
_mm256_movemask_epi8(__m256i __a)
{
  return __builtin_ia32_pmovmskb256((__v32qi)__a);
}

But it appears that the problems I was having trying to build on windows previously can be fixed by using the proper extension name (AVX512BW vs AVX512F), this builds clean on both and (afaict) uses the correct builtins.

[gchatelet]

libc/src/string/memory_utils/op_x86.h

30

Ha yes indeed, they are sometimes macros sometimes builtins...

Anyways it's great to know that using the proper preprocessor definitions fixes it!

BTW do we have windows CI machines? I don't think our bazel config currently handles windows build but I can have a look if needed.

[michaelrj]

libc/src/string/memory_utils/op_x86.h

30

We don't currently have Windows CI working, and the Windows builds I've tried have all been cmake/ninja.

[gchatelet]

LGTM just make sure to keep AVX512F for memmove, memcpy, memset and bzero.

libc/src/string/CMakeLists.txt

385

Only bcmp and memcmp require AVX512BW, the other ones only require AVX512F

[michaelrj]

rebase and address comments to fix which CPU extensions are relevant to which operations.

[michaelrj]

I'm going to land this as is for now, but I have identified a minor issue: When building variants of the memory functions that don't need AVX512BW, it doesn't seem to set the compiler flag for it. This causes a lot of warnings when building memcpy (for example) that boil down to "the macro _mm512_cmpneq_epi8_mask is being redefined". I don't think this will cause any real issues since that builtin isn't used for that function, but it will cause some noise in the logs.

libc/cmake/modules/LLVMLibCCheckCpuFeatures.cmake

 # ------------------------------------------------------------------------------
 # Cpu features definition and flags
 # ------------------------------------------------------------------------------
 
 # Initialize ALL_CPU_FEATURES as empty list.
 set(ALL_CPU_FEATURES "")
 
 if(${LIBC_TARGET_ARCHITECTURE_IS_X86})
-  set(ALL_CPU_FEATURES SSE2 SSE4_2 AVX2 AVX512F FMA)
+  set(ALL_CPU_FEATURES SSE2 SSE4_2 AVX2 AVX512F AVX512BW FMA)
   set(LIBC_COMPILE_OPTIONS_NATIVE -march=native)
 elseif(${LIBC_TARGET_ARCHITECTURE_IS_AARCH64})
   set(LIBC_COMPILE_OPTIONS_NATIVE -mcpu=native)
 endif()
 
 # Making sure ALL_CPU_FEATURES is sorted.
 list(SORT ALL_CPU_FEATURES)
 

libc/src/string/CMakeLists.txt

add_implementation(bcmp ${bcmp_name}
     ${ARGN}
   )
 endfunction()
 
 if(${LIBC_TARGET_ARCHITECTURE_IS_X86})
   add_bcmp(bcmp_x86_64_opt_sse2   COMPILE_OPTIONS -march=k8             REQUIRE SSE2)
   add_bcmp(bcmp_x86_64_opt_sse4   COMPILE_OPTIONS -march=nehalem        REQUIRE SSE4_2)
   add_bcmp(bcmp_x86_64_opt_avx2   COMPILE_OPTIONS -march=haswell        REQUIRE AVX2)
-  add_bcmp(bcmp_x86_64_opt_avx512 COMPILE_OPTIONS -march=skylake-avx512 REQUIRE AVX512F)
+  add_bcmp(bcmp_x86_64_opt_avx512 COMPILE_OPTIONS -march=skylake-avx512 REQUIRE AVX512BW)
   add_bcmp(bcmp_opt_host          COMPILE_OPTIONS ${LIBC_COMPILE_OPTIONS_NATIVE})
   add_bcmp(bcmp)
 else()
   add_bcmp(bcmp_opt_host          COMPILE_OPTIONS ${LIBC_COMPILE_OPTIONS_NATIVE})
   add_bcmp(bcmp)
 endif()
 
 # ------------------------------------------------------------------------------
add_implementation(bzero ${bzero_name}
     ${ARGN}
   )
 endfunction()
 
 if(${LIBC_TARGET_ARCHITECTURE_IS_X86})
   add_bzero(bzero_x86_64_opt_sse2   COMPILE_OPTIONS -march=k8             REQUIRE SSE2)
   add_bzero(bzero_x86_64_opt_sse4   COMPILE_OPTIONS -march=nehalem        REQUIRE SSE4_2)
   add_bzero(bzero_x86_64_opt_avx2   COMPILE_OPTIONS -march=haswell        REQUIRE AVX2)
   add_bzero(bzero_x86_64_opt_avx512 COMPILE_OPTIONS -march=skylake-avx512 REQUIRE AVX512F)
   add_bzero(bzero_opt_host          COMPILE_OPTIONS ${LIBC_COMPILE_OPTIONS_NATIVE})
   add_bzero(bzero)
 else()
   add_bzero(bzero_opt_host          COMPILE_OPTIONS ${LIBC_COMPILE_OPTIONS_NATIVE})
   add_bzero(bzero)
 endif()
 
 # ------------------------------------------------------------------------------
add_implementation(memcmp ${memcmp_name}
     ${ARGN}
   )
 endfunction()
 
 if(${LIBC_TARGET_ARCHITECTURE_IS_X86})
   add_memcmp(memcmp_x86_64_opt_sse2   COMPILE_OPTIONS -march=k8             REQUIRE SSE2)
   add_memcmp(memcmp_x86_64_opt_sse4   COMPILE_OPTIONS -march=nehalem        REQUIRE SSE4_2)
   add_memcmp(memcmp_x86_64_opt_avx2   COMPILE_OPTIONS -march=haswell        REQUIRE AVX2)
-  add_memcmp(memcmp_x86_64_opt_avx512 COMPILE_OPTIONS -march=skylake-avx512 REQUIRE AVX512F)
+  add_memcmp(memcmp_x86_64_opt_avx512 COMPILE_OPTIONS -march=skylake-avx512 REQUIRE AVX512BW)
   add_memcmp(memcmp_opt_host          COMPILE_OPTIONS ${LIBC_COMPILE_OPTIONS_NATIVE})
   add_memcmp(memcmp)
 elseif(${LIBC_TARGET_ARCHITECTURE_IS_AARCH64})
   add_memcmp(memcmp_opt_host          COMPILE_OPTIONS ${LIBC_COMPILE_OPTIONS_NATIVE})
   add_memcmp(memcmp)
 else()
   add_memcmp(memcmp_opt_host          COMPILE_OPTIONS ${LIBC_COMPILE_OPTIONS_NATIVE})
   add_memcmp(memcmp)

libc/src/string/memory_utils/op_x86.h

 #include "src/__support/architectures.h"
 
 #if defined(LLVM_LIBC_ARCH_X86_64)
 
 #include "src/__support/common.h"
 #include "src/string/memory_utils/op_builtin.h"
 #include "src/string/memory_utils/op_generic.h"
 
-#ifdef __SSE2__
+#if defined(__AVX512BW__) || defined(__AVX512F__) || defined(__AVX2__) ||      \
+    defined(__SSE2__)
 #include <immintrin.h>
-#else
+#endif
+
 // Define fake functions to prevent the compiler from failing on undefined
-// functions in case SSE2 is not present.
+// functions in case the CPU extension is not present.
+#ifndef __AVX512BW__
 #define _mm512_cmpneq_epi8_mask(A, B) 0
-#define _mm_movemask_epi8(A) 0
+#endif
+#ifndef __AVX2__
 #define _mm256_movemask_epi8(A) 0
-#endif //  __SSE2__
+#endif
+#ifndef __SSE2__
+#define _mm_movemask_epi8(A) 0
+#endif
 
 namespace __llvm_libc::x86 {
 
 // A set of constants to check compile time features.
 static inline constexpr bool kSse2 = LLVM_LIBC_IS_DEFINED(__SSE2__);
 static inline constexpr bool kAvx = LLVM_LIBC_IS_DEFINED(__AVX__);
 static inline constexpr bool kAvx2 = LLVM_LIBC_IS_DEFINED(__AVX2__);
 static inline constexpr bool kAvx512F = LLVM_LIBC_IS_DEFINED(__AVX512F__);