Message ID | 20221122090114.38090-1-christophe.lyon@arm.com |
---|---|
State | New |
Headers | show |
Series | aarch64: Fix test_dfp_17.c for big-endian [PR 107604] | expand |
Christophe Lyon via Gcc-patches <gcc-patches@gcc.gnu.org> 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
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?
Richard Earnshaw via Gcc-patches <gcc-patches@gcc.gnu.org> 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
On 22/11/2022 11:21, Richard Sandiford wrote: > Richard Earnshaw via Gcc-patches <gcc-patches@gcc.gnu.org> 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.
On 11/22/22 12:33, Richard Earnshaw wrote: > > > On 22/11/2022 11:21, Richard Sandiford wrote: >> Richard Earnshaw via Gcc-patches <gcc-patches@gcc.gnu.org> 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.
On 11/22/22 12:33, Richard Earnshaw wrote: > > > On 22/11/2022 11:21, Richard Sandiford wrote: >> Richard Earnshaw via Gcc-patches <gcc-patches@gcc.gnu.org> 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.
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 <gcc-patches@gcc.gnu.org> 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.
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