Page MenuHomePhabricator

[clang] Override the Fuchsia platform ABI using the triple environment
AcceptedPublic

Authored by leonardchan on Dec 21 2020, 3:36 PM.

Details

Summary

This is an alternative to D85802 which added a flag for selecting between C++ ABIs at compile-time. This instead allows for overriding the platform ABI on a Fuchsia system, rather than overriding the C++ ABI.

This will be used as a way to integrate code built with Clang with code built by other compilers that don't contain Fuchsia-specific ABI changes. The immediate use case is for generating multilibs that use the generic Itanium ABI to be used by code built by GCC.

Diff Detail

Event Timeline

leonardchan created this revision.Dec 21 2020, 3:36 PM
leonardchan requested review of this revision.Dec 21 2020, 3:36 PM
rnk added a comment.Jan 6 2021, 11:21 AM

Generally makes sense, but I had a concern.

clang/include/clang/Basic/LangOptions.h
333 ↗(On Diff #313210)

Why isn't this part of the LangOptions.def xmacro list?

phosek added a comment.Jan 6 2021, 1:52 PM

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.

rnk added a comment.Jan 6 2021, 1:58 PM

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?

phosek added a comment.Jan 6 2021, 3:42 PM
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?

We've been considering the use of WASM as a binary format in Fuchsia at which point we'd need to decide which C++ ABI to use in those cases. There are no concrete plans yet, but I want to make sure that whatever solution we come up with doesn't result in multiple new flags down the line.

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.

@mcgrathr Would you be fine with using the triple instead of a new flag in this case?

It's fine to have a target tuple translate to a default C++ ABI.
But the C++ ABI selection is fundamentally not a target flavor thing. It's just a C++ ABI thing.
So using the target tuple as the sole mechanism to determine C++ ABI is fundamentally wrong.

rnk added a comment.Jan 8 2021, 11:05 AM

It's fine to have a target tuple translate to a default C++ ABI.
But the C++ ABI selection is fundamentally not a target flavor thing. It's just a C++ ABI thing.
So using the target tuple as the sole mechanism to determine C++ ABI is fundamentally wrong.

Generally speaking, the C++ ABI corresponds to the target tuple. I don't want to give the user separate control of the C++ ABI without a compelling use case. If a command line flag is available, users will set it, they will request the AArch64 or Microsoft C++ ABI on x64, they will create a novel, never-before-previously-tested configuration, and when it doesn't work, they will file bugs about it. I aim to head that off at the pass. The two compelling use cases that I've heard of so far are:

  1. vanilla Itanium C++ ABI code on Fuchsia
  2. targeting fuchsia using wasm, some Fuchsia C++ ABI features may be needed

I think both of those cases can be handled with target tuples: *-*-fuchsia-itanium or webassembly-*-fuchsia
Do you think we need another way of expressing this?

You've said you want to avoid expressing the C++ ABI as a generic switch orthogonal to target because people can use it when it's not what they actually want or isn't actually useful to attempt. This is true of all switches affecting ABI and they haven't all been rejected on this basis. But fine, consistency is the hobgoblin of small minds. If the concern there is either user confusion or support burden for configurations nobody wants to test, then both of those are easily addressed by having -fc++abi= constrain the settings it allows based on target. For Fuchsia targets, we want to maintain and test both "fuchsia" and "compat" (or "itanium") modes. If for other targets the only setting allowed is the one that's the default, or if using non-default settings generates a scare warning or suchlike, that's fine. That seems to be the natural way to constrain the explicit control of the C++ ABI to the supported cases, while not conflating control of the C++ ABI with the language-independent platform ABI being targeted.

You've said you want to avoid a proliferation of explicit switches for specific per-target C++ ABI selections. This is in fact the original rationale for -fc++abi=. Instead you've proposed a proliferation of specific per-target pseudo-tuples. This is exactly equivalent in material effect except that it's more hidden because possible tuples don't appear in --help output like separate switches do. It's worse in the abstract sense of correctness and in the self-documenting sense of being misleading and confusing by conflating the language-specific ABI with the platform ABI. You've cited the precedent of the mingw tuples. I'm not sufficiently familiar with the mingw cases to know whether they in fact are about nothing but the language ABI or are in fact actually about properly different platform ABIs in some other sense. If the latter, then your proposal exemplifies and worsens the kind of confusion between platform ABI (which tuples are explicitly about and always have been) with language ABI (which is outside the scope of what tuples were specified to describe) that I'm objecting to. If the former, then this bad precedent is a sign this problem has already caused the confusion I warned about and IMHO a precedent of doing the wrong thing is not a reason to compound the error.

leonardchan retitled this revision from [clang] Add -ffuchsia-c++-abi flag to explicitly use the Fuchsia C++ ABI to [clang] Override the Fuchsia platform ABI using the triple environment.
leonardchan edited the summary of this revision. (Show Details)

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.

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 the context of representing the fourth component of the triple to represent the *platform* ABI rather than exclusively the C++ ABI, I think this would make sense. Using this other component would essentially just mean adding another layer of default flag configurations whose default values differ from those set by the first three components. I think we'd only want to have this extra layer if we have a sizable number of flags that we'd want separate from the default triple.

For our case, I think this mainly includes

  • the C++ ABI (which encompasses "return this" on ctors/dtors and relative vtables),
  • SS/SCS
  • and maybe the PIC-friendly lookup tables if that ends up getting hidden behind a switch

but there could be new ABI changes down the line that could fall under this (or others that exist now but don't come to mind right now). Unless I'm missing some other features, this list seems fairly small and might not be worth tucking this under something that's meant to control a set of flags, but if we care more about being preventative from adding new flags, then it seems fine to just stick with using the triple.

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/

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/

I don't think we would use compat multilib in that case, instead we would simply use multiarch since these are now distinct targets. That is we would use just ${BUILD_DIR}/bin/../lib/x86_64-fuchsia-gnu/c++.

The Fuchsia platform ABI includes the SS & SCS ABI. There is no compatibility issue with those. PIC-friendly optimizations are purely an optimization and correctness issue for any PIC/PIE code, and is not an ABI issue.
The only thing where we have an incompatible difference in ABI between our compiler environments is the C++ ABI.

rnk added a comment.Jan 21 2021, 1:42 PM

I guess the spelling -fc++-abi= is intuitive, discoverable, and easy to use. I'd be OK going back to that, as long as the compiler knows which ABIs work on which targets and rejects the unsupported configurations.

Regarding the idea of separating the platform ABI from the C++ ABI, I can't say I really believe in the idea of language neutral platform ABIs. The platform ABI always incorporates aspects of the most widely used source languages, typically C. This is where LLVM gets into trouble when non-C frontends want to pass _Complex values or structs as arguments, for example. Whether you think of C++ features as being part of the platform or not probably depends on your view on whether C++ support is a critical part of the platform. I tend to think of C++ as pretty critical, but I'm a C++ programmer working on C++ ABI support, so that's my view.

Regarding reducing global ABI switches, yes, I would like to minimize them. We have many, and they have costs.

phosek accepted this revision.Jan 21 2021, 1:46 PM

LGTM

This revision is now accepted and ready to land.Jan 21 2021, 1:46 PM

I am also a C++ programmer working on C++ ABI support. I'm glad to be working on a platform that made many intentional design decisions to enable keeping the C++ ABI out of the platform ABI. It makes it vastly easier to do interesting work on C++ ABIs such as what Leo is doing. It also makes it significantly easier to do interesting work on platform ABIs, which I also do.