[PATCH] D129135: [doc][ReleaseNotes] Document AArch64 SVE ABI fix from D127209

2022-07-07 Thread Peter Waller via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2db2a4e11240: [doc][ReleaseNotes] Document AArch64 SVE ABI 
fix from D127209 (authored by peterwaller-arm).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129135

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -503,6 +503,14 @@
   (e.g. ``int : 0``) no longer prevents the structure from being considered a
   homogeneous floating-point or vector aggregate. The new behavior agrees with
   the AAPCS specification, and matches the similar bug fix in GCC 12.1.
+- Targeting AArch64, since D127209 LLVM now only preserves the z8-z23
+  and p4-p15 registers across a call if the registers z0-z7 or p0-p3 are
+  used to pass data into or out of a subroutine. The new behavior
+  matches the AAPCS. Previously LLVM preserved z8-z23 and p4-p15 across
+  a call if the callee had an SVE type anywhere in its signature. This
+  would cause an incorrect use of the caller-preserved z8-z23 and p4-p15
+  ABI for example if the 9th argument or greater were the first SVE type
+  in the signature of a function.
 - All copy constructors can now be trivial if they are not user-provided,
   regardless of the type qualifiers of the argument of the defaulted 
constructor,
   fixing dr2171.


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -503,6 +503,14 @@
   (e.g. ``int : 0``) no longer prevents the structure from being considered a
   homogeneous floating-point or vector aggregate. The new behavior agrees with
   the AAPCS specification, and matches the similar bug fix in GCC 12.1.
+- Targeting AArch64, since D127209 LLVM now only preserves the z8-z23
+  and p4-p15 registers across a call if the registers z0-z7 or p0-p3 are
+  used to pass data into or out of a subroutine. The new behavior
+  matches the AAPCS. Previously LLVM preserved z8-z23 and p4-p15 across
+  a call if the callee had an SVE type anywhere in its signature. This
+  would cause an incorrect use of the caller-preserved z8-z23 and p4-p15
+  ABI for example if the 9th argument or greater were the first SVE type
+  in the signature of a function.
 - All copy constructors can now be trivial if they are not user-provided,
   regardless of the type qualifiers of the argument of the defaulted constructor,
   fixing dr2171.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129135: [doc][ReleaseNotes] Document AArch64 SVE ABI fix from D127209

2022-07-05 Thread Peter Waller via Phabricator via cfe-commits 
peterwaller-arm updated this revision to Diff 442264.
peterwaller-arm added a comment.

s/p0-p4/p0-p3/, thanks for the keen eye, rsandifo-arm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129135

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -499,6 +499,14 @@
   (e.g. ``int : 0``) no longer prevents the structure from being considered a
   homogeneous floating-point or vector aggregate. The new behavior agrees with
   the AAPCS specification, and matches the similar bug fix in GCC 12.1.
+- Targeting AArch64, since D127209 LLVM now only preserves the z8-z23
+  and p4-p15 registers across a call if the registers z0-z7 or p0-p3 are
+  used to pass data into or out of a subroutine. The new behavior
+  matches the AAPCS. Previously LLVM preserved z8-z23 and p4-p15 across
+  a call if the callee had an SVE type anywhere in its signature. This
+  would cause an incorrect use of the caller-preserved z8-z23 and p4-p15
+  ABI for example if the 9th argument or greater were the first SVE type
+  in the signature of a function.
 - All copy constructors can now be trivial if they are not user-provided,
   regardless of the type qualifiers of the argument of the defaulted 
constructor,
   fixing dr2171.


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -499,6 +499,14 @@
   (e.g. ``int : 0``) no longer prevents the structure from being considered a
   homogeneous floating-point or vector aggregate. The new behavior agrees with
   the AAPCS specification, and matches the similar bug fix in GCC 12.1.
+- Targeting AArch64, since D127209 LLVM now only preserves the z8-z23
+  and p4-p15 registers across a call if the registers z0-z7 or p0-p3 are
+  used to pass data into or out of a subroutine. The new behavior
+  matches the AAPCS. Previously LLVM preserved z8-z23 and p4-p15 across
+  a call if the callee had an SVE type anywhere in its signature. This
+  would cause an incorrect use of the caller-preserved z8-z23 and p4-p15
+  ABI for example if the 9th argument or greater were the first SVE type
+  in the signature of a function.
 - All copy constructors can now be trivial if they are not user-provided,
   regardless of the type qualifiers of the argument of the defaulted constructor,
   fixing dr2171.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129135: [doc][ReleaseNotes] Document AArch64 SVE ABI fix from D127209

