In D76802#2514717, @phosek wrote:

In D76802#2201170, @pcc wrote:

In D76802#2201164, @phosek wrote:

In D76802#2200926, @pcc wrote:

In D76802#2200906, @phosek wrote:

@davidxl does this look good to you? This feature is now gated on -counter-associated-metadata flag (I'd welcome suggestions for better name if you have some) because we require recent lld. We might be able to set that flag in the frontend when we know that the latest lld is being used in the future.

Can you find a better way of communicating this infomation than via an llvm::cl flag (e.g. use a function attribute)? The flag won't be compatible with LTO.

Do I understand your suggestion correctly that we would set function attribute when -fprofile-*-generate -fuse-lld is used that would enable the use of metadata in the backend?

Yes, that's what I had in mind.

I was thinking that maybe a better approach would be to provide a new flag akin to -fbinutils-version introduced D85474, for example -flld-version. We would then emit the metadata unconditionally. In the backend, we would then check if LLD version is greater than 12 or if binutils version is greater than 2.36, and if not we would avoid using SHF_LINK_ORDER in this case effectively ignoring the metadata. During LTO, LLD would pass its own version to the backend via TargetOptions. WDYT?

In D76802#2201170, @pcc wrote:

In D76802#2201164, @phosek wrote:

In D76802#2200926, @pcc wrote:

In D76802#2200906, @phosek wrote:

@davidxl does this look good to you? This feature is now gated on -counter-associated-metadata flag (I'd welcome suggestions for better name if you have some) because we require recent lld. We might be able to set that flag in the frontend when we know that the latest lld is being used in the future.

Can you find a better way of communicating this infomation than via an llvm::cl flag (e.g. use a function attribute)? The flag won't be compatible with LTO.

Do I understand your suggestion correctly that we would set function attribute when -fprofile-*-generate -fuse-lld is used that would enable the use of metadata in the backend?

Yes, that's what I had in mind.

Note that there's one additional issue right now. The TableGen cross-compilation sub-invokes CMake which fails with:

CMake Error at /src/clang-llvm/llvm-project/libc/CMakeLists.txt:49 (message):

LGTM

In D93668#2510463, @leonardchan wrote:

Also, this probably isn't very important, but just as a heads up: if we were to depend on the environment component in the triple as the flag for selecting which multilib to use, then the multilib would be stored in a directory containing that environment.

So assuming we would check for the flag -target x86_64-fuchsia-gnu the multilib path would be something like:

${BUILD_DIR}/bin/../lib/x86_64-fuchsia-gnu/c++/compat+noexcept/

rather than

${BUILD_DIR}/bin/../lib/x86_64-fuchsia/c++/compat+noexcept/

@keith do you plan on landing this?

In D94461#2491947, @plotfi wrote:

In D94461#2491946, @plotfi wrote:

1. I think for unification, the first thing would be unifying the text abi file format. Currently, the YAML format used by ifs is very similar to the one used in InterfaceStub(elfabi), however there are not compatible. One thing is that ifs embeds llvm triples in the text ifs file while elfabi does not do that. Elfabi get platform and endianness information from command line instead of getting them from the text stub file. We did that because there are use cases that we generate ELF stubs for multiple platforms using a single set of text abi files. Is there a specific reason why ifs text files require llvm triples? Is it feasible to make llvm-ifs to get the platform information from the command line options?

The triple-ness was added because originally IFS was intended to be a tool that the clang driver invokes. The idea was that you build some project with IFS enabled and you automatically get the stubs for shipping or for CI purposes to track what new exposure was done in recent builds of said project. I think it would be ok to add a flag to override the triple if that isn't there already. @compnerd
thoughts?

I meant to add, as part of llvm-ifs being invoked by clang the idea was that clang would just forward its triple and llvm-ifs would decide what to do with it because we wanted the ifs format to be kinda platform agnostic but we wanted llvm-ifs to potentially be able to generate stubs for multiple platforms in the future (like MS .libs and tbds etc too).

I agree that if we want to allow selecting C++ ABI only, a flag like -fc++abi= with some additional checks to disallow invalid combinations is better. The question is whether we want to allow that in the first place? The motivation behind this change is to support generating code that's ABI compatible with the code generated by GCC. That could be expressed using the target triple (one idea is to use *-*-fuchsia-gnu), which would cover both C++ ABI as well as other aspects of the ABI (for example it could also disable SafeStack/ShadowCallStack) so developers don't need to concern themselves with all the details of the ABI, which could evolve over time expanding the set of flags you'd need to pass to the compiler to generate code that's ABI compatible with GCC.

LGTM

LGTM

LGTM

In D93668#2482995, @rnk wrote:

I guess a triple of -fuchsia-itanium would be a reasonable way of expressing this.

Why would we want a feature flag for the wasm C++ ABI? Is there a use case for using the webassembly C++ ABI on non-wasm ISAs?

I'd prefer to use the target triple rather than introducing a custom flag.

This is fine with me, but I wonder we should avoid setting this property for every target and instead rely on CMAKE_BUILD_WITH_INSTALL_RPATH (we could set CMAKE_BUILD_WITH_INSTALL_RPATH to ON if unset to preserve the current behavior).

In D92792#2475182, @stephan.dollberg wrote:

Ping @phosek

LGTM

LGTM, thanks!

In D92291#2423417, @leonardchan wrote:

@phosek This shouldn't affect supporting riscv with our toolchain, right?

LGTM

LGTM

LGTM

In D91555#2397902, @pcc wrote:

What is the use case?

LGTM we may consider implementing -fsource-dir in terms of -fprofile-prefix-map as suggested in D87928 so I think we should go ahead and land this.

In D87928#2394913, @dexonsmith wrote:

I missed this before, and it's an interesting problem.

I have a few questions:

• I'm not clear on the expected interaction between the current working directory of the compiler and -fsource-dir. What happens if they're different?

LGTM

Ping? Is it OK to land this?

LGTM

I agree with your analysis and I'd also prefer Solution (1), I've rebased the change and included a simple test.

LGTM

LGTM

@rnk @vsk does this look good to you as well?

LGTM with a few nits.

I don't think this is going to help, lit tasks are running inside the console Ninja pool so they're already limited to a single job. Instead, we would need to pass -j to lit through LIT_OPTS environment variable, see http://llvm.org/docs/CommandGuide/lit.html#general-options.

In D88922#2372946, @mstorsjo wrote:

Any update regarding the fact that it now builds ubsan_type_hash_win.cpp with -nostdinc++, despite the fact that the file does include <typeinfo>?

I have made the same change which is slightly simpler, see D90695, so LGTM.

LGTM

What's the plan regarding updating scan-build-py to match the upstream? It seems like this issue has already been address in the upstream version.

LGTM

Thanks a lot for landing this!

LGTM

LGTM

After many experiments, I finally got something that seems to be building with the buildbot_android.sh script.

LGTM

LGTM

LGTM do you want me to land this change for you?

In D89432#2350758, @plotfi wrote:

Nice. Will you folks be updating llvm-ifs to use this support instead of yaml2obj soon? If so add me to the review, I will be happy to take a look.

LGTM but someone else should take a look as well.

I already talked to @ldionne over chat. D88922 is a larger cleanup and I'm still trying to get it to a working state, but I think we should land this change in the meantime to address the current failures.

LGTM

@ldionne I've started doing a related cleanup of compiler-rt in D88922 but I had to revert it because it broke various builders that use custom flags and I'm still working through those issues.

LGTM

@ldionne do you plan to land this change?

I apologize for the late response, I somehow missed the earlier responses. We have successfully used this feature in Fuchsia and found it useful, but I agree that the issues raised need to be addressed. Unfortunately @paulkirth is no longer working on this project. I hope that someone from our team can take a look but it might take a few weeks. If you prefer, we could revert this change and then reland an improved version in the future?

In D88922#2334106, @vitalybuka wrote:

In D88922#2334076, @phosek wrote:

In D88922#2331869, @vitalybuka wrote:

FAILED: lib/fuzzer/CMakeFiles/RTfuzzer.i386.dir/FuzzerFork.cpp.o 
/usr/local/google/home/vitalybuka/slow/bbot/llvm_build64/bin/clang++   -I/usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/compiler-rt/lib/fuzzer/../../include --target=i686-linux-android --sysroot=/usr/local/google/home/vitalybuka/slow/bbot/android_ndk/standalone-i686/sysroot -B/usr/local/google/home/vitalybuka/slow/bbot/android_ndk/standalone-i686  -Wall -Werror -std=c++14 -Wno-unused-parameter -O3 -DNDEBUG    -fPIC -fno-builtin -fno-exceptions -fomit-frame-pointer -funwind-tables -fno-stack-protector -fno-sanitize=safe-stack -fvisibility=hidden -fno-lto -O3 -gline-tables-only -Wno-gnu -Wno-variadic-macros -Wno-c99-extensions -Wno-non-virtual-dtor -Dthread_local=__thread -MD -MT lib/fuzzer/CMakeFiles/RTfuzzer.i386.dir/FuzzerFork.cpp.o -MF lib/fuzzer/CMakeFiles/RTfuzzer.i386.dir/FuzzerFork.cpp.o.d -o lib/fuzzer/CMakeFiles/RTfuzzer.i386.dir/FuzzerFork.cpp.o -c /usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/compiler-rt/lib/fuzzer/FuzzerFork.cpp
In file included from /usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/compiler-rt/lib/fuzzer/FuzzerFork.cpp:11:
In file included from /usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/compiler-rt/lib/fuzzer/FuzzerCommand.h:15:
/usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/compiler-rt/lib/fuzzer/FuzzerDefs.h:14:10: fatal error: 'cassert' file not found
#include <cassert>

I couldn't reproduce this failure locally. There's also no -nostdinc++ flag being used here so it's not clear to me why this is failing. Do you know if Android provides C++ standard library as part of its sysroot?

did you try zorg script ?