[ARM] Fix fragile arm fpu attribute tests.

Message ID 20171212172909.GA12902@arm.com
State New
Headers show
Series
  • [ARM] Fix fragile arm fpu attribute tests.
Related show

Commit Message

Tamar Christina Dec. 12, 2017, 5:29 p.m.
Hi All,

The previous test made use of arm_neon.h which made the whole test
rather fragile and only applicable to some of the arm targets.

So instead I make use of different fpus now to test the generation of
fmla instructions. The actual instruction itself is not tested as all
we care about if that the proper .fpu directives are generated.

Regtested on arm-none-eabi and arm-none-linux-gnueabihf
with no regressions.

Ok for trunk?


gcc/testsuite/
2017-12-12  Tamar Christina  <tamar.christina@arm.com>

	PR target/82641
	* gcc.target/arm/pragma_fpu_attribute.c: New.
	* gcc.target/arm/pragma_fpu_attribute_2.c: New.

--

Comments

Christophe Lyon Dec. 13, 2017, 8:49 a.m. | #1
On 12 December 2017 at 18:29, Tamar Christina <tamar.christina@arm.com> wrote:
> Hi All,

>

> The previous test made use of arm_neon.h which made the whole test

> rather fragile and only applicable to some of the arm targets.

>

> So instead I make use of different fpus now to test the generation of

> fmla instructions. The actual instruction itself is not tested as all

> we care about if that the proper .fpu directives are generated.

>

> Regtested on arm-none-eabi and arm-none-linux-gnueabihf

> with no regressions.

>

> Ok for trunk?

>

>

> gcc/testsuite/

> 2017-12-12  Tamar Christina  <tamar.christina@arm.com>

>

>         PR target/82641

>         * gcc.target/arm/pragma_fpu_attribute.c: New.

>         * gcc.target/arm/pragma_fpu_attribute_2.c: New.

>

Sorry, it seems your patch does not apply against ToT, and
the ChangeLog looks incorrect (these are not new files)

Christophe
Tamar Christina Dec. 14, 2017, 10:56 a.m. | #2
The 12/13/2017 08:49, Christophe Lyon wrote:
> On 12 December 2017 at 18:29, Tamar Christina <tamar.christina@arm.com> wrote:

> > Hi All,

> >

> > The previous test made use of arm_neon.h which made the whole test

> > rather fragile and only applicable to some of the arm targets.

> >

> > So instead I make use of different fpus now to test the generation of

> > fmla instructions. The actual instruction itself is not tested as all

> > we care about if that the proper .fpu directives are generated.

> >

> > Regtested on arm-none-eabi and arm-none-linux-gnueabihf

> > with no regressions.

> >

> > Ok for trunk?

> >

> >

> > gcc/testsuite/

> > 2017-12-12  Tamar Christina  <tamar.christina@arm.com>

> >

> >         PR target/82641

> >         * gcc.target/arm/pragma_fpu_attribute.c: New.

> >         * gcc.target/arm/pragma_fpu_attribute_2.c: New.


Hi Christophe,

My apologies, I have rebased the patch.
New Changelog:

gcc/testsuite/
2017-12-14  Tamar Christina  <tamar.christina@arm.com>

	PR target/82641
	* gcc.target/arm/pragma_fpu_attribute.c: Rewrite to use
	no NEON.
	* gcc.target/arm/pragma_fpu_attribute_2.c: Likewise.

> >

> Sorry, it seems your patch does not apply against ToT, and

> the ChangeLog looks incorrect (these are not new files)

> 

> Christophe


Thanks,
Tamar

--
diff --git a/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute.c b/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute.c
index f47c745855e4acc099afd554838dcf7d031f798c..8191deac25965564a3c78dc08959a5e5637a0066 100644
--- a/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute.c
+++ b/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute.c
@@ -1,18 +1,19 @@
 /* Test for target attribute assembly extension generations.  */
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_arch_v8a_ok } */
-/* { dg-additional-options "-std=gnu99 -mfpu=vfpv3-d16" } */
+/* { dg-additional-options "-std=gnu99" } */
 
 #include <stdint.h>
-#include <arm_neon.h>
 
-extern uint32_t bar();
+extern uint32_t bar ();
 
-__attribute__((target("fpu=crypto-neon-fp-armv8"))) poly64x1_t vsricw(poly64x1_t crc, uint32_t val)
+#pragma GCC target("fpu=vfpv3-d16")
+
+extern float fmaf (float, float, float);
+
+float
+__attribute__((target("fpu=vfpv4"))) vfma32 (float x, float y, float z)
 {
-    poly64x1_t res;
-    asm("vsri %0, %1, %2" : "=r"(res) : "r"(crc), "r"(val));
-    return res;
+  return fmaf (x, y, z);
 }
 
 uint32_t restored ()