2022-07-05 Thread Richard Sandiford via Phabricator via cfe-commits 
rsandifo-arm added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:503
+- Targeting AArch64, since D127209 LLVM now only preserves the z8-z23
+  and p4-p15 registers across a call if the registers z0-z7 or p0-p4 are
+  used to pass data into or out of a subroutine. The new behavior

Should be p0-p3 rather than p0-p4.  LGTM with that change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129135

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

[PATCH] D129135: [doc][ReleaseNotes] Document AArch64 SVE ABI fix from D127209

2022-07-05 Thread David Spickett via Phabricator via cfe-commits 
DavidSpickett accepted this revision.
DavidSpickett added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129135

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

[PATCH] D129135: [doc][ReleaseNotes] Document AArch64 SVE ABI fix from D127209

2022-07-05 Thread Peter Waller via Phabricator via cfe-commits 
peterwaller-arm updated this revision to Diff 442253.
peterwaller-arm marked an inline comment as done.
peterwaller-arm edited the summary of this revision.
peterwaller-arm added a comment.

- Update commit message: "This affects" -> "The fix affects"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129135

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -499,6 +499,14 @@
   (e.g. ``int : 0``) no longer prevents the structure from being considered a
   homogeneous floating-point or vector aggregate. The new behavior agrees with
   the AAPCS specification, and matches the similar bug fix in GCC 12.1.
+- Targeting AArch64, since D127209 LLVM now only preserves the z8-z23
+  and p4-p15 registers across a call if the registers z0-z7 or p0-p4 are
+  used to pass data into or out of a subroutine. The new behavior
+  matches the AAPCS. Previously LLVM preserved z8-z23 and p4-p15 across
+  a call if the callee had an SVE type anywhere in its signature. This
+  would cause an incorrect use of the caller-preserved z8-z23 and p4-p15
+  ABI for example if the 9th argument or greater were the first SVE type
+  in the signature of a function.
 - All copy constructors can now be trivial if they are not user-provided,
   regardless of the type qualifiers of the argument of the defaulted 
constructor,
   fixing dr2171.


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -499,6 +499,14 @@
   (e.g. ``int : 0``) no longer prevents the structure from being considered a
   homogeneous floating-point or vector aggregate. The new behavior agrees with
   the AAPCS specification, and matches the similar bug fix in GCC 12.1.
+- Targeting AArch64, since D127209 LLVM now only preserves the z8-z23
+  and p4-p15 registers across a call if the registers z0-z7 or p0-p4 are
+  used to pass data into or out of a subroutine. The new behavior
+  matches the AAPCS. Previously LLVM preserved z8-z23 and p4-p15 across
+  a call if the callee had an SVE type anywhere in its signature. This
+  would cause an incorrect use of the caller-preserved z8-z23 and p4-p15
+  ABI for example if the 9th argument or greater were the first SVE type
+  in the signature of a function.
 - All copy constructors can now be trivial if they are not user-provided,
   regardless of the type qualifiers of the argument of the defaulted constructor,
   fixing dr2171.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129135: [doc][ReleaseNotes] Document AArch64 SVE ABI fix from D127209

2022-07-05 Thread Peter Waller via Phabricator via cfe-commits 
peterwaller-arm marked an inline comment as done.
peterwaller-arm added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:503
+- Targeting AArch64, LLVM now only preserves the z8-z23 registers across
+  a call if the registers z0-z7 are used to pass data into or out of a
+  subroutine. This new behavior now matches the AAPCS. Previously LLVM

rsandifo-arm wrote:
> Pedantically, I think it should be “the registers z0-z7 or p0-p3”.
I've now switched to writing out all of the registers every time, hope that's 
clearer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129135

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

[PATCH] D129135: [doc][ReleaseNotes] Document AArch64 SVE ABI fix from D127209

2022-07-05 Thread Peter Waller via Phabricator via cfe-commits 
peterwaller-arm updated this revision to Diff 442251.
peterwaller-arm added a comment.

- Cite D127209  in the release note per 
tschuett comment.
- Be more specific about p0-p4, remove 'by analogy', per rsandifo-arm comment.
- Clarify 9th argument or greater, per DavidSpicket comment.

Thank you everyone for your inputs, hope this addresses everything so far, 
happy to receive further input.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129135

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -499,6 +499,14 @@
   (e.g. ``int : 0``) no longer prevents the structure from being considered a
   homogeneous floating-point or vector aggregate. The new behavior agrees with
   the AAPCS specification, and matches the similar bug fix in GCC 12.1.
