[libc-commits] [PATCH] D137868: [libc] Fix builtin definition for memory functions

[Guillaume Chatelet via Phabricator via libc-commits]

gchatelet added inline comments.


================
Comment at: libc/cmake/modules/LLVMLibCCheckCpuFeatures.cmake:9
 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)
----------------
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



================
Comment at: libc/src/string/memory_utils/op_x86.h:29
+// functions in case the builtin is not present.
+#ifndef _mm512_cmpneq_epi8_mask
 #define _mm512_cmpneq_epi8_mask(A, B) 0
----------------
lntue wrote:
> 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?
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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137868/new/

https://reviews.llvm.org/D137868