This is an archive of the discontinued LLVM Phabricator instance.

[libc] Add a set of elementary operations
AbandonedPublic

Authored by gchatelet on Jun 14 2021, 6:58 AM.

Details

Reviewers
courbet
Summary

Resubmission of D100646 now making sure that we handle cases were __builtin_memcpy_inline is not available.

Diff Detail

Event Timeline

gchatelet created this revision.Jun 14 2021, 6:58 AM
gchatelet requested review of this revision.Jun 14 2021, 6:58 AM
gchatelet updated this revision to Diff 351868.Jun 14 2021, 7:34 AM
  • fix base commit
courbet added inline comments.Jun 14 2021, 7:42 AM
libc/src/string/memory_utils/elements.h
355

Do you really need to guard this with #idef __clang__ ?

358

why not #if __has_builtin(__builtin_memcpy) ?

gchatelet marked 2 inline comments as done.Jun 14 2021, 8:18 AM
gchatelet added inline comments.
libc/src/string/memory_utils/elements.h
355

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.

358

why not #if __has_builtin(__builtin_memcpy) ?

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

LGTM for libc/src/string/memory_utils/elements.h

gchatelet abandoned this revision.Jun 16 2021, 7:08 AM
gchatelet marked 2 inline comments as done.

This has been submitted as rG7fff39d9b046.

libc/test/src/string/memory_utils/elements_test.cpp