Re: [PATCH] aarch64: Fix test_dfp_17.c for big-endian [PR 107604]

2022-11-22 Thread Christophe Lyon via Gcc-patches




On 11/22/22 12:33, Richard Earnshaw wrote:



On 22/11/2022 11:21, Richard Sandiford wrote:

Richard Earnshaw via Gcc-patches  writes:

On 22/11/2022 09:01, Christophe Lyon via Gcc-patches wrote:

gcc.target/aarch64/aapcs64/test_dfp_17.c has been failing on
big-endian, because the _Decimal32 on-stack argument is not padded in
the same direction depending on endianness.

This patch fixes the testcase so that it expects the argument in the
right stack location, similarly to what other tests do in the same
directory.

gcc/testsuite/ChangeLog:

PR target/107604
* gcc.target/aarch64/aapcs64/test_dfp_17.c: Fix for big-endian.
---
   gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c | 4 
   1 file changed, 4 insertions(+)

diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c 
b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c

index 22dc462bf7c..3c45f715cf7 100644
--- a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c
+++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c
@@ -32,6 +32,10 @@ struct z b = { 9.0dd, 10.0dd, 11.0dd, 12.0dd };
 ANON(struct z, a, D1)
 ANON(struct z, b, STACK)
 ANON(int , 5, W0)
+#ifndef __AAPCS64_BIG_ENDIAN__
 ANON(_Decimal32, f1, STACK+32) /* Note: no promotion to 
_Decimal64.  */

+#else
+  ANON(_Decimal32, f1, STACK+36) /* Note: no promotion to 
_Decimal64.  */

+#endif
 LAST_ANON(_Decimal64, 0.5dd, STACK+40)
   #endif


Why would a Decimal32 change stack placement based on the endianness?
Isn't it a 4-byte object?


Yes, but PARM_BOUNDARY (64) sets a minimum alignment for all stack 
arguments.


Richard


Ah, OK.


Indeed, it was not immediately obvious to me either when looking at 
aarch64_layout_arg. aarch64_function_arg_padding comes into play, too.




I wonder if we should have a new macro in the tests, something like 
ANON_PADDED to describe this case and that works things out more 
automagically for big-endian.

Maybe, there are quite a few tests under aapcs64 which have a similar
#ifndef __AAPCS64_BIG_ENDIAN__



I notice the new ANON definition is not correctly indented.

R.


Re: [PATCH] aarch64: Fix test_dfp_17.c for big-endian [PR 107604]

2022-11-22 Thread Richard Earnshaw via Gcc-patches




On 22/11/2022 13:09, Christophe Lyon wrote:



On 11/22/22 12:33, Richard Earnshaw wrote:



On 22/11/2022 11:21, Richard Sandiford wrote:

Richard Earnshaw via Gcc-patches  writes:

On 22/11/2022 09:01, Christophe Lyon via Gcc-patches wrote:
gcc.target/aarch64/aapcs64/test_dfp_17.c has been failing on 
big-endian, because the _Decimal32 on-stack argument is not

padded in the same direction depending on endianness.

This patch fixes the testcase so that it expects the argument
in the right stack location, similarly to what other tests do
in the same directory.

gcc/testsuite/ChangeLog:

PR target/107604 * gcc.target/aarch64/aapcs64/test_dfp_17.c:
Fix for big-endian. --- 
gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c | 4

 1 file changed, 4 insertions(+)

diff --git
a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c 
b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c index

22dc462bf7c..3c45f715cf7 100644 ---
a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c +++
b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c @@
-32,6 +32,10 @@ struct z b = { 9.0dd, 10.0dd, 11.0dd, 12.0dd
}; ANON(struct z, a, D1) ANON(struct z, b, STACK) ANON(int , 5,
W0) +#ifndef __AAPCS64_BIG_ENDIAN__ ANON(_Decimal32, f1,
STACK+32) /* Note: no promotion to _Decimal64.  */ +#else +
ANON(_Decimal32, f1, STACK+36) /* Note: no promotion to 
_Decimal64.  */ +#endif LAST_ANON(_Decimal64, 0.5dd, STACK+40) #endif