+- Targeting AArch64, since D127209 LLVM now only preserves the z8-z23
+  and p4-p15 registers across a call if the registers z0-z7 or p0-p4 are
+  used to pass data into or out of a subroutine. The new behavior
+  matches the AAPCS. Previously LLVM preserved z8-z23 and p4-p15 across
+  a call if the callee had an SVE type anywhere in its signature. This
+  would cause an incorrect use of the caller-preserved z8-z23 and p4-p15
+  ABI for example if the 9th argument or greater were the first SVE type
+  in the signature of a function.
 - All copy constructors can now be trivial if they are not user-provided,
   regardless of the type qualifiers of the argument of the defaulted 
constructor,
   fixing dr2171.


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -499,6 +499,14 @@
   (e.g. ``int : 0``) no longer prevents the structure from being considered a
   homogeneous floating-point or vector aggregate. The new behavior agrees with
   the AAPCS specification, and matches the similar bug fix in GCC 12.1.
+- Targeting AArch64, since D127209 LLVM now only preserves the z8-z23
+  and p4-p15 registers across a call if the registers z0-z7 or p0-p4 are
+  used to pass data into or out of a subroutine. The new behavior
+  matches the AAPCS. Previously LLVM preserved z8-z23 and p4-p15 across
+  a call if the callee had an SVE type anywhere in its signature. This
+  would cause an incorrect use of the caller-preserved z8-z23 and p4-p15
+  ABI for example if the 9th argument or greater were the first SVE type
+  in the signature of a function.
 - All copy constructors can now be trivial if they are not user-provided,
   regardless of the type qualifiers of the argument of the defaulted constructor,
   fixing dr2171.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129135: [doc][ReleaseNotes] Document AArch64 SVE ABI fix from D127209

2022-07-05 Thread David Spickett via Phabricator via cfe-commits 
DavidSpickett added a comment.

> My intent was to express that the issue is present where it's the 9th or 
> later arguments.

So I take from that that it does not matter if arguments 1-8 were all integers 
and so did not get put in SIMD/floating point registers. I think I am confusing 
what the ABI says (and what llvm *now does*) with the way the previous llvm 
behaviour. So the statement answers the question "of the functions in my 
project which ones might change", it's the ones where the first SVE type 
argument was argument 9 or greater.

Should the release note also say that the key is the *first* SVE type parameter 
being argument 9, not just having an SVE type in argument 9 at all?

Commit msg:

> This affects functions where the first SVE parameter appears in the 9th or 
> later arguments, and the function does not return an SVE type.

Note:

> This would cause an incorrect use of the caller-preserved z8-z23 ABI for 
> example if the 9th argument to a function were an SVE type.

Or in other words what about arguments 1-8, can they be SVE and still hit this 
issue? I realise this is not supposed to be a full on super detailed report but 
mentioning because the "first" stood out to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129135

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

[PATCH] D129135: [doc][ReleaseNotes] Document AArch64 SVE ABI fix from D127209

2022-07-05 Thread Richard Sandiford via Phabricator via cfe-commits 
rsandifo-arm added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:503
+- Targeting AArch64, LLVM now only preserves the z8-z23 registers across
+  a call if the registers z0-z7 are used to pass data into or out of a
+  subroutine. This new behavior now matches the AAPCS. Previously LLVM

Pedantically, I think it should be “the registers z0-z7 or p0-p3”.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129135

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

[PATCH] D129135: [doc][ReleaseNotes] Document AArch64 SVE ABI fix from D127209

2022-07-05 Thread Thorsten via Phabricator via cfe-commits 
tschuett added a comment.

Could you cite a Diff where this was changed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129135

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

[PATCH] D129135: [doc][ReleaseNotes] Document AArch64 SVE ABI fix from D127209

2022-07-05 Thread Peter Waller via Phabricator via cfe-commits 
peterwaller-arm added a comment.

In D129135#3629860 , @peterwaller-arm 
wrote:

> but I can't identify a specific worse issue at present.

Thinking a little harder, one breaking case is where a 
callee-compiled-with-wrong-logic looks at its signature, concludes it does not 
need to preserve the regs (because the SVE type is in the 9th argument, for 
example), but is called by a different compiler which correctly implements the 
ABI, so neither side preserves registers which require preservation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129135

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

[PATCH] D129135: [doc][ReleaseNotes] Document AArch64 SVE ABI fix from D127209

2022-07-05 Thread Peter Waller via Phabricator via cfe-commits 
peterwaller-arm added a comment.

