[PATCH] D89490: Introduce __attribute__((darwin_abi))

[John McCall via Phabricator via llvm-commits]

rjmccall added a comment.

Sorry, it's just been a busy month for me.

In D89490#2539950 <https://reviews.llvm.org/D89490#2539950>, @aguinet wrote:

> In D89490#2516255 <https://reviews.llvm.org/D89490#2516255>, @rjmccall wrote:
>
>> In D89490#2514695 <https://reviews.llvm.org/D89490#2514695>, @aguinet wrote:
>>
>>>> I may be over-reacting to the way the patch seemed to be touching on the C++ ABI in multiple places.  My understanding is that `ms_abi` is just a calling-convention attribute; it's basically "use the (default) calling convention that MSVC would use for this function".  If that's all you want, then this is reasonable, although I am worried about creating a new attribute for every system that Wine chooses to target.
>>>
>>> I literally based this patch on how ms_abi was implemented. It's unfortunately more than just teaching clang to change the calling convention on LLVM IR functions. The fact that ABI implementations are spread all over the place between various places in LLVM is, as far as I remember, a known problem discussed many times on llvm-dev, and looks like a hard one to fix.
>>
>> Right, I understand that — I've even given an LLVM talk about it.  I was just confused about your intent because you made some effort to match other parts of the ABI besides what we might traditionally consider the calling convention.  I suppose some of those changes could be thought of as feature-specific calling conventions, but I don't usually think of them that way.
>
> We could have a long discussion on what do we exactly call a "calling convention" :) IIRC I did implement a few C++ bits, but they could be removed from this patch for further implementation.
>
>>>> About "darwin": technically, every Apple platform has a different ABI.  Our current ARM64 platforms do all agree about things like the calling convention, at least if you count newer watches (which use a 32-on-64 ABI in userspace) as not ARM64.  That is not true of other architectures, most notably on ARM32, where the 32-bit iOS ABI is very different from the armv7k Apple Watch ABI; and it is certainly conceivable that Apple might release a new ARM64 platform in the future with a different calling convention.  The more technically correct and future-proof thing would be to use the OS name (or maybe even the triple!) in the attribute, probably as an argument to the attribute, like `__attribute__((target_abi("arm64-apple-ios")))`.
>>>
>>> I'm a bit afraid that `__attribute__((target_abi(XX)))` would conflict with the existing `__attribute__((ms_abi))`.
>>
>> They don't *conflict*.  It's a more general scheme than the existing attribute, but the existing attribute can be thought of as a shorthand for one case of that more general scheme, so I don't see a problem here.  I would like to not have to add a new attribute for every OS that Wine decides to support.
>
> You're right, let me rephrase what I wanted to say: with this, there will be two ways to target the windows ABI. I guess we can be fine with it, but that adds some kind of complexity that needs to be explained to the end user.
>
> There is also the support the equivalent of the `__builtin_ms_va_list` type that needs to be considered, which would make a type that could look like `__builtin_abi_va_list_INSERT_TRIPLE_HERE`, with the fact that we can't have '-' in types. It might be better to do something similar than `ext_vector_type`, and have something like:
>
>   typedef va_list __attribute__((target_abi("arm64-apple-ios"))) arm64_ios_abi_va_list; 

Ah.  That's tricky because `va_list` is not normally abstracted in the compiler representation; the target just gives an implicit prefix that's parsed before the translation unit, and that defines the `va_list` type.  If you want easy OS ABI portability here, you'll need to come up with a way in the source language to spell the different `va_list` types and to select which `va_next` implementation you want (`va_start` has to match the current function, but `va_next` doesn't).  Fortunately we lower `va_next` in the frontend, so LLVM only needs to know about `va_start`, and that's always determined by the current function.

> Another question I would have with that setup is how much this is "transposed" into the LLVM world. My feeling is that can be, at least as a first implementation, just a "frontend" to assign the darwin or MS calling conventions to functions.

I think it really depends on how far you want to take this.  If you just want to control register/stack conventions, then it seems to me that adding LLVM CCs that are explicit about which rules to use (and making Clang do its side of the lowering properly) is pretty reasonable.  LLVM is already pretty well abstracted to be flexible about CCs.  I'm not sure if it's well-abstracted to be flexible about varargs support, but that's still probably not too invasive.  If you want to start doing function-specific overrides for features with a more complex lowering, like the C++ ABI or exception handling, that's asking a lot more.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89490/new/

https://reviews.llvm.org/D89490