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

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

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

In D89490#2539950 , @aguinet wrote:

> In D89490#2516255 , @rjmccall wrote:
>
>> In 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 

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

[Adrien Guinet via Phabricator via cfe-commits]

aguinet added a comment.

@rjmccall @mstorsjo @aaron.ballman any advice on what to do next? Should I 
bring this discussion back to llvm-dev?

I don't want this to justs stall here. I would like to have a clear decision on 
why it is or it is not a good idea to merge this in LLVM.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Adrien Guinet via Phabricator via cfe-commits]

aguinet added a comment.

So, what could be the futur of that patch? Would that be okay to merge with a 
`target_abi` Clang attribute?

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Adrien Guinet via Phabricator via cfe-commits]

aguinet added a comment.

In D89490#2516255 , @rjmccall wrote:

> In 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; 

This means that `__builtin_ms_abi_va_list` would just be a typedef (and that 
might have some implication on legacy code?).

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 ABI to functions.

>> About Apple that would create as much ABIs as products, I guess we have a 
>> living example: is the ABI for OSX/ARM64 different than the ABI for 
>> iOS/ARM64? I can't seem find any official documentation about this :/ (only 
>> talking about arm64 here, but I get your points about armv7 targets).
>
> No, we've used the same basic ABI on iOS, macOS, and tvOS for ARM64.  But 
> that's not surprising because those products were all released within a 
> relatively short period of time from each other, so we don't have much of a 
> laundry list of things we'd like to improve about them.

Okay, thanks for the information!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In 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.

>> 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.

> About Apple that would create as much ABIs as products, I guess we have a 
> living example: is the ABI for OSX/ARM64 different than the ABI for 
> iOS/ARM64? I can't seem find any official documentation about this :/ (only 
> talking about arm64 here, but I get your points about armv7 targets).

No, we've used the same basic ABI on iOS, macOS, and tvOS for ARM64.  But 
that's not surprising because those products were all released within a 
relatively short period of time from each other, so we don't have much of a 
laundry list of things we'd like to improve about them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Adrien Guinet via Phabricator via cfe-commits]

aguinet added a comment.

> 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.