@@ -20,5 +21,5 @@ uint32_t restored ()
   return bar();
 }
 
-/* { dg-final { scan-assembler-times {\.fpu\s+crypto-neon-fp-armv8} 1 } } */
+/* { dg-final { scan-assembler-times {\.fpu\s+vfpv4} 1 } } */
 /* { dg-final { scan-assembler-times {\.fpu\s+vfpv3-d16} 1 } } */
diff --git a/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute_2.c b/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute_2.c
index f23fd83779e57e48c0035b6688a21850d12cb4ab..b50d4107b56ed7abd8b95fd2a3a08df8ab54410a 100644
--- a/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute_2.c
+++ b/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute_2.c
@@ -1,20 +1,22 @@
 /* Test for #pragma assembly extension generations.  */
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_arch_v8a_ok } */
-/* { dg-additional-options "-std=gnu99 -mfpu=vfpv3-d16" } */
+/* { dg-additional-options "-std=gnu99" } */
 
 #include <stdint.h>
 #include <arm_neon.h>
 
+#pragma GCC target("fpu=vfpv3-d16")
+
 extern uint32_t bar();
 
 #pragma GCC push_options
-#pragma GCC target("fpu=crypto-neon-fp-armv8")
-poly64x1_t vsricw(poly64x1_t crc, uint32_t val)
+#pragma GCC target("fpu=vfpv4")
+extern float fmaf (float, float, float);
+
+float
+vfma32 (float x, float y, float z)
 {
-    poly64x1_t res;
-    asm("vsri %0, %1, %2" : "=r"(res) : "r"(crc), "r"(val));
-    return res;
+  return fmaf (x, y, z);
 }
 #pragma GCC pop_options
 
@@ -23,5 +25,5 @@ uint32_t restored ()
   return bar();
 }
 
-/* { dg-final { scan-assembler-times {\.fpu\s+crypto-neon-fp-armv8} 1 } } */
+/* { dg-final { scan-assembler-times {\.fpu\s+vfpv4} 1 } } */
 /* { dg-final { scan-assembler-times {\.fpu\s+vfpv3-d16} 1 } } */
Christophe Lyon Dec. 14, 2017, 8:46 p.m. | #3
On 14 December 2017 at 11:56, Tamar Christina <Tamar.Christina@arm.com> wrote:
> The 12/13/2017 08:49, Christophe Lyon wrote:

>> On 12 December 2017 at 18:29, Tamar Christina <tamar.christina@arm.com> wrote:

>> > Hi All,

>> >

>> > The previous test made use of arm_neon.h which made the whole test

>> > rather fragile and only applicable to some of the arm targets.

>> >

>> > So instead I make use of different fpus now to test the generation of

>> > fmla instructions. The actual instruction itself is not tested as all

>> > we care about if that the proper .fpu directives are generated.

>> >

>> > Regtested on arm-none-eabi and arm-none-linux-gnueabihf

>> > with no regressions.

>> >

>> > Ok for trunk?

>> >

>> >

>> > gcc/testsuite/

>> > 2017-12-12  Tamar Christina  <tamar.christina@arm.com>

>> >

>> >         PR target/82641

>> >         * gcc.target/arm/pragma_fpu_attribute.c: New.

>> >         * gcc.target/arm/pragma_fpu_attribute_2.c: New.

>

> Hi Christophe,

>

> My apologies, I have rebased the patch.

> New Changelog:

>

> gcc/testsuite/

> 2017-12-14  Tamar Christina  <tamar.christina@arm.com>

>

>         PR target/82641

>         * gcc.target/arm/pragma_fpu_attribute.c: Rewrite to use

>         no NEON.

>         * gcc.target/arm/pragma_fpu_attribute_2.c: Likewise.

>


Hi,

Sorry I think there is still something wrong with this patch.
In pragma_fpu_attribute_2.c, you are not removing
#include <arm_neon.h>
as the ChangeLog seems to imply?

So, with this patch, there are problems on arm-none-linux-gnueabi and
arm-none-eabi:
FAIL:    gcc.target/arm/pragma_fpu_attribute.c scan-assembler-times
\\.fpu\\s+vfpv3-d16 1 (found 0 times)
FAIL:    gcc.target/arm/pragma_fpu_attribute.c scan-assembler-times
\\.fpu\\s+vfpv4 1 (found 0 times)

and pragma_fpu_attribute_2.c still fails to compile:
In file included from /gcc/testsuite/gcc.target/arm/pragma_fpu_attribute_2.c:6:
/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-arm-none-eabi/gcc3/gcc/include/arm_neon.h:31:2:
error: #error "NEON intrinsics not available with the soft-float ABI.
Please use -mfloat-abi=softfp or -mfloat-abi=hard"