Why would a Decimal32 change stack placement based on the
endianness? Isn't it a 4-byte object?


Yes, but PARM_BOUNDARY (64) sets a minimum alignment for all stack
 arguments.

Richard


Ah, OK.
Indeed, it was not immediately obvious to me either, when looking at 
aarch64_layout_arg. aarch64_function_arg_padding comes into play, too.




I wonder if we should have a new macro in the tests, something like 
ANON_PADDED to describe this case and that works things out more 
automagically for big-endian.

Maybe. There are many other tests under aapcs64/ which have a similar
#ifndef __AAPCS64_BIG_ENDIAN__



Yes, it could be used to clean all those up as well.




I notice the new ANON definition is not correctly indented.

It looks OK on my side (2 spaces).


Never mind then, it must be a quirk of how the diff is displayed.


Thanks,

Christophe



R.


Re: [PATCH] aarch64: Fix test_dfp_17.c for big-endian [PR 107604]

2022-11-22 Thread Christophe Lyon via Gcc-patches




On 11/22/22 12:33, Richard Earnshaw wrote:



On 22/11/2022 11:21, Richard Sandiford wrote:

Richard Earnshaw via Gcc-patches  writes:

On 22/11/2022 09:01, Christophe Lyon via Gcc-patches wrote:
gcc.target/aarch64/aapcs64/test_dfp_17.c has been failing on 
big-endian, because the _Decimal32 on-stack argument is not

padded in the same direction depending on endianness.

This patch fixes the testcase so that it expects the argument
in the right stack location, similarly to what other tests do
in the same directory.

gcc/testsuite/ChangeLog:

PR target/107604 * gcc.target/aarch64/aapcs64/test_dfp_17.c:
Fix for big-endian. --- 
gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c | 4

 1 file changed, 4 insertions(+)

diff --git
a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c 
b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c index

22dc462bf7c..3c45f715cf7 100644 ---
a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c +++
b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c @@
-32,6 +32,10 @@ struct z b = { 9.0dd, 10.0dd, 11.0dd, 12.0dd
}; ANON(struct z, a, D1) ANON(struct z, b, STACK) ANON(int , 5,
W0) +#ifndef __AAPCS64_BIG_ENDIAN__ ANON(_Decimal32, f1,
STACK+32) /* Note: no promotion to _Decimal64.  */ +#else +
ANON(_Decimal32, f1, STACK+36) /* Note: no promotion to 
_Decimal64.  */ +#endif LAST_ANON(_Decimal64, 0.5dd, STACK+40) 
#endif


Why would a Decimal32 change stack placement based on the
endianness? Isn't it a 4-byte object?


Yes, but PARM_BOUNDARY (64) sets a minimum alignment for all stack
 arguments.

Richard


Ah, OK.
Indeed, it was not immediately obvious to me either, when looking at 
aarch64_layout_arg. aarch64_function_arg_padding comes into play, too.




I wonder if we should have a new macro in the tests, something like 
ANON_PADDED to describe this case and that works things out more 
automagically for big-endian.

Maybe. There are many other tests under aapcs64/ which have a similar
#ifndef __AAPCS64_BIG_ENDIAN__



I notice the new ANON definition is not correctly indented.

It looks OK on my side (2 spaces).

Thanks,

Christophe



R.


Re: [PATCH] aarch64: Fix test_dfp_17.c for big-endian [PR 107604]

2022-11-22 Thread Richard Earnshaw via Gcc-patches




On 22/11/2022 11:21, Richard Sandiford wrote:

Richard Earnshaw via Gcc-patches  writes:

On 22/11/2022 09:01, Christophe Lyon via Gcc-patches wrote:

gcc.target/aarch64/aapcs64/test_dfp_17.c has been failing on
big-endian, because the _Decimal32 on-stack argument is not padded in
the same direction depending on endianness.

