Almost there! Just a few more comments on my end.

Hi. It seems this commit is causing this failure on our aarch64 bulder (https://luci-milo.appspot.com/p/fuchsia/builders/ci/clang-linux-arm64/b8857452110716658368):

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.

In D93668#2492081, @phosek wrote:

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.

In D91466#2395297, @pcc wrote:

How does Zircon handle tagged addresses in syscalls? Are they handled equivalently to Linux's tagged address ABI?

Locally I'm able to build and run the bringup.arm64 config (that is launch the shell and run some commands).

Looking good. Just a few more comments on my end.

In D93668#2482986, @phosek wrote:

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

With dedicated flags, you might eventually end up in a similar situation as D85802, that is in the extreme case you might end up with -f[no-]fuchsia-c++-abi, -f[no-]webassembly-c++-abi, etc. which is not any better than -fc++-abi=.

With target triple, I can imagine using either <arch>-unknown-fuchsia-itanium or <arch>-unknown-fuchsia-gnu, where the former would mean targeting Fuchsia with Itanium C++ ABI while the latter would mean using GCC compatible ABI (which would imply Itanium C++ ABI). Both of these are already used by MinGW for the same purpose so there's a precedent and we don't need to invent anything new.

Ok this time I remembered the email. Unsure why the bug isn't automatically closed though.

Committed in 155fca3cae275562e626d3e4fbfac70b4b75d2e7. (Sorry, I forgot to change the author email in the commit)

Sorry. I accidentally missed this in my emails. Will commit these.

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

LGTM. Thanks

Thanks for catching this. LGTM

Removed formatting for existing code.

In D91747#2412212, @rnk wrote:

Does rG64802d48d51d651bd2e4567b2f228f8795569542 fix it?

Hi, it seems that p2.cpp is failing on our production builder (https://luci-milo.appspot.com/p/fuchsia/builders/prod/clang-linux-x64/b8862835022166171808):

LGTM but you should probably also wait for @aaron.ballman or @rsmith to also chime in.

*ping* any more comments on this patch?

*ping* Any more comments on this patch?

@rjmccall This might've changed a bit since you last accepted the revision. Will wait a couple of days before submitting if you have any more comments on this.

Hi. I suspect it might be this patch or D90984 that might be leading to the test failure we're seeing on our arm64 builders

Welp, it turns out that if I went with the approach of adding the constant manually everywhere the macro was used, I'd be adding a lot of special cases everywhere. The only case I needed to cover was in Core which depends on Value.def:

/usr/local/google/home/leonardchan/llvm-monorepo/llvm-project-1/llvm/lib/IR/Core.cpp:832:12: note: expanded from macro 'HANDLE_VALUE'
    return LLVM##Name##ValueKind;
           ^
<scratch space>:239:1: note: expanded from here
LLVMDSOLocalEquivalentValueKind

The only way I could think of preventing the constant from being used here while still being used in other places HANDLE_VALUE/CONSTANT are used is a "banlist" approach via more macros in Value.def. Open to other ideas if it doesn't look that good.

In D91468#2395487, @Meinersbur wrote:

Is this obsolete after D91470 has been committed?

LGTM

Are there any OpenACC specific tests?

There was an inconsistency in the LLVMBuild.txt for LLVM Frontend: OpenACC wasn't listed but it was part of the CMake infrastructure. A side effect of the llvmbuildectomy has been to remove that inconsistency and register OpenACC as a regular component.

Looking at the error log, there is a missing libLLVMFrontendOpenACC.a
But I cannot see any reference to FrontendOpenACC in the build log, just as if it wasn't build, which is odd.
I failed to reproduce your issue, but does that OpenACC stuff rings any bell? Can you force building that component and see what happens?

Hi, it looks like this might be leading to some test failures on our toolchain builders:

LGTM

LGTM

In D77248#2390009, @MaskRay wrote:

Apologies for my late reply. A couple of comments inlined

See RFC for more details.

Neither the RFC nor the patch mentions the immediate use case. It is possible that the merit is buries in a deep message but for a few feature the description itself should justify its value (dblaikie mentioned that:

We should do this because I need it" (we shouldn't be implementing features for especially niche use cases/if they don't generalize) isn't always a compelling motivation but "we should do this because someone might need it" isn't either (we shouldn't be implementing features that have no users).

Please add the use cases.

Updated some renamed variables that I didn't check prior.

Rebased

In D90959#2383910, @gkistanova wrote:

Specifying 32 jobs as a hard coded value in the build factory does not look right. You may want to parametrize this somehow.

Updated to use LIT_OPTS.

In D90864#2376653, @vitalybuka wrote:

Sorry, feel free to land fixes like this without review.

Abandoned since https://reviews.llvm.org/D90695 fixes this

@MaskRay any more comments?

In D90695#2371840, @vitalybuka wrote:

Sorry for that.
Does fuchsia has any build bots? Would be nice to get automatic email about such breaks.

Unsure if this is the best way to go about it, so open to other solutions.

In D77248#2358172, @MaskRay wrote:

llc -filetype=obj doesn't. Please fix it. It is a bit unfortunate but the ELFObjectWriter generally has more restrictions than the assembler writer.

I'd recommend splitting this into two patches, one for LLVM changes and another for Clang changes.

*ping* for other comments. I'll probably commit this at the end of the week unless others have more to add.

Update to use dso_local_equivalent instead of emitting stubs in the frontend. dso_local_equivalent should emit a stub automatically if one is required for the target (ie. it does not support PLTs).

Updated to use the -fexperimental-relative-c++-ab-vtables flag now that D85802 was reverted. This will be used for the purpose of testing the relative vtables ABI in Fuchsia CQ builders to ensure all tests are run, accounting for flakes. The end goal will involve not having a relative vtables multilib, but instead making it the default for the fuchsia ABI, and instead having a separate Itanium multilib for supporting GCC.