I'm not sure why you don't see this when testing on arm-none-eabi?

If you want to see more details:
http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/255624-rb8655.patch-2/report-build-info.html
(ignore the lines with "interrupted", this means there was a problem
on the host during the build)

Christophe


>> >

>> Sorry, it seems your patch does not apply against ToT, and

>> the ChangeLog looks incorrect (these are not new files)

>>

>> Christophe

>

> Thanks,

> Tamar

>

> --
Tamar Christina Dec. 21, 2017, 2:24 p.m. | #4
The 12/14/2017 20:46, Christophe Lyon wrote:
> On 14 December 2017 at 11:56, Tamar Christina <Tamar.Christina@arm.com> wrote:

> > The 12/13/2017 08:49, Christophe Lyon wrote:

> >> On 12 December 2017 at 18:29, Tamar Christina <tamar.christina@arm.com> wrote:

> >> > Hi All,

> >> >

> >> > The previous test made use of arm_neon.h which made the whole test

> >> > rather fragile and only applicable to some of the arm targets.

> >> >

> >> > So instead I make use of different fpus now to test the generation of

> >> > fmla instructions. The actual instruction itself is not tested as all

> >> > we care about if that the proper .fpu directives are generated.

> >> >

> >> > Regtested on arm-none-eabi and arm-none-linux-gnueabihf

> >> > with no regressions.

> >> >

> >> > Ok for trunk?

> >> >

> >> >

> >> > gcc/testsuite/

> >> > 2017-12-12  Tamar Christina  <tamar.christina@arm.com>

> >> >

> >> >         PR target/82641

> >> >         * gcc.target/arm/pragma_fpu_attribute.c: New.

> >> >         * gcc.target/arm/pragma_fpu_attribute_2.c: New.

> >

> > Hi Christophe,

> >

> > My apologies, I have rebased the patch.

> > New Changelog:

> >

> > gcc/testsuite/

> > 2017-12-14  Tamar Christina  <tamar.christina@arm.com>

> >

> >         PR target/82641

> >         * gcc.target/arm/pragma_fpu_attribute.c: Rewrite to use

> >         no NEON.

> >         * gcc.target/arm/pragma_fpu_attribute_2.c: Likewise.

> >

> 

> Hi,

> 

> Sorry I think there is still something wrong with this patch.

> In pragma_fpu_attribute_2.c, you are not removing

> #include <arm_neon.h>

> as the ChangeLog seems to imply?

>


Sorry that was extremely sloppy of me. I noticed the changelog after sending
but hadn't noticed the #include being left in.
 
> So, with this patch, there are problems on arm-none-linux-gnueabi and

> arm-none-eabi:

> FAIL:    gcc.target/arm/pragma_fpu_attribute.c scan-assembler-times

> \\.fpu\\s+vfpv3-d16 1 (found 0 times)

> FAIL:    gcc.target/arm/pragma_fpu_attribute.c scan-assembler-times

> \\.fpu\\s+vfpv4 1 (found 0 times)

> 

> and pragma_fpu_attribute_2.c still fails to compile:

> In file included from /gcc/testsuite/gcc.target/arm/pragma_fpu_attribute_2.c:6:

> /aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-arm-none-eabi/gcc3/gcc/include/arm_neon.h:31:2:

> error: #error "NEON intrinsics not available with the soft-float ABI.

> Please use -mfloat-abi=softfp or -mfloat-abi=hard"

> 

> I'm not sure why you don't see this when testing on arm-none-eabi?


It's because I don't have a compiler configured with only -mfloat-abi=soft. So when I run
the tests it's always able to just change the ABI. I resorted to manually testing it.

I've now prevented the tests from running at all on soft float only targets. This should fix
the problem once and for all.

Regtested on arm-none-eabi, arm-none-linux-gnueabi and arm-none-linux-gnueabihf.

Thanks and sorry for the noise,
Tamar

Ok for trunk?

gcc/testsuite/
2017-12-21  Tamar Christina  <tamar.christina@arm.com>

	PR target/82641
	* gcc.target/arm/pragma_fpu_attribute.c: Rewrite to use
	no NEON and require softfp or hard float-abi.
	* gcc.target/arm/pragma_fpu_attribute_2.c: Likewise.

> 

> If you want to see more details:

> http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/255624-rb8655.patch-2/report-build-info.html

> (ignore the lines with "interrupted", this means there was a problem

> on the host during the build)

> 

> Christophe

> 

> 

> >> >

> >> Sorry, it seems your patch does not apply against ToT, and

> >> the ChangeLog looks incorrect (these are not new files)

> >>

> >> Christophe

