LGTM

LGTM

Addressed the comment and also added a test for Linux.

Done

LGTM

Ping?

LGTM

We're seeing a slightly different test failure on aarch64-linux-gnu:

FAIL: LLVM :: Transforms/Attributor/liveness.ll (27821 of 35033)
******************** TEST 'LLVM :: Transforms/Attributor/liveness.ll' FAILED ********************
Script:
--
: 'RUN: at line 2';   /b/s/w/ir/k/recipe_cleanup/clange57EIw/llvm_build_dir/bin/opt -attributor --attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=4 -S < /b/s/w/ir/k/llvm-project/llvm/test/Transforms/Attributor/liveness.ll | /b/s/w/ir/k/recipe_cleanup/clange57EIw/llvm_build_dir/bin/FileCheck /b/s/w/ir/k/llvm-project/llvm/test/Transforms/Attributor/liveness.ll --check-prefixes=CHECK,OLDPM
: 'RUN: at line 3';   /b/s/w/ir/k/recipe_cleanup/clange57EIw/llvm_build_dir/bin/opt -passes=attributor --attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=4 -S < /b/s/w/ir/k/llvm-project/llvm/test/Transforms/Attributor/liveness.ll | /b/s/w/ir/k/recipe_cleanup/clange57EIw/llvm_build_dir/bin/FileCheck /b/s/w/ir/k/llvm-project/llvm/test/Transforms/Attributor/liveness.ll --check-prefixes=CHECK,NEWPM
--
Exit Code: 2

As an alternative, I've also considered iterating over all dependencies inside add_llvm_install_targets and see if the prefix of that target is install- and handling it specially, but that seems like a more error prone solution.

LGTM

In D70450#1793699, @phosek wrote:

This change seems to have broken -ffixed-xX, we've noticed that our kernel started crashing with tip-of-tree Clang. After debugging, we found out that compiler would generate stores to x15 even though we compile with -ffixed-x15 which breaks the kernel because x15 is used to store per-cpu state. Would it be possible to revert or fix this issue ASAP please?

This change seems to have broken -ffixed-xX, we've noticed that our kernel started crashing with tip-of-tree Clang. After debugging, we found out that compiler would generate stores to x15 even though we compile with -ffixed-x15 which breaks the kernel because x15 is used to store per-cpu state. Would it be possible to revert or fix this issue ASAP please?

LGTM

In D71737#1793355, @JamesNagurne wrote:

This foreach loop goes over each variable, yes?
If so, then I'm not sure this change does what is required.

If the list variableNames is:

RUNTIMES_target+multi_XYZ
RUNTIMES_target_XYZ

This loop will:

1. variableName = RUNTIMES_target+multi_XYZ
2. Check if {name} matches, it does
3. Add -DXYZ=value to the list
4. variableName = RUNTIMES_target_XYZ
5. Check if {name} matches, it doesn't
6. Else, check if {target} matches, it does
7. Add -DXYZ=value to the list, overriding the expected value in 3 above
   
   Edit: That is to say, only one path will ever be executed per iteration of this loop, so the addition of else() doesn't change the behavior.

In D70744#1760792, @mstorsjo wrote:

If I read this correctly, this would break existing setups that build builtins only, by pointing cmake to compiler-rt/lib/builtins? Is it possible to handle the transition more gracefully?

This is an alternative to D71570.

In D71570#1792087, @phosek wrote:

I'm fine with this change in general, but I'm concerned that no matter what order we choose, this might break users who rely on the opposite order. Maybe it'd be better to modify the logic to skip the second loop if the first loop matches, that way we wouldn't rely on any particular order.

I'm fine with this change in general, but I'm concerned that no matter what order we choose, this might break users who rely on the opposite order. Maybe it'd be better to modify the logic to skip the second loop if the first loop matches, that way we wouldn't rely on any particular order.

I changed the name to LLVM_SYMBOLIZER_OPTS/LLVM_ADDR2LINE_OPTS since the short form seems to be more common among other tools.

In D71668#1790886, @jhenderson wrote:

I like the idea, but the behaviour needs adding to the llvm-symbolizer command guide. You may also want to consider adding it to the section on differences in llvm-addr2line.

Test case added.

This seems to be failing on aarch64-linux-gnu:

