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

[John McCall via Phabricator via llvm-commits]

rjmccall added a comment.

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

> In D89490#2482531 <https://reviews.llvm.org/D89490#2482531>, @rjmccall wrote:
>
>> I'm not calling Wine a niche use-case, I'm calling this feature a niche use-case.  The lack of this feature has not blocked Wine from being a successful project.  The feature has to stand on its own and be more broadly useful than the momentary convenience of a few developers.
>
> I am not saying this **exact** feature is needed by Wine. What I am doing is a parallel with `__attribute__((ms_abi))`, which is exactly the same as this exact feature, but to target the MS ABI from a foreign OS (instead of the Darwin one). Wine just can't work without that attribute.
>
> Let's imagine that, at the time people had wanted to introduce `__attribute__((ms_abi))`, we would have told them "this is a niche, hack <insert compiler representation> instead". Do you really think that would have been a serious solution?
>
> I'd say that we are in a chicken and egg problem: we don't have users for this, but maybe because we don't have the feature. At my company, we already have internal tools that are using this feature, and have lots of hopes it will simplify some of our more complex internal CI and debugging processes. We plan to open source all of this, because we also believe that will also help others with the same kind of issues. But I agree that we might be wrong, I honestly don't know.

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.

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")))`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490