This patch fixes the testcase so that it expects the argument in the
right stack location, similarly to what other tests do in the same
directory.

gcc/testsuite/ChangeLog:

PR target/107604
* gcc.target/aarch64/aapcs64/test_dfp_17.c: Fix for big-endian.
---
   gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c | 4 
   1 file changed, 4 insertions(+)

diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c 
b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c
index 22dc462bf7c..3c45f715cf7 100644
--- a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c
+++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c
@@ -32,6 +32,10 @@ struct z b = { 9.0dd, 10.0dd, 11.0dd, 12.0dd };
 ANON(struct z, a, D1)
 ANON(struct z, b, STACK)
 ANON(int , 5, W0)
+#ifndef __AAPCS64_BIG_ENDIAN__
 ANON(_Decimal32, f1, STACK+32) /* Note: no promotion to _Decimal64.  */
+#else
+  ANON(_Decimal32, f1, STACK+36) /* Note: no promotion to _Decimal64.  */
+#endif
 LAST_ANON(_Decimal64, 0.5dd, STACK+40)
   #endif


Why would a Decimal32 change stack placement based on the endianness?
Isn't it a 4-byte object?


Yes, but PARM_BOUNDARY (64) sets a minimum alignment for all stack arguments.

Richard


Ah, OK.

I wonder if we should have a new macro in the tests, something like 
ANON_PADDED to describe this case and that works things out more 
automagically for big-endian.


I notice the new ANON definition is not correctly indented.

R.


Re: [PATCH] aarch64: Fix test_dfp_17.c for big-endian [PR 107604]

2022-11-22 Thread Richard Sandiford via Gcc-patches
Richard Earnshaw via Gcc-patches  writes:
> On 22/11/2022 09:01, Christophe Lyon via Gcc-patches wrote:
>> gcc.target/aarch64/aapcs64/test_dfp_17.c has been failing on
>> big-endian, because the _Decimal32 on-stack argument is not padded in
>> the same direction depending on endianness.
>> 
>> This patch fixes the testcase so that it expects the argument in the
>> right stack location, similarly to what other tests do in the same
>> directory.
>> 
>> gcc/testsuite/ChangeLog:
>> 
>>  PR target/107604
>>  * gcc.target/aarch64/aapcs64/test_dfp_17.c: Fix for big-endian.
>> ---
>>   gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c | 4 
>>   1 file changed, 4 insertions(+)
>> 
>> diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c 
>> b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c
>> index 22dc462bf7c..3c45f715cf7 100644
>> --- a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c
>> +++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c
>> @@ -32,6 +32,10 @@ struct z b = { 9.0dd, 10.0dd, 11.0dd, 12.0dd };
>> ANON(struct z, a, D1)
>> ANON(struct z, b, STACK)
>> ANON(int , 5, W0)
>> +#ifndef __AAPCS64_BIG_ENDIAN__
>> ANON(_Decimal32, f1, STACK+32) /* Note: no promotion to _Decimal64.  */
>> +#else
>> +  ANON(_Decimal32, f1, STACK+36) /* Note: no promotion to _Decimal64.  */
>> +#endif
>> LAST_ANON(_Decimal64, 0.5dd, STACK+40)
>>   #endif
>
> Why would a Decimal32 change stack placement based on the endianness? 
> Isn't it a 4-byte object?

Yes, but PARM_BOUNDARY (64) sets a minimum alignment for all stack arguments.

Richard


Re: [PATCH] aarch64: Fix test_dfp_17.c for big-endian [PR 107604]

2022-11-22 Thread Richard Earnshaw via Gcc-patches




On 22/11/2022 09:01, Christophe Lyon via Gcc-patches wrote:

gcc.target/aarch64/aapcs64/test_dfp_17.c has been failing on
big-endian, because the _Decimal32 on-stack argument is not padded in
the same direction depending on endianness.