> 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))`. Maybe, not to conflict with the MS ABI 
implementation, we could introduce `__attribute__((darwin_abi("arm64-ios")))` 
(and arm64-tvos, arm64-osx, ...)?

About Apple that would create as much ABIs as products, I guess we have a 
living example: is the ABI for OSX/ARM64 different than the ABI for iOS/ARM64? 
I can't seem find any official documentation about this :/ (only talking about 
arm64 here, but I get your points about armv7 targets).

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D89490#2483792 , @aguinet wrote:

> In 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 
>  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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Adrien Guinet via Phabricator via cfe-commits]

aguinet added a comment.

In 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 
 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. 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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

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.

Thinking about it some more, it might be much easier to hack LLVM IR output 
than assembly output.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Adrien Guinet via Phabricator via cfe-commits]

aguinet added a comment.

In D89490#2481306 , @rjmccall wrote:

> I do feel like this is a terrible idea, sorry.  It's a *very* niche use case 
> to be dedicating a new compiler feature to, and it's a feature that demands a 
> lot from the internal compiler architecture in ways that other features don't.

I'd say it's as much as a niche use case as `__attribute__((ms_abi))`? But the 
"niche use case" is Wine, which I believe has a consequent user base. I'm not 
saying the goal of this patch is to be able to do Wine for ARM64/iOS, but I 
believe that will help make very helpful tools for developers that target that 
OS (and don't have access to development devices, that is, basically everyone 
not working at Apple).

> If you really need to build code with the Darwin ABI that runs on Linux, can 
> you not achieve it by building with `-S` and then hacking up the resulting 
> assembly files?

After having written this patch and looked at the differences in assembly, that 
seemed far from trivial to achieve. But I'd be happy to look at POC of this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I do feel like this is a terrible idea, sorry.  It's a *very* niche use case to 
be dedicating a new compiler feature to, and it's a feature that demands a lot 
from the internal compiler architecture in ways that other features don't.

If you really need to build code with the Darwin ABI that runs on Linux, can 
you not achieve it by building with `-S` and then hacking up the resulting 
assembly files?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

The frontend parts LGTM, but you should wait for someone else to review the 
backend parts before landing.




Comment at: clang/test/Sema/callingconv-darwin_abi.c:10
+
+void(__attribute__((darwin_abi)) * pfoo2)(void) = foo; // 
expected-warning{{incompatible function pointer types}}

aguinet wrote:
> aaron.ballman wrote:
> > You should also add some tests like:
> > ```
> > __attribute__((darwin_abi)) int i; // error
> > __attribute__((darwin_abi(12))) void func(void); // error
> > ```
> If I am correct, this is tested in `clang/test/Sema/darwin_abi-sysv_abi.c`
So it is! Sorry for the noise. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Adrien Guinet via Phabricator via cfe-commits]

aguinet added inline comments.



Comment at: clang/test/Sema/callingconv-darwin_abi.c:10
+
+void(__attribute__((darwin_abi)) * pfoo2)(void) = foo; // 
expected-warning{{incompatible function pointer types}}

aaron.ballman wrote:
> You should also add some tests like:
> ```
> __attribute__((darwin_abi)) int i; // error
> __attribute__((darwin_abi(12))) void func(void); // error
> ```
If I am correct, this is tested in `clang/test/Sema/darwin_abi-sysv_abi.c`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Adrien Guinet via Phabricator via cfe-commits]

aguinet updated this revision to Diff 314565.
aguinet marked 4 inline comments as done.
aguinet added a comment.

Rebase on current master, and take into account @aaron.ballman 's fixes 
(thanks!).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/Specifiers.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/darwin_abi.c
  clang/test/CodeGen/darwin_abi_empty_structs.cpp
  clang/test/CodeGen/darwin_abi_vaarg.c
  clang/test/CodeGen/debug-info-cc.c
  clang/test/CodeGenCXX/darwinabi-returnthis.cpp
  clang/test/Sema/callingconv-darwin_abi.c
  clang/test/Sema/darwin_abi-sysv_abi.c
  clang/test/Sema/darwin_abi-win64.c
  clang/test/Sema/no_callconv.cpp
  clang/test/Sema/varargs-aarch64.c
  clang/tools/libclang/CXType.cpp
  llvm/include/llvm/BinaryFormat/Dwarf.def
  llvm/include/llvm/IR/CallingConv.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/Target/AArch64/AArch64CallingConvention.cpp
  llvm/lib/Target/AArch64/AArch64FastISel.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.h
  llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp

Index: llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
===
--- llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
+++ llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
@@ -474,8 +474,8 @@
   uint64_t StackOffset = Handler.StackUsed;
   if (F.isVarArg()) {
 auto  = MF.getSubtarget();
-if (!Subtarget.isTargetDarwin()) {
-// FIXME: we need to reimplement saveVarArgsRegisters from
+if (!Subtarget.isCallingConvDarwin(MF.getFunction().getCallingConv())) {
+  // FIXME: we need to reimplement saveVarArgsRegisters from
   // AArch64ISelLowering.
   return false;
 }
Index: llvm/lib/Target/AArch64/AArch64Subtarget.h
===
--- llvm/lib/Target/AArch64/AArch64Subtarget.h
+++ llvm/lib/Target/AArch64/AArch64Subtarget.h
@@ -566,6 +566,12 @@
 }
   }
 
+  bool isCallingConvDarwin(CallingConv::ID CC) const {
+if (CC == CallingConv::AArch64Darwin)
+  return true;
+return isTargetDarwin();
+  }
+
   void mirFileLoaded(MachineFunction ) const override;
 
   // Return the known range for the bit length of SVE data registers. A value
Index: llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
===
--- llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
+++ llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
@@ -73,7 +73,6 @@
 const MCPhysReg *
 AArch64RegisterInfo::getCalleeSavedRegs(const MachineFunction *MF) const {
   assert(MF && "Invalid MachineFunction pointer.");
-
   if (MF->getFunction().getCallingConv() == CallingConv::GHC)
 // GHC set of callee saved regs is empty as all those regs are
 // used for passing STG regs around
@@ -86,22 +85,25 @@
   if (MF->getSubtarget().isTargetDarwin())
 return getDarwinCalleeSavedRegs(MF);
 
-  if (MF->getFunction().getCallingConv() == CallingConv::CFGuard_Check)
+  const auto FCC = MF->getFunction().getCallingConv();
+  if (FCC == CallingConv::AArch64Darwin)
+return CSR_Darwin_AArch64_AAPCS_SaveList;
+  if (FCC == CallingConv::CFGuard_Check)
 return CSR_Win_AArch64_CFGuard_Check_SaveList;
   if (MF->getSubtarget().isTargetWindows())
 return CSR_Win_AArch64_AAPCS_SaveList;
-  if (MF->getFunction().getCallingConv() == CallingConv::AArch64_VectorCall)
+  if (FCC == CallingConv::AArch64_VectorCall)
 return CSR_AArch64_AAVPCS_SaveList;
-  if (MF->getFunction().getCallingConv() == CallingConv::AArch64_SVE_VectorCall)
+  if (FCC == CallingConv::AArch64_SVE_VectorCall)
 return CSR_AArch64_SVE_AAPCS_SaveList;
   if (MF->getSubtarget().getTargetLowering()
   ->supportSwiftError() &&
   MF->getFunction().getAttributes().hasAttrSomewhere(
   Attribute::SwiftError))
 return CSR_AArch64_AAPCS_SwiftError_SaveList;
-  if (MF->getFunction().getCallingConv() == CallingConv::PreserveMost)
+  if (FCC == CallingConv::PreserveMost)
 return CSR_AArch64_RT_MostRegs_SaveList;
-  if (MF->getFunction().getCallingConv() == CallingConv::Win64)
+  if (FCC == CallingConv::Win64)
 // This is for OSes 

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

[Adrien Guinet via Phabricator via cfe-commits]

aguinet added a comment.

In D89490#2477937 , @emaste wrote:

>> For now, only Linux/ARM64 is supported/tested.
>
> Is there any reason this is Linux-specific (as far as support; I understand 
> if it's not easy for you to test on non-Linux arm64).

It's mainly due to this: it was easier to first test support on Linux. I think 
support for Windows/ARM64 could appear in another patch.


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

https://reviews.llvm.org/D89490

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ed Maste via Phabricator via cfe-commits]

emaste added a comment.

> For now, only Linux/ARM64 is supported/tested.

Is there any reason this is Linux-specific (as far as support; I understand if 
it's not easy for you to test on non-Linux arm64).


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

https://reviews.llvm.org/D89490

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Some minor nits while double-checking the attribute bits.




Comment at: clang/include/clang/Basic/AttrDocs.td:2334
+
+Here is the list of whatthis attribute supports, quoting the aformentioned
+document:





Comment at: clang/test/CodeGen/darwin_abi.c:1
+// RUN: %clang_cc1 -triple aarch64-pc-linux %s -emit-llvm -o - |FileCheck %s
+// Checks that correct LLVM IR is generated for functions that target





Comment at: clang/test/CodeGen/darwin_abi_empty_structs.cpp:4
+// empty structures unless those structures have a nontrivial destructor or
+// copy constructor." when using darwin_abi
+





Comment at: clang/test/CodeGen/darwin_abi_vaarg.c:1
+// RUN: %clang_cc1 -triple aarch64-pc-linux -emit-llvm %s -o - |FileCheck %s
+// Check that va_arg used inside a function with the darwin_abi attribute still





Comment at: clang/test/Sema/callingconv-darwin_abi.c:10
+
+void(__attribute__((darwin_abi)) * pfoo2)(void) = foo; // 
expected-warning{{incompatible function pointer types}}

You should also add some tests like:
```
__attribute__((darwin_abi)) int i; // error
__attribute__((darwin_abi(12))) void func(void); // error
```


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

https://reviews.llvm.org/D89490

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Adrien Guinet via Phabricator via cfe-commits]

aguinet updated this revision to Diff 313842.
aguinet added a comment.

Replace a just introduced `const auto` usage.


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

https://reviews.llvm.org/D89490

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/Specifiers.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/darwin_abi.c
  clang/test/CodeGen/darwin_abi_empty_structs.cpp
  clang/test/CodeGen/darwin_abi_vaarg.c
  clang/test/CodeGen/debug-info-cc.c
  clang/test/CodeGenCXX/darwinabi-returnthis.cpp
  clang/test/Sema/callingconv-darwin_abi.c
  clang/test/Sema/darwin_abi-sysv_abi.c
  clang/test/Sema/darwin_abi-win64.c
  clang/test/Sema/no_callconv.cpp
  clang/test/Sema/varargs-aarch64.c
  clang/tools/libclang/CXType.cpp
  llvm/include/llvm/BinaryFormat/Dwarf.def
  llvm/include/llvm/IR/CallingConv.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/Target/AArch64/AArch64CallingConvention.cpp
  llvm/lib/Target/AArch64/AArch64FastISel.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.h
  llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
  llvm/test/CodeGen/AArch64/darwin_abi.ll
  llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll

Index: llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll
@@ -0,0 +1,39 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-pc-linux | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-pc-linux"
+
+%struct.__va_list = type { i8*, i8*, i8*, i32, i32 }
+
+define dso_local aarch64_darwincc void @foo(i32 %n, ...) local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:sub sp, sp, #48 // =48
+; CHECK-NEXT:stp x29, x30, [sp, #32] // 16-byte Folded Spill
+; CHECK-NEXT:add x29, sp, #32 // =32
+; CHECK-NEXT:add x8, x29, #16 // =16
+; CHECK-NEXT:mov x1, sp
+; CHECK-NEXT:str xzr, [sp, #24]
+; CHECK-NEXT:str x8, [sp]
+; CHECK-NEXT:bl vfoo
+; CHECK-NEXT:ldp x29, x30, [sp, #32] // 16-byte Folded Reload
+; CHECK-NEXT:add sp, sp, #48 // =48
+; CHECK-NEXT:ret
+entry:
+  %va = alloca %struct.__va_list, align 8
+  %0 = bitcast %struct.__va_list* %va to i8*
+  call void @llvm.va_start(i8* nonnull %0)
+  call void @vfoo(i32 %n, %struct.__va_list* nonnull %va) #1
+  call void @llvm.va_end(i8* nonnull %0)
+  ret void
+}
+
+declare void @llvm.va_start(i8*) #1
+
+declare dso_local void @vfoo(i32, %struct.__va_list*) local_unnamed_addr #0
+
+declare void @llvm.va_end(i8*) #1
+
+attributes #0 = { nounwind "disable-tail-calls"="false" "frame-pointer"="non-leaf" "target-cpu"="generic" }
+attributes #1 = { nounwind }
Index: llvm/test/CodeGen/AArch64/darwin_abi.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/darwin_abi.ll
@@ -0,0 +1,29 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-pc-linux | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-pc-linux"
+
+define dso_local aarch64_darwincc signext i16 @f1(i16 signext %a) local_unnamed_addr #0 {
+; CHECK-LABEL: f1:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:add w8, w0, #1 // =1
+; CHECK-NEXT:sxth w0, w8
+; CHECK-NEXT:ret
+entry:
+  %add = add i16 %a, 1
+  ret i16 %add
+}
+
+define dso_local aarch64_darwincc zeroext i16 @f2(i16 zeroext %a) local_unnamed_addr #0 {
+; CHECK-LABEL: f2:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:add w8, w0, #1 // =1
+; CHECK-NEXT:and w0, w8, #0x
+; CHECK-NEXT:ret
+entry:
+  %add = add i16 %a, 1
+  ret i16 %add
+}
+
+attributes #0 = { norecurse nounwind readnone "disable-tail-calls"="false" "frame-pointer"="non-leaf" "target-cpu"="generic" }
Index: llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
===
--- llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
+++ llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
@@ -474,8 +474,8 @@
   uint64_t StackOffset = Handler.StackUsed;
   if (F.isVarArg()) {
 auto  = 

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

[Adrien Guinet via Phabricator via cfe-commits]

aguinet updated this revision to Diff 313841.
aguinet added a comment.

Clang format fixes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/Specifiers.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/darwin_abi.c
  clang/test/CodeGen/darwin_abi_empty_structs.cpp
  clang/test/CodeGen/darwin_abi_vaarg.c
  clang/test/CodeGen/debug-info-cc.c
  clang/test/CodeGenCXX/darwinabi-returnthis.cpp
  clang/test/Sema/callingconv-darwin_abi.c
  clang/test/Sema/darwin_abi-sysv_abi.c
  clang/test/Sema/darwin_abi-win64.c
  clang/test/Sema/no_callconv.cpp
  clang/test/Sema/varargs-aarch64.c
  clang/tools/libclang/CXType.cpp
  llvm/include/llvm/BinaryFormat/Dwarf.def
  llvm/include/llvm/IR/CallingConv.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/Target/AArch64/AArch64CallingConvention.cpp
  llvm/lib/Target/AArch64/AArch64FastISel.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.h
  llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
  llvm/test/CodeGen/AArch64/darwin_abi.ll
  llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll

Index: llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll
@@ -0,0 +1,39 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-pc-linux | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-pc-linux"
+
+%struct.__va_list = type { i8*, i8*, i8*, i32, i32 }
+
+define dso_local aarch64_darwincc void @foo(i32 %n, ...) local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:sub sp, sp, #48 // =48
+; CHECK-NEXT:stp x29, x30, [sp, #32] // 16-byte Folded Spill
+; CHECK-NEXT:add x29, sp, #32 // =32
+; CHECK-NEXT:add x8, x29, #16 // =16
+; CHECK-NEXT:mov x1, sp
+; CHECK-NEXT:str xzr, [sp, #24]
+; CHECK-NEXT:str x8, [sp]
+; CHECK-NEXT:bl vfoo
+; CHECK-NEXT:ldp x29, x30, [sp, #32] // 16-byte Folded Reload
+; CHECK-NEXT:add sp, sp, #48 // =48
+; CHECK-NEXT:ret
+entry:
+  %va = alloca %struct.__va_list, align 8
+  %0 = bitcast %struct.__va_list* %va to i8*
+  call void @llvm.va_start(i8* nonnull %0)
+  call void @vfoo(i32 %n, %struct.__va_list* nonnull %va) #1
+  call void @llvm.va_end(i8* nonnull %0)
+  ret void
+}
+
+declare void @llvm.va_start(i8*) #1
+
+declare dso_local void @vfoo(i32, %struct.__va_list*) local_unnamed_addr #0
+
+declare void @llvm.va_end(i8*) #1
+
+attributes #0 = { nounwind "disable-tail-calls"="false" "frame-pointer"="non-leaf" "target-cpu"="generic" }
+attributes #1 = { nounwind }
Index: llvm/test/CodeGen/AArch64/darwin_abi.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/darwin_abi.ll
@@ -0,0 +1,29 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-pc-linux | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-pc-linux"
+
+define dso_local aarch64_darwincc signext i16 @f1(i16 signext %a) local_unnamed_addr #0 {
+; CHECK-LABEL: f1:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:add w8, w0, #1 // =1
+; CHECK-NEXT:sxth w0, w8
+; CHECK-NEXT:ret
+entry:
+  %add = add i16 %a, 1
+  ret i16 %add
+}
+
+define dso_local aarch64_darwincc zeroext i16 @f2(i16 zeroext %a) local_unnamed_addr #0 {
+; CHECK-LABEL: f2:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:add w8, w0, #1 // =1
+; CHECK-NEXT:and w0, w8, #0x
+; CHECK-NEXT:ret
+entry:
+  %add = add i16 %a, 1
+  ret i16 %add
+}
+
+attributes #0 = { norecurse nounwind readnone "disable-tail-calls"="false" "frame-pointer"="non-leaf" "target-cpu"="generic" }
Index: llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
===
--- llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
+++ llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
@@ -474,8 +474,8 @@
   uint64_t StackOffset = Handler.StackUsed;
   if (F.isVarArg()) {
 auto  = 

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

[Adrien Guinet via Phabricator via cfe-commits]

aguinet updated this revision to Diff 313838.
aguinet added a comment.

Rebased on current master branch, and added clang sema tests cc @aaron.ballman .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/Specifiers.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/darwin_abi.c
  clang/test/CodeGen/darwin_abi_empty_structs.cpp
  clang/test/CodeGen/darwin_abi_vaarg.c
  clang/test/CodeGen/debug-info-cc.c
  clang/test/CodeGenCXX/darwinabi-returnthis.cpp
  clang/test/Sema/callingconv-darwin_abi.c
  clang/test/Sema/darwin_abi-sysv_abi.c
  clang/test/Sema/darwin_abi-win64.c
  clang/test/Sema/no_callconv.cpp
  clang/test/Sema/varargs-aarch64.c
  clang/tools/libclang/CXType.cpp
  llvm/include/llvm/BinaryFormat/Dwarf.def
  llvm/include/llvm/IR/CallingConv.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/Target/AArch64/AArch64CallingConvention.cpp
  llvm/lib/Target/AArch64/AArch64FastISel.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.h
  llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
  llvm/test/CodeGen/AArch64/darwin_abi.ll
  llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll

Index: llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll
@@ -0,0 +1,39 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-pc-linux | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-pc-linux"
+
+%struct.__va_list = type { i8*, i8*, i8*, i32, i32 }
+
+define dso_local aarch64_darwincc void @foo(i32 %n, ...) local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:sub sp, sp, #48 // =48
+; CHECK-NEXT:stp x29, x30, [sp, #32] // 16-byte Folded Spill
+; CHECK-NEXT:add x29, sp, #32 // =32
+; CHECK-NEXT:add x8, x29, #16 // =16
+; CHECK-NEXT:mov x1, sp
+; CHECK-NEXT:str xzr, [sp, #24]
+; CHECK-NEXT:str x8, [sp]
+; CHECK-NEXT:bl vfoo
+; CHECK-NEXT:ldp x29, x30, [sp, #32] // 16-byte Folded Reload
+; CHECK-NEXT:add sp, sp, #48 // =48
+; CHECK-NEXT:ret
+entry:
+  %va = alloca %struct.__va_list, align 8
+  %0 = bitcast %struct.__va_list* %va to i8*
+  call void @llvm.va_start(i8* nonnull %0)
+  call void @vfoo(i32 %n, %struct.__va_list* nonnull %va) #1
+  call void @llvm.va_end(i8* nonnull %0)
+  ret void
+}
+
+declare void @llvm.va_start(i8*) #1
+
+declare dso_local void @vfoo(i32, %struct.__va_list*) local_unnamed_addr #0
+
+declare void @llvm.va_end(i8*) #1
+
+attributes #0 = { nounwind "disable-tail-calls"="false" "frame-pointer"="non-leaf" "target-cpu"="generic" }
+attributes #1 = { nounwind }
Index: llvm/test/CodeGen/AArch64/darwin_abi.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/darwin_abi.ll
@@ -0,0 +1,29 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-pc-linux | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-pc-linux"
+
+define dso_local aarch64_darwincc signext i16 @f1(i16 signext %a) local_unnamed_addr #0 {
+; CHECK-LABEL: f1:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:add w8, w0, #1 // =1
+; CHECK-NEXT:sxth w0, w8
+; CHECK-NEXT:ret
+entry:
+  %add = add i16 %a, 1
+  ret i16 %add
+}
+
+define dso_local aarch64_darwincc zeroext i16 @f2(i16 zeroext %a) local_unnamed_addr #0 {
+; CHECK-LABEL: f2:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:add w8, w0, #1 // =1
+; CHECK-NEXT:and w0, w8, #0x
+; CHECK-NEXT:ret
+entry:
+  %add = add i16 %a, 1
+  ret i16 %add
+}
+
+attributes #0 = { norecurse nounwind readnone "disable-tail-calls"="false" "frame-pointer"="non-leaf" "target-cpu"="generic" }
Index: llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
===
--- llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
+++ llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
@@ -474,8 +474,8 @@
   uint64_t StackOffset 

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

[Adrien Guinet via Phabricator via cfe-commits]

aguinet added a comment.

Thanks @aaron.ballman for the comments!

I fixed all your comments in the new patch. I will upload a new one with the 
Sema tests!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Adrien Guinet via Phabricator via cfe-commits]

aguinet updated this revision to Diff 310006.
aguinet marked 3 inline comments as done.
aguinet added a comment.

Multiple things:

- remove clang-format tags
- remove const usage
- enhanced attribute documentation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/Specifiers.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/darwin_abi.c
  clang/test/CodeGen/darwin_abi_empty_structs.cpp
  clang/test/CodeGen/darwin_abi_vaarg.c
  clang/test/CodeGen/debug-info-cc.c
  clang/test/CodeGenCXX/darwinabi-returnthis.cpp
  clang/tools/libclang/CXType.cpp
  llvm/include/llvm/BinaryFormat/Dwarf.def
  llvm/include/llvm/IR/CallingConv.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/Target/AArch64/AArch64CallingConvention.cpp
  llvm/lib/Target/AArch64/AArch64FastISel.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.h
  llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
  llvm/test/CodeGen/AArch64/darwin_abi.ll
  llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll

Index: llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll
@@ -0,0 +1,39 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-pc-linux | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-pc-linux"
+
+%struct.__va_list = type { i8*, i8*, i8*, i32, i32 }
+
+define dso_local aarch64_darwincc void @foo(i32 %n, ...) local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:sub sp, sp, #48 // =48
+; CHECK-NEXT:stp x29, x30, [sp, #32] // 16-byte Folded Spill
+; CHECK-NEXT:add x29, sp, #32 // =32
+; CHECK-NEXT:add x8, x29, #16 // =16
+; CHECK-NEXT:mov x1, sp
+; CHECK-NEXT:str xzr, [sp, #24]
+; CHECK-NEXT:str x8, [sp]
+; CHECK-NEXT:bl vfoo
+; CHECK-NEXT:ldp x29, x30, [sp, #32] // 16-byte Folded Reload
+; CHECK-NEXT:add sp, sp, #48 // =48
+; CHECK-NEXT:ret
+entry:
+  %va = alloca %struct.__va_list, align 8
+  %0 = bitcast %struct.__va_list* %va to i8*
+  call void @llvm.va_start(i8* nonnull %0)
+  call void @vfoo(i32 %n, %struct.__va_list* nonnull %va) #1
+  call void @llvm.va_end(i8* nonnull %0)
+  ret void
+}
+
+declare void @llvm.va_start(i8*) #1
+
+declare dso_local void @vfoo(i32, %struct.__va_list*) local_unnamed_addr #0
+
+declare void @llvm.va_end(i8*) #1
+
+attributes #0 = { nounwind "disable-tail-calls"="false" "frame-pointer"="non-leaf" "target-cpu"="generic" }
+attributes #1 = { nounwind }
Index: llvm/test/CodeGen/AArch64/darwin_abi.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/darwin_abi.ll
@@ -0,0 +1,29 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-pc-linux | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-pc-linux"
+
+define dso_local aarch64_darwincc signext i16 @f1(i16 signext %a) local_unnamed_addr #0 {
+; CHECK-LABEL: f1:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:add w8, w0, #1 // =1
+; CHECK-NEXT:sxth w0, w8
+; CHECK-NEXT:ret
+entry:
+  %add = add i16 %a, 1
+  ret i16 %add
+}
+
+define dso_local aarch64_darwincc zeroext i16 @f2(i16 zeroext %a) local_unnamed_addr #0 {
+; CHECK-LABEL: f2:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:add w8, w0, #1 // =1
+; CHECK-NEXT:and w0, w8, #0x
+; CHECK-NEXT:ret
+entry:
+  %add = add i16 %a, 1
+  ret i16 %add
+}
+
+attributes #0 = { norecurse nounwind readnone "disable-tail-calls"="false" "frame-pointer"="non-leaf" "target-cpu"="generic" }
Index: llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
===
--- llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
+++ llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
@@ -474,8 +474,8 @@
   uint64_t StackOffset = Handler.StackUsed;
   if (F.isVarArg()) {
 auto  = MF.getSubtarget();
-if (!Subtarget.isTargetDarwin()) {
-// 

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: rjmccall.
aaron.ballman added a subscriber: rjmccall.
aaron.ballman added a comment.

You should add some frontend Sema tests for the attribute. The usual tests are: 
correct usage without diagnostics (as both a declaration attribute and a type 
attribute), applying the attribute to the wrong subject, passing arguments to 
the attribute when it doesn't accept any.

Adding @rjmccall to see if he spots any Darwin-specific concerns, but the 
frontend side of things looks reasonable to me aside from a few nits.




Comment at: clang/include/clang/Basic/AttrDocs.td:2258
+  let Content = [{
+On Linux ARM64 targets, this attribute changes the calling convention of
+a function to match the default convention used on Apple ARM64. This

aguinet wrote:
> aaron.ballman wrote:
> > Adding more details about what that ABI actually is, or linking to a place 
> > where the user can learn more on their own, would be appreciated.
> Do you know if we can we put http links into this documentation? Would this 
> render correctly in the final clang documentation? 
Yup! The attribute documentation content bits form a .rst file that gets 
compiled by Sphinx, so you can use any Sphinx markup you'd like 
(https://www.sphinx-doc.org/en/master/usage/restructuredtext/index.html).



Comment at: clang/include/clang/Basic/Specifiers.h:269
 
+  // clang-format off
   /// CallingConv - Specifies the calling convention that a function uses.

aguinet wrote:
> aaron.ballman wrote:
> > My personal opinion is that these formatting markers are noise. However, 
> > there's a fair number of enumerations this could cover and disabling the 
> > format behavior may be reasonable.
> > 
> > I think I'd rather see the formatting comments removed from this patch. 
> > They can be added in a different commit but perhaps the file can be 
> > restructured so that we don't feel the need to sprinkle so many comments 
> > around.
> I don't have strong opinions about this, but so we agree that `clang-format` 
> won't be happy about this patch but we're fine with it?
Yup, that's fine -- the linter is already noisy in these sort of areas as it 
stands, so this won't add any new noise we're not already used to seeing. 
Reviewers are usually pretty good about asking for specific instances of 
clang-format complaints to be resolved if they're important.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:70
+// Returns true if AArch64 Darwin ABI is explicitly used.
+const bool IsCtorOrDtor = (isa(GD.getDecl()) ||
+   (isa(GD.getDecl()) &&

aguinet wrote:
> aaron.ballman wrote:
> > We don't typically go with top-level `const` on non-pointer/reference 
> > types, so I'd drop the `const`.
> I am not sure to understand the reasoning behind it. IMHO it's interesting 
> when reading the code to know that this value will never change accross the 
> function. Is there an issue I am missing?
It's not that it's an unreasonable coding style, it's that we don't want to 
introduce new inconsistencies in the existing code. If the local code already 
does it, we let it slide, but otherwise, we typically don't do it for local 
variables or parameters (but will sometimes do it for member declarations 
because that can lead to better codegen).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Adrien Guinet via Phabricator via cfe-commits]

aguinet updated this revision to Diff 308203.
aguinet added a comment.

Relax checks in `CodeGenCXX/darwinabi-returnthis.cpp` Clang test (to adapt to 
new attributes), and removes some useless brackets in if statements.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/Specifiers.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/darwin_abi.c
  clang/test/CodeGen/darwin_abi_empty_structs.cpp
  clang/test/CodeGen/darwin_abi_vaarg.c
  clang/test/CodeGen/debug-info-cc.c
  clang/test/CodeGenCXX/darwinabi-returnthis.cpp
  clang/tools/libclang/CXType.cpp
  llvm/include/llvm/BinaryFormat/Dwarf.def
  llvm/include/llvm/IR/CallingConv.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/Target/AArch64/AArch64CallingConvention.cpp
  llvm/lib/Target/AArch64/AArch64FastISel.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.h
  llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
  llvm/test/CodeGen/AArch64/darwin_abi.ll
  llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll

Index: llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll
@@ -0,0 +1,39 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-pc-linux | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-pc-linux"
+
+%struct.__va_list = type { i8*, i8*, i8*, i32, i32 }
+
+define dso_local aarch64_darwincc void @foo(i32 %n, ...) local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:sub sp, sp, #48 // =48
+; CHECK-NEXT:stp x29, x30, [sp, #32] // 16-byte Folded Spill
+; CHECK-NEXT:add x29, sp, #32 // =32
+; CHECK-NEXT:add x8, x29, #16 // =16
+; CHECK-NEXT:mov x1, sp
+; CHECK-NEXT:str xzr, [sp, #24]
+; CHECK-NEXT:str x8, [sp]
+; CHECK-NEXT:bl vfoo
+; CHECK-NEXT:ldp x29, x30, [sp, #32] // 16-byte Folded Reload
+; CHECK-NEXT:add sp, sp, #48 // =48
+; CHECK-NEXT:ret
+entry:
+  %va = alloca %struct.__va_list, align 8
+  %0 = bitcast %struct.__va_list* %va to i8*
+  call void @llvm.va_start(i8* nonnull %0)
+  call void @vfoo(i32 %n, %struct.__va_list* nonnull %va) #1
+  call void @llvm.va_end(i8* nonnull %0)
+  ret void
+}
+
+declare void @llvm.va_start(i8*) #1
+
+declare dso_local void @vfoo(i32, %struct.__va_list*) local_unnamed_addr #0
+
+declare void @llvm.va_end(i8*) #1
+
+attributes #0 = { nounwind "disable-tail-calls"="false" "frame-pointer"="non-leaf" "target-cpu"="generic" }
+attributes #1 = { nounwind }
Index: llvm/test/CodeGen/AArch64/darwin_abi.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/darwin_abi.ll
@@ -0,0 +1,29 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-pc-linux | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-pc-linux"
+
+define dso_local aarch64_darwincc signext i16 @f1(i16 signext %a) local_unnamed_addr #0 {
+; CHECK-LABEL: f1:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:add w8, w0, #1 // =1
+; CHECK-NEXT:sxth w0, w8
+; CHECK-NEXT:ret
+entry:
+  %add = add i16 %a, 1
+  ret i16 %add
+}
+
+define dso_local aarch64_darwincc zeroext i16 @f2(i16 zeroext %a) local_unnamed_addr #0 {
+; CHECK-LABEL: f2:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:add w8, w0, #1 // =1
+; CHECK-NEXT:and w0, w8, #0x
+; CHECK-NEXT:ret
+entry:
+  %add = add i16 %a, 1
+  ret i16 %add
+}
+
+attributes #0 = { norecurse nounwind readnone "disable-tail-calls"="false" "frame-pointer"="non-leaf" "target-cpu"="generic" }
Index: llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
===
--- llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
+++ llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
@@ -474,8 +474,8 @@
   uint64_t StackOffset = Handler.StackUsed;
   if (F.isVarArg()) {
 auto  = MF.getSubtarget();
-if (!Subtarget.isTargetDarwin()) {
-   

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

[Adrien Guinet via Phabricator via cfe-commits]

aguinet added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1481
+def DarwinABI : DeclOrTypeAttr {
+  let Spellings = [GCC<"darwin_abi">];
+//  let Subjects = [Function, ObjCMethod];

aaron.ballman wrote:
> I suspect this should be using the `Clang` spelling as I don't believe GCC 
> supports this attribute.
Changed in new patch.



Comment at: clang/include/clang/Basic/AttrDocs.td:2258
+  let Content = [{
+On Linux ARM64 targets, this attribute changes the calling convention of
+a function to match the default convention used on Apple ARM64. This

aaron.ballman wrote:
> Adding more details about what that ABI actually is, or linking to a place 
> where the user can learn more on their own, would be appreciated.
Do you know if we can we put http links into this documentation? Would this 
render correctly in the final clang documentation? 



Comment at: clang/include/clang/Basic/Specifiers.h:269
 
+  // clang-format off
   /// CallingConv - Specifies the calling convention that a function uses.

aaron.ballman wrote:
> My personal opinion is that these formatting markers are noise. However, 
> there's a fair number of enumerations this could cover and disabling the 
> format behavior may be reasonable.
> 
> I think I'd rather see the formatting comments removed from this patch. They 
> can be added in a different commit but perhaps the file can be restructured 
> so that we don't feel the need to sprinkle so many comments around.
I don't have strong opinions about this, but so we agree that `clang-format` 
won't be happy about this patch but we're fine with it?



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:70
+// Returns true if AArch64 Darwin ABI is explicitly used.
+const bool IsCtorOrDtor = (isa(GD.getDecl()) ||
+   (isa(GD.getDecl()) &&

aaron.ballman wrote:
> We don't typically go with top-level `const` on non-pointer/reference types, 
> so I'd drop the `const`.
I am not sure to understand the reasoning behind it. IMHO it's interesting when 
reading the code to know that this value will never change accross the 
function. Is there an issue I am missing?



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:78-79
+cast(GD.getDecl())->getType()->getAs();
+const auto FCC = FTy->getCallConv();
+return FCC == CallingConv::CC_AArch64Darwin;
+  }

aaron.ballman wrote:
> `return FTy->getCallConv() == CallingConv::CC_Aarch64Darwin;`
> 
> (Removes a questionable use of `auto` and a top-level `const` at the same 
> time.)
Changed in new patch!



Comment at: clang/lib/CodeGen/TargetInfo.cpp:5449
   bool isDarwinPCS() const { return Kind == DarwinPCS; }
+  bool isDarwinPCS(const unsigned CConv) const {
+return isDarwinPCS() || CConv == llvm::CallingConv::AArch64Darwin;

aaron.ballman wrote:
> Drop the top-level `const` in the parameter.
Changed in new patch!



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4640
+  case ParsedAttr::AT_DarwinABI:
+CC = Context.getTargetInfo().getTriple().isOSDarwin() ? CC_C
+  : CC_AArch64Darwin;

aaron.ballman wrote:
> Similar confusion on my part here -- if the OS is Darwin, we want to default 
> to cdecl?
Yes! The idea is to implement the C ABI of Darwin/AArch64 on foreign OSes. So, 
if we are already targeting Darwin, just use the C ABI. This is the same logic 
used just above with `AT_MSABI`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Adrien Guinet via Phabricator via cfe-commits]

aguinet updated this revision to Diff 308202.
aguinet marked 10 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/Specifiers.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/darwin_abi.c
  clang/test/CodeGen/darwin_abi_empty_structs.cpp
  clang/test/CodeGen/darwin_abi_vaarg.c
  clang/test/CodeGen/debug-info-cc.c
  clang/test/CodeGenCXX/darwinabi-returnthis.cpp
  clang/tools/libclang/CXType.cpp
  llvm/include/llvm/BinaryFormat/Dwarf.def
  llvm/include/llvm/IR/CallingConv.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/Target/AArch64/AArch64CallingConvention.cpp
  llvm/lib/Target/AArch64/AArch64FastISel.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.h
  llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
  llvm/test/CodeGen/AArch64/darwin_abi.ll
  llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll

Index: llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll
@@ -0,0 +1,39 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-pc-linux | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-pc-linux"
+
+%struct.__va_list = type { i8*, i8*, i8*, i32, i32 }
+
+define dso_local aarch64_darwincc void @foo(i32 %n, ...) local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:sub sp, sp, #48 // =48
+; CHECK-NEXT:stp x29, x30, [sp, #32] // 16-byte Folded Spill
+; CHECK-NEXT:add x29, sp, #32 // =32
+; CHECK-NEXT:add x8, x29, #16 // =16
+; CHECK-NEXT:mov x1, sp
+; CHECK-NEXT:str xzr, [sp, #24]
+; CHECK-NEXT:str x8, [sp]
+; CHECK-NEXT:bl vfoo
+; CHECK-NEXT:ldp x29, x30, [sp, #32] // 16-byte Folded Reload
+; CHECK-NEXT:add sp, sp, #48 // =48
+; CHECK-NEXT:ret
+entry:
+  %va = alloca %struct.__va_list, align 8
+  %0 = bitcast %struct.__va_list* %va to i8*
+  call void @llvm.va_start(i8* nonnull %0)
+  call void @vfoo(i32 %n, %struct.__va_list* nonnull %va) #1
+  call void @llvm.va_end(i8* nonnull %0)
+  ret void
+}
+
+declare void @llvm.va_start(i8*) #1
+
+declare dso_local void @vfoo(i32, %struct.__va_list*) local_unnamed_addr #0
+
+declare void @llvm.va_end(i8*) #1
+
+attributes #0 = { nounwind "disable-tail-calls"="false" "frame-pointer"="non-leaf" "target-cpu"="generic" }
+attributes #1 = { nounwind }
Index: llvm/test/CodeGen/AArch64/darwin_abi.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/darwin_abi.ll
@@ -0,0 +1,29 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-pc-linux | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-pc-linux"
+
+define dso_local aarch64_darwincc signext i16 @f1(i16 signext %a) local_unnamed_addr #0 {
+; CHECK-LABEL: f1:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:add w8, w0, #1 // =1
+; CHECK-NEXT:sxth w0, w8
+; CHECK-NEXT:ret
+entry:
+  %add = add i16 %a, 1
+  ret i16 %add
+}
+
+define dso_local aarch64_darwincc zeroext i16 @f2(i16 zeroext %a) local_unnamed_addr #0 {
+; CHECK-LABEL: f2:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:add w8, w0, #1 // =1
+; CHECK-NEXT:and w0, w8, #0x
+; CHECK-NEXT:ret
+entry:
+  %add = add i16 %a, 1
+  ret i16 %add
+}
+
+attributes #0 = { norecurse nounwind readnone "disable-tail-calls"="false" "frame-pointer"="non-leaf" "target-cpu"="generic" }
Index: llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
===
--- llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
+++ llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
@@ -474,8 +474,8 @@
   uint64_t StackOffset = Handler.StackUsed;
   if (F.isVarArg()) {
 auto  = MF.getSubtarget();
-if (!Subtarget.isTargetDarwin()) {
-// FIXME: we need to reimplement saveVarArgsRegisters from
+if 

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:238
+  if (D->hasAttr())
+return IsDarwin ? CC_C : CC_AArch64Darwin;
+

mstorsjo wrote:
> aaron.ballman wrote:
> > Can you help me understand this change a bit better? If the declaration 
> > uses the Darwin ABI and the platform is Darwin, you want to use the cdecl 
> > convention?
> This (here and in the other similar places) matches the existing logic for 
> the `ms_abi` attribute; if you're on a platform where the attribute selects 
> what already is the default (what `CC_C` implies on this platform), we just 
> use that instead of the more explicit calling convention.
A, thank you for the clarification, that makes sense now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:238
+  if (D->hasAttr())
+return IsDarwin ? CC_C : CC_AArch64Darwin;
+

aaron.ballman wrote:
> Can you help me understand this change a bit better? If the declaration uses 
> the Darwin ABI and the platform is Darwin, you want to use the cdecl 
> convention?
This (here and in the other similar places) matches the existing logic for the 
`ms_abi` attribute; if you're on a platform where the attribute selects what 
already is the default (what `CC_C` implies on this platform), we just use that 
instead of the more explicit calling convention.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1481
+def DarwinABI : DeclOrTypeAttr {
+  let Spellings = [GCC<"darwin_abi">];
+//  let Subjects = [Function, ObjCMethod];

I suspect this should be using the `Clang` spelling as I don't believe GCC 
supports this attribute.



Comment at: clang/include/clang/Basic/AttrDocs.td:2258
+  let Content = [{
+On Linux ARM64 targets, this attribute changes the calling convention of
+a function to match the default convention used on Apple ARM64. This

Adding more details about what that ABI actually is, or linking to a place 
where the user can learn more on their own, would be appreciated.



Comment at: clang/include/clang/Basic/Specifiers.h:269
 
+  // clang-format off
   /// CallingConv - Specifies the calling convention that a function uses.

My personal opinion is that these formatting markers are noise. However, 
there's a fair number of enumerations this could cover and disabling the format 
behavior may be reasonable.

I think I'd rather see the formatting comments removed from this patch. They 
can be added in a different commit but perhaps the file can be restructured so 
that we don't feel the need to sprinkle so many comments around.



Comment at: clang/lib/AST/Type.cpp:3115
 
+// clang-format off
 StringRef FunctionType::getNameForCallConv(CallingConv CC) {

I'd remove these as well.



Comment at: clang/lib/CodeGen/CGCall.cpp:46
 
+// clang-format off
 unsigned CodeGenTypes::ClangCallConvToLLVMCallConv(CallingConv CC) {

These too.



Comment at: clang/lib/CodeGen/CGCall.cpp:238
+  if (D->hasAttr())
+return IsDarwin ? CC_C : CC_AArch64Darwin;
+

Can you help me understand this change a bit better? If the declaration uses 
the Darwin ABI and the platform is Darwin, you want to use the cdecl convention?



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:70
+// Returns true if AArch64 Darwin ABI is explicitly used.
+const bool IsCtorOrDtor = (isa(GD.getDecl()) ||
+   (isa(GD.getDecl()) &&

We don't typically go with top-level `const` on non-pointer/reference types, so 
I'd drop the `const`.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:78-79
+cast(GD.getDecl())->getType()->getAs();
+const auto FCC = FTy->getCallConv();
+return FCC == CallingConv::CC_AArch64Darwin;
+  }

`return FTy->getCallConv() == CallingConv::CC_Aarch64Darwin;`

(Removes a questionable use of `auto` and a top-level `const` at the same time.)



Comment at: clang/lib/CodeGen/TargetInfo.cpp:5449
   bool isDarwinPCS() const { return Kind == DarwinPCS; }
+  bool isDarwinPCS(const unsigned CConv) const {
+return isDarwinPCS() || CConv == llvm::CallingConv::AArch64Darwin;

Drop the top-level `const` in the parameter.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4640
+  case ParsedAttr::AT_DarwinABI:
+CC = Context.getTargetInfo().getTriple().isOSDarwin() ? CC_C
+  : CC_AArch64Darwin;

Similar confusion on my part here -- if the OS is Darwin, we want to default to 
cdecl?



Comment at: llvm/lib/IR/AsmWriter.cpp:379
+Out << "aarch64_darwincc";
+break;
   case CallingConv::SPIR_FUNC: Out << "spir_func"; break;

mstorsjo wrote:
> aguinet wrote:
> > mstorsjo wrote:
> > > The new code here has a different indentation than the rest; the same in 
> > > a couple other places throughout the patch.
> > As clang-format changed that formatting for me, should I reformat the whole 
> > switch-case to be "clang-format compatible", or should we set this section 
> > as ignored?
> As the manually laid out custom formatting here is kinda beneficial, I think 
> it's preferred to keep it as is. If there are annotations to make 
> clang-format stay off of it, that's probably good (but I don't know what our 
> policy regarding use of such annotations is, if we have any).
FWIW, I'm on the side of "don't use the comments to turn off clang-format" 
because it adds an extra two lines of noise every time we have to use the 
markup. I'd rather see either clang-format improved for the cases that cause us 
pain or the code change to be fine with what's produced by clang-format for 
most cases (there will always be one-offs where it makes sense to use the 
comments, I'm sure).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Adrien Guinet via Phabricator via cfe-commits]

aguinet updated this revision to Diff 298569.
aguinet added a comment.

One missing formatting case...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/Specifiers.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/darwin_abi.c
  clang/test/CodeGen/darwin_abi_empty_structs.cpp
  clang/test/CodeGen/darwin_abi_vaarg.c
  clang/test/CodeGen/debug-info-cc.c
  clang/test/CodeGenCXX/darwinabi-returnthis.cpp
  clang/tools/libclang/CXType.cpp
  llvm/include/llvm/BinaryFormat/Dwarf.def
  llvm/include/llvm/IR/CallingConv.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/Target/AArch64/AArch64CallingConvention.cpp
  llvm/lib/Target/AArch64/AArch64FastISel.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.h
  llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
  llvm/test/CodeGen/AArch64/darwin_abi.ll
  llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll

Index: llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll
@@ -0,0 +1,39 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-pc-linux | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-pc-linux"
+
+%struct.__va_list = type { i8*, i8*, i8*, i32, i32 }
+
+define dso_local aarch64_darwincc void @foo(i32 %n, ...) local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:sub sp, sp, #48 // =48
+; CHECK-NEXT:stp x29, x30, [sp, #32] // 16-byte Folded Spill
+; CHECK-NEXT:add x29, sp, #32 // =32
+; CHECK-NEXT:add x8, x29, #16 // =16
+; CHECK-NEXT:mov x1, sp
+; CHECK-NEXT:str xzr, [sp, #24]
+; CHECK-NEXT:str x8, [sp]
+; CHECK-NEXT:bl vfoo
+; CHECK-NEXT:ldp x29, x30, [sp, #32] // 16-byte Folded Reload
+; CHECK-NEXT:add sp, sp, #48 // =48
+; CHECK-NEXT:ret
+entry:
+  %va = alloca %struct.__va_list, align 8
+  %0 = bitcast %struct.__va_list* %va to i8*
+  call void @llvm.va_start(i8* nonnull %0)
+  call void @vfoo(i32 %n, %struct.__va_list* nonnull %va) #1
+  call void @llvm.va_end(i8* nonnull %0)
+  ret void
+}
+
+declare void @llvm.va_start(i8*) #1
+
+declare dso_local void @vfoo(i32, %struct.__va_list*) local_unnamed_addr #0
+
+declare void @llvm.va_end(i8*) #1
+
+attributes #0 = { nounwind "disable-tail-calls"="false" "frame-pointer"="non-leaf" "target-cpu"="generic" }
+attributes #1 = { nounwind }
Index: llvm/test/CodeGen/AArch64/darwin_abi.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/darwin_abi.ll
@@ -0,0 +1,29 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-pc-linux | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-pc-linux"
+
+define dso_local aarch64_darwincc signext i16 @f1(i16 signext %a) local_unnamed_addr #0 {
+; CHECK-LABEL: f1:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:add w8, w0, #1 // =1
+; CHECK-NEXT:sxth w0, w8
+; CHECK-NEXT:ret
+entry:
+  %add = add i16 %a, 1
+  ret i16 %add
+}
+
+define dso_local aarch64_darwincc zeroext i16 @f2(i16 zeroext %a) local_unnamed_addr #0 {
+; CHECK-LABEL: f2:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:add w8, w0, #1 // =1
+; CHECK-NEXT:and w0, w8, #0x
+; CHECK-NEXT:ret
+entry:
+  %add = add i16 %a, 1
+  ret i16 %add
+}
+
+attributes #0 = { norecurse nounwind readnone "disable-tail-calls"="false" "frame-pointer"="non-leaf" "target-cpu"="generic" }
Index: llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
===
--- llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
+++ llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
@@ -474,8 +474,8 @@
   uint64_t StackOffset = Handler.StackUsed;
   if (F.isVarArg()) {
 auto  = MF.getSubtarget();
-if (!Subtarget.isTargetDarwin()) {
-// FIXME: we need to reimplement saveVarArgsRegisters from
+if 

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

[Adrien Guinet via Phabricator via cfe-commits]

aguinet updated this revision to Diff 298568.
aguinet added a comment.

Add some clang-format tags, and restore back the "original" formatting for 
these cases


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/Specifiers.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/darwin_abi.c
  clang/test/CodeGen/darwin_abi_empty_structs.cpp
  clang/test/CodeGen/darwin_abi_vaarg.c
  clang/test/CodeGen/debug-info-cc.c
  clang/test/CodeGenCXX/darwinabi-returnthis.cpp
  clang/tools/libclang/CXType.cpp
  llvm/include/llvm/BinaryFormat/Dwarf.def
  llvm/include/llvm/IR/CallingConv.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/Target/AArch64/AArch64CallingConvention.cpp
  llvm/lib/Target/AArch64/AArch64FastISel.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.h
  llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
  llvm/test/CodeGen/AArch64/darwin_abi.ll
  llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll

Index: llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll
@@ -0,0 +1,39 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-pc-linux | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-pc-linux"
+
+%struct.__va_list = type { i8*, i8*, i8*, i32, i32 }
+
+define dso_local aarch64_darwincc void @foo(i32 %n, ...) local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:sub sp, sp, #48 // =48
+; CHECK-NEXT:stp x29, x30, [sp, #32] // 16-byte Folded Spill
+; CHECK-NEXT:add x29, sp, #32 // =32
+; CHECK-NEXT:add x8, x29, #16 // =16
+; CHECK-NEXT:mov x1, sp
+; CHECK-NEXT:str xzr, [sp, #24]
+; CHECK-NEXT:str x8, [sp]
+; CHECK-NEXT:bl vfoo
+; CHECK-NEXT:ldp x29, x30, [sp, #32] // 16-byte Folded Reload
+; CHECK-NEXT:add sp, sp, #48 // =48
+; CHECK-NEXT:ret
+entry:
+  %va = alloca %struct.__va_list, align 8
+  %0 = bitcast %struct.__va_list* %va to i8*
+  call void @llvm.va_start(i8* nonnull %0)
+  call void @vfoo(i32 %n, %struct.__va_list* nonnull %va) #1
+  call void @llvm.va_end(i8* nonnull %0)
+  ret void
+}
+
+declare void @llvm.va_start(i8*) #1
+
+declare dso_local void @vfoo(i32, %struct.__va_list*) local_unnamed_addr #0
+
+declare void @llvm.va_end(i8*) #1
+
+attributes #0 = { nounwind "disable-tail-calls"="false" "frame-pointer"="non-leaf" "target-cpu"="generic" }
+attributes #1 = { nounwind }
Index: llvm/test/CodeGen/AArch64/darwin_abi.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/darwin_abi.ll
@@ -0,0 +1,29 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-pc-linux | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-pc-linux"
+
+define dso_local aarch64_darwincc signext i16 @f1(i16 signext %a) local_unnamed_addr #0 {
+; CHECK-LABEL: f1:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:add w8, w0, #1 // =1
+; CHECK-NEXT:sxth w0, w8
+; CHECK-NEXT:ret
+entry:
+  %add = add i16 %a, 1
+  ret i16 %add
+}
+
+define dso_local aarch64_darwincc zeroext i16 @f2(i16 zeroext %a) local_unnamed_addr #0 {
+; CHECK-LABEL: f2:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:add w8, w0, #1 // =1
+; CHECK-NEXT:and w0, w8, #0x
+; CHECK-NEXT:ret
+entry:
+  %add = add i16 %a, 1
+  ret i16 %add
+}
+
+attributes #0 = { norecurse nounwind readnone "disable-tail-calls"="false" "frame-pointer"="non-leaf" "target-cpu"="generic" }
Index: llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
===
--- llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
+++ llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
@@ -474,8 +474,8 @@
   uint64_t StackOffset = Handler.StackUsed;
   if (F.isVarArg()) {
 auto  = MF.getSubtarget();
-if (!Subtarget.isTargetDarwin()) {
-// FIXME: we need to reimplement saveVarArgsRegisters from

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

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.h:876
   SDValue LowerAAPCS_VASTART(SDValue Op, SelectionDAG ) const;
+  SDValue LowerAAPCSFromDarwin_VASTART(SDValue Op, SelectionDAG ) const;
   SDValue LowerDarwin_VASTART(SDValue Op, SelectionDAG ) const;

aguinet wrote:
> mstorsjo wrote:
> > aguinet wrote:
> > > Same problem as with clang-format: clang-tidy suggests 
> > > "lowerAapcsFromDarwinVastart", which would mean modifying the whole file 
> > > for consistency. Should we set clang-tidy to ignore this file?
> > I think the churn generally isn't considered worth it regarding such 
> > things; such changes can be quite disruptive to downstream users (with a 
> > lot of non-upstream code) for little benefit. Same thing here, not sure 
> > what the policy is regarding annotations.
> I do agree that a big diff just for this is counter productive. There are few 
> places where we already have clang-format annotations, like 
> https://github.com/llvm/llvm-project/blob/master/llvm/lib/TextAPI/MachO/TextStub.cpp#L262
>  . Maybe we can add one here?
Sure, I guess that can be done.

Normally, I don't strictly follow what clang-format suggests blindly, but only 
selectively keep the bits that look sensible and don't unnecessarily munge 
otherwise untouched code. But annotations to avoid manually filtering it too 
much probably is better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Adrien Guinet via Phabricator via cfe-commits]

aguinet added inline comments.



Comment at: llvm/lib/AsmParser/LLParser.cpp:2147
+CC = CallingConv::Tail;
+break;
   case lltok::kw_cc: {

Again here this "big" diff is a result of clang-format. We can see that 
"kw_aarch64_sve_vector_pcs" has been "clang-formated" but not the rest. I would 
prefer just add a one-line diff and maybe also add annotations?



Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.h:876
   SDValue LowerAAPCS_VASTART(SDValue Op, SelectionDAG ) const;
+  SDValue LowerAAPCSFromDarwin_VASTART(SDValue Op, SelectionDAG ) const;
   SDValue LowerDarwin_VASTART(SDValue Op, SelectionDAG ) const;

mstorsjo wrote:
> aguinet wrote:
> > Same problem as with clang-format: clang-tidy suggests 
> > "lowerAapcsFromDarwinVastart", which would mean modifying the whole file 
> > for consistency. Should we set clang-tidy to ignore this file?
> I think the churn generally isn't considered worth it regarding such things; 
> such changes can be quite disruptive to downstream users (with a lot of 
> non-upstream code) for little benefit. Same thing here, not sure what the 
> policy is regarding annotations.
I do agree that a big diff just for this is counter productive. There are few 
places where we already have clang-format annotations, like 
https://github.com/llvm/llvm-project/blob/master/llvm/lib/TextAPI/MachO/TextStub.cpp#L262
 . Maybe we can add one here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Adrien Guinet via Phabricator via cfe-commits]

aguinet updated this revision to Diff 298548.
aguinet added a comment.

Fix one clang-tidy warning


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/Specifiers.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/darwin_abi.c
  clang/test/CodeGen/darwin_abi_empty_structs.cpp
  clang/test/CodeGen/darwin_abi_vaarg.c
  clang/test/CodeGen/debug-info-cc.c
  clang/test/CodeGenCXX/darwinabi-returnthis.cpp
  clang/tools/libclang/CXType.cpp
  llvm/include/llvm/BinaryFormat/Dwarf.def
  llvm/include/llvm/IR/CallingConv.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/Target/AArch64/AArch64CallingConvention.cpp
  llvm/lib/Target/AArch64/AArch64FastISel.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.h
  llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
  llvm/test/CodeGen/AArch64/darwin_abi.ll
  llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll

Index: llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll
@@ -0,0 +1,39 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-pc-linux | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-pc-linux"
+
+%struct.__va_list = type { i8*, i8*, i8*, i32, i32 }
+
+define dso_local aarch64_darwincc void @foo(i32 %n, ...) local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:sub sp, sp, #48 // =48
+; CHECK-NEXT:stp x29, x30, [sp, #32] // 16-byte Folded Spill
+; CHECK-NEXT:add x29, sp, #32 // =32
+; CHECK-NEXT:add x8, x29, #16 // =16
+; CHECK-NEXT:mov x1, sp
+; CHECK-NEXT:str xzr, [sp, #24]
+; CHECK-NEXT:str x8, [sp]
+; CHECK-NEXT:bl vfoo
+; CHECK-NEXT:ldp x29, x30, [sp, #32] // 16-byte Folded Reload
+; CHECK-NEXT:add sp, sp, #48 // =48
+; CHECK-NEXT:ret
+entry:
+  %va = alloca %struct.__va_list, align 8
+  %0 = bitcast %struct.__va_list* %va to i8*
+  call void @llvm.va_start(i8* nonnull %0)
+  call void @vfoo(i32 %n, %struct.__va_list* nonnull %va) #1
+  call void @llvm.va_end(i8* nonnull %0)
+  ret void
+}
+
+declare void @llvm.va_start(i8*) #1
+
+declare dso_local void @vfoo(i32, %struct.__va_list*) local_unnamed_addr #0
+
+declare void @llvm.va_end(i8*) #1
+
+attributes #0 = { nounwind "disable-tail-calls"="false" "frame-pointer"="non-leaf" "target-cpu"="generic" }
+attributes #1 = { nounwind }
Index: llvm/test/CodeGen/AArch64/darwin_abi.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/darwin_abi.ll
@@ -0,0 +1,29 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-pc-linux | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-pc-linux"
+
+define dso_local aarch64_darwincc signext i16 @f1(i16 signext %a) local_unnamed_addr #0 {
+; CHECK-LABEL: f1:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:add w8, w0, #1 // =1
+; CHECK-NEXT:sxth w0, w8
+; CHECK-NEXT:ret
+entry:
+  %add = add i16 %a, 1
+  ret i16 %add
+}
+
+define dso_local aarch64_darwincc zeroext i16 @f2(i16 zeroext %a) local_unnamed_addr #0 {
+; CHECK-LABEL: f2:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:add w8, w0, #1 // =1
+; CHECK-NEXT:and w0, w8, #0x
+; CHECK-NEXT:ret
+entry:
+  %add = add i16 %a, 1
+  ret i16 %add
+}
+
+attributes #0 = { norecurse nounwind readnone "disable-tail-calls"="false" "frame-pointer"="non-leaf" "target-cpu"="generic" }
Index: llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
===
--- llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
+++ llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
@@ -474,8 +474,8 @@
   uint64_t StackOffset = Handler.StackUsed;
   if (F.isVarArg()) {
 auto  = MF.getSubtarget();
-if (!Subtarget.isTargetDarwin()) {
-// FIXME: we need to reimplement saveVarArgsRegisters from
+if 

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

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added inline comments.



Comment at: llvm/lib/IR/AsmWriter.cpp:379
+Out << "aarch64_darwincc";
+break;
   case CallingConv::SPIR_FUNC: Out << "spir_func"; break;

aguinet wrote:
> mstorsjo wrote:
> > The new code here has a different indentation than the rest; the same in a 
> > couple other places throughout the patch.
> As clang-format changed that formatting for me, should I reformat the whole 
> switch-case to be "clang-format compatible", or should we set this section as 
> ignored?
As the manually laid out custom formatting here is kinda beneficial, I think 
it's preferred to keep it as is. If there are annotations to make clang-format 
stay off of it, that's probably good (but I don't know what our policy 
regarding use of such annotations is, if we have any).



Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.h:876
   SDValue LowerAAPCS_VASTART(SDValue Op, SelectionDAG ) const;
+  SDValue LowerAAPCSFromDarwin_VASTART(SDValue Op, SelectionDAG ) const;
   SDValue LowerDarwin_VASTART(SDValue Op, SelectionDAG ) const;

aguinet wrote:
> Same problem as with clang-format: clang-tidy suggests 
> "lowerAapcsFromDarwinVastart", which would mean modifying the whole file for 
> consistency. Should we set clang-tidy to ignore this file?
I think the churn generally isn't considered worth it regarding such things; 
such changes can be quite disruptive to downstream users (with a lot of 
non-upstream code) for little benefit. Same thing here, not sure what the 
policy is regarding annotations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Adrien Guinet via Phabricator via cfe-commits]

aguinet added inline comments.



Comment at: llvm/lib/IR/AsmWriter.cpp:379
+Out << "aarch64_darwincc";
+break;
   case CallingConv::SPIR_FUNC: Out << "spir_func"; break;

mstorsjo wrote:
> The new code here has a different indentation than the rest; the same in a 
> couple other places throughout the patch.
As clang-format changed that formatting for me, should I reformat the whole 
switch-case to be "clang-format compatible", or should we set this section as 
ignored?



Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.h:876
   SDValue LowerAAPCS_VASTART(SDValue Op, SelectionDAG ) const;
+  SDValue LowerAAPCSFromDarwin_VASTART(SDValue Op, SelectionDAG ) const;
   SDValue LowerDarwin_VASTART(SDValue Op, SelectionDAG ) const;

Same problem as with clang-format: clang-tidy suggests 
"lowerAapcsFromDarwinVastart", which would mean modifying the whole file for 
consistency. Should we set clang-tidy to ignore this file?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Adrien Guinet via Phabricator via cfe-commits]

aguinet updated this revision to Diff 298461.
aguinet added a comment.

Update format + llvm tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/Specifiers.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/darwin_abi.c
  clang/test/CodeGen/darwin_abi_empty_structs.cpp
  clang/test/CodeGen/darwin_abi_vaarg.c
  clang/test/CodeGen/debug-info-cc.c
  clang/test/CodeGenCXX/darwinabi-returnthis.cpp
  clang/tools/libclang/CXType.cpp
  llvm/include/llvm/BinaryFormat/Dwarf.def
  llvm/include/llvm/IR/CallingConv.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/Target/AArch64/AArch64CallingConvention.cpp
  llvm/lib/Target/AArch64/AArch64FastISel.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.h
  llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
  llvm/test/CodeGen/AArch64/darwin_abi.ll
  llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll

Index: llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/darwin_abi_vararg.ll
@@ -0,0 +1,39 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-pc-linux | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-pc-linux"
+
+%struct.__va_list = type { i8*, i8*, i8*, i32, i32 }
+
+define dso_local aarch64_darwincc void @foo(i32 %n, ...) local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:sub sp, sp, #48 // =48
+; CHECK-NEXT:stp x29, x30, [sp, #32] // 16-byte Folded Spill
+; CHECK-NEXT:add x29, sp, #32 // =32
+; CHECK-NEXT:add x8, x29, #16 // =16
+; CHECK-NEXT:mov x1, sp
+; CHECK-NEXT:str xzr, [sp, #24]
+; CHECK-NEXT:str x8, [sp]
+; CHECK-NEXT:bl vfoo
+; CHECK-NEXT:ldp x29, x30, [sp, #32] // 16-byte Folded Reload
+; CHECK-NEXT:add sp, sp, #48 // =48
+; CHECK-NEXT:ret
+entry:
+  %va = alloca %struct.__va_list, align 8
+  %0 = bitcast %struct.__va_list* %va to i8*
+  call void @llvm.va_start(i8* nonnull %0)
+  call void @vfoo(i32 %n, %struct.__va_list* nonnull %va) #1
+  call void @llvm.va_end(i8* nonnull %0)
+  ret void
+}
+
+declare void @llvm.va_start(i8*) #1
+
+declare dso_local void @vfoo(i32, %struct.__va_list*) local_unnamed_addr #0
+
+declare void @llvm.va_end(i8*) #1
+
+attributes #0 = { nounwind "disable-tail-calls"="false" "frame-pointer"="non-leaf" "target-cpu"="generic" }
+attributes #1 = { nounwind }
Index: llvm/test/CodeGen/AArch64/darwin_abi.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/darwin_abi.ll
@@ -0,0 +1,29 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-pc-linux | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-pc-linux"
+
+define dso_local aarch64_darwincc signext i16 @f1(i16 signext %a) local_unnamed_addr #0 {
+; CHECK-LABEL: f1:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:add w8, w0, #1 // =1
+; CHECK-NEXT:sxth w0, w8
+; CHECK-NEXT:ret
+entry:
+  %add = add i16 %a, 1
+  ret i16 %add
+}
+
+define dso_local aarch64_darwincc zeroext i16 @f2(i16 zeroext %a) local_unnamed_addr #0 {
+; CHECK-LABEL: f2:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:add w8, w0, #1 // =1
+; CHECK-NEXT:and w0, w8, #0x
+; CHECK-NEXT:ret
+entry:
+  %add = add i16 %a, 1
+  ret i16 %add
+}
+
+attributes #0 = { norecurse nounwind readnone "disable-tail-calls"="false" "frame-pointer"="non-leaf" "target-cpu"="generic" }
Index: llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
===
--- llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
+++ llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
@@ -474,8 +474,8 @@
   uint64_t StackOffset = Handler.StackUsed;
   if (F.isVarArg()) {
 auto  = MF.getSubtarget();
-if (!Subtarget.isTargetDarwin()) {
-// FIXME: we need to reimplement saveVarArgsRegisters from
+if 

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

[Adrien Guinet via Phabricator via cfe-commits]

aguinet added a comment.

In D89490#2333073 , @mstorsjo wrote:

> I see that the llvm side of the patch lacks tests?

Sorry I forgot to add them indeed... I will push a patch with these.

About the formatting, git clang-format HEAD~1 did this, and it looks by the 
automated test that some issues are still there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

I see that the llvm side of the patch lacks tests?




Comment at: llvm/lib/IR/AsmWriter.cpp:379
+Out << "aarch64_darwincc";
+break;
   case CallingConv::SPIR_FUNC: Out << "spir_func"; break;

The new code here has a different indentation than the rest; the same in a 
couple other places throughout the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Adrien Guinet via Phabricator via cfe-commits]

aguinet created this revision.
aguinet added reviewers: t.p.northover, mstorsjo, thegameg.
Herald added subscribers: llvm-commits, cfe-commits, arphaman, dexonsmith, 
steven_wu, hiraditya, kristof.beyls, krytarowski.
Herald added a reviewer: aaron.ballman.
Herald added projects: clang, LLVM.
aguinet requested review of this revision.

This patch introduces the "darwin_abi" function attribute, to be able to 
compile functions for the Apple ARM64 ABI when targeting other ARM64 OSes. For 
now, only Linux/ARM64 is supported/tested.

I explained the motivation behind this and some limitations in this mail on 
llvm-dev: http://lists.llvm.org/pipermail/llvm-dev/2020-October/145680.html . 
Please note that, in this mail, I call this attribute "apple_abi". I decided to 
change it to "darwin_abi", because everything Apple-related is called "darwin" 
in clang/llvm. That being said, I don't have any strong opinion on this, and 
will be willing to hear any argument in favoir of one or the other.

It does not allow to target all the differences that exist in the Darwin ARM64 
ABI against the standard AAPCS one (see [1] for the exhaustive list).

What I beleive is implemented:

- targeting the Darwin AAPCS ABI for C functions, especially those with 
variadic arguments. This means everything in section "Pass Arguments to 
Functions Correctly" and "Update Code that Passes Arguments to Variadic 
Functions" in [1].

- "The ABI requires the complete object (C1) and base-object (C2) constructors 
to return this to their callers. Similarly, the complete object (D1 
) and base-object (D2 
) destructors return this. This behavior matches 
the ARM 32-bit C++ ABI." (quoting [1]). I am not sure this would be useful to 
anyone, but that was not that hard to implement it, so I put it. The C++ 
support isn't complete in this patch though (see below), so maybe it's better 
to remove it.

- "When passing parameters to a function, Apple platforms ignore empty 
structures unless those structures have a nontrivial destructor or copy 
constructor. When passing such nontrivial structures, treat them as aggregates 
with one byte member in the generic manner." (quoting [1])

- properly forwarding variadic arguments from a "darwin_abi function" to a 
linux one using va_list, by zeroing the relevant fields in the linux va_list 
structure to make sure only stack arguments will be used (cf. discussions in 
the aforementioned ML thread)

What isn't implemented and isn't supported (quoting [1]):

- "The ABI provides a fixed layout of two size_t words for array cookies, with 
no extra alignment requirements. This behavior matches the ARM 32-bit C++ ABI." 
=> this would require allowing the "darwin_abi" to be set on class types, and I 
think the overall feature would make a non-trivial patch. I prefer doing the C 
ABI right and eventually implement this afterwards.

- "A pointer to a function declared as extern “C” isn’t interchangeable with a 
function declared as extern “c++”. This behavior differs from the ARM64 ABI, in 
which the functions are interchangeable." => I'm not sure to find where this is 
tested in clang, let alone this would be possible to be tested.

- My understanding is that we should keep the Itanium mangling, even for 
functions with the "darwin_abi" attributes, so I didn't implemented the 
mangling differences. For instance, if I understood correctly, setting the 
"ms_abi" on a C++ function doesn't use the MSVC mangling.

- Everything remaining in the "Handle C++ Differences" section in [1]

[1] 
https://developer.apple.com/documentation/xcode/writing_arm64_code_for_apple_platforms
 for reference


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89490

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/Specifiers.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/darwin_abi.c
  clang/test/CodeGen/darwin_abi_empty_structs.cpp
  clang/test/CodeGen/darwin_abi_vaarg.c
  clang/test/CodeGen/debug-info-cc.c
  clang/test/CodeGenCXX/darwinabi-returnthis.cpp
  clang/tools/libclang/CXType.cpp
  llvm/include/llvm/BinaryFormat/Dwarf.def
  llvm/include/llvm/IR/CallingConv.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/Target/AArch64/AArch64CallingConvention.cpp
  llvm/lib/Target/AArch64/AArch64FastISel.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.h
  llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp