Much neater than my initial suggestion. LGTM

Minor suggestions, feel free to ignore.

Will fix the outstanding issues and commit tomorrow. Thanks for reviewing and testing.

fix typo

Drop APPLE special case

Fix bfloat16 case

Thanks, LGTM

Also handle bfloat16 (didn't notice this before since I was building with clang 14)

Thanks for working on this! How bad is the test diff if you remove the hardcoded --version=1 from lit.local.cfg?

I believe D145237 fixes the issue.

I've managed to reproduce this error on Linux, it turns out the real problem is that these checks are being done once even though the library is built multiple times with differing flags. I think the only way to fix this issue is to actually start using ExternalProject_Add (or adding an ARCH prefix for all the checks).

cmake -GNinja -S compiler-rt/lib/builtins -B cmake-build-builtins -DLLVM_CONFIG_PATH=NOTFOUND -DCMAKE_DISABLE_FIND_PACKAGE_LLVM=TRUE && ninja -C cmake-build-builtins

@emaste it looks like you landed an older diff?

LGTM

I would check the output of the no longer crashing tests but otherwise LGTM. Please wait for @arsenm to confirm as well before committing though.

The __builtin_align_{up,down} code generation could also make use of this IIRC. I think the reason I didn't do this initially was concerns about ptrmask not being optimized as well.

In D143437#4145600, @krzysz00 wrote:

@arsenm, I can try to put together tests for some of the remaining stuff (like the ICmp merge). However, unless I'm forgetting something, the remaaining untested bits are the ones that call accumulateConstantOffset(), which, per https://github.com/llvm/llvm-project/blob/main/llvm/lib/IR/Operator.cpp#L94-L96 , will crash if you mix up pointer bitwidth and index bitwidth. I'm not sure that fixing up those crashes (when most of the other code in LLVM has been fixed for this) justifies a test?

Thanks for fixing a hardcoded AS 0! Code change looks correct to me but the test case can be reduced further.

LGTM, but I'll let @dim and @emaste decide if FreeBSD 8 code is still needed.

I am almost certain backtrace() does work on FreeBSD, so there should be a __linux__ check as well.
The comment says it only exists on glibc systems, but on FreeBSD libexecinfo provides this functionality.

Thanks for this patch! In the CHERI fork I added a rather ugly workaround in getIngPtrType() for this instead of updating all callers (https://github.com/CTSRD-CHERI/llvm-project/blob/19d402e23fcaa197e1d40547da403dc17e13c7ae/llvm/lib/IR/DataLayout.cpp#L862). I will try applying the patch to CHERI LLVM to see if there are any further places that need updating.

It sounds to me that the logic from b919c7d should have been restricted to actual LLVM intrinsics that are overloaded on the pointer args ( e.g. llvm.memcpy which was the test in that commit). Builtins that are just library calls with known semantics probably need the cast even for opencl?

I like this approach, it's a lot less hacky than my attempt to default function signatures to on. I'll rebase that to check for version>1 instead.

In D142422#4076305, @kib wrote:

use SIZEOF_LONG == 8 instead of LP64 for CHERI.

Using __LP64__ means we won't use umtx for CHERI architectures. Can we change this to __SIZEOF_LONG__ == 8?

Looks like this is obsolete now that D140985 landed, will rebase the remaining patch stack.

The difference in codegen is rather surprising, but looking at the diffs they are all semantically the same.

This commit breaks the build for me (and some of the buildbots): https://lab.llvm.org/buildbot/#/builders/16/builds/41419/steps/5/logs/stdio

In D138196#4029503, @smeenai wrote:

In D138196#4019550, @thakis wrote:

cstdlib only has the following using ::aligned_alloc _LIBCPP_USING_IF_EXISTS;, so I assume it would also fail with std::aligned_alloc(). I don't have a chromium checkout, but maybe the C library that your build is using doesn't provide aligned_alloc() ? Or maybe it requires an additional macro to expose it? Otherwise you could add another opt-out case to libcxx/include/__config (like the macos < 10.15 one from this commit).

It's broken on android (bionic) and chromeos (some glibc, I think?).

libcxx/test/std/depr/depr.c.headers/stdlib_h.aligned_alloc.compile.pass.cpp claims "::aligned_alloc is available starting with Android P (API 28)". We support API level 24, so we can't use that yet.

So I think this breaks most people doing Android builds, so it'd probably be good if _LIBCPP_HAS_NO_C11_ALIGNED_ALLOC was set correctly automatically for at least Android.

(But we'll just define _LIBCPP_HAS_NO_C11_ALIGNED_ALLOC ourselves for now.)

CC @danalbert and @rprichard. (We hit the same breakage as well.)

Haven't updated all tests this affects yet, wanted to check if this is an acceptable change before spending more time on it.

Found using D141060

Rebase, fix unsused variable.

In D138196#4013628, @thakis wrote:

This breaks our build (c++20): https://ci.chromium.org/ui/p/chromium/builders/try/android-binary-size/1337773/overview

Since this includes cstdlib instead of stdlib.h, is :: the right qualification? Should it look in std instead?

Looks like I was too slow - already committed as rG5b97612a11e88c5a035611a824479490b22c9905

This fix should hopefully be correct, but opening a review in case there is something else that I need to change.

In D140212#4007438, @asb wrote:

Some general thoughts:

Although I understand the concerns about churn from updating auto-generated tests, I was concerned that an approach like this might have greater disadvantages than just landing a patch that regenerates everything (after all, churn in auto-generated tests has far fewer practical issues vs e.g. clang-formatting - conflicts are easily resolved by regenerating).

Though looking through the implementation, it doesn't seem overly complicated and seems like a sensible path forwards. I wouldn't be supportive of using such an approach for all future behaviour changes though - the complexity would just accrete and it makes the behaviour of the tool harder to predict.

rebase for CI

In D138196#4009494, @ldionne wrote:

Gentle ping before this becomes too hard to rebase :-)

Rebase and update version guard to macos 10.15

Seeing as there are around 300 tests inside llvm/ that use --function-signature and 200+ in clang/, maybe this needs a "only for new tests" workaround such as D140212.
The other option would be to bulk-update all affected tests once this lands, but I'm not sure how the maintainers of the affected tests feel about this.

In D140218#4001835, @jdoerfert wrote:

I don't understand why we would remove the flag for _cc_. I feel all these patches are going exactly in the opposite direction I would think one would go.

I like the idea of defaulting to --function-signature for new tests a lot, so I've implemented that in D140212

In D128084#3999857, @emaste wrote:

Quick status update, with D140117 applied I still have the following failures:

Failed Tests (31):
  llvm-libc++-shared.cfg.in :: libcxx/atomics/bit-int.verify.cpp
  llvm-libc++-shared.cfg.in :: libcxx/experimental/fexperimental-library.compile.pass.cpp
  llvm-libc++-shared.cfg.in :: std/algorithms/alg.modifying.operations/alg.transform/ranges.transform.pass.cpp
  llvm-libc++-shared.cfg.in :: std/input.output/stream.buffers/streambuf/streambuf.protected/streambuf.put.area/pbump2gig.pass.cpp
  llvm-libc++-shared.cfg.in :: std/thread/futures/futures.task/futures.task.members/ctad.static.compile.pass.cpp
  llvm-libc++-shared.cfg.in :: std/time/time.cal/time.cal.month/time.cal.month.nonmembers/ostream.pass.cpp
  llvm-libc++-shared.cfg.in :: std/time/time.syn/formatter.day.pass.cpp
  llvm-libc++-shared.cfg.in :: std/time/time.syn/formatter.duration.pass.cpp
  llvm-libc++-shared.cfg.in :: std/time/time.syn/formatter.month.pass.cpp
  llvm-libc++-shared.cfg.in :: std/time/time.syn/formatter.weekday.pass.cpp
  llvm-libc++-shared.cfg.in :: std/time/time.syn/formatter.year.pass.cpp
  llvm-libc++-shared.cfg.in :: std/utilities/expected/expected.expected/assign/assign.U.pass.cpp
  llvm-libc++-shared.cfg.in :: std/utilities/expected/expected.expected/assign/assign.copy.pass.cpp
  llvm-libc++-shared.cfg.in :: std/utilities/expected/expected.expected/assign/assign.move.pass.cpp
  llvm-libc++-shared.cfg.in :: std/utilities/expected/expected.expected/assign/assign.unexpected.copy.pass.cpp
  llvm-libc++-shared.cfg.in :: std/utilities/expected/expected.expected/assign/assign.unexpected.move.pass.cpp
  llvm-libc++-shared.cfg.in :: std/utilities/expected/expected.expected/assign/emplace.pass.cpp
  llvm-libc++-shared.cfg.in :: std/utilities/expected/expected.expected/ctor/ctor.inplace_init_list.pass.cpp
  llvm-libc++-shared.cfg.in :: std/utilities/expected/expected.expected/ctor/ctor.unexpect_init_list.pass.cpp
  llvm-libc++-shared.cfg.in :: std/utilities/expected/expected.expected/dtor.pass.cpp
  llvm-libc++-shared.cfg.in :: std/utilities/expected/expected.expected/swap/free.swap.pass.cpp
  llvm-libc++-shared.cfg.in :: std/utilities/expected/expected.void/assign/assign.copy.pass.cpp
  llvm-libc++-shared.cfg.in :: std/utilities/expected/expected.void/assign/assign.move.pass.cpp
  llvm-libc++-shared.cfg.in :: std/utilities/expected/expected.void/assign/assign.unexpected.copy.pass.cpp
  llvm-libc++-shared.cfg.in :: std/utilities/expected/expected.void/assign/assign.unexpected.move.pass.cpp
  llvm-libc++-shared.cfg.in :: std/utilities/expected/expected.void/assign/emplace.pass.cpp
  llvm-libc++-shared.cfg.in :: std/utilities/expected/expected.void/ctor/ctor.unexpect_init_list.pass.cpp
  llvm-libc++-shared.cfg.in :: std/utilities/expected/expected.void/dtor.pass.cpp
  llvm-libc++-shared.cfg.in :: std/utilities/expected/expected.void/swap/free.swap.pass.cpp
  llvm-libc++-shared.cfg.in :: std/utilities/expected/expected.void/swap/member.swap.pass.cpp
  llvm-libc++-shared.cfg.in :: std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/ctad.static.compile.pass.cpp

I'll iterate on adjusting XFAILs before submitting a final version of this change

As this is a rather simple cleanup, I'll go ahead and merge this next week unless there is any objection until then.

rebase