| Message ID | CADnVucBSHO=JziDqVeNv257myMCoPJraO0Y5tzzZUu2Ob1Hb+g@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | ARM testsuite: force hardfp for addr-modes-float.c | expand |
Hi Charles, On 27/11/17 17:03, Charles Baylis wrote: > Some of the new tests in addr-modes-float.c, which were introduced for > the rework of addressing modes costs [1] fail when GCC is configured > to default to a softfp calling convention. Fix this by annotating the > test functions with __attribute__((pcs("aapcs-vfp"))). The usual approach to this problem is to add an -mfloat-abi=hard to the dg-options of the test (the tests are not dg-run, so there's no link-time mismatch concerns). Any particular reason to use the pcs attribute instead? Thanks, Kyrill > Thanks to Christophe for pointing this out. > > [1] https://gcc.gnu.org/ml/gcc-patches/2017-11/msg02149.html > > <date> Charles Baylis <charles.baylis@linaro.org> > > * gcc.target/arm/addr-modes-float.c (ATTR): New define. > (POST_STORE): Pass ATTR as 2nd argument. > (POST_LOAD): Likewise. > (POST_STORE_VEC): Likewise. > * gcc.target/arm/addr-modes-int.c (ATTR): New define. > (PRE_STORE): Pass ATTR as 2nd argument. > (POST_STORE): Likewise. > (PRE_LOAD): Likewise. > (POST_LOAD): Likewise. > * gcc.target/arm/addr-modes.h (PRE_STORE): New parameter. > (POST_STORE): Likewise. > (POST_STORE_VEC): Likewise. > (PRE_LOAD): Likewise. > (POST_LOAD): Likewise. > (POST_LOAD_VEC): Likewise.
On 27 November 2017 at 17:47, Kyrill Tkachov <kyrylo.tkachov@foss.arm.com> wrote: > Hi Charles, > > On 27/11/17 17:03, Charles Baylis wrote: >> >> Some of the new tests in addr-modes-float.c, which were introduced for >> the rework of addressing modes costs [1] fail when GCC is configured >> to default to a softfp calling convention. Fix this by annotating the >> test functions with __attribute__((pcs("aapcs-vfp"))). > > > The usual approach to this problem is to add an -mfloat-abi=hard to the > dg-options > of the test (the tests are not dg-run, so there's no link-time mismatch > concerns). > Any particular reason to use the pcs attribute instead? With the way I have GCC configured, it doesn't work to do this when including certain system headers, such as arm_neon.h. In file included from /home/cbaylis/tools/sysroot-arm-unknown-linux-gnueabi-git/usr/include/features.h:447, from /home/cbaylis/tools/sysroot-arm-unknown-linux-gnueabi-git/usr/include/bits/libc-header-start.h:33, from /home/cbaylis/tools/sysroot-arm-unknown-linux-gnueabi-git/usr/include/stdint.h:26, from /home/cbaylis/buildarea/gcc/build2/gcc/include/stdint.h:9, from /home/cbaylis/buildarea/gcc/build2/gcc/include/arm_fp16.h:34, from /home/cbaylis/buildarea/gcc/build2/gcc/include/arm_neon.h:41, from /home/cbaylis/srcarea/gcc/gcc-git/gcc/testsuite/gcc.target/arm/addr-modes-float.c:6: /home/cbaylis/tools/sysroot-arm-unknown-linux-gnueabi-git/usr/include/gnu/stubs.h:10:11: fatal error: gnu/stubs-hard.h: No such file or directory compilation terminated.
On 27/11/17 19:23, Charles Baylis wrote: > On 27 November 2017 at 17:47, Kyrill Tkachov > <kyrylo.tkachov@foss.arm.com> wrote: >> Hi Charles, >> >> On 27/11/17 17:03, Charles Baylis wrote: >>> Some of the new tests in addr-modes-float.c, which were introduced for >>> the rework of addressing modes costs [1] fail when GCC is configured >>> to default to a softfp calling convention. Fix this by annotating the >>> test functions with __attribute__((pcs("aapcs-vfp"))). >> >> The usual approach to this problem is to add an -mfloat-abi=hard to the >> dg-options >> of the test (the tests are not dg-run, so there's no link-time mismatch >> concerns). >> Any particular reason to use the pcs attribute instead? > With the way I have GCC configured, it doesn't work to do this when > including certain system headers, such as arm_neon.h. > > In file included from > /home/cbaylis/tools/sysroot-arm-unknown-linux-gnueabi-git/usr/include/features.h:447, > from > /home/cbaylis/tools/sysroot-arm-unknown-linux-gnueabi-git/usr/include/bits/libc-header-start.h:33, > from > /home/cbaylis/tools/sysroot-arm-unknown-linux-gnueabi-git/usr/include/stdint.h:26, > from /home/cbaylis/buildarea/gcc/build2/gcc/include/stdint.h:9, > from > /home/cbaylis/buildarea/gcc/build2/gcc/include/arm_fp16.h:34, > from > /home/cbaylis/buildarea/gcc/build2/gcc/include/arm_neon.h:41, > from > /home/cbaylis/srcarea/gcc/gcc-git/gcc/testsuite/gcc.target/arm/addr-modes-float.c:6: > /home/cbaylis/tools/sysroot-arm-unknown-linux-gnueabi-git/usr/include/gnu/stubs.h:10:11: > fatal error: gnu/stubs-hard.h: No such file or directory > compilation terminated. So is it the case that you don't run any arm tests that include arm_neon.h in your configuration? If so, then I would be fine with leaving this test unsupported on this configuration. By the way, I notice that in addr-modes-float.c the arm_neon_ok check is placed before the dg-add-options. I don't remember the arcane rules exactly, but I think the effective target check should go before it, so that the test gets skipped properly. Thanks, Kyrill
On 30 November 2017 at 15:56, Kyrill Tkachov <kyrylo.tkachov@foss.arm.com> wrote: > > So is it the case that you don't run any arm tests that include arm_neon.h > in your configuration? No, it is only the case that any arm test which includes arm_neon.h (in fact, any system header) *and* uses dg-add-options -mfloat-abi=hard fails on my configuration (And -mfloat-abi=softfp fails in my configurations which default to hardfp). [1] The only test which currently has -mfloat-abi=hard and #include <arm_neon.h> is gcc.target/arm/pr51534.c, and it FAILs in my arm-unknown-linux-gnueabi configuration. > If so, then I would be fine with leaving this test unsupported on this > configuration. I don't see why, when the test can simply be fixed with attribute((pcs)), but if you prefer I can respin the patch accordingly. > By the way, I notice that in addr-modes-float.c the arm_neon_ok check is > placed before the dg-add-options. > I don't remember the arcane rules exactly, but I think the effective target > check should go before it, so that the test gets skipped properly. OK, I can respin the patch with that change. [1] full details as follows: $ arm-unknown-linux-gnueabi-gcc -v COLLECT_GCC=/home/cbaylis/tools//tools-arm-unknown-linux-gnueabi-git/bin/arm-unknown-linux-gnueabi-gcc COLLECT_LTO_WRAPPER=/home/cbaylis/tools/tools-arm-unknown-linux-gnueabi-git/bin/../libexec/gcc/arm-unknown-linux-gnueabi/8.0.0/lto-wrapper Target: arm-unknown-linux-gnueabi Configured with: /home/cbaylis/srcarea/gcc/gcc-git/configure --prefix=/home/cbaylis/tools//tools-arm-unknown-linux-gnueabi-git --target=arm-unknown-linux-gnueabi --enable-languages=c,c++ --with-sysroot=/home/cbaylis/tools//sysroot-arm-unknown-linux-gnueabi-git --with-arch=armv7-a --with-tune=cortex-a9 --with-fpu=vfpv3-d16 --with-float=softfp --with-mode=thumb Thread model: posix gcc version 8.0.0 20171124 (experimental) (GCC) $ cat tn.c #include <stdio.h> $ arm-unknown-linux-gnueabi-gcc -mfloat-abi=hard tn.c In file included from /home/cbaylis/tools/sysroot-arm-unknown-linux-gnueabi-git/usr/include/features.h:447, from /home/cbaylis/tools/sysroot-arm-unknown-linux-gnueabi-git/usr/include/bits/libc-header-start.h:33, from /home/cbaylis/tools/sysroot-arm-unknown-linux-gnueabi-git/usr/include/stdio.h:27, from tn.c:2: /home/cbaylis/tools/sysroot-arm-unknown-linux-gnueabi-git/usr/include/gnu/stubs.h:10:11: fatal error: gnu/stubs-hard.h: Dosiero aŭ dosierujo ne ekzistas # include <gnu/stubs-hard.h> ^~~~~~~~~~~~~~~~~~ compilation terminated.
On 01/12/17 15:43, Charles Baylis wrote: > On 30 November 2017 at 15:56, Kyrill Tkachov > <kyrylo.tkachov@foss.arm.com> wrote: > >> So is it the case that you don't run any arm tests that include arm_neon.h >> in your configuration? > No, it is only the case that any arm test which includes arm_neon.h > (in fact, any system header) *and* uses dg-add-options > -mfloat-abi=hard fails on my configuration (And -mfloat-abi=softfp > fails in my configurations which default to hardfp). [1] Yes, you're right. > The only test which currently has -mfloat-abi=hard and #include > <arm_neon.h> is gcc.target/arm/pr51534.c, and it FAILs in my > arm-unknown-linux-gnueabi configuration. > >> If so, then I would be fine with leaving this test unsupported on this >> configuration. > I don't see why, when the test can simply be fixed with > attribute((pcs)), but if you prefer I can respin the patch > accordingly. No need, I think adding the pcs attribute to this test is the simpler solution here. So your patch is ok as is, sorry for the noise... >> By the way, I notice that in addr-modes-float.c the arm_neon_ok check is >> placed before the dg-add-options. >> I don't remember the arcane rules exactly, but I think the effective target >> check should go before it, so that the test gets skipped properly. > OK, I can respin the patch with that change. This can be done as a follow up if you want, or if you make this change as part of this patch they are pre-approved. Thanks, Kyrill > [1] full details as follows: > > $ arm-unknown-linux-gnueabi-gcc -v > COLLECT_GCC=/home/cbaylis/tools//tools-arm-unknown-linux-gnueabi-git/bin/arm-unknown-linux-gnueabi-gcc > COLLECT_LTO_WRAPPER=/home/cbaylis/tools/tools-arm-unknown-linux-gnueabi-git/bin/../libexec/gcc/arm-unknown-linux-gnueabi/8.0.0/lto-wrapper > Target: arm-unknown-linux-gnueabi > Configured with: /home/cbaylis/srcarea/gcc/gcc-git/configure > --prefix=/home/cbaylis/tools//tools-arm-unknown-linux-gnueabi-git > --target=arm-unknown-linux-gnueabi --enable-languages=c,c++ > --with-sysroot=/home/cbaylis/tools//sysroot-arm-unknown-linux-gnueabi-git > --with-arch=armv7-a --with-tune=cortex-a9 --with-fpu=vfpv3-d16 > --with-float=softfp --with-mode=thumb > Thread model: posix > gcc version 8.0.0 20171124 (experimental) (GCC) > > $ cat tn.c > #include <stdio.h> > > $ arm-unknown-linux-gnueabi-gcc -mfloat-abi=hard tn.c > In file included from > /home/cbaylis/tools/sysroot-arm-unknown-linux-gnueabi-git/usr/include/features.h:447, > from > /home/cbaylis/tools/sysroot-arm-unknown-linux-gnueabi-git/usr/include/bits/libc-header-start.h:33, > from > /home/cbaylis/tools/sysroot-arm-unknown-linux-gnueabi-git/usr/include/stdio.h:27, > from tn.c:2: > /home/cbaylis/tools/sysroot-arm-unknown-linux-gnueabi-git/usr/include/gnu/stubs.h:10:11: > fatal error: gnu/stubs-hard.h: Dosiero aŭ dosierujo ne ekzistas > # include <gnu/stubs-hard.h> > ^~~~~~~~~~~~~~~~~~ > compilation terminated.
From c8743026e53429131e6677aaca7b0840ecc11e25 Mon Sep 17 00:00:00 2001 From: Charles Baylis <charles.baylis@linaro.org> Date: Fri, 24 Nov 2017 16:24:18 +0000 Subject: [PATCH] [ARM] testsuite: force hardfp in addr-modes-float.c gcc/testsuite/ChangeLog: <date> Charles Baylis <charles.baylis@linaro.org> * gcc.target/arm/addr-modes-float.c (ATTR): New define. (POST_STORE): Pass ATTR as 2nd argument. (POST_LOAD): Likewise. (POST_STORE_VEC): Likewise. * gcc.target/arm/addr-modes-int.c (ATTR): New define. (PRE_STORE): Pass ATTR as 2nd argument. (POST_STORE): Likewise. (PRE_LOAD): Likewise. (POST_LOAD): Likewise. * gcc.target/arm/addr-modes.h (PRE_STORE): New parameter. (POST_STORE): Likewise. (POST_STORE_VEC): Likewise. (PRE_LOAD): Likewise. (POST_LOAD): Likewise. (POST_LOAD_VEC): Likewise. Change-Id: I7f85e811194098da8f1b7d243653d7873f132fff --- gcc/testsuite/gcc.target/arm/addr-modes-float.c | 26 +++++++++--------- gcc/testsuite/gcc.target/arm/addr-modes-int.c | 35 ++++++++++++++----------- gcc/testsuite/gcc.target/arm/addr-modes.h | 30 ++++++++++----------- 3 files changed, 48 insertions(+), 43 deletions(-) diff --git a/gcc/testsuite/gcc.target/arm/addr-modes-float.c b/gcc/testsuite/gcc.target/arm/addr-modes-float.c index 3b4235c..300a2bea 100644 --- a/gcc/testsuite/gcc.target/arm/addr-modes-float.c +++ b/gcc/testsuite/gcc.target/arm/addr-modes-float.c @@ -7,35 +7,37 @@ #include "addr-modes.h" -POST_STORE(float) +#define ATTR __attribute__((__pcs__("aapcs-vfp"))) + +POST_STORE(float, ATTR) /* { dg-final { scan-assembler "vstmia.32" } } */ -POST_STORE(double) +POST_STORE(double, ATTR) /* { dg-final { scan-assembler "vstmia.64" } } */ -POST_LOAD(float) +POST_LOAD(float, ATTR) /* { dg-final { scan-assembler "vldmia.32" } } */ -POST_LOAD(double) +POST_LOAD(double, ATTR) /* { dg-final { scan-assembler "vldmia.64" } } */ -POST_STORE_VEC (int8_t, int8x8_t, vst1_s8) +POST_STORE_VEC (int8_t, int8x8_t, vst1_s8, ATTR) /* { dg-final { scan-assembler "vst1.8\t\{.*\}, \\\[r\[0-9\]+\\\]!" } } */ -POST_STORE_VEC (int8_t, int8x16_t, vst1q_s8) +POST_STORE_VEC (int8_t, int8x16_t, vst1q_s8, ATTR) /* { dg-final { scan-assembler "vst1.8\t\{.*\[-,\]d.*\}, \\\[r\[0-9\]+\\\]!" } } */ -POST_STORE_VEC (int8_t, int8x8x2_t, vst2_s8) +POST_STORE_VEC (int8_t, int8x8x2_t, vst2_s8, ATTR) /* { dg-final { scan-assembler "vst2.8\t\{.*\}, \\\[r\[0-9\]+\\\]!" } } */ -POST_STORE_VEC (int8_t, int8x16x2_t, vst2q_s8) +POST_STORE_VEC (int8_t, int8x16x2_t, vst2q_s8, ATTR) /* { dg-final { scan-assembler "vst2.8\t\{.*-d.*\}, \\\[r\[0-9\]+\\\]!" } } */ -POST_STORE_VEC (int8_t, int8x8x3_t, vst3_s8) +POST_STORE_VEC (int8_t, int8x8x3_t, vst3_s8, ATTR) /* { dg-final { scan-assembler "vst3.8\t\{.*\}, \\\[r\[0-9\]+\\\]!" } } */ -POST_STORE_VEC (int8_t, int8x16x3_t, vst3q_s8) +POST_STORE_VEC (int8_t, int8x16x3_t, vst3q_s8, ATTR) /* { dg-final { scan-assembler "vst3.8\t\{d\[02468\], d\[02468\], d\[02468\]\}, \\\[r\[0-9\]+\\\]!" } } */ /* { dg-final { scan-assembler "vst3.8\t\{d\[13579\], d\[13579\], d\[13579\]\}, \\\[r\[0-9\]+\\\]!" { xfail *-*-* } } } */ -POST_STORE_VEC (int8_t, int8x8x4_t, vst4_s8) +POST_STORE_VEC (int8_t, int8x8x4_t, vst4_s8, ATTR) /* { dg-final { scan-assembler "vst4.8\t\{.*\}, \\\[r\[0-9\]+\\\]!" } } */ -POST_STORE_VEC (int8_t, int8x16x4_t, vst4q_s8) +POST_STORE_VEC (int8_t, int8x16x4_t, vst4q_s8, ATTR) /* { dg-final { scan-assembler "vst4.8\t\{d\[02468\], d\[02468\], d\[02468\], d\[02468\]\}, \\\[r\[0-9\]+\\\]!" } } */ /* { dg-final { scan-assembler "vst4.8\t\{d\[13579\], d\[13579\], d\[13579\], d\[13579\]\}, \\\[r\[0-9\]+\\\]!" { xfail *-*-* } } } */ diff --git a/gcc/testsuite/gcc.target/arm/addr-modes-int.c b/gcc/testsuite/gcc.target/arm/addr-modes-int.c index e3e1e6a..90b7425 100644 --- a/gcc/testsuite/gcc.target/arm/addr-modes-int.c +++ b/gcc/testsuite/gcc.target/arm/addr-modes-int.c @@ -7,40 +7,43 @@ typedef long long ll; -PRE_STORE(char) +/* no special function attribute required */ +#define ATTR /* */ + +PRE_STORE(char, ATTR) /* { dg-final { scan-assembler "strb.*#1]!" } } */ -PRE_STORE(short) +PRE_STORE(short, ATTR) /* { dg-final { scan-assembler "strh.*#2]!" } } */ -PRE_STORE(int) +PRE_STORE(int, ATTR) /* { dg-final { scan-assembler "str.*#4]!" } } */ -PRE_STORE(ll) +PRE_STORE(ll, ATTR) /* { dg-final { scan-assembler "strd.*#8]!" } } */ -POST_STORE(char) +POST_STORE(char, ATTR) /* { dg-final { scan-assembler "strb.*], #1" } } */ -POST_STORE(short) +POST_STORE(short, ATTR) /* { dg-final { scan-assembler "strh.*], #2" } } */ -POST_STORE(int) +POST_STORE(int, ATTR) /* { dg-final { scan-assembler "str.*], #4" } } */ -POST_STORE(ll) +POST_STORE(ll, ATTR) /* { dg-final { scan-assembler "strd.*], #8" } } */ -PRE_LOAD(char) +PRE_LOAD(char, ATTR) /* { dg-final { scan-assembler "ldrb.*#1]!" } } */ -PRE_LOAD(short) +PRE_LOAD(short, ATTR) /* { dg-final { scan-assembler "ldrsh.*#2]!" } } */ -PRE_LOAD(int) +PRE_LOAD(int, ATTR) /* { dg-final { scan-assembler "ldr.*#4]!" } } */ -PRE_LOAD(ll) +PRE_LOAD(ll, ATTR) /* { dg-final { scan-assembler "ldrd.*#8]!" } } */ -POST_LOAD(char) +POST_LOAD(char, ATTR) /* { dg-final { scan-assembler "ldrb.*], #1" } } */ -POST_LOAD(short) +POST_LOAD(short, ATTR) /* { dg-final { scan-assembler "ldrsh.*], #2" } } */ -POST_LOAD(int) +POST_LOAD(int, ATTR) /* { dg-final { scan-assembler "ldr.*], #4" } } */ -POST_LOAD(ll) +POST_LOAD(ll, ATTR) /* { dg-final { scan-assembler "ldrd.*], #8" } } */ /* { dg-final { scan-assembler-not "\tadd" } } */ diff --git a/gcc/testsuite/gcc.target/arm/addr-modes.h b/gcc/testsuite/gcc.target/arm/addr-modes.h index eac4678..9844c6a 100644 --- a/gcc/testsuite/gcc.target/arm/addr-modes.h +++ b/gcc/testsuite/gcc.target/arm/addr-modes.h @@ -1,22 +1,22 @@ -#define PRE_STORE(T) \ - T * \ +#define PRE_STORE(T, ATTR) \ + ATTR T * \ T ## _pre_store (T *p, T v) \ { \ *++p = v; \ return p; \ } \ -#define POST_STORE(T) \ - T * \ +#define POST_STORE(T, ATTR) \ + ATTR T * \ T ## _post_store (T *p, T v) \ { \ *p++ = v; \ return p; \ } -#define POST_STORE_VEC(T, VT, OP) \ - T * \ +#define POST_STORE_VEC(T, VT, OP, ATTR) \ + ATTR T * \ VT ## _post_store (T * p, VT v) \ { \ OP (p, v); \ @@ -24,29 +24,29 @@ return p; \ } -#define PRE_LOAD(T) \ - void \ +#define PRE_LOAD(T, ATTR) \ + ATTR void \ T ## _pre_load (T *p) \ { \ - extern void f ## T (T*,T); \ + ATTR extern void f ## T (T*,T); \ T x = *++p; \ f ## T (p, x); \ } -#define POST_LOAD(T) \ - void \ +#define POST_LOAD(T, ATTR) \ + ATTR void \ T ## _post_load (T *p) \ { \ - extern void f ## T (T*,T); \ + ATTR extern void f ## T (T*,T); \ T x = *p++; \ f ## T (p, x); \ } -#define POST_LOAD_VEC(T, VT, OP) \ - void \ +#define POST_LOAD_VEC(T, VT, OP, ATTR) \ + ATTR void \ VT ## _post_load (T * p) \ { \ - extern void f ## T (T*,T); \ + ATTR extern void f ## T (T*,T); \ VT x = OP (p, v); \ p += sizeof (VT) / sizeof (T); \ f ## T (p, x); \ -- 2.7.4
Some of the new tests in addr-modes-float.c, which were introduced for the rework of addressing modes costs [1] fail when GCC is configured to default to a softfp calling convention. Fix this by annotating the test functions with __attribute__((pcs("aapcs-vfp"))). Thanks to Christophe for pointing this out. [1] https://gcc.gnu.org/ml/gcc-patches/2017-11/msg02149.html <date> Charles Baylis <charles.baylis@linaro.org> * gcc.target/arm/addr-modes-float.c (ATTR): New define. (POST_STORE): Pass ATTR as 2nd argument. (POST_LOAD): Likewise. (POST_STORE_VEC): Likewise. * gcc.target/arm/addr-modes-int.c (ATTR): New define. (PRE_STORE): Pass ATTR as 2nd argument. (POST_STORE): Likewise. (PRE_LOAD): Likewise. (POST_LOAD): Likewise. * gcc.target/arm/addr-modes.h (PRE_STORE): New parameter. (POST_STORE): Likewise. (POST_STORE_VEC): Likewise. (PRE_LOAD): Likewise. (POST_LOAD): Likewise. (POST_LOAD_VEC): Likewise.