> >

> > Thanks,

> > Tamar

> >

> > --


--
diff --git a/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute.c b/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute.c
index f47c745855e4acc099afd554838dcf7d031f798c..174be85f3f777e4f9a4a1c4a85f706021ce4dd7c 100644
--- a/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute.c
+++ b/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute.c
@@ -1,18 +1,20 @@
 /* Test for target attribute assembly extension generations.  */
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_arch_v8a_ok } */
-/* { dg-additional-options "-std=gnu99 -mfpu=vfpv3-d16" } */
+/* { dg-require-effective-target arm_fp_ok } */
+/* { dg-add-options arm_fp } */
 
 #include <stdint.h>
-#include <arm_neon.h>
 
-extern uint32_t bar();
+extern uint32_t bar ();
 
-__attribute__((target("fpu=crypto-neon-fp-armv8"))) poly64x1_t vsricw(poly64x1_t crc, uint32_t val)
+#pragma GCC target("fpu=vfpv3-d16")
+
+extern float fmaf (float, float, float);
+
+float
+__attribute__((target("fpu=vfpv4"))) vfma32 (float x, float y, float z)
 {
-    poly64x1_t res;
-    asm("vsri %0, %1, %2" : "=r"(res) : "r"(crc), "r"(val));
-    return res;
+  return fmaf (x, y, z);
 }
 
 uint32_t restored ()
@@ -20,5 +22,5 @@ uint32_t restored ()
   return bar();
 }
 
-/* { dg-final { scan-assembler-times {\.fpu\s+crypto-neon-fp-armv8} 1 } } */
+/* { dg-final { scan-assembler-times {\.fpu\s+vfpv4} 1 } } */
 /* { dg-final { scan-assembler-times {\.fpu\s+vfpv3-d16} 1 } } */
diff --git a/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute_2.c b/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute_2.c
index f23fd83779e57e48c0035b6688a21850d12cb4ab..add40ddc6b8a8645023784e9a29ea547c613698d 100644
--- a/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute_2.c
+++ b/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute_2.c
@@ -1,20 +1,22 @@
 /* Test for #pragma assembly extension generations.  */
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_arch_v8a_ok } */
-/* { dg-additional-options "-std=gnu99 -mfpu=vfpv3-d16" } */
+/* { dg-require-effective-target arm_fp_ok } */
+/* { dg-add-options arm_fp } */
 
 #include <stdint.h>
-#include <arm_neon.h>
+
+#pragma GCC target("fpu=vfpv3-d16")
 
 extern uint32_t bar();
 
 #pragma GCC push_options
-#pragma GCC target("fpu=crypto-neon-fp-armv8")
-poly64x1_t vsricw(poly64x1_t crc, uint32_t val)
+#pragma GCC target("fpu=vfpv4")
+extern float fmaf (float, float, float);
+
+float
+vfma32 (float x, float y, float z)
 {
-    poly64x1_t res;
-    asm("vsri %0, %1, %2" : "=r"(res) : "r"(crc), "r"(val));
-    return res;
+  return fmaf (x, y, z);
 }
 #pragma GCC pop_options
 
@@ -23,5 +25,5 @@ uint32_t restored ()
   return bar();
 }
 
-/* { dg-final { scan-assembler-times {\.fpu\s+crypto-neon-fp-armv8} 1 } } */
+/* { dg-final { scan-assembler-times {\.fpu\s+vfpv4} 1 } } */
 /* { dg-final { scan-assembler-times {\.fpu\s+vfpv3-d16} 1 } } */
Christophe Lyon Dec. 21, 2017, 9:38 p.m. | #5
On 21 December 2017 at 15:24, Tamar Christina <Tamar.Christina@arm.com> wrote:
> The 12/14/2017 20:46, Christophe Lyon wrote:

>> On 14 December 2017 at 11:56, Tamar Christina <Tamar.Christina@arm.com> wrote:

>> > The 12/13/2017 08:49, Christophe Lyon wrote:

>> >> On 12 December 2017 at 18:29, Tamar Christina <tamar.christina@arm.com> wrote:

>> >> > Hi All,

>> >> >

>> >> > The previous test made use of arm_neon.h which made the whole test

>> >> > rather fragile and only applicable to some of the arm targets.

>> >> >

>> >> > So instead I make use of different fpus now to test the generation of

>> >> > fmla instructions. The actual instruction itself is not tested as all

>> >> > we care about if that the proper .fpu directives are generated.

>> >> >

>> >> > Regtested on arm-none-eabi and arm-none-linux-gnueabihf

>> >> > with no regressions.

>> >> >

>> >> > Ok for trunk?

>> >> >

>> >> >

>> >> > gcc/testsuite/

>> >> > 2017-12-12  Tamar Christina  <tamar.christina@arm.com>

>> >> >

>> >> >         PR target/82641

>> >> >         * gcc.target/arm/pragma_fpu_attribute.c: New.

>> >> >         * gcc.target/arm/pragma_fpu_attribute_2.c: New.

>> >

>> > Hi Christophe,

>> >

>> > My apologies, I have rebased the patch.

>> > New Changelog:

>> >

>> > gcc/testsuite/

>> > 2017-12-14  Tamar Christina  <tamar.christina@arm.com>

>> >

>> >         PR target/82641

>> >         * gcc.target/arm/pragma_fpu_attribute.c: Rewrite to use

>> >         no NEON.

>> >         * gcc.target/arm/pragma_fpu_attribute_2.c: Likewise.

>> >

>>

>> Hi,

>>

>> Sorry I think there is still something wrong with this patch.

>> In pragma_fpu_attribute_2.c, you are not removing

>> #include <arm_neon.h>

>> as the ChangeLog seems to imply?

>>

>

> Sorry that was extremely sloppy of me. I noticed the changelog after sending

> but hadn't noticed the #include being left in.

>

>> So, with this patch, there are problems on arm-none-linux-gnueabi and

>> arm-none-eabi:

>> FAIL:    gcc.target/arm/pragma_fpu_attribute.c scan-assembler-times

>> \\.fpu\\s+vfpv3-d16 1 (found 0 times)

>> FAIL:    gcc.target/arm/pragma_fpu_attribute.c scan-assembler-times

>> \\.fpu\\s+vfpv4 1 (found 0 times)

>>

>> and pragma_fpu_attribute_2.c still fails to compile:

>> In file included from /gcc/testsuite/gcc.target/arm/pragma_fpu_attribute_2.c:6:

>> /aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-arm-none-eabi/gcc3/gcc/include/arm_neon.h:31:2:

>> error: #error "NEON intrinsics not available with the soft-float ABI.

>> Please use -mfloat-abi=softfp or -mfloat-abi=hard"

>>

>> I'm not sure why you don't see this when testing on arm-none-eabi?

>

> It's because I don't have a compiler configured with only -mfloat-abi=soft. So when I run

> the tests it's always able to just change the ABI. I resorted to manually testing it.

>

> I've now prevented the tests from running at all on soft float only targets. This should fix

> the problem once and for all.

>

> Regtested on arm-none-eabi, arm-none-linux-gnueabi and arm-none-linux-gnueabihf.

>

> Thanks and sorry for the noise,

> Tamar

>

> Ok for trunk?

>

> gcc/testsuite/

> 2017-12-21  Tamar Christina  <tamar.christina@arm.com>

>

>         PR target/82641

>         * gcc.target/arm/pragma_fpu_attribute.c: Rewrite to use

>         no NEON and require softfp or hard float-abi.

>         * gcc.target/arm/pragma_fpu_attribute_2.c: Likewise.

>

FWIW, this version passes validation on my side.
Thanks

>>

>> If you want to see more details:

>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/255624-rb8655.patch-2/report-build-info.html

>> (ignore the lines with "interrupted", this means there was a problem

>> on the host during the build)

>>

>> Christophe

>>

>>

>> >> >

>> >> Sorry, it seems your patch does not apply against ToT, and

>> >> the ChangeLog looks incorrect (these are not new files)

>> >>

>> >> Christophe

>> >

>> > Thanks,

>> > Tamar

>> >

>> > --

>

> --
Tamar Christina Jan. 9, 2018, 10:16 a.m. | #6
Ping,

Is the fix ok for trunk?

Thanks,
Tamar

> -----Original Message-----

> From: Christophe Lyon [mailto:christophe.lyon@linaro.org]

> Sent: Thursday, December 21, 2017 21:39

> To: Tamar Christina <Tamar.Christina@arm.com>

> Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>; Ramana Radhakrishnan

> <Ramana.Radhakrishnan@arm.com>; Richard Earnshaw

> <Richard.Earnshaw@arm.com>; nickc@redhat.com; Kyrylo Tkachov

> <Kyrylo.Tkachov@arm.com>

> Subject: Re: [PATCH][GCC][ARM] Fix fragile arm fpu attribute tests.

> 

> On 21 December 2017 at 15:24, Tamar Christina <Tamar.Christina@arm.com>

> wrote:

> > The 12/14/2017 20:46, Christophe Lyon wrote:

> >> On 14 December 2017 at 11:56, Tamar Christina

> <Tamar.Christina@arm.com> wrote:

> >> > The 12/13/2017 08:49, Christophe Lyon wrote:

> >> >> On 12 December 2017 at 18:29, Tamar Christina

> <tamar.christina@arm.com> wrote:

> >> >> > Hi All,

> >> >> >

> >> >> > The previous test made use of arm_neon.h which made the whole

> >> >> > test rather fragile and only applicable to some of the arm targets.

> >> >> >

> >> >> > So instead I make use of different fpus now to test the

> >> >> > generation of fmla instructions. The actual instruction itself

> >> >> > is not tested as all we care about if that the proper .fpu directives are

> generated.

> >> >> >

> >> >> > Regtested on arm-none-eabi and arm-none-linux-gnueabihf with no

> >> >> > regressions.

> >> >> >

> >> >> > Ok for trunk?

> >> >> >

> >> >> >

> >> >> > gcc/testsuite/

> >> >> > 2017-12-12  Tamar Christina  <tamar.christina@arm.com>

> >> >> >

> >> >> >         PR target/82641

> >> >> >         * gcc.target/arm/pragma_fpu_attribute.c: New.

> >> >> >         * gcc.target/arm/pragma_fpu_attribute_2.c: New.

> >> >

> >> > Hi Christophe,

> >> >

> >> > My apologies, I have rebased the patch.

> >> > New Changelog:

> >> >

> >> > gcc/testsuite/

> >> > 2017-12-14  Tamar Christina  <tamar.christina@arm.com>

> >> >

> >> >         PR target/82641

> >> >         * gcc.target/arm/pragma_fpu_attribute.c: Rewrite to use

> >> >         no NEON.

> >> >         * gcc.target/arm/pragma_fpu_attribute_2.c: Likewise.

> >> >

> >>

> >> Hi,

> >>

> >> Sorry I think there is still something wrong with this patch.

> >> In pragma_fpu_attribute_2.c, you are not removing #include

> >> <arm_neon.h> as the ChangeLog seems to imply?

> >>

> >

> > Sorry that was extremely sloppy of me. I noticed the changelog after

> sending

> > but hadn't noticed the #include being left in.

> >

> >> So, with this patch, there are problems on arm-none-linux-gnueabi and

> >> arm-none-eabi:

> >> FAIL:    gcc.target/arm/pragma_fpu_attribute.c scan-assembler-times

> >> \\.fpu\\s+vfpv3-d16 1 (found 0 times)

> >> FAIL:    gcc.target/arm/pragma_fpu_attribute.c scan-assembler-times

> >> \\.fpu\\s+vfpv4 1 (found 0 times)

> >>

> >> and pragma_fpu_attribute_2.c still fails to compile:

> >> In file included from

> /gcc/testsuite/gcc.target/arm/pragma_fpu_attribute_2.c:6:

> >> /aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-arm-none-

> eabi/gcc3/gcc/include/arm_neon.h:31:2:

> >> error: #error "NEON intrinsics not available with the soft-float ABI.

> >> Please use -mfloat-abi=softfp or -mfloat-abi=hard"

> >>

> >> I'm not sure why you don't see this when testing on arm-none-eabi?

> >

> > It's because I don't have a compiler configured with only -mfloat-abi=soft.

> So when I run

> > the tests it's always able to just change the ABI. I resorted to manually

> testing it.

> >

> > I've now prevented the tests from running at all on soft float only targets.

> This should fix

> > the problem once and for all.

> >

> > Regtested on arm-none-eabi, arm-none-linux-gnueabi and arm-none-

> linux-gnueabihf.

> >

> > Thanks and sorry for the noise,

> > Tamar

> >

> > Ok for trunk?

> >

> > gcc/testsuite/

> > 2017-12-21  Tamar Christina  <tamar.christina@arm.com>

> >

> >         PR target/82641

> >         * gcc.target/arm/pragma_fpu_attribute.c: Rewrite to use

> >         no NEON and require softfp or hard float-abi.

> >         * gcc.target/arm/pragma_fpu_attribute_2.c: Likewise.

> >

> FWIW, this version passes validation on my side.

> Thanks

> 

> >>

> >> If you want to see more details:

> >> http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-

> patches/255624-rb8655.patch-2/report-build-info.html

> >> (ignore the lines with "interrupted", this means there was a problem

> >> on the host during the build)

> >>

> >> Christophe

> >>

> >>

> >> >> >

> >> >> Sorry, it seems your patch does not apply against ToT, and

> >> >> the ChangeLog looks incorrect (these are not new files)

> >> >>

> >> >> Christophe

> >> >

> >> > Thanks,

> >> > Tamar

> >> >

> >> > --

> >

> > --
Kyrill Tkachov Jan. 9, 2018, 10:18 a.m. | #7
On 09/01/18 10:16, Tamar Christina wrote:
> Ping,

>

> Is the fix ok for trunk?


Hi Tamar,

Yes, thanks for pinging this.
I had reviewed it before the break but had forgotten to send an ok out.
Please commit.

Kyrill

>

> Thanks,

> Tamar

>

> > -----Original Message-----

> > From: Christophe Lyon [mailto:christophe.lyon@linaro.org]

> > Sent: Thursday, December 21, 2017 21:39

> > To: Tamar Christina <Tamar.Christina@arm.com>

> > Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>; Ramana Radhakrishnan

> > <Ramana.Radhakrishnan@arm.com>; Richard Earnshaw

> > <Richard.Earnshaw@arm.com>; nickc@redhat.com; Kyrylo Tkachov

> > <Kyrylo.Tkachov@arm.com>

> > Subject: Re: [PATCH][GCC][ARM] Fix fragile arm fpu attribute tests.

> >

> > On 21 December 2017 at 15:24, Tamar Christina <Tamar.Christina@arm.com>

> > wrote:

> > > The 12/14/2017 20:46, Christophe Lyon wrote:

> > >> On 14 December 2017 at 11:56, Tamar Christina

> > <Tamar.Christina@arm.com> wrote:

> > >> > The 12/13/2017 08:49, Christophe Lyon wrote:

> > >> >> On 12 December 2017 at 18:29, Tamar Christina

> > <tamar.christina@arm.com> wrote:

> > >> >> > Hi All,

> > >> >> >

> > >> >> > The previous test made use of arm_neon.h which made the whole

> > >> >> > test rather fragile and only applicable to some of the arm 

> targets.

> > >> >> >

> > >> >> > So instead I make use of different fpus now to test the

> > >> >> > generation of fmla instructions. The actual instruction itself

> > >> >> > is not tested as all we care about if that the proper .fpu 

> directives are

> > generated.

> > >> >> >

> > >> >> > Regtested on arm-none-eabi and arm-none-linux-gnueabihf with no

> > >> >> > regressions.

> > >> >> >

> > >> >> > Ok for trunk?

> > >> >> >

> > >> >> >

> > >> >> > gcc/testsuite/

> > >> >> > 2017-12-12  Tamar Christina <tamar.christina@arm.com>

> > >> >> >

> > >> >> >         PR target/82641

> > >> >> >         * gcc.target/arm/pragma_fpu_attribute.c: New.

> > >> >> >         * gcc.target/arm/pragma_fpu_attribute_2.c: New.

> > >> >

> > >> > Hi Christophe,

> > >> >

> > >> > My apologies, I have rebased the patch.

> > >> > New Changelog:

> > >> >

> > >> > gcc/testsuite/

> > >> > 2017-12-14  Tamar Christina <tamar.christina@arm.com>

> > >> >

> > >> >         PR target/82641

> > >> >         * gcc.target/arm/pragma_fpu_attribute.c: Rewrite to use

> > >> >         no NEON.

> > >> >         * gcc.target/arm/pragma_fpu_attribute_2.c: Likewise.

> > >> >

> > >>

> > >> Hi,

> > >>

> > >> Sorry I think there is still something wrong with this patch.

> > >> In pragma_fpu_attribute_2.c, you are not removing #include

> > >> <arm_neon.h> as the ChangeLog seems to imply?

> > >>

> > >

> > > Sorry that was extremely sloppy of me. I noticed the changelog after

> > sending

> > > but hadn't noticed the #include being left in.

> > >

> > >> So, with this patch, there are problems on arm-none-linux-gnueabi and

> > >> arm-none-eabi:

> > >> FAIL:    gcc.target/arm/pragma_fpu_attribute.c scan-assembler-times

> > >> \\.fpu\\s+vfpv3-d16 1 (found 0 times)

> > >> FAIL:    gcc.target/arm/pragma_fpu_attribute.c scan-assembler-times

> > >> \\.fpu\\s+vfpv4 1 (found 0 times)

> > >>

> > >> and pragma_fpu_attribute_2.c still fails to compile:

> > >> In file included from

> > /gcc/testsuite/gcc.target/arm/pragma_fpu_attribute_2.c:6:

> > >> /aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-arm-none-

> > eabi/gcc3/gcc/include/arm_neon.h:31:2:

> > >> error: #error "NEON intrinsics not available with the soft-float ABI.

> > >> Please use -mfloat-abi=softfp or -mfloat-abi=hard"

> > >>

> > >> I'm not sure why you don't see this when testing on arm-none-eabi?

> > >

> > > It's because I don't have a compiler configured with only 

> -mfloat-abi=soft.

> > So when I run

> > > the tests it's always able to just change the ABI. I resorted to 

> manually

> > testing it.

> > >

> > > I've now prevented the tests from running at all on soft float 

> only targets.

> > This should fix

> > > the problem once and for all.