******************** TEST 'Clang :: CodeGenCXX/mangle-ptr-size-address-space.cpp' FAILED ********************
Script:
--
: 'RUN: at line 1';   /b/s/w/ir/k/recipe_cleanup/clangdgOoVq/llvm_build_dir/bin/clang -cc1 -internal-isystem /b/s/w/ir/k/recipe_cleanup/clangdgOoVq/llvm_build_dir/lib/clang/10.0.0/include -nostdsysteminc -fms-extensions -emit-llvm -triple aarch64-unknown-linux-gnu -o - /b/s/w/ir/k/llvm-project/clang/test/CodeGenCXX/mangle-ptr-size-address-space.cpp | /b/s/w/ir/k/recipe_cleanup/clangdgOoVq/llvm_build_dir/bin/FileCheck /b/s/w/ir/k/llvm-project/clang/test/CodeGenCXX/mangle-ptr-size-address-space.cpp --check-prefixes=CHECK
: 'RUN: at line 2';   /b/s/w/ir/k/recipe_cleanup/clangdgOoVq/llvm_build_dir/bin/clang -cc1 -internal-isystem /b/s/w/ir/k/recipe_cleanup/clangdgOoVq/llvm_build_dir/lib/clang/10.0.0/include -nostdsysteminc -fms-extensions -emit-llvm -triple x86_64-windows-msvc -o - /b/s/w/ir/k/llvm-project/clang/test/CodeGenCXX/mangle-ptr-size-address-space.cpp | /b/s/w/ir/k/recipe_cleanup/clangdgOoVq/llvm_build_dir/bin/FileCheck /b/s/w/ir/k/llvm-project/clang/test/CodeGenCXX/mangle-ptr-size-address-space.cpp --check-prefixes=WIN
--
Exit Code: 1

In D71135#1773192, @mgorny wrote:

If it causes link-time error, why CMake checks don't catch that?

We're seeing a different failure on our 2-stage bots (in second stage):

/b/s/w/ir/k/llvm-project/llvm/test/TableGen/GICombinerEmitter/parse-match-pattern.td:30:17: error: CHECK-LABEL: expected string not found in input
// CHECK-LABEL: Parsed rule defs/match for 'trivial'
                ^
<stdin>:1:1: note: scanning from here
llvm-tblgen: for the -d option: may not occur within a group!
^
<stdin>:3:11: note: possible intended match here
llvm-tblgen: Did you mean '-d'?
          ^

I'm not sure if the detection is working correctly, this is failing on our macOS toolchain bots:

FAIL: Clang :: Driver/darwin-ld-platform-version-ios.c (5284 of 16450)
******************** TEST 'Clang :: Driver/darwin-ld-platform-version-ios.c' FAILED ********************
Script:
--
: 'RUN: at line 1';   touch /b/s/w/ir/k/recipe_cleanup/clangybBdXS/llvm_build_dir/tools/clang/test/Driver/Output/darwin-ld-platform-version-ios.c.tmp.o
: 'RUN: at line 3';   /b/s/w/ir/k/recipe_cleanup/clangybBdXS/llvm_build_dir/bin/clang -target arm64-apple-ios12.3 -isysroot /b/s/w/ir/k/llvm-project/clang/test/Driver/Inputs/iPhoneOS13.0.sdk -### /b/s/w/ir/k/recipe_cleanup/clangybBdXS/llvm_build_dir/tools/clang/test/Driver/Output/darwin-ld-platform-version-ios.c.tmp.o 2>&1    | /b/s/w/ir/k/recipe_cleanup/clangybBdXS/llvm_build_dir/bin/FileCheck /b/s/w/ir/k/llvm-project/clang/test/Driver/darwin-ld-platform-version-ios.c
: 'RUN: at line 5';   /b/s/w/ir/k/recipe_cleanup/clangybBdXS/llvm_build_dir/bin/clang -target x86_64-apple-ios13-simulator -isysroot /b/s/w/ir/k/llvm-project/clang/test/Driver/Inputs/iPhoneOS13.0.sdk -### /b/s/w/ir/k/recipe_cleanup/clangybBdXS/llvm_build_dir/tools/clang/test/Driver/Output/darwin-ld-platform-version-ios.c.tmp.o 2>&1    | /b/s/w/ir/k/recipe_cleanup/clangybBdXS/llvm_build_dir/bin/FileCheck --check-prefix=SIMUL /b/s/w/ir/k/llvm-project/clang/test/Driver/darwin-ld-platform-version-ios.c
--
Exit Code: 1

LGTM

LGTM

LGTM

LGTM

We're seeing an assertion error in Clang when compiling compiler-rt builtins for aarch64-linux-gnu with this change:

LGTM

LGTM

Test added.

Sorry about the late response, I was mostly out last week. This recently came up again so I'd like to look into it but I'll have to rethink the change and come up with a better approach.

LGTM, thanks!

LGTM, thanks! Would it be possible to make the same change to libc++abi and libc++ as well (since they also use dependent library feature)?

@rupprecht added the --fallback-debug-path option in rL353730 which is AFAIK used for similar purpose as --debug-file-directory, except that it supports only a single directory, whereas in our use case we'd like to support searching through multiple directories.

LGTM

I assume that builtins and crt would be eventually moved to their own top-level directory (e.g. builtins or just compiler-rt) separate from sanitizers at which point this new CMakeLists.txt would become the root of that directory.

In D70575#1756072, @leonardchan wrote:

Could you add a test to show that with a fuchsia target we end up returning this from constructors + destructors and ensure that this ABI is used?

In D69740#1752348, @vsk wrote:

I think this looks good overall. The only high-level item that seems to be missing is Fuchsia testing -- are there any bots for this?