In D129135#3629833 , @DavidSpickett 
wrote:

> From reading the release note my understanding is that before this fix the 
> caller of a function would store `z0-z7` in situations where it did not need 
> to.

I believe you are correct, with an inkling of doubt in the back of my mind, but 
I can't identify a specific worse issue at present.

> Wouldn't the type of the preceding 8 arguments also be relevant?

My intent was to express that the issue is present where it's the 9th or later 
arguments.

> This is because the function would return in `z0` therefore the caller must 
> preserve at least `z0` and the ABI tells you to preserve `z0-z7` to do that?

Yes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129135

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

[PATCH] D129135: [doc][ReleaseNotes] Document AArch64 SVE ABI fix from D127209

2022-07-05 Thread David Spickett via Phabricator via cfe-commits 
DavidSpickett added a comment.

From reading the release note my understanding is that before this fix the 
caller of a function would store `z0-z7` in situations where it did not need 
to. Which seems low impact unless you were doing something that read the 
previous stack frame (but nevertheless a difference from the AAPCS).

If that's what you were trying to get across then great!

> This affects functions where the first SVE parameter appears in the 9th or 
> later arguments

I'm reading 
https://github.com/ARM-software/abi-aa/blob/main/aapcs64/aapcs64.rst#642parameter-passing-rules.
 Wouldn't the type of the preceding 8 arguments also be relevant?

Though I was surprised to see that "NSRN" covers floating point and SVE 
registers, so I'm hardly an expert here.

> and the function does not return an SVE type.

This is because the function would return in `z0` therefore the caller must 
preserve at least `z0` and the ABI tells you to preserve `z0-z7` to do that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129135

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

[PATCH] D129135: [doc][ReleaseNotes] Document AArch64 SVE ABI fix from D127209

2022-07-05 Thread Daniel Kiss via Phabricator via cfe-commits 
danielkiss accepted this revision.
danielkiss added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129135

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

[PATCH] D129135: [doc][ReleaseNotes] Document AArch64 SVE ABI fix from D127209

2022-07-05 Thread Peter Waller via Phabricator via cfe-commits 
peterwaller-arm created this revision.
peterwaller-arm added reviewers: rsandifo-arm, kristof.beyls.
Herald added subscribers: ctetreau, tschuett.
Herald added a project: All.
peterwaller-arm requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

D127209  fixed LLVM to bring it in line with 
the AAPCS. This affects
functions where the first SVE parameter appears in the 9th or later
arguments, and the function does not return an SVE type.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129135

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -499,6 +499,15 @@
   (e.g. ``int : 0``) no longer prevents the structure from being considered a
   homogeneous floating-point or vector aggregate. The new behavior agrees with
   the AAPCS specification, and matches the similar bug fix in GCC 12.1.
+- Targeting AArch64, LLVM now only preserves the z8-z23 registers across
+  a call if the registers z0-z7 are used to pass data into or out of a
+  subroutine. This new behavior now matches the AAPCS. Previously LLVM
+  preserved z8-z23 across a call if the callee had an SVE type anywhere
+  in its signature. This would cause an incorrect use of the
+  caller-preserved z8-z23 ABI for example if the 9th argument to a
+  function were an SVE type. The analogous issue and fix applies to
+  predicate register arguments (p0-p3 for passing between subroutines,
+  and p4-15 preserved).
 - All copy constructors can now be trivial if they are not user-provided,
   regardless of the type qualifiers of the argument of the defaulted 
constructor,
   fixing dr2171.


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -499,6 +499,15 @@
   (e.g. ``int : 0``) no longer prevents the structure from being considered a
   homogeneous floating-point or vector aggregate. The new behavior agrees with
   the AAPCS specification, and matches the similar bug fix in GCC 12.1.
+- Targeting AArch64, LLVM now only preserves the z8-z23 registers across
+  a call if the registers z0-z7 are used to pass data into or out of a
+  subroutine. This new behavior now matches the AAPCS. Previously LLVM
+  preserved z8-z23 across a call if the callee had an SVE type anywhere
+  in its signature. This would cause an incorrect use of the
+  caller-preserved z8-z23 ABI for example if the 9th argument to a
+  function were an SVE type. The analogous issue and fix applies to
+  predicate register arguments (p0-p3 for passing between subroutines,
+  and p4-15 preserved).
 - All copy constructors can now be trivial if they are not user-provided,
   regardless of the type qualifiers of the argument of the defaulted constructor,
   fixing dr2171.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits