[libc-commits] [PATCH] D104226: [libc] Add a set of elementary operations

[Guillaume Chatelet via Phabricator via libc-commits]

gchatelet marked 2 inline comments as done.
gchatelet added inline comments.


================
Comment at: libc/src/string/memory_utils/elements.h:355
+#ifdef __clang__
+#if __has_builtin(__builtin_memcpy_inline)
+#define USE_BUILTIN_MEMCPY_INLINE
----------------
courbet wrote:
> Do you really need to guard this with `#idef __clang__ ?`
So unfortunately `__has_builtin` only exists from GCC 10 on.
Before this it errors https://godbolt.org/z/5bvbro8jW

For clang it's supported from clang 3.0.

I can update it slightly by using `#if defined(__clang) || (defined(__GNUC__) && __GNUC__ >= 10)` but it doesn't add much value since GCC does not implement `__builtin_memcpy_inline` anyways.
Also I would still need to fallback to `__builtin_memcpy` in the GCC < 10 case below.


================
Comment at: libc/src/string/memory_utils/elements.h:358
+#endif
+#elif defined(__GNUC__)
+#define USE_BUILTIN_MEMCPY
----------------
courbet wrote:
> why not `#if __has_builtin(__builtin_memcpy)` ?
> why not `#if __has_builtin(__builtin_memcpy)` ?

As I explained above `__has_builtin` does not exist for most versions of GCC


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104226