> > >

> > > Regtested on arm-none-eabi, arm-none-linux-gnueabi and arm-none-

> > linux-gnueabihf.

> > >

> > > Thanks and sorry for the noise,

> > > Tamar

> > >

> > > Ok for trunk?

> > >

> > > gcc/testsuite/

> > > 2017-12-21  Tamar Christina <tamar.christina@arm.com>

> > >

> > >         PR target/82641

> > >         * gcc.target/arm/pragma_fpu_attribute.c: Rewrite to use

> > >         no NEON and require softfp or hard float-abi.

> > >         * gcc.target/arm/pragma_fpu_attribute_2.c: Likewise.

> > >

> > FWIW, this version passes validation on my side.

> > Thanks

> >

> > >>

> > >> If you want to see more details:

> > >> 

> http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test- 

> <http://people.linaro.org/%7Echristophe.lyon/cross-validation/gcc-test->

> > patches/255624-rb8655.patch-2/report-build-info.html

> > >> (ignore the lines with "interrupted", this means there was a problem

> > >> on the host during the build)

> > >>

> > >> Christophe

> > >>

> > >>

> > >> >> >

> > >> >> Sorry, it seems your patch does not apply against ToT, and

> > >> >> the ChangeLog looks incorrect (these are not new files)

> > >> >>

> > >> >> Christophe

> > >> >

> > >> > Thanks,

> > >> > Tamar

> > >> >

> > >> > --

> > >

> > > --

Patch

diff --git a/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute.c b/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute.c
index 5f039d9bfb2b14f9134f138527fc395b8e273bbb..8191deac25965564a3c78dc08959a5e5637a0066 100644
--- a/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute.c
+++ b/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute.c
@@ -1,21 +1,19 @@ 
 /* Test for target attribute assembly extension generations.  */
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_arch_v8a_ok } */
-/* { dg-require-effective-target arm_neon_ok } */
 /* { dg-additional-options "-std=gnu99" } */
 
 #include <stdint.h>
-#include <arm_neon.h>
+
+extern uint32_t bar ();
 
 #pragma GCC target("fpu=vfpv3-d16")
 
-extern uint32_t bar();
+extern float fmaf (float, float, float);
 
-__attribute__((target("fpu=crypto-neon-fp-armv8"))) poly64x1_t vsricw(poly64x1_t crc, uint32_t val)
+float
+__attribute__((target("fpu=vfpv4"))) vfma32 (float x, float y, float z)
 {
-    poly64x1_t res;
-    asm("vsri %0, %1, %2" : "=r"(res) : "r"(crc), "r"(val));
-    return res;
+  return fmaf (x, y, z);
 }
 
 uint32_t restored ()
@@ -23,5 +21,5 @@  uint32_t restored ()
   return bar();
 }
 
-/* { dg-final { scan-assembler-times {\.fpu\s+crypto-neon-fp-armv8} 1 } } */
+/* { dg-final { scan-assembler-times {\.fpu\s+vfpv4} 1 } } */
 /* { dg-final { scan-assembler-times {\.fpu\s+vfpv3-d16} 1 } } */
diff --git a/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute_2.c b/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute_2.c
index b710de38612707b9109966f7bbc694a913121cb6..b50d4107b56ed7abd8b95fd2a3a08df8ab54410a 100644
--- a/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute_2.c
+++ b/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute_2.c
@@ -1,7 +1,5 @@ 
 /* Test for #pragma assembly extension generations.  */
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_arch_v8a_ok } */
-/* { dg-require-effective-target arm_neon_ok } */
 /* { dg-additional-options "-std=gnu99" } */
 
 #include <stdint.h>
@@ -12,12 +10,13 @@ 
 extern uint32_t bar();
 
 #pragma GCC push_options
-#pragma GCC target("fpu=crypto-neon-fp-armv8")
-poly64x1_t vsricw(poly64x1_t crc, uint32_t val)
+#pragma GCC target("fpu=vfpv4")
+extern float fmaf (float, float, float);
+
+float
+vfma32 (float x, float y, float z)
 {
-    poly64x1_t res;
-    asm("vsri %0, %1, %2" : "=r"(res) : "r"(crc), "r"(val));
-    return res;
+  return fmaf (x, y, z);
 }
 #pragma GCC pop_options
 
@@ -26,5 +25,5 @@  uint32_t restored ()
   return bar();
 }
 
-/* { dg-final { scan-assembler-times {\.fpu\s+crypto-neon-fp-armv8} 1 } } */
+/* { dg-final { scan-assembler-times {\.fpu\s+vfpv4} 1 } } */
 /* { dg-final { scan-assembler-times {\.fpu\s+vfpv3-d16} 1 } } */