This patch fixes the testcase so that it expects the argument in the
right stack location, similarly to what other tests do in the same
directory.

gcc/testsuite/ChangeLog:

PR target/107604
* gcc.target/aarch64/aapcs64/test_dfp_17.c: Fix for big-endian.
---
  gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c 
b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c
index 22dc462bf7c..3c45f715cf7 100644
--- a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c
+++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c
@@ -32,6 +32,10 @@ struct z b = { 9.0dd, 10.0dd, 11.0dd, 12.0dd };
ANON(struct z, a, D1)
ANON(struct z, b, STACK)
ANON(int , 5, W0)
+#ifndef __AAPCS64_BIG_ENDIAN__
ANON(_Decimal32, f1, STACK+32) /* Note: no promotion to _Decimal64.  */
+#else
+  ANON(_Decimal32, f1, STACK+36) /* Note: no promotion to _Decimal64.  */
+#endif
LAST_ANON(_Decimal64, 0.5dd, STACK+40)
  #endif


Why would a Decimal32 change stack placement based on the endianness? 
Isn't it a 4-byte object?


Re: [PATCH] aarch64: Fix test_dfp_17.c for big-endian [PR 107604]

2022-11-22 Thread Richard Sandiford via Gcc-patches
Christophe Lyon via Gcc-patches  writes:
> gcc.target/aarch64/aapcs64/test_dfp_17.c has been failing on
> big-endian, because the _Decimal32 on-stack argument is not padded in
> the same direction depending on endianness.
>
> This patch fixes the testcase so that it expects the argument in the
> right stack location, similarly to what other tests do in the same
> directory.
>
> gcc/testsuite/ChangeLog:
>
>   PR target/107604
>   * gcc.target/aarch64/aapcs64/test_dfp_17.c: Fix for big-endian.

OK, thanks.

Richard

> ---
>  gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c 
> b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c
> index 22dc462bf7c..3c45f715cf7 100644
> --- a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c
> +++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c
> @@ -32,6 +32,10 @@ struct z b = { 9.0dd, 10.0dd, 11.0dd, 12.0dd };
>ANON(struct z, a, D1)
>ANON(struct z, b, STACK)
>ANON(int , 5, W0)
> +#ifndef __AAPCS64_BIG_ENDIAN__
>ANON(_Decimal32, f1, STACK+32) /* Note: no promotion to _Decimal64.  */
> +#else
> +  ANON(_Decimal32, f1, STACK+36) /* Note: no promotion to _Decimal64.  */
> +#endif
>LAST_ANON(_Decimal64, 0.5dd, STACK+40)
>  #endif


[PATCH] aarch64: Fix test_dfp_17.c for big-endian [PR 107604]

2022-11-22 Thread Christophe Lyon via Gcc-patches
gcc.target/aarch64/aapcs64/test_dfp_17.c has been failing on
big-endian, because the _Decimal32 on-stack argument is not padded in
the same direction depending on endianness.

This patch fixes the testcase so that it expects the argument in the
right stack location, similarly to what other tests do in the same
directory.

gcc/testsuite/ChangeLog:

PR target/107604
* gcc.target/aarch64/aapcs64/test_dfp_17.c: Fix for big-endian.
---
 gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c 
b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c
index 22dc462bf7c..3c45f715cf7 100644
--- a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c
+++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_dfp_17.c
@@ -32,6 +32,10 @@ struct z b = { 9.0dd, 10.0dd, 11.0dd, 12.0dd };
   ANON(struct z, a, D1)
   ANON(struct z, b, STACK)
   ANON(int , 5, W0)
+#ifndef __AAPCS64_BIG_ENDIAN__
   ANON(_Decimal32, f1, STACK+32) /* Note: no promotion to _Decimal64.  */
+#else
+  ANON(_Decimal32, f1, STACK+36) /* Note: no promotion to _Decimal64.  */
+#endif
   LAST_ANON(_Decimal64, 0.5dd, STACK+40)
 #endif
-- 
2.34.1