[testsuite][ARM target attributes] Fix effective_target tests

[testsuite][ARM target attributes] Fix effective_target tests

[Christophe Lyon]

[-- Attachment #1: Type: text/plain, Size: 3927 bytes --]

Hi,

After the recent commits from Christian adding target attributes
support for ARM FPU settings,  I've noticed that some of the tests
were failing because of incorrect assumptions wrt to the default
cpu/fpu/float-abi of the compiler.

This patch fixes the problems I've noticed in the following way:
- do not force -mfloat-abi=softfp in dg-options, to avoid conflicts
when gcc is configured --with-float=hard

- change arm_vfp_ok such that it tries several -mfpu/-mfloat-abi
flags, checks that __ARM_FP is defined and __ARM_NEON_FP is not
defined

- introduce arm_fp_ok, which is similar but does not enforce fpu setting

- add a new effective_target: arm_crypto_pragma_ok to check that
setting this fpu via a pragma is actually supported by the current
"multilib". This is different from checking the command-line option
because the pragma might conflict with the command-line options in
use.

The updates in the testcases are as follows:
- attr-crypto.c, we have to make sure that the defaut fpu does not
conflict with the one forced by pragma. That's why I use the arm_vfp
options/effective_target. This is needed if gcc has been configured
--with-fpu=neon-fp16, as the pragma fpu=crypto-neon-fp-armv8 would
conflict.

- attr-neon-builtin-fail.c: use arm_fp to force the appropriate
float-abi setting. Enforcing fpu is not needed here.

- attr-neon-fp16.c: similar, I also removed arm_neon_ok since it was
not necessary to make the test pass in my testing. On second thought,
I'm wondering whether I should leave it and make the test unsupported
in more cases (such as when forcing -march=armv5t, although it does
pass with this patch)

- attr-neon2.c: use arm_vfp to force the appropriate float-abi
setting. Enforcing mfpu=vfp is needed to avoid conflict with the
pragma target fpu=neon (for instance if the toolchain default is
neon-fp16)

- attr-neon3.c: similar

Tested on a variety of configurations, see:
http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/230929-target-attr/report-build-info.html

Note that the regressions reported fall into 3 categories:
- when forcing march=armv5t: tests are now unsupported because I
modified arm_crypto_ok to require arm_v8_neon_ok instead of arm32.

- the warning reported by attr-neon-builtin-fail.c moved from line 12
to 14 and is thus seen as a regression + one improvement

- finally, attr-neon-fp16.c causes an ICE on armeb compilers, for
which I need to post a bugzilla.


TBH, I'm a bit concerned by the complexity of all these multilib-like
conditions. I'm confident that I'm still missing some combinations :-)

And with new target attributes coming, new architectures etc... all
this logic is likely to become even more complex.

That being said, OK for trunk?

Christophe


2015-11-27  Christophe Lyon  <christophe.lyon@linaro.org>

    * lib/target-supports.exp
    (check_effective_target_arm_vfp_ok_nocache): New.
    (check_effective_target_arm_vfp_ok): Call the new
    check_effective_target_arm_vfp_ok_nocache function.
    (check_effective_target_arm_fp_ok_nocache): New.
    (check_effective_target_arm_fp_ok): New.
    (add_options_for_arm_fp): New.
    (check_effective_target_arm_crypto_ok_nocache): Require
    target_arm_v8_neon_ok instead of arm32.
    (check_effective_target_arm_crypto_pragma_ok_nocache): New.
    (check_effective_target_arm_crypto_pragma_ok): New.
    (add_options_for_arm_vfp): New.
    * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok effective
    target. Do not force -mfloat-abi=softfp, use arm_vfp effective
    target instead.
    * gcc.target/arm/attr-neon-builtin-fail.c: Do not force
    -mfloat-abi=softfp, use arm_fp effective target instead.
    * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove arm_neon_ok
    dependency.
    * gcc.target/arm/attr-neon2.c: Do not force -mfloat-abi=softfp,
    use arm_vfp effective target instead.
    * gcc.target/arm/attr-neon3.c: Likewise.

[-- Attachment #2: target-attr.txt --]
[-- Type: text/plain, Size: 9010 bytes --]

diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 254c4e3..886ad66 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -2664,17 +2664,34 @@ proc check_effective_target_arm_vect_no_misalign { } {
 
 
 # Return 1 if this is an ARM target supporting -mfpu=vfp
-# -mfloat-abi=softfp.  Some multilibs may be incompatible with these
-# options.
+# -mfloat-abi=softfp or equivalent options.  Some multilibs may be
+# incompatible with these options.  Also set et_arm_vfp_flags to the
+# best options to add.
 
-proc check_effective_target_arm_vfp_ok { } {
+proc check_effective_target_arm_vfp_ok_nocache { } {
+    global et_arm_vfp_flags
+    set et_arm_vfp_flags ""
     if { [check_effective_target_arm32] } {
-	return [check_no_compiler_messages arm_vfp_ok object {
-	    int dummy;
-	} "-mfpu=vfp -mfloat-abi=softfp"]
-    } else {
-	return 0
+	foreach flags {"-mfpu=vfp" "-mfpu=vfp -mfloat-abi=softfp" "-mfpu=vfp -mfloat-abi=hard"} {
+	    if { [check_no_compiler_messages_nocache arm_vfp_ok object {
+		#ifndef __ARM_FP
+		#error __ARM_FP not defined
+		#endif
+		#ifdef __ARM_NEON_FP
+		#error __ARM_NEON_FP defined
+		#endif
+	    } "$flags" ] } {
+		set et_arm_vfp_flags $flags
+		return 1
+	    }
+	}
     }
+    return 0
+}
+
+proc check_effective_target_arm_vfp_ok { } {
+    return [check_cached_effective_target arm_vfp_ok \
+		check_effective_target_arm_vfp_ok_nocache]
 }
 
 # Return 1 if this is an ARM target supporting -mfpu=vfp3
@@ -2721,6 +2738,47 @@ proc check_effective_target_arm_hard_vfp_ok { } {
     }
 }
 
+# Return 1 if this is an ARM target defining __ARM_FP. We may need
+# -mfloat-abi=softfp or equivalent options.  Some multilibs may be
+# incompatible with these options.  Also set et_arm_fp_flags to the
+# best options to add.
+
+proc check_effective_target_arm_fp_ok_nocache { } {
+    global et_arm_fp_flags
+    set et_arm_fp_flags ""
+    if { [check_effective_target_arm32] } {
+	foreach flags {"" "-mfloat-abi=softfp" "-mfloat-abi=hard"} {
+	    if { [check_no_compiler_messages_nocache arm_fp_ok object {
+		#ifndef __ARM_FP
+		#error __ARM_FP not defined
+		#endif
+	    } "$flags"] } {
+		set et_arm_fp_flags $flags
+		return 1
+	    }
+	}
+    }
+
+    return 0
+}
+
+proc check_effective_target_arm_fp_ok { } {
+    return [check_cached_effective_target arm_fp_ok \
+		check_effective_target_arm_fp_ok_nocache]
+}
+
+# Add the options needed to define __ARM_FP.  We need either
+# -mfloat-abi=softfp or -mfloat-abi=hard, but if one is already
+# specified by the multilib, use it.
+
+proc add_options_for_arm_fp { flags } {
+    if { ! [check_effective_target_arm_fp_ok] } {
+	return "$flags"
+    }
+    global et_arm_fp_flags
+    return "$flags $et_arm_fp_flags"
+}
+
 # Return 1 if this is an ARM target that supports DSP multiply with
 # current multilib flags.
 
@@ -2753,7 +2811,7 @@ proc check_effective_target_arm_unaligned { } {
 proc check_effective_target_arm_crypto_ok_nocache { } {
     global et_arm_crypto_flags
     set et_arm_crypto_flags ""
-    if { [check_effective_target_arm32] } {
+    if { [check_effective_target_arm_v8_neon_ok] } {
 	foreach flags {"" "-mfloat-abi=softfp" "-mfpu=crypto-neon-fp-armv8" "-mfpu=crypto-neon-fp-armv8 -mfloat-abi=softfp"} {
 	    if { [check_no_compiler_messages_nocache arm_crypto_ok object {
 		#include "arm_neon.h"
@@ -2779,6 +2837,54 @@ proc check_effective_target_arm_crypto_ok { } {
 		check_effective_target_arm_crypto_ok_nocache]
 }
 
+# Return 1 if this is an ARM target supporting pragma target
+# -mfpu=crypto-neon-fp-armv8 -mfloat-abi=softfp or equivalent options.
+# Some multilibs may be incompatible with these options.  Also set
+# et_arm_crypto_pragma_flags to the best options to add.
+
+proc check_effective_target_arm_crypto_pragma_ok_nocache { } {
+    global et_arm_crypto_pragma_flags
+    set et_arm_crypto_pragma_flags ""
+    if { [check_effective_target_arm_v8_neon_ok] } {
+	foreach flags {"" "-mfloat-abi=softfp" "-mfpu=crypto-neon-fp-armv8" "-mfpu=crypto-neon-fp-armv8 -mfloat-abi=softfp"} {
+	    if { [check_no_compiler_messages_nocache arm_crypto_pragmaok object {
+		#pragma GCC target ("fpu=crypto-neon-fp-armv8")
+		#include "arm_neon.h"
+		uint8x16_t
+		foo (uint8x16_t a, uint8x16_t b)
+		{
+	          return vaeseq_u8 (a, b);
+		}
+	    } "[add_options_for_arm_v8_neon ""] $flags"] } {
+		set et_arm_crypto_pragma_flags "[add_options_for_arm_v8_neon ""] $flags"
+		return 1
+	    }
+	}
+    }
+
+    return 0
+}
+
+# Return 1 if this is an ARM target supporting pragma target
+# -mfpu=crypto-neon-fp-armv8.
+
+proc check_effective_target_arm_crypto_pragma_ok { } {
+    return [check_cached_effective_target arm_crypto_pragma_ok \
+		check_effective_target_arm_crypto_pragma_ok_nocache]
+}
+
+# Add the options needed for VFP.  We need either -mfloat-abi=softfp
+# or -mfloat-abi=hard, but if one is already specified by the
+# multilib, use it.
+
+proc add_options_for_arm_vfp { flags } {
+    if { ! [check_effective_target_arm_vfp_ok] } {
+	return "$flags"
+    }
+    global et_arm_vfp_flags
+    return "$flags $et_arm_vfp_flags"
+}
+
 # Add options for crypto extensions.
 proc add_options_for_arm_crypto { flags } {
     if { ! [check_effective_target_arm_crypto_ok] } {
@@ -2897,8 +3003,8 @@ proc check_effective_target_arm_crc_ok { } {
 
 # Return 1 if this is an ARM target supporting -mfpu=neon-fp16
 # -mfloat-abi=softfp or equivalent options.  Some multilibs may be
-# incompatible with these options.  Also set et_arm_neon_flags to the
-# best options to add.
+# incompatible with these options.  Also set et_arm_neon_fp16_flags to
+# the best options to add.
 
 proc check_effective_target_arm_neon_fp16_ok_nocache { } {
     global et_arm_neon_fp16_flags
diff --git a/gcc/testsuite/gcc.target/arm/attr-crypto.c b/gcc/testsuite/gcc.target/arm/attr-crypto.c
index 1db5984..b703fbc 100644
--- a/gcc/testsuite/gcc.target/arm/attr-crypto.c
+++ b/gcc/testsuite/gcc.target/arm/attr-crypto.c
@@ -1,6 +1,10 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_crypto_ok } */
-/* { dg-options "-O2 -mfloat-abi=softfp" } */
+/* Make sure we can force fpu=vfp before switching using the
+   pragma.  */
+/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_crypto_pragma_ok } */
+/* { dg-options "-O2 -march=armv8-a" } */
+/* { dg-add-options arm_vfp } */
 
 #pragma GCC target ("fpu=crypto-neon-fp-armv8")
 
diff --git a/gcc/testsuite/gcc.target/arm/attr-neon-builtin-fail.c b/gcc/testsuite/gcc.target/arm/attr-neon-builtin-fail.c
index 6ac32fc..05dc579 100644
--- a/gcc/testsuite/gcc.target/arm/attr-neon-builtin-fail.c
+++ b/gcc/testsuite/gcc.target/arm/attr-neon-builtin-fail.c
@@ -1,7 +1,9 @@
 /* Check that calling a neon builtin from a function compiled with vfp fails.  */
 /* { dg-do compile } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-require-effective-target arm_neon_ok } */
-/* { dg-options "-O2 -mfloat-abi=softfp" } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_fp } */
 
 #include <arm_neon.h>
 
@@ -12,6 +14,5 @@ foo (uint8x16_t *p)
   *p = vmovq_n_u8 (3); /* { dg-message "called from here" } */
 }
 
-/* { dg-error "inlining failed in call to always_inline" "" { target *-*-* } 0 }
- */
+/* { dg-error "inlining failed in call to always_inline" "" { target *-*-* } 0 } */
 
diff --git a/gcc/testsuite/gcc.target/arm/attr-neon-fp16.c b/gcc/testsuite/gcc.target/arm/attr-neon-fp16.c
index 3cf8918..984992f 100644
--- a/gcc/testsuite/gcc.target/arm/attr-neon-fp16.c
+++ b/gcc/testsuite/gcc.target/arm/attr-neon-fp16.c
@@ -1,6 +1,7 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_neon_ok } */
-/* { dg-options "-mfp16-format=ieee -mfloat-abi=softfp" } */
+/* { dg-require-effective-target arm_fp_ok } */
+/* { dg-options "-mfp16-format=ieee" } */
+/* { dg-add-options arm_fp } */
 
 #include "arm_neon.h"
 
diff --git a/gcc/testsuite/gcc.target/arm/attr-neon2.c b/gcc/testsuite/gcc.target/arm/attr-neon2.c
index 819fad4..79133e5 100644
--- a/gcc/testsuite/gcc.target/arm/attr-neon2.c
+++ b/gcc/testsuite/gcc.target/arm/attr-neon2.c
@@ -1,6 +1,8 @@
 /* { dg-do compile } */
 /* { dg-require-effective-target arm_neon_ok } */
-/* { dg-options "-O2 -mfloat-abi=softfp -mfpu=vfp" } */
+/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_vfp } */
 
 #pragma GCC target ("fpu=neon")
 #include <arm_neon.h>
diff --git a/gcc/testsuite/gcc.target/arm/attr-neon3.c b/gcc/testsuite/gcc.target/arm/attr-neon3.c
index 30a1479..0d31fb5 100644
--- a/gcc/testsuite/gcc.target/arm/attr-neon3.c
+++ b/gcc/testsuite/gcc.target/arm/attr-neon3.c
@@ -1,6 +1,8 @@
 /* { dg-do compile } */
 /* { dg-require-effective-target arm_crypto_ok } */
-/* { dg-options "-O2 -mfloat-abi=softfp -mfpu=vfp" } */
+/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_vfp } */
 
 #include <arm_neon.h>
 

Re: [testsuite][ARM target attributes] Fix effective_target tests

[Christophe Lyon]

Ping?


On 27 November 2015 at 14:00, Christophe Lyon
<christophe.lyon@linaro.org> wrote:
> Hi,
>
> After the recent commits from Christian adding target attributes
> support for ARM FPU settings,  I've noticed that some of the tests
> were failing because of incorrect assumptions wrt to the default
> cpu/fpu/float-abi of the compiler.
>
> This patch fixes the problems I've noticed in the following way:
> - do not force -mfloat-abi=softfp in dg-options, to avoid conflicts
> when gcc is configured --with-float=hard
>
> - change arm_vfp_ok such that it tries several -mfpu/-mfloat-abi
> flags, checks that __ARM_FP is defined and __ARM_NEON_FP is not
> defined
>
> - introduce arm_fp_ok, which is similar but does not enforce fpu setting
>
> - add a new effective_target: arm_crypto_pragma_ok to check that
> setting this fpu via a pragma is actually supported by the current
> "multilib". This is different from checking the command-line option
> because the pragma might conflict with the command-line options in
> use.
>
> The updates in the testcases are as follows:
> - attr-crypto.c, we have to make sure that the defaut fpu does not
> conflict with the one forced by pragma. That's why I use the arm_vfp
> options/effective_target. This is needed if gcc has been configured
> --with-fpu=neon-fp16, as the pragma fpu=crypto-neon-fp-armv8 would
> conflict.
>
> - attr-neon-builtin-fail.c: use arm_fp to force the appropriate
> float-abi setting. Enforcing fpu is not needed here.
>
> - attr-neon-fp16.c: similar, I also removed arm_neon_ok since it was
> not necessary to make the test pass in my testing. On second thought,
> I'm wondering whether I should leave it and make the test unsupported
> in more cases (such as when forcing -march=armv5t, although it does
> pass with this patch)
>
> - attr-neon2.c: use arm_vfp to force the appropriate float-abi
> setting. Enforcing mfpu=vfp is needed to avoid conflict with the
> pragma target fpu=neon (for instance if the toolchain default is
> neon-fp16)
>
> - attr-neon3.c: similar
>
> Tested on a variety of configurations, see:
> http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/230929-target-attr/report-build-info.html
>
> Note that the regressions reported fall into 3 categories:
> - when forcing march=armv5t: tests are now unsupported because I
> modified arm_crypto_ok to require arm_v8_neon_ok instead of arm32.
>
> - the warning reported by attr-neon-builtin-fail.c moved from line 12
> to 14 and is thus seen as a regression + one improvement
>
> - finally, attr-neon-fp16.c causes an ICE on armeb compilers, for
> which I need to post a bugzilla.
>
I've created https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68620 for this.

>
> TBH, I'm a bit concerned by the complexity of all these multilib-like
> conditions. I'm confident that I'm still missing some combinations :-)
>
> And with new target attributes coming, new architectures etc... all
> this logic is likely to become even more complex.
>
> That being said, OK for trunk?
>
> Christophe
>
>
> 2015-11-27  Christophe Lyon  <christophe.lyon@linaro.org>
>
>     * lib/target-supports.exp
>     (check_effective_target_arm_vfp_ok_nocache): New.
>     (check_effective_target_arm_vfp_ok): Call the new
>     check_effective_target_arm_vfp_ok_nocache function.
>     (check_effective_target_arm_fp_ok_nocache): New.
>     (check_effective_target_arm_fp_ok): New.
>     (add_options_for_arm_fp): New.
>     (check_effective_target_arm_crypto_ok_nocache): Require
>     target_arm_v8_neon_ok instead of arm32.
>     (check_effective_target_arm_crypto_pragma_ok_nocache): New.
>     (check_effective_target_arm_crypto_pragma_ok): New.
>     (add_options_for_arm_vfp): New.
>     * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok effective
>     target. Do not force -mfloat-abi=softfp, use arm_vfp effective
>     target instead.
>     * gcc.target/arm/attr-neon-builtin-fail.c: Do not force
>     -mfloat-abi=softfp, use arm_fp effective target instead.
>     * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove arm_neon_ok
>     dependency.
>     * gcc.target/arm/attr-neon2.c: Do not force -mfloat-abi=softfp,
>     use arm_vfp effective target instead.
>     * gcc.target/arm/attr-neon3.c: Likewise.

Re: [testsuite][ARM target attributes] Fix effective_target tests

[Kyrill Tkachov]

Hi Christophe,

On 27/11/15 13:00, Christophe Lyon wrote:
> Hi,
>
> After the recent commits from Christian adding target attributes
> support for ARM FPU settings,  I've noticed that some of the tests
> were failing because of incorrect assumptions wrt to the default
> cpu/fpu/float-abi of the compiler.
>
> This patch fixes the problems I've noticed in the following way:
> - do not force -mfloat-abi=softfp in dg-options, to avoid conflicts
> when gcc is configured --with-float=hard
>
> - change arm_vfp_ok such that it tries several -mfpu/-mfloat-abi
> flags, checks that __ARM_FP is defined and __ARM_NEON_FP is not
> defined
>
> - introduce arm_fp_ok, which is similar but does not enforce fpu setting
>
> - add a new effective_target: arm_crypto_pragma_ok to check that
> setting this fpu via a pragma is actually supported by the current
> "multilib". This is different from checking the command-line option
> because the pragma might conflict with the command-line options in
> use.
>
> The updates in the testcases are as follows:
> - attr-crypto.c, we have to make sure that the defaut fpu does not
> conflict with the one forced by pragma. That's why I use the arm_vfp
> options/effective_target. This is needed if gcc has been configured
> --with-fpu=neon-fp16, as the pragma fpu=crypto-neon-fp-armv8 would
> conflict.
>
> - attr-neon-builtin-fail.c: use arm_fp to force the appropriate
> float-abi setting. Enforcing fpu is not needed here.
>
> - attr-neon-fp16.c: similar, I also removed arm_neon_ok since it was
> not necessary to make the test pass in my testing. On second thought,
> I'm wondering whether I should leave it and make the test unsupported
> in more cases (such as when forcing -march=armv5t, although it does
> pass with this patch)
>
> - attr-neon2.c: use arm_vfp to force the appropriate float-abi
> setting. Enforcing mfpu=vfp is needed to avoid conflict with the
> pragma target fpu=neon (for instance if the toolchain default is
> neon-fp16)
>
> - attr-neon3.c: similar
>
> Tested on a variety of configurations, see:
> http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/230929-target-attr/report-build-info.html
>
> Note that the regressions reported fall into 3 categories:
> - when forcing march=armv5t: tests are now unsupported because I
> modified arm_crypto_ok to require arm_v8_neon_ok instead of arm32.
>
> - the warning reported by attr-neon-builtin-fail.c moved from line 12
> to 14 and is thus seen as a regression + one improvement
>
> - finally, attr-neon-fp16.c causes an ICE on armeb compilers, for
> which I need to post a bugzilla.
>
>
> TBH, I'm a bit concerned by the complexity of all these multilib-like
> conditions. I'm confident that I'm still missing some combinations :-)
>
> And with new target attributes coming, new architectures etc... all
> this logic is likely to become even more complex.
>
> That being said, OK for trunk?

I'd like us to clean up and reorganise the gcc.target/arm testsuite
at some point to make it more robust with respect to the tons of multilib
options and configurations we can have for arm. It's becoming very frustrating
to test feature-specific stuff :(

This is ok in the meantime.
Sorry for the delay.

Thanks for handling this!
Kyrill

> Christophe
>
>
> 2015-11-27  Christophe Lyon  <christophe.lyon@linaro.org>
>
>      * lib/target-supports.exp
>      (check_effective_target_arm_vfp_ok_nocache): New.
>      (check_effective_target_arm_vfp_ok): Call the new
>      check_effective_target_arm_vfp_ok_nocache function.
>      (check_effective_target_arm_fp_ok_nocache): New.
>      (check_effective_target_arm_fp_ok): New.
>      (add_options_for_arm_fp): New.
>      (check_effective_target_arm_crypto_ok_nocache): Require
>      target_arm_v8_neon_ok instead of arm32.
>      (check_effective_target_arm_crypto_pragma_ok_nocache): New.
>      (check_effective_target_arm_crypto_pragma_ok): New.
>      (add_options_for_arm_vfp): New.
>      * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok effective
>      target. Do not force -mfloat-abi=softfp, use arm_vfp effective
>      target instead.
>      * gcc.target/arm/attr-neon-builtin-fail.c: Do not force
>      -mfloat-abi=softfp, use arm_fp effective target instead.
>      * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove arm_neon_ok
>      dependency.
>      * gcc.target/arm/attr-neon2.c: Do not force -mfloat-abi=softfp,
>      use arm_vfp effective target instead.
>      * gcc.target/arm/attr-neon3.c: Likewise.

Re: [testsuite][ARM target attributes] Fix effective_target tests

[Christophe Lyon]

On 8 December 2015 at 11:50, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
> Hi Christophe,
>
>
> On 27/11/15 13:00, Christophe Lyon wrote:
>>
>> Hi,
>>
>> After the recent commits from Christian adding target attributes
>> support for ARM FPU settings,  I've noticed that some of the tests
>> were failing because of incorrect assumptions wrt to the default
>> cpu/fpu/float-abi of the compiler.
>>
>> This patch fixes the problems I've noticed in the following way:
>> - do not force -mfloat-abi=softfp in dg-options, to avoid conflicts
>> when gcc is configured --with-float=hard
>>
>> - change arm_vfp_ok such that it tries several -mfpu/-mfloat-abi
>> flags, checks that __ARM_FP is defined and __ARM_NEON_FP is not
>> defined
>>
>> - introduce arm_fp_ok, which is similar but does not enforce fpu setting
>>
>> - add a new effective_target: arm_crypto_pragma_ok to check that
>> setting this fpu via a pragma is actually supported by the current
>> "multilib". This is different from checking the command-line option
>> because the pragma might conflict with the command-line options in
>> use.
>>
>> The updates in the testcases are as follows:
>> - attr-crypto.c, we have to make sure that the defaut fpu does not
>> conflict with the one forced by pragma. That's why I use the arm_vfp
>> options/effective_target. This is needed if gcc has been configured
>> --with-fpu=neon-fp16, as the pragma fpu=crypto-neon-fp-armv8 would
>> conflict.
>>
>> - attr-neon-builtin-fail.c: use arm_fp to force the appropriate
>> float-abi setting. Enforcing fpu is not needed here.
>>
>> - attr-neon-fp16.c: similar, I also removed arm_neon_ok since it was
>> not necessary to make the test pass in my testing. On second thought,
>> I'm wondering whether I should leave it and make the test unsupported
>> in more cases (such as when forcing -march=armv5t, although it does
>> pass with this patch)
>>
>> - attr-neon2.c: use arm_vfp to force the appropriate float-abi
>> setting. Enforcing mfpu=vfp is needed to avoid conflict with the
>> pragma target fpu=neon (for instance if the toolchain default is
>> neon-fp16)
>>
>> - attr-neon3.c: similar
>>
>> Tested on a variety of configurations, see:
>>
>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/230929-target-attr/report-build-info.html
>>
>> Note that the regressions reported fall into 3 categories:
>> - when forcing march=armv5t: tests are now unsupported because I
>> modified arm_crypto_ok to require arm_v8_neon_ok instead of arm32.
>>
>> - the warning reported by attr-neon-builtin-fail.c moved from line 12
>> to 14 and is thus seen as a regression + one improvement
>>
>> - finally, attr-neon-fp16.c causes an ICE on armeb compilers, for
>> which I need to post a bugzilla.
>>
>>
>> TBH, I'm a bit concerned by the complexity of all these multilib-like
>> conditions. I'm confident that I'm still missing some combinations :-)
>>
>> And with new target attributes coming, new architectures etc... all
>> this logic is likely to become even more complex.
>>
>> That being said, OK for trunk?
>
>
> I'd like us to clean up and reorganise the gcc.target/arm testsuite
> at some point to make it more robust with respect to the tons of multilib
> options and configurations we can have for arm. It's becoming very
> frustrating
> to test feature-specific stuff :(
>
> This is ok in the meantime.
> Sorry for the delay.
>

Thanks for the approval, glad to see I'm not the only one willing to see
improvements in this area :)

Committed as r231403.

Christophe.

> Thanks for handling this!
> Kyrill
>
>
>> Christophe
>>
>>
>> 2015-11-27  Christophe Lyon  <christophe.lyon@linaro.org>
>>
>>      * lib/target-supports.exp
>>      (check_effective_target_arm_vfp_ok_nocache): New.
>>      (check_effective_target_arm_vfp_ok): Call the new
>>      check_effective_target_arm_vfp_ok_nocache function.
>>      (check_effective_target_arm_fp_ok_nocache): New.
>>      (check_effective_target_arm_fp_ok): New.
>>      (add_options_for_arm_fp): New.
>>      (check_effective_target_arm_crypto_ok_nocache): Require
>>      target_arm_v8_neon_ok instead of arm32.
>>      (check_effective_target_arm_crypto_pragma_ok_nocache): New.
>>      (check_effective_target_arm_crypto_pragma_ok): New.
>>      (add_options_for_arm_vfp): New.
>>      * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok effective
>>      target. Do not force -mfloat-abi=softfp, use arm_vfp effective
>>      target instead.
>>      * gcc.target/arm/attr-neon-builtin-fail.c: Do not force
>>      -mfloat-abi=softfp, use arm_fp effective target instead.
>>      * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove arm_neon_ok
>>      dependency.
>>      * gcc.target/arm/attr-neon2.c: Do not force -mfloat-abi=softfp,
>>      use arm_vfp effective target instead.
>>      * gcc.target/arm/attr-neon3.c: Likewise.
>
>

Re: [testsuite][ARM target attributes] Fix effective_target tests

[Kyrill Tkachov]

Hi Christophe,

On 08/12/15 11:18, Christophe Lyon wrote:
> On 8 December 2015 at 11:50, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
>> Hi Christophe,
>>
>>
>> On 27/11/15 13:00, Christophe Lyon wrote:
>>> Hi,
>>>
>>> After the recent commits from Christian adding target attributes
>>> support for ARM FPU settings,  I've noticed that some of the tests
>>> were failing because of incorrect assumptions wrt to the default
>>> cpu/fpu/float-abi of the compiler.
>>>
>>> This patch fixes the problems I've noticed in the following way:
>>> - do not force -mfloat-abi=softfp in dg-options, to avoid conflicts
>>> when gcc is configured --with-float=hard
>>>
>>> - change arm_vfp_ok such that it tries several -mfpu/-mfloat-abi
>>> flags, checks that __ARM_FP is defined and __ARM_NEON_FP is not
>>> defined
>>>
>>> - introduce arm_fp_ok, which is similar but does not enforce fpu setting
>>>
>>> - add a new effective_target: arm_crypto_pragma_ok to check that
>>> setting this fpu via a pragma is actually supported by the current
>>> "multilib". This is different from checking the command-line option
>>> because the pragma might conflict with the command-line options in
>>> use.
>>>
>>> The updates in the testcases are as follows:
>>> - attr-crypto.c, we have to make sure that the defaut fpu does not
>>> conflict with the one forced by pragma. That's why I use the arm_vfp
>>> options/effective_target. This is needed if gcc has been configured
>>> --with-fpu=neon-fp16, as the pragma fpu=crypto-neon-fp-armv8 would
>>> conflict.
>>>
>>> - attr-neon-builtin-fail.c: use arm_fp to force the appropriate
>>> float-abi setting. Enforcing fpu is not needed here.
>>>
>>> - attr-neon-fp16.c: similar, I also removed arm_neon_ok since it was
>>> not necessary to make the test pass in my testing. On second thought,
>>> I'm wondering whether I should leave it and make the test unsupported
>>> in more cases (such as when forcing -march=armv5t, although it does
>>> pass with this patch)
>>>
>>> - attr-neon2.c: use arm_vfp to force the appropriate float-abi
>>> setting. Enforcing mfpu=vfp is needed to avoid conflict with the
>>> pragma target fpu=neon (for instance if the toolchain default is
>>> neon-fp16)
>>>
>>> - attr-neon3.c: similar
>>>
>>> Tested on a variety of configurations, see:
>>>
>>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/230929-target-attr/report-build-info.html
>>>
>>> Note that the regressions reported fall into 3 categories:
>>> - when forcing march=armv5t: tests are now unsupported because I
>>> modified arm_crypto_ok to require arm_v8_neon_ok instead of arm32.
>>>
>>> - the warning reported by attr-neon-builtin-fail.c moved from line 12
>>> to 14 and is thus seen as a regression + one improvement
>>>
>>> - finally, attr-neon-fp16.c causes an ICE on armeb compilers, for
>>> which I need to post a bugzilla.
>>>
>>>
>>> TBH, I'm a bit concerned by the complexity of all these multilib-like
>>> conditions. I'm confident that I'm still missing some combinations :-)
>>>
>>> And with new target attributes coming, new architectures etc... all
>>> this logic is likely to become even more complex.
>>>
>>> That being said, OK for trunk?
>>
>> I'd like us to clean up and reorganise the gcc.target/arm testsuite
>> at some point to make it more robust with respect to the tons of multilib
>> options and configurations we can have for arm. It's becoming very
>> frustrating
>> to test feature-specific stuff :(
>>
>> This is ok in the meantime.
>> Sorry for the delay.
>>
> Thanks for the approval, glad to see I'm not the only one willing to see
> improvements in this area :)
>
> Committed as r231403.

With this patch we're seeing legitimate PASSes go to NA.
For example:

gcc.target/arm/vfp-1.c
gcc.target/arm/vfp-ldmdbs.c
and other vfp tests in arm.exp.
This is, for example, for the variant-mthumb/-march=armv8-a/-mfpu=crypto-neon-fp-armv8/-mfloat-abi=hard

I suspect under your new predicates they'd need to be changed to check for arm_fp_ok rather than arm_vfp_ok.

We want to avoid removing any PASSes.
Can you please test some more to make sure we don't remove any legitimate passes with your patch?

Also, Ramana reminded me offline that any new predicates in target-supports.exp
need documenting in sourcebuild.txt.

In light of that, can you please revert your patch and address the issues above
so that we can be confident we don't lose existing legitimate test coverage?

Sorry for not catching these sooner.
Kyrill

> Christophe.
>
>> Thanks for handling this!
>> Kyrill
>>
>>
>>> Christophe
>>>
>>>
>>> 2015-11-27  Christophe Lyon  <christophe.lyon@linaro.org>
>>>
>>>       * lib/target-supports.exp
>>>       (check_effective_target_arm_vfp_ok_nocache): New.
>>>       (check_effective_target_arm_vfp_ok): Call the new
>>>       check_effective_target_arm_vfp_ok_nocache function.
>>>       (check_effective_target_arm_fp_ok_nocache): New.
>>>       (check_effective_target_arm_fp_ok): New.
>>>       (add_options_for_arm_fp): New.
>>>       (check_effective_target_arm_crypto_ok_nocache): Require
>>>       target_arm_v8_neon_ok instead of arm32.
>>>       (check_effective_target_arm_crypto_pragma_ok_nocache): New.
>>>       (check_effective_target_arm_crypto_pragma_ok): New.
>>>       (add_options_for_arm_vfp): New.
>>>       * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok effective
>>>       target. Do not force -mfloat-abi=softfp, use arm_vfp effective
>>>       target instead.
>>>       * gcc.target/arm/attr-neon-builtin-fail.c: Do not force
>>>       -mfloat-abi=softfp, use arm_fp effective target instead.
>>>       * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove arm_neon_ok
>>>       dependency.
>>>       * gcc.target/arm/attr-neon2.c: Do not force -mfloat-abi=softfp,
>>>       use arm_vfp effective target instead.
>>>       * gcc.target/arm/attr-neon3.c: Likewise.
>>

Re: [testsuite][ARM target attributes] Fix effective_target tests

[Christophe Lyon]

On 10 December 2015 at 13:30, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
> Hi Christophe,
>
>
> On 08/12/15 11:18, Christophe Lyon wrote:
>>
>> On 8 December 2015 at 11:50, Kyrill Tkachov <kyrylo.tkachov@arm.com>
>> wrote:
>>>
>>> Hi Christophe,
>>>
>>>
>>> On 27/11/15 13:00, Christophe Lyon wrote:
>>>>
>>>> Hi,
>>>>
>>>> After the recent commits from Christian adding target attributes
>>>> support for ARM FPU settings,  I've noticed that some of the tests
>>>> were failing because of incorrect assumptions wrt to the default
>>>> cpu/fpu/float-abi of the compiler.
>>>>
>>>> This patch fixes the problems I've noticed in the following way:
>>>> - do not force -mfloat-abi=softfp in dg-options, to avoid conflicts
>>>> when gcc is configured --with-float=hard
>>>>
>>>> - change arm_vfp_ok such that it tries several -mfpu/-mfloat-abi
>>>> flags, checks that __ARM_FP is defined and __ARM_NEON_FP is not
>>>> defined
>>>>
>>>> - introduce arm_fp_ok, which is similar but does not enforce fpu setting
>>>>
>>>> - add a new effective_target: arm_crypto_pragma_ok to check that
>>>> setting this fpu via a pragma is actually supported by the current
>>>> "multilib". This is different from checking the command-line option
>>>> because the pragma might conflict with the command-line options in
>>>> use.
>>>>
>>>> The updates in the testcases are as follows:
>>>> - attr-crypto.c, we have to make sure that the defaut fpu does not
>>>> conflict with the one forced by pragma. That's why I use the arm_vfp
>>>> options/effective_target. This is needed if gcc has been configured
>>>> --with-fpu=neon-fp16, as the pragma fpu=crypto-neon-fp-armv8 would
>>>> conflict.
>>>>
>>>> - attr-neon-builtin-fail.c: use arm_fp to force the appropriate
>>>> float-abi setting. Enforcing fpu is not needed here.
>>>>
>>>> - attr-neon-fp16.c: similar, I also removed arm_neon_ok since it was
>>>> not necessary to make the test pass in my testing. On second thought,
>>>> I'm wondering whether I should leave it and make the test unsupported
>>>> in more cases (such as when forcing -march=armv5t, although it does
>>>> pass with this patch)
>>>>
>>>> - attr-neon2.c: use arm_vfp to force the appropriate float-abi
>>>> setting. Enforcing mfpu=vfp is needed to avoid conflict with the
>>>> pragma target fpu=neon (for instance if the toolchain default is
>>>> neon-fp16)
>>>>
>>>> - attr-neon3.c: similar
>>>>
>>>> Tested on a variety of configurations, see:
>>>>
>>>>
>>>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/230929-target-attr/report-build-info.html
>>>>
>>>> Note that the regressions reported fall into 3 categories:
>>>> - when forcing march=armv5t: tests are now unsupported because I
>>>> modified arm_crypto_ok to require arm_v8_neon_ok instead of arm32.
>>>>
>>>> - the warning reported by attr-neon-builtin-fail.c moved from line 12
>>>> to 14 and is thus seen as a regression + one improvement
>>>>
>>>> - finally, attr-neon-fp16.c causes an ICE on armeb compilers, for
>>>> which I need to post a bugzilla.
>>>>
>>>>
>>>> TBH, I'm a bit concerned by the complexity of all these multilib-like
>>>> conditions. I'm confident that I'm still missing some combinations :-)
>>>>
>>>> And with new target attributes coming, new architectures etc... all
>>>> this logic is likely to become even more complex.
>>>>
>>>> That being said, OK for trunk?
>>>
>>>
>>> I'd like us to clean up and reorganise the gcc.target/arm testsuite
>>> at some point to make it more robust with respect to the tons of multilib
>>> options and configurations we can have for arm. It's becoming very
>>> frustrating
>>> to test feature-specific stuff :(
>>>
>>> This is ok in the meantime.
>>> Sorry for the delay.
>>>
>> Thanks for the approval, glad to see I'm not the only one willing to see
>> improvements in this area :)
>>
>> Committed as r231403.
>
>
> With this patch we're seeing legitimate PASSes go to NA.
> For example:
>
> gcc.target/arm/vfp-1.c
> gcc.target/arm/vfp-ldmdbs.c
> and other vfp tests in arm.exp.
> This is, for example, for the
> variant-mthumb/-march=armv8-a/-mfpu=crypto-neon-fp-armv8/-mfloat-abi=hard
>
Hmm I'm attempting to cover such a configuration in my matrix of validations:
http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/231403/report-build-info.html

The difference is that I use similar flags at GCC configure time, while you
override them when running the testsuite:
--target arm-none-linux-gnueabihf --with-mode=thum --with-cpu=cortex-a57
--with-fpu=crypto-neon-fp-armv8

I this case, I do not see the regressions.

> I suspect under your new predicates they'd need to be changed to check for
> arm_fp_ok rather than arm_vfp_ok.
Probably, yes.

> We want to avoid removing any PASSes.
> Can you please test some more to make sure we don't remove any legitimate
> passes with your patch?
Is that the only combination in which you saw less PASSes?

> Also, Ramana reminded me offline that any new predicates in
> target-supports.exp
> need documenting in sourcebuild.txt.
>
> In light of that, can you please revert your patch and address the issues
> above
> so that we can be confident we don't lose existing legitimate test coverage?
OK.

> Sorry for not catching these sooner.
No problem, there are so many combinations.
I'm not sure how to define a reasonable set.

Christophe.

> Kyrill
>
>
>> Christophe.
>>
>>> Thanks for handling this!
>>> Kyrill
>>>
>>>
>>>> Christophe
>>>>
>>>>
>>>> 2015-11-27  Christophe Lyon  <christophe.lyon@linaro.org>
>>>>
>>>>       * lib/target-supports.exp
>>>>       (check_effective_target_arm_vfp_ok_nocache): New.
>>>>       (check_effective_target_arm_vfp_ok): Call the new
>>>>       check_effective_target_arm_vfp_ok_nocache function.
>>>>       (check_effective_target_arm_fp_ok_nocache): New.
>>>>       (check_effective_target_arm_fp_ok): New.
>>>>       (add_options_for_arm_fp): New.
>>>>       (check_effective_target_arm_crypto_ok_nocache): Require
>>>>       target_arm_v8_neon_ok instead of arm32.
>>>>       (check_effective_target_arm_crypto_pragma_ok_nocache): New.
>>>>       (check_effective_target_arm_crypto_pragma_ok): New.
>>>>       (add_options_for_arm_vfp): New.
>>>>       * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok effective
>>>>       target. Do not force -mfloat-abi=softfp, use arm_vfp effective
>>>>       target instead.
>>>>       * gcc.target/arm/attr-neon-builtin-fail.c: Do not force
>>>>       -mfloat-abi=softfp, use arm_fp effective target instead.
>>>>       * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove arm_neon_ok
>>>>       dependency.
>>>>       * gcc.target/arm/attr-neon2.c: Do not force -mfloat-abi=softfp,
>>>>       use arm_vfp effective target instead.
>>>>       * gcc.target/arm/attr-neon3.c: Likewise.
>>>
>>>
>

Re: [testsuite][ARM target attributes] Fix effective_target tests

[Kyrill Tkachov]

On 10/12/15 13:04, Christophe Lyon wrote:
> On 10 December 2015 at 13:30, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
>> Hi Christophe,
>>
>>
>> On 08/12/15 11:18, Christophe Lyon wrote:
>>> On 8 December 2015 at 11:50, Kyrill Tkachov <kyrylo.tkachov@arm.com>
>>> wrote:
>>>> Hi Christophe,
>>>>
>>>>
>>>> On 27/11/15 13:00, Christophe Lyon wrote:
>>>>> Hi,
>>>>>
>>>>> After the recent commits from Christian adding target attributes
>>>>> support for ARM FPU settings,  I've noticed that some of the tests
>>>>> were failing because of incorrect assumptions wrt to the default
>>>>> cpu/fpu/float-abi of the compiler.
>>>>>
>>>>> This patch fixes the problems I've noticed in the following way:
>>>>> - do not force -mfloat-abi=softfp in dg-options, to avoid conflicts
>>>>> when gcc is configured --with-float=hard
>>>>>
>>>>> - change arm_vfp_ok such that it tries several -mfpu/-mfloat-abi
>>>>> flags, checks that __ARM_FP is defined and __ARM_NEON_FP is not
>>>>> defined
>>>>>
>>>>> - introduce arm_fp_ok, which is similar but does not enforce fpu setting
>>>>>
>>>>> - add a new effective_target: arm_crypto_pragma_ok to check that
>>>>> setting this fpu via a pragma is actually supported by the current
>>>>> "multilib". This is different from checking the command-line option
>>>>> because the pragma might conflict with the command-line options in
>>>>> use.
>>>>>
>>>>> The updates in the testcases are as follows:
>>>>> - attr-crypto.c, we have to make sure that the defaut fpu does not
>>>>> conflict with the one forced by pragma. That's why I use the arm_vfp
>>>>> options/effective_target. This is needed if gcc has been configured
>>>>> --with-fpu=neon-fp16, as the pragma fpu=crypto-neon-fp-armv8 would
>>>>> conflict.
>>>>>
>>>>> - attr-neon-builtin-fail.c: use arm_fp to force the appropriate
>>>>> float-abi setting. Enforcing fpu is not needed here.
>>>>>
>>>>> - attr-neon-fp16.c: similar, I also removed arm_neon_ok since it was
>>>>> not necessary to make the test pass in my testing. On second thought,
>>>>> I'm wondering whether I should leave it and make the test unsupported
>>>>> in more cases (such as when forcing -march=armv5t, although it does
>>>>> pass with this patch)
>>>>>
>>>>> - attr-neon2.c: use arm_vfp to force the appropriate float-abi
>>>>> setting. Enforcing mfpu=vfp is needed to avoid conflict with the
>>>>> pragma target fpu=neon (for instance if the toolchain default is
>>>>> neon-fp16)
>>>>>
>>>>> - attr-neon3.c: similar
>>>>>
>>>>> Tested on a variety of configurations, see:
>>>>>
>>>>>
>>>>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/230929-target-attr/report-build-info.html
>>>>>
>>>>> Note that the regressions reported fall into 3 categories:
>>>>> - when forcing march=armv5t: tests are now unsupported because I
>>>>> modified arm_crypto_ok to require arm_v8_neon_ok instead of arm32.
>>>>>
>>>>> - the warning reported by attr-neon-builtin-fail.c moved from line 12
>>>>> to 14 and is thus seen as a regression + one improvement
>>>>>
>>>>> - finally, attr-neon-fp16.c causes an ICE on armeb compilers, for
>>>>> which I need to post a bugzilla.
>>>>>
>>>>>
>>>>> TBH, I'm a bit concerned by the complexity of all these multilib-like
>>>>> conditions. I'm confident that I'm still missing some combinations :-)
>>>>>
>>>>> And with new target attributes coming, new architectures etc... all
>>>>> this logic is likely to become even more complex.
>>>>>
>>>>> That being said, OK for trunk?
>>>>
>>>> I'd like us to clean up and reorganise the gcc.target/arm testsuite
>>>> at some point to make it more robust with respect to the tons of multilib
>>>> options and configurations we can have for arm. It's becoming very
>>>> frustrating
>>>> to test feature-specific stuff :(
>>>>
>>>> This is ok in the meantime.
>>>> Sorry for the delay.
>>>>
>>> Thanks for the approval, glad to see I'm not the only one willing to see
>>> improvements in this area :)
>>>
>>> Committed as r231403.
>>
>> With this patch we're seeing legitimate PASSes go to NA.
>> For example:
>>
>> gcc.target/arm/vfp-1.c
>> gcc.target/arm/vfp-ldmdbs.c
>> and other vfp tests in arm.exp.
>> This is, for example, for the
>> variant-mthumb/-march=armv8-a/-mfpu=crypto-neon-fp-armv8/-mfloat-abi=hard
>>
> Hmm I'm attempting to cover such a configuration in my matrix of validations:
> http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/231403/report-build-info.html
>
> The difference is that I use similar flags at GCC configure time, while you
> override them when running the testsuite:
> --target arm-none-linux-gnueabihf --with-mode=thum --with-cpu=cortex-a57
> --with-fpu=crypto-neon-fp-armv8
>
> I this case, I do not see the regressions.

My gcc is arm-none-eabi configured with
--with-float=hard --with-cpu=cortex-a15 --with-fpu=neon-vfpv4 --with-float=hard

>> I suspect under your new predicates they'd need to be changed to check for
>> arm_fp_ok rather than arm_vfp_ok.
> Probably, yes.
>

In the test log where it checks the effective target it fails due to:
arm_vfp_ok27268.c:6:4: error: #error __ARM_NEON_FP defined
it's compiling the check with
-mthumb -march=armv8-a -mfpu=crypto-neon-fp-armv8 -mfloat-abi=hard -o arm_vfp_ok27268.o arm_vfp_ok27268.c

>> We want to avoid removing any PASSes.
>> Can you please test some more to make sure we don't remove any legitimate
>> passes with your patch?
> Is that the only combination in which you saw less PASSes?

That's the one that was brought to my attention, but I think the issue here is just the
tests that check for arm_vfp_ok rather than the new arm_fp_ok and set -mfpu=vfp explicitly.
They appear unsupported if testing with an explicit neon option in mfpu, I think.

>> Also, Ramana reminded me offline that any new predicates in
>> target-supports.exp
>> need documenting in sourcebuild.txt.
>>
>> In light of that, can you please revert your patch and address the issues
>> above
>> so that we can be confident we don't lose existing legitimate test coverage?
> OK.
>
>> Sorry for not catching these sooner.
> No problem, there are so many combinations.
> I'm not sure how to define a reasonable set.

So, I think the action plan here is to audit the tests that need to be changed
to arm_fp_ok and add the documentation for the new predicate checks.

Thanks,
Kyrill

> Christophe.
>
>> Kyrill
>>
>>
>>> Christophe.
>>>
>>>> Thanks for handling this!
>>>> Kyrill
>>>>
>>>>
>>>>> Christophe
>>>>>
>>>>>
>>>>> 2015-11-27  Christophe Lyon  <christophe.lyon@linaro.org>
>>>>>
>>>>>        * lib/target-supports.exp
>>>>>        (check_effective_target_arm_vfp_ok_nocache): New.
>>>>>        (check_effective_target_arm_vfp_ok): Call the new
>>>>>        check_effective_target_arm_vfp_ok_nocache function.
>>>>>        (check_effective_target_arm_fp_ok_nocache): New.
>>>>>        (check_effective_target_arm_fp_ok): New.
>>>>>        (add_options_for_arm_fp): New.
>>>>>        (check_effective_target_arm_crypto_ok_nocache): Require
>>>>>        target_arm_v8_neon_ok instead of arm32.
>>>>>        (check_effective_target_arm_crypto_pragma_ok_nocache): New.
>>>>>        (check_effective_target_arm_crypto_pragma_ok): New.
>>>>>        (add_options_for_arm_vfp): New.
>>>>>        * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok effective
>>>>>        target. Do not force -mfloat-abi=softfp, use arm_vfp effective
>>>>>        target instead.
>>>>>        * gcc.target/arm/attr-neon-builtin-fail.c: Do not force
>>>>>        -mfloat-abi=softfp, use arm_fp effective target instead.
>>>>>        * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove arm_neon_ok
>>>>>        dependency.
>>>>>        * gcc.target/arm/attr-neon2.c: Do not force -mfloat-abi=softfp,
>>>>>        use arm_vfp effective target instead.
>>>>>        * gcc.target/arm/attr-neon3.c: Likewise.
>>>>

Re: [testsuite][ARM target attributes] Fix effective_target tests

[Christophe Lyon]

On 10 December 2015 at 14:14, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
>
> On 10/12/15 13:04, Christophe Lyon wrote:
>>
>> On 10 December 2015 at 13:30, Kyrill Tkachov <kyrylo.tkachov@arm.com>
>> wrote:
>>>
>>> Hi Christophe,
>>>
>>>
>>> On 08/12/15 11:18, Christophe Lyon wrote:
>>>>
>>>> On 8 December 2015 at 11:50, Kyrill Tkachov <kyrylo.tkachov@arm.com>
>>>> wrote:
>>>>>
>>>>> Hi Christophe,
>>>>>
>>>>>
>>>>> On 27/11/15 13:00, Christophe Lyon wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> After the recent commits from Christian adding target attributes
>>>>>> support for ARM FPU settings,  I've noticed that some of the tests
>>>>>> were failing because of incorrect assumptions wrt to the default
>>>>>> cpu/fpu/float-abi of the compiler.
>>>>>>
>>>>>> This patch fixes the problems I've noticed in the following way:
>>>>>> - do not force -mfloat-abi=softfp in dg-options, to avoid conflicts
>>>>>> when gcc is configured --with-float=hard
>>>>>>
>>>>>> - change arm_vfp_ok such that it tries several -mfpu/-mfloat-abi
>>>>>> flags, checks that __ARM_FP is defined and __ARM_NEON_FP is not
>>>>>> defined
>>>>>>
>>>>>> - introduce arm_fp_ok, which is similar but does not enforce fpu
>>>>>> setting
>>>>>>
>>>>>> - add a new effective_target: arm_crypto_pragma_ok to check that
>>>>>> setting this fpu via a pragma is actually supported by the current
>>>>>> "multilib". This is different from checking the command-line option
>>>>>> because the pragma might conflict with the command-line options in
>>>>>> use.
>>>>>>
>>>>>> The updates in the testcases are as follows:
>>>>>> - attr-crypto.c, we have to make sure that the defaut fpu does not
>>>>>> conflict with the one forced by pragma. That's why I use the arm_vfp
>>>>>> options/effective_target. This is needed if gcc has been configured
>>>>>> --with-fpu=neon-fp16, as the pragma fpu=crypto-neon-fp-armv8 would
>>>>>> conflict.
>>>>>>
>>>>>> - attr-neon-builtin-fail.c: use arm_fp to force the appropriate
>>>>>> float-abi setting. Enforcing fpu is not needed here.
>>>>>>
>>>>>> - attr-neon-fp16.c: similar, I also removed arm_neon_ok since it was
>>>>>> not necessary to make the test pass in my testing. On second thought,
>>>>>> I'm wondering whether I should leave it and make the test unsupported
>>>>>> in more cases (such as when forcing -march=armv5t, although it does
>>>>>> pass with this patch)
>>>>>>
>>>>>> - attr-neon2.c: use arm_vfp to force the appropriate float-abi
>>>>>> setting. Enforcing mfpu=vfp is needed to avoid conflict with the
>>>>>> pragma target fpu=neon (for instance if the toolchain default is
>>>>>> neon-fp16)
>>>>>>
>>>>>> - attr-neon3.c: similar
>>>>>>
>>>>>> Tested on a variety of configurations, see:
>>>>>>
>>>>>>
>>>>>>
>>>>>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/230929-target-attr/report-build-info.html
>>>>>>
>>>>>> Note that the regressions reported fall into 3 categories:
>>>>>> - when forcing march=armv5t: tests are now unsupported because I
>>>>>> modified arm_crypto_ok to require arm_v8_neon_ok instead of arm32.
>>>>>>
>>>>>> - the warning reported by attr-neon-builtin-fail.c moved from line 12
>>>>>> to 14 and is thus seen as a regression + one improvement
>>>>>>
>>>>>> - finally, attr-neon-fp16.c causes an ICE on armeb compilers, for
>>>>>> which I need to post a bugzilla.
>>>>>>
>>>>>>
>>>>>> TBH, I'm a bit concerned by the complexity of all these multilib-like
>>>>>> conditions. I'm confident that I'm still missing some combinations :-)
>>>>>>
>>>>>> And with new target attributes coming, new architectures etc... all
>>>>>> this logic is likely to become even more complex.
>>>>>>
>>>>>> That being said, OK for trunk?
>>>>>
>>>>>
>>>>> I'd like us to clean up and reorganise the gcc.target/arm testsuite
>>>>> at some point to make it more robust with respect to the tons of
>>>>> multilib
>>>>> options and configurations we can have for arm. It's becoming very
>>>>> frustrating
>>>>> to test feature-specific stuff :(
>>>>>
>>>>> This is ok in the meantime.
>>>>> Sorry for the delay.
>>>>>
>>>> Thanks for the approval, glad to see I'm not the only one willing to see
>>>> improvements in this area :)
>>>>
>>>> Committed as r231403.
>>>
>>>
>>> With this patch we're seeing legitimate PASSes go to NA.
>>> For example:
>>>
>>> gcc.target/arm/vfp-1.c
>>> gcc.target/arm/vfp-ldmdbs.c
>>> and other vfp tests in arm.exp.
>>> This is, for example, for the
>>> variant-mthumb/-march=armv8-a/-mfpu=crypto-neon-fp-armv8/-mfloat-abi=hard
>>>
>> Hmm I'm attempting to cover such a configuration in my matrix of
>> validations:
>>
>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/231403/report-build-info.html
>>
>> The difference is that I use similar flags at GCC configure time, while
>> you
>> override them when running the testsuite:
>> --target arm-none-linux-gnueabihf --with-mode=thum --with-cpu=cortex-a57
>> --with-fpu=crypto-neon-fp-armv8
>>
>> I this case, I do not see the regressions.
>
>
> My gcc is arm-none-eabi configured with
> --with-float=hard --with-cpu=cortex-a15 --with-fpu=neon-vfpv4
> --with-float=hard
>
>>> I suspect under your new predicates they'd need to be changed to check
>>> for
>>> arm_fp_ok rather than arm_vfp_ok.
>>
>> Probably, yes.
>>
>
> In the test log where it checks the effective target it fails due to:
> arm_vfp_ok27268.c:6:4: error: #error __ARM_NEON_FP defined
> it's compiling the check with
> -mthumb -march=armv8-a -mfpu=crypto-neon-fp-armv8 -mfloat-abi=hard -o
> arm_vfp_ok27268.o arm_vfp_ok27268.c
>
>>> We want to avoid removing any PASSes.
>>> Can you please test some more to make sure we don't remove any legitimate
>>> passes with your patch?
>>
>> Is that the only combination in which you saw less PASSes?
>
>
> That's the one that was brought to my attention, but I think the issue here
> is just the
> tests that check for arm_vfp_ok rather than the new arm_fp_ok and set
> -mfpu=vfp explicitly.
> They appear unsupported if testing with an explicit neon option in mfpu, I
> think.
>
>>> Also, Ramana reminded me offline that any new predicates in
>>> target-supports.exp
>>> need documenting in sourcebuild.txt.
>>>
>>> In light of that, can you please revert your patch and address the issues
>>> above
>>> so that we can be confident we don't lose existing legitimate test
>>> coverage?
>>
>> OK.
>>
>>> Sorry for not catching these sooner.
>>
>> No problem, there are so many combinations.
>> I'm not sure how to define a reasonable set.
>
>
> So, I think the action plan here is to audit the tests that need to be
> changed
> to arm_fp_ok and add the documentation for the new predicate checks.
>

I came up with a new version where I now use only the new arm_fp_ok, so
it might be easier just to update arm_vfp_ok and not introduce arm_fp_ok.

However, I'm still not happy with this version because it has problems with
a few tests that use target attributes such as attr-crypto.c, which starts with:
#pragma GCC target ("fpu=crypto-neon-fp-armv8")

This pragma fails if the compiler+options have set an incompatible
fpu before processing the pragma.

So for instance, if gcc has been configured --with-fpu=neon, compiling the test
with no fpu option fails (neon is not compatible with crypto-neon-fp-armv8
because the latter has fp16)

Now, if we use effective_target arm_vfp_ok + dg_option to force -mfpu=vfp,
we end up compiling with -mfpu=vfp and the pragma passes.

But if, in addition, we force the testflags to set -mfpu=crypto-neon-fp-armv8
as you do, the effective_target arm_vfp_ok test now fails because it is
compiled with -mfpu=vfp -mfpu=crypto-neon-fp-armv8, which defines
__ARM_NEON_FP

In short, the problem is how to make sure that the fpu setting is always
compatible with the pragma, whatever the gcc configuration and multilib
options used.

Thanks,

Christophe.

> Thanks,
> Kyrill
>
>
>> Christophe.
>>
>>> Kyrill
>>>
>>>
>>>> Christophe.
>>>>
>>>>> Thanks for handling this!
>>>>> Kyrill
>>>>>
>>>>>
>>>>>> Christophe
>>>>>>
>>>>>>
>>>>>> 2015-11-27  Christophe Lyon  <christophe.lyon@linaro.org>
>>>>>>
>>>>>>        * lib/target-supports.exp
>>>>>>        (check_effective_target_arm_vfp_ok_nocache): New.
>>>>>>        (check_effective_target_arm_vfp_ok): Call the new
>>>>>>        check_effective_target_arm_vfp_ok_nocache function.
>>>>>>        (check_effective_target_arm_fp_ok_nocache): New.
>>>>>>        (check_effective_target_arm_fp_ok): New.
>>>>>>        (add_options_for_arm_fp): New.
>>>>>>        (check_effective_target_arm_crypto_ok_nocache): Require
>>>>>>        target_arm_v8_neon_ok instead of arm32.
>>>>>>        (check_effective_target_arm_crypto_pragma_ok_nocache): New.
>>>>>>        (check_effective_target_arm_crypto_pragma_ok): New.
>>>>>>        (add_options_for_arm_vfp): New.
>>>>>>        * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok
>>>>>> effective
>>>>>>        target. Do not force -mfloat-abi=softfp, use arm_vfp effective
>>>>>>        target instead.
>>>>>>        * gcc.target/arm/attr-neon-builtin-fail.c: Do not force
>>>>>>        -mfloat-abi=softfp, use arm_fp effective target instead.
>>>>>>        * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove arm_neon_ok
>>>>>>        dependency.
>>>>>>        * gcc.target/arm/attr-neon2.c: Do not force -mfloat-abi=softfp,
>>>>>>        use arm_vfp effective target instead.
>>>>>>        * gcc.target/arm/attr-neon3.c: Likewise.
>>>>>
>>>>>
>

Re: [testsuite][ARM target attributes] Fix effective_target tests

[Christophe Lyon]

[-- Attachment #1: Type: text/plain, Size: 13126 bytes --]

Hi,

Here is an updated version of this patch.
I did test it with
-mthumb/-march=armv8-a/-mfpu=crypto-neon-fp-armv8/-mfloat-abi=hard in
addition to my usual set of options.

Compared to the previous version:
- I added some doc in sourcebuild.texi
- I no longer modify arm_vfp_ok...
- I replaced all uses of arm_vfp with the new arm_fp because I found
that the existing tests do not actually need to pass -mfpu=vfp: this
is implicitly set as the default when using -mfloat-abi={softfp|hard}
- I chose not to remove arm_vfp_ok because we may need it in the
future, if a test really needs vfp (as opposed to neon for instance)
- in gcc.target/arm/attr-crypto.c I force the initial fpu to be vfp
via pragma instead, so that the next pragma fpu
fpu=crypto-neon-fp-armv8 is always compatible, regardless of the
command-line options/default fpu
- same for attr-neon2.c and attr-neon3.c
- I updated cmp-2.c, unsigned-float.c, vfp-1.c, vfp-ldmdbd.c,
vfp-ldmdbs.c, vfp-ldmiad.c, vfp-ldmias.c, vfp-stmdbd.c, vfp-stmdbs.c,
vfp-stmiad.c, vfp-stmias.c, vnmul-[1234].c to use the new arm_fp
effective target instead of arm_vfp. This is so that they don't need
to use -mfpu=vfp and can use the new dg-add-options arm_fp

The validation results show (in addition to what I originally reported):
- attr-crypto.c and attr-neon3.c now ICE in some cases. This is PR68895.
- depending on the GCC configuration (e.g. --with-fpu=neon)
attr-neon3.c may fail. This is PR68896.

OK?

Christophe

2015-12-17  Christophe Lyon  <christophe.lyon@linaro.org>

    * doc/sourcebuild.texi (arm_fp_ok): Document new entry.
    (arm_fp): Likewise.
    * lib/target-supports.exp
    (check_effective_target_arm_fp_ok_nocache): New.
    (check_effective_target_arm_fp_ok): New.
    (add_options_for_arm_fp): New.
    (check_effective_target_arm_crypto_ok_nocache): Require
    target_arm_v8_neon_ok instead of arm32.
    (check_effective_target_arm_crypto_pragma_ok_nocache): New.
    (check_effective_target_arm_crypto_pragma_ok): New.
    (add_options_for_arm_vfp): New.
    * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok effective
    target. Do not force -mfloat-abi=softfp, use arm_fp_ok effective
    target instead. Force initial fpu to vfp.
    * gcc.target/arm/attr-neon-builtin-fail.c: Do not force
    -mfloat-abi=softfp, use arm_fp_ok effective target instead.
    * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove arm_neon_ok
    dependency.
    * gcc.target/arm/attr-neon2.c: Do not force -mfloat-abi=softfp,
    use arm_vfp effective target instead. Force initial fpu to vfp.
    * gcc.target/arm/attr-neon3.c: Likewise.
    * gcc.target/arm/cmp-2.c: Use arm_fp_ok effective target instead of
    arm_vfp_ok.
    * gcc.target/arm/unsigned-float.c: Likewise.
    * gcc.target/arm/vfp-1.c: Likewise.
    * gcc.target/arm/vfp-ldmdbd.c: Likewise.
    * gcc.target/arm/vfp-ldmdbs.c: Likewise.
    * gcc.target/arm/vfp-ldmiad.c: Likewise.
    * gcc.target/arm/vfp-ldmias.c: Likewise.
    * gcc.target/arm/vfp-stmdbd.c: Likewise.
    * gcc.target/arm/vfp-stmdbs.c: Likewise.
    * gcc.target/arm/vfp-stmiad.c: Likewise.
    * gcc.target/arm/vfp-stmias.c: Likewise.
    * gcc.target/arm/vnmul-1.c: Likewise.
    * gcc.target/arm/vnmul-2.c: Likewise.
    * gcc.target/arm/vnmul-3.c: Likewise.
    * gcc.target/arm/vnmul-4.c: Likewise.



On 10 December 2015 at 20:52, Christophe Lyon
<christophe.lyon@linaro.org> wrote:
> On 10 December 2015 at 14:14, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
>>
>> On 10/12/15 13:04, Christophe Lyon wrote:
>>>
>>> On 10 December 2015 at 13:30, Kyrill Tkachov <kyrylo.tkachov@arm.com>
>>> wrote:
>>>>
>>>> Hi Christophe,
>>>>
>>>>
>>>> On 08/12/15 11:18, Christophe Lyon wrote:
>>>>>
>>>>> On 8 December 2015 at 11:50, Kyrill Tkachov <kyrylo.tkachov@arm.com>
>>>>> wrote:
>>>>>>
>>>>>> Hi Christophe,
>>>>>>
>>>>>>
>>>>>> On 27/11/15 13:00, Christophe Lyon wrote:
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> After the recent commits from Christian adding target attributes
>>>>>>> support for ARM FPU settings,  I've noticed that some of the tests
>>>>>>> were failing because of incorrect assumptions wrt to the default
>>>>>>> cpu/fpu/float-abi of the compiler.
>>>>>>>
>>>>>>> This patch fixes the problems I've noticed in the following way:
>>>>>>> - do not force -mfloat-abi=softfp in dg-options, to avoid conflicts
>>>>>>> when gcc is configured --with-float=hard
>>>>>>>
>>>>>>> - change arm_vfp_ok such that it tries several -mfpu/-mfloat-abi
>>>>>>> flags, checks that __ARM_FP is defined and __ARM_NEON_FP is not
>>>>>>> defined
>>>>>>>
>>>>>>> - introduce arm_fp_ok, which is similar but does not enforce fpu
>>>>>>> setting
>>>>>>>
>>>>>>> - add a new effective_target: arm_crypto_pragma_ok to check that
>>>>>>> setting this fpu via a pragma is actually supported by the current
>>>>>>> "multilib". This is different from checking the command-line option
>>>>>>> because the pragma might conflict with the command-line options in
>>>>>>> use.
>>>>>>>
>>>>>>> The updates in the testcases are as follows:
>>>>>>> - attr-crypto.c, we have to make sure that the defaut fpu does not
>>>>>>> conflict with the one forced by pragma. That's why I use the arm_vfp
>>>>>>> options/effective_target. This is needed if gcc has been configured
>>>>>>> --with-fpu=neon-fp16, as the pragma fpu=crypto-neon-fp-armv8 would
>>>>>>> conflict.
>>>>>>>
>>>>>>> - attr-neon-builtin-fail.c: use arm_fp to force the appropriate
>>>>>>> float-abi setting. Enforcing fpu is not needed here.
>>>>>>>
>>>>>>> - attr-neon-fp16.c: similar, I also removed arm_neon_ok since it was
>>>>>>> not necessary to make the test pass in my testing. On second thought,
>>>>>>> I'm wondering whether I should leave it and make the test unsupported
>>>>>>> in more cases (such as when forcing -march=armv5t, although it does
>>>>>>> pass with this patch)
>>>>>>>
>>>>>>> - attr-neon2.c: use arm_vfp to force the appropriate float-abi
>>>>>>> setting. Enforcing mfpu=vfp is needed to avoid conflict with the
>>>>>>> pragma target fpu=neon (for instance if the toolchain default is
>>>>>>> neon-fp16)
>>>>>>>
>>>>>>> - attr-neon3.c: similar
>>>>>>>
>>>>>>> Tested on a variety of configurations, see:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/230929-target-attr/report-build-info.html
>>>>>>>
>>>>>>> Note that the regressions reported fall into 3 categories:
>>>>>>> - when forcing march=armv5t: tests are now unsupported because I
>>>>>>> modified arm_crypto_ok to require arm_v8_neon_ok instead of arm32.
>>>>>>>
>>>>>>> - the warning reported by attr-neon-builtin-fail.c moved from line 12
>>>>>>> to 14 and is thus seen as a regression + one improvement
>>>>>>>
>>>>>>> - finally, attr-neon-fp16.c causes an ICE on armeb compilers, for
>>>>>>> which I need to post a bugzilla.
>>>>>>>
>>>>>>>
>>>>>>> TBH, I'm a bit concerned by the complexity of all these multilib-like
>>>>>>> conditions. I'm confident that I'm still missing some combinations :-)
>>>>>>>
>>>>>>> And with new target attributes coming, new architectures etc... all
>>>>>>> this logic is likely to become even more complex.
>>>>>>>
>>>>>>> That being said, OK for trunk?
>>>>>>
>>>>>>
>>>>>> I'd like us to clean up and reorganise the gcc.target/arm testsuite
>>>>>> at some point to make it more robust with respect to the tons of
>>>>>> multilib
>>>>>> options and configurations we can have for arm. It's becoming very
>>>>>> frustrating
>>>>>> to test feature-specific stuff :(
>>>>>>
>>>>>> This is ok in the meantime.
>>>>>> Sorry for the delay.
>>>>>>
>>>>> Thanks for the approval, glad to see I'm not the only one willing to see
>>>>> improvements in this area :)
>>>>>
>>>>> Committed as r231403.
>>>>
>>>>
>>>> With this patch we're seeing legitimate PASSes go to NA.
>>>> For example:
>>>>
>>>> gcc.target/arm/vfp-1.c
>>>> gcc.target/arm/vfp-ldmdbs.c
>>>> and other vfp tests in arm.exp.
>>>> This is, for example, for the
>>>> variant-mthumb/-march=armv8-a/-mfpu=crypto-neon-fp-armv8/-mfloat-abi=hard
>>>>
>>> Hmm I'm attempting to cover such a configuration in my matrix of
>>> validations:
>>>
>>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/231403/report-build-info.html
>>>
>>> The difference is that I use similar flags at GCC configure time, while
>>> you
>>> override them when running the testsuite:
>>> --target arm-none-linux-gnueabihf --with-mode=thum --with-cpu=cortex-a57
>>> --with-fpu=crypto-neon-fp-armv8
>>>
>>> I this case, I do not see the regressions.
>>
>>
>> My gcc is arm-none-eabi configured with
>> --with-float=hard --with-cpu=cortex-a15 --with-fpu=neon-vfpv4
>> --with-float=hard
>>
>>>> I suspect under your new predicates they'd need to be changed to check
>>>> for
>>>> arm_fp_ok rather than arm_vfp_ok.
>>>
>>> Probably, yes.
>>>
>>
>> In the test log where it checks the effective target it fails due to:
>> arm_vfp_ok27268.c:6:4: error: #error __ARM_NEON_FP defined
>> it's compiling the check with
>> -mthumb -march=armv8-a -mfpu=crypto-neon-fp-armv8 -mfloat-abi=hard -o
>> arm_vfp_ok27268.o arm_vfp_ok27268.c
>>
>>>> We want to avoid removing any PASSes.
>>>> Can you please test some more to make sure we don't remove any legitimate
>>>> passes with your patch?
>>>
>>> Is that the only combination in which you saw less PASSes?
>>
>>
>> That's the one that was brought to my attention, but I think the issue here
>> is just the
>> tests that check for arm_vfp_ok rather than the new arm_fp_ok and set
>> -mfpu=vfp explicitly.
>> They appear unsupported if testing with an explicit neon option in mfpu, I
>> think.
>>
>>>> Also, Ramana reminded me offline that any new predicates in
>>>> target-supports.exp
>>>> need documenting in sourcebuild.txt.
>>>>
>>>> In light of that, can you please revert your patch and address the issues
>>>> above
>>>> so that we can be confident we don't lose existing legitimate test
>>>> coverage?
>>>
>>> OK.
>>>
>>>> Sorry for not catching these sooner.
>>>
>>> No problem, there are so many combinations.
>>> I'm not sure how to define a reasonable set.
>>
>>
>> So, I think the action plan here is to audit the tests that need to be
>> changed
>> to arm_fp_ok and add the documentation for the new predicate checks.
>>
>
> I came up with a new version where I now use only the new arm_fp_ok, so
> it might be easier just to update arm_vfp_ok and not introduce arm_fp_ok.
>
> However, I'm still not happy with this version because it has problems with
> a few tests that use target attributes such as attr-crypto.c, which starts with:
> #pragma GCC target ("fpu=crypto-neon-fp-armv8")
>
> This pragma fails if the compiler+options have set an incompatible
> fpu before processing the pragma.
>
> So for instance, if gcc has been configured --with-fpu=neon, compiling the test
> with no fpu option fails (neon is not compatible with crypto-neon-fp-armv8
> because the latter has fp16)
>
> Now, if we use effective_target arm_vfp_ok + dg_option to force -mfpu=vfp,
> we end up compiling with -mfpu=vfp and the pragma passes.
>
> But if, in addition, we force the testflags to set -mfpu=crypto-neon-fp-armv8
> as you do, the effective_target arm_vfp_ok test now fails because it is
> compiled with -mfpu=vfp -mfpu=crypto-neon-fp-armv8, which defines
> __ARM_NEON_FP
>
> In short, the problem is how to make sure that the fpu setting is always
> compatible with the pragma, whatever the gcc configuration and multilib
> options used.
>
> Thanks,
>
> Christophe.
>
>> Thanks,
>> Kyrill
>>
>>
>>> Christophe.
>>>
>>>> Kyrill
>>>>
>>>>
>>>>> Christophe.
>>>>>
>>>>>> Thanks for handling this!
>>>>>> Kyrill
>>>>>>
>>>>>>
>>>>>>> Christophe
>>>>>>>
>>>>>>>
>>>>>>> 2015-11-27  Christophe Lyon  <christophe.lyon@linaro.org>
>>>>>>>
>>>>>>>        * lib/target-supports.exp
>>>>>>>        (check_effective_target_arm_vfp_ok_nocache): New.
>>>>>>>        (check_effective_target_arm_vfp_ok): Call the new
>>>>>>>        check_effective_target_arm_vfp_ok_nocache function.
>>>>>>>        (check_effective_target_arm_fp_ok_nocache): New.
>>>>>>>        (check_effective_target_arm_fp_ok): New.
>>>>>>>        (add_options_for_arm_fp): New.
>>>>>>>        (check_effective_target_arm_crypto_ok_nocache): Require
>>>>>>>        target_arm_v8_neon_ok instead of arm32.
>>>>>>>        (check_effective_target_arm_crypto_pragma_ok_nocache): New.
>>>>>>>        (check_effective_target_arm_crypto_pragma_ok): New.
>>>>>>>        (add_options_for_arm_vfp): New.
>>>>>>>        * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok
>>>>>>> effective
>>>>>>>        target. Do not force -mfloat-abi=softfp, use arm_vfp effective
>>>>>>>        target instead.
>>>>>>>        * gcc.target/arm/attr-neon-builtin-fail.c: Do not force
>>>>>>>        -mfloat-abi=softfp, use arm_fp effective target instead.
>>>>>>>        * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove arm_neon_ok
>>>>>>>        dependency.
>>>>>>>        * gcc.target/arm/attr-neon2.c: Do not force -mfloat-abi=softfp,
>>>>>>>        use arm_vfp effective target instead.
>>>>>>>        * gcc.target/arm/attr-neon3.c: Likewise.
>>>>>>
>>>>>>
>>

[-- Attachment #2: target-attr-v5.patch --]
[-- Type: text/x-patch, Size: 18450 bytes --]

diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi
index 61de4a5..facde56 100644
--- a/gcc/doc/sourcebuild.texi
+++ b/gcc/doc/sourcebuild.texi
@@ -1514,6 +1514,12 @@ ARM target generates 32-bit code.
 @item arm_eabi
 ARM target adheres to the ABI for the ARM Architecture.
 
+@item arm_fp_ok
+@anchor{arm_fp_ok}
+ARM target defines @code{__ARM_FP} using @code{-mfloat-abi=softfp} or
+equivalent options..  Some multilibs may be incompatible with these
+options.
+
 @item arm_hf_eabi
 ARM target adheres to the VFP and Advanced SIMD Register Arguments
 variant of the ABI for the ARM Architecture (as selected with
@@ -2035,6 +2040,11 @@ The supported values of @var{feature} for directive @code{dg-add-options}
 are:
 
 @table @code
+@item arm_fp
+@code{__ARM_FP} definition.  Only ARM targets support this feature, and only then
+in certain modes; see the @ref{arm_fp_ok,,arm_fp_ok effective target
+keyword}.
+
 @item arm_neon
 NEON support.  Only ARM targets support this feature, and only then
 in certain modes; see the @ref{arm_neon_ok,,arm_neon_ok effective target
diff --git a/gcc/testsuite/gcc.target/arm/attr-crypto.c b/gcc/testsuite/gcc.target/arm/attr-crypto.c
index 1db5984..3b4b7c5 100644
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 4e349e9..6ab9270 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -2721,6 +2721,47 @@ proc check_effective_target_arm_hard_vfp_ok { } {
     }
 }
 
+# Return 1 if this is an ARM target defining __ARM_FP. We may need
+# -mfloat-abi=softfp or equivalent options.  Some multilibs may be
+# incompatible with these options.  Also set et_arm_fp_flags to the
+# best options to add.
+
+proc check_effective_target_arm_fp_ok_nocache { } {
+    global et_arm_fp_flags
+    set et_arm_fp_flags ""
+    if { [check_effective_target_arm32] } {
+	foreach flags {"" "-mfloat-abi=softfp" "-mfloat-abi=hard"} {
+	    if { [check_no_compiler_messages_nocache arm_fp_ok object {
+		#ifndef __ARM_FP
+		#error __ARM_FP not defined
+		#endif
+	    } "$flags"] } {
+		set et_arm_fp_flags $flags
+		return 1
+	    }
+	}
+    }
+
+    return 0
+}
+
+proc check_effective_target_arm_fp_ok { } {
+    return [check_cached_effective_target arm_fp_ok \
+		check_effective_target_arm_fp_ok_nocache]
+}
+
+# Add the options needed to define __ARM_FP.  We need either
+# -mfloat-abi=softfp or -mfloat-abi=hard, but if one is already
+# specified by the multilib, use it.
+
+proc add_options_for_arm_fp { flags } {
+    if { ! [check_effective_target_arm_fp_ok] } {
+	return "$flags"
+    }
+    global et_arm_fp_flags
+    return "$flags $et_arm_fp_flags"
+}
+
 # Return 1 if this is an ARM target that supports DSP multiply with
 # current multilib flags.
 
@@ -2753,7 +2794,7 @@ proc check_effective_target_arm_unaligned { } {
 proc check_effective_target_arm_crypto_ok_nocache { } {
     global et_arm_crypto_flags
     set et_arm_crypto_flags ""
-    if { [check_effective_target_arm32] } {
+    if { [check_effective_target_arm_v8_neon_ok] } {
 	foreach flags {"" "-mfloat-abi=softfp" "-mfpu=crypto-neon-fp-armv8" "-mfpu=crypto-neon-fp-armv8 -mfloat-abi=softfp"} {
 	    if { [check_no_compiler_messages_nocache arm_crypto_ok object {
 		#include "arm_neon.h"
@@ -2779,6 +2820,42 @@ proc check_effective_target_arm_crypto_ok { } {
 		check_effective_target_arm_crypto_ok_nocache]
 }
 
+# Return 1 if this is an ARM target supporting pragma target
+# -mfpu=crypto-neon-fp-armv8 -mfloat-abi=softfp or equivalent options.
+# Some multilibs may be incompatible with these options.  Also set
+# et_arm_crypto_pragma_flags to the best options to add.
+
+proc check_effective_target_arm_crypto_pragma_ok_nocache { } {
+    global et_arm_crypto_pragma_flags
+    set et_arm_crypto_pragma_flags ""
+    if { [check_effective_target_arm_v8_neon_ok] } {
+	foreach flags {"" "-mfloat-abi=softfp" "-mfpu=crypto-neon-fp-armv8" "-mfpu=crypto-neon-fp-armv8 -mfloat-abi=softfp"} {
+	    if { [check_no_compiler_messages_nocache arm_crypto_pragmaok object {
+		#pragma GCC target ("fpu=crypto-neon-fp-armv8")
+		#include "arm_neon.h"
+		uint8x16_t
+		foo (uint8x16_t a, uint8x16_t b)
+		{
+	          return vaeseq_u8 (a, b);
+		}
+	    } "[add_options_for_arm_v8_neon ""] $flags"] } {
+		set et_arm_crypto_pragma_flags "[add_options_for_arm_v8_neon ""] $flags"
+		return 1
+	    }
+	}
+    }
+
+    return 0
+}
+
+# Return 1 if this is an ARM target supporting pragma target
+# -mfpu=crypto-neon-fp-armv8.
+
+proc check_effective_target_arm_crypto_pragma_ok { } {
+    return [check_cached_effective_target arm_crypto_pragma_ok \
+		check_effective_target_arm_crypto_pragma_ok_nocache]
+}
+
 # Add options for crypto extensions.
 proc add_options_for_arm_crypto { flags } {
     if { ! [check_effective_target_arm_crypto_ok] } {
@@ -2907,8 +2984,8 @@ proc check_effective_target_arm_crc_ok { } {
 
 # Return 1 if this is an ARM target supporting -mfpu=neon-fp16
 # -mfloat-abi=softfp or equivalent options.  Some multilibs may be
-# incompatible with these options.  Also set et_arm_neon_flags to the
-# best options to add.
+# incompatible with these options.  Also set et_arm_neon_fp16_flags to
+# the best options to add.
 
 proc check_effective_target_arm_neon_fp16_ok_nocache { } {
     global et_arm_neon_fp16_flags
--- a/gcc/testsuite/gcc.target/arm/attr-crypto.c
+++ b/gcc/testsuite/gcc.target/arm/attr-crypto.c
@@ -1,6 +1,14 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_crypto_ok } */
-/* { dg-options "-O2 -mfloat-abi=softfp" } */
+/* Make sure we can force fpu=vfp before switching using the
+   pragma.  */
+/* { dg-require-effective-target arm_fp_ok } */
+/* { dg-require-effective-target arm_crypto_pragma_ok } */
+/* { dg-options "-O2 -march=armv8-a" } */
+/* { dg-add-options arm_fp } */
+
+/* Reset fpu to a value compatible with the next pragmas.  */
+#pragma GCC target ("fpu=vfp")
+#pragma GCC push_options
 
 #pragma GCC target ("fpu=crypto-neon-fp-armv8")
 
@@ -28,7 +36,7 @@ foo (void)
   return res[0];
 }
 
-#pragma GCC reset_options
+#pragma GCC pop_options
 
 /* Check that the FP version is correctly reset.  */
 
diff --git a/gcc/testsuite/gcc.target/arm/attr-neon-builtin-fail.c b/gcc/testsuite/gcc.target/arm/attr-neon-builtin-fail.c
index 6ac32fc..05dc579 100644
--- a/gcc/testsuite/gcc.target/arm/attr-neon-builtin-fail.c
+++ b/gcc/testsuite/gcc.target/arm/attr-neon-builtin-fail.c
@@ -1,7 +1,9 @@
 /* Check that calling a neon builtin from a function compiled with vfp fails.  */
 /* { dg-do compile } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-require-effective-target arm_neon_ok } */
-/* { dg-options "-O2 -mfloat-abi=softfp" } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_fp } */
 
 #include <arm_neon.h>
 
@@ -12,6 +14,5 @@ foo (uint8x16_t *p)
   *p = vmovq_n_u8 (3); /* { dg-message "called from here" } */
 }
 
-/* { dg-error "inlining failed in call to always_inline" "" { target *-*-* } 0 }
- */
+/* { dg-error "inlining failed in call to always_inline" "" { target *-*-* } 0 } */
 
diff --git a/gcc/testsuite/gcc.target/arm/attr-neon-fp16.c b/gcc/testsuite/gcc.target/arm/attr-neon-fp16.c
index 3cf8918..984992f 100644
--- a/gcc/testsuite/gcc.target/arm/attr-neon-fp16.c
+++ b/gcc/testsuite/gcc.target/arm/attr-neon-fp16.c
@@ -1,6 +1,7 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_neon_ok } */
-/* { dg-options "-mfp16-format=ieee -mfloat-abi=softfp" } */
+/* { dg-require-effective-target arm_fp_ok } */
+/* { dg-options "-mfp16-format=ieee" } */
+/* { dg-add-options arm_fp } */
 
 #include "arm_neon.h"
 
diff --git a/gcc/testsuite/gcc.target/arm/attr-neon2.c b/gcc/testsuite/gcc.target/arm/attr-neon2.c
index 819fad4..2966825 100644
--- a/gcc/testsuite/gcc.target/arm/attr-neon2.c
+++ b/gcc/testsuite/gcc.target/arm/attr-neon2.c
@@ -1,6 +1,12 @@
 /* { dg-do compile } */
 /* { dg-require-effective-target arm_neon_ok } */
-/* { dg-options "-O2 -mfloat-abi=softfp -mfpu=vfp" } */
+/* { dg-require-effective-target arm_fp_ok } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_fp } */
+
+/* Reset fpu to a value compatible with the next pragmas.  */
+#pragma GCC target ("fpu=vfp")
+#pragma GCC push_options
 
 #pragma GCC target ("fpu=neon")
 #include <arm_neon.h>
@@ -12,7 +18,7 @@ my (int8x8_t __a, int8x8_t __b)
   return __a + __b;
 }
 
-#pragma GCC reset_options
+#pragma GCC pop_options
 
 /* Check that command line option is restored.  */
 int8x8_t 
diff --git a/gcc/testsuite/gcc.target/arm/attr-neon3.c b/gcc/testsuite/gcc.target/arm/attr-neon3.c
index 30a1479..17e429a 100644
--- a/gcc/testsuite/gcc.target/arm/attr-neon3.c
+++ b/gcc/testsuite/gcc.target/arm/attr-neon3.c
@@ -1,6 +1,12 @@
 /* { dg-do compile } */
 /* { dg-require-effective-target arm_crypto_ok } */
-/* { dg-options "-O2 -mfloat-abi=softfp -mfpu=vfp" } */
+/* { dg-require-effective-target arm_fp_ok } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_fp } */
+
+/* Reset fpu to a value compatible with the next pragmas.  */
+#pragma GCC target ("fpu=vfp")
+#pragma GCC push_options
 
 #include <arm_neon.h>
 
diff --git a/gcc/testsuite/gcc.target/arm/cmp-2.c b/gcc/testsuite/gcc.target/arm/cmp-2.c
index ed6b609..70e4509 100644
--- a/gcc/testsuite/gcc.target/arm/cmp-2.c
+++ b/gcc/testsuite/gcc.target/arm/cmp-2.c
@@ -1,7 +1,8 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
-/* { dg-options "-O -mfpu=vfp -mfloat-abi=softfp" } */
+/* { dg-options "-O" } */
+/* { dg-add-options arm_fp } */
 /* { dg-final { scan-assembler-not "\tbl\t" } } */
 /* { dg-final { scan-assembler-not "__aeabi" } } */
 int x, y;
diff --git a/gcc/testsuite/gcc.target/arm/unsigned-float.c b/gcc/testsuite/gcc.target/arm/unsigned-float.c
index b9ed681..e1cda0c 100644
--- a/gcc/testsuite/gcc.target/arm/unsigned-float.c
+++ b/gcc/testsuite/gcc.target/arm/unsigned-float.c
@@ -1,8 +1,9 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
 /* { dg-options "-march=armv7-a -O1" } */
-/* { dg-additional-options "-mfloat-abi=softfp" { target { ! { arm_hf_eabi } } } } */
+/* { dg-add-options arm_fp } */
+
 
 #include <stdint.h>
 
diff --git a/gcc/testsuite/gcc.target/arm/vfp-1.c b/gcc/testsuite/gcc.target/arm/vfp-1.c
index 9aa5302..7add1b8 100644
--- a/gcc/testsuite/gcc.target/arm/vfp-1.c
+++ b/gcc/testsuite/gcc.target/arm/vfp-1.c
@@ -1,6 +1,7 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -mfpu=vfp -mfloat-abi=softfp -ffp-contract=off" } */
-/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
+/* { dg-options "-O2 -ffp-contract=off" } */
+/* { dg-add-options arm_fp } */
 /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
 
 extern float fabsf (float);
diff --git a/gcc/testsuite/gcc.target/arm/vfp-ldmdbd.c b/gcc/testsuite/gcc.target/arm/vfp-ldmdbd.c
index 7041579..3489a2a 100644
--- a/gcc/testsuite/gcc.target/arm/vfp-ldmdbd.c
+++ b/gcc/testsuite/gcc.target/arm/vfp-ldmdbd.c
@@ -1,7 +1,8 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
-/* { dg-options "-O2 -mfpu=vfp -mfloat-abi=softfp" } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_fp } */
 
 extern void bar (double);
 
diff --git a/gcc/testsuite/gcc.target/arm/vfp-ldmdbs.c b/gcc/testsuite/gcc.target/arm/vfp-ldmdbs.c
index 0187c01..8fda405 100644
--- a/gcc/testsuite/gcc.target/arm/vfp-ldmdbs.c
+++ b/gcc/testsuite/gcc.target/arm/vfp-ldmdbs.c
@@ -1,7 +1,8 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
-/* { dg-options "-O2 -mfpu=vfp -mfloat-abi=softfp" } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_fp } */
 
 extern void bar (float);
 
diff --git a/gcc/testsuite/gcc.target/arm/vfp-ldmiad.c b/gcc/testsuite/gcc.target/arm/vfp-ldmiad.c
index 9c22f1f..422e3ed 100644
--- a/gcc/testsuite/gcc.target/arm/vfp-ldmiad.c
+++ b/gcc/testsuite/gcc.target/arm/vfp-ldmiad.c
@@ -1,7 +1,8 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
-/* { dg-options "-O2 -mfpu=vfp -mfloat-abi=softfp" } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_fp } */
 
 extern void bar (double);
 
diff --git a/gcc/testsuite/gcc.target/arm/vfp-ldmias.c b/gcc/testsuite/gcc.target/arm/vfp-ldmias.c
index 92051fd..31d2ee1 100644
--- a/gcc/testsuite/gcc.target/arm/vfp-ldmias.c
+++ b/gcc/testsuite/gcc.target/arm/vfp-ldmias.c
@@ -1,7 +1,8 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
-/* { dg-options "-O2 -mfpu=vfp -mfloat-abi=softfp" } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_fp } */
 
 extern void bar (float);
 
diff --git a/gcc/testsuite/gcc.target/arm/vfp-stmdbd.c b/gcc/testsuite/gcc.target/arm/vfp-stmdbd.c
index 53383b5..686fe86 100644
--- a/gcc/testsuite/gcc.target/arm/vfp-stmdbd.c
+++ b/gcc/testsuite/gcc.target/arm/vfp-stmdbd.c
@@ -1,7 +1,8 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
-/* { dg-options "-O2 -mfpu=vfp -mfloat-abi=softfp" } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_fp } */
 
 void
 foo (double *p, double a, double b, int n)
diff --git a/gcc/testsuite/gcc.target/arm/vfp-stmdbs.c b/gcc/testsuite/gcc.target/arm/vfp-stmdbs.c
index 6570def..dbb30ec 100644
--- a/gcc/testsuite/gcc.target/arm/vfp-stmdbs.c
+++ b/gcc/testsuite/gcc.target/arm/vfp-stmdbs.c
@@ -1,7 +1,8 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
-/* { dg-options "-O2 -mfpu=vfp -mfloat-abi=softfp" } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_fp } */
 
 void
 foo (float *p, float a, float b, int n)
diff --git a/gcc/testsuite/gcc.target/arm/vfp-stmiad.c b/gcc/testsuite/gcc.target/arm/vfp-stmiad.c
index 28e9d73..665fa7a 100644
--- a/gcc/testsuite/gcc.target/arm/vfp-stmiad.c
+++ b/gcc/testsuite/gcc.target/arm/vfp-stmiad.c
@@ -1,7 +1,8 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
-/* { dg-options "-O2 -mfpu=vfp -mfloat-abi=softfp" } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_fp } */
 
 void
 foo (double *p, double a, double b, int n)
diff --git a/gcc/testsuite/gcc.target/arm/vfp-stmias.c b/gcc/testsuite/gcc.target/arm/vfp-stmias.c
index efa5fbe..90940e5 100644
--- a/gcc/testsuite/gcc.target/arm/vfp-stmias.c
+++ b/gcc/testsuite/gcc.target/arm/vfp-stmias.c
@@ -1,7 +1,8 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
-/* { dg-options "-O2 -mfpu=vfp -mfloat-abi=softfp" } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_fp } */
 
 void
 foo (float *p, float a, float b, int n)
diff --git a/gcc/testsuite/gcc.target/arm/vnmul-1.c b/gcc/testsuite/gcc.target/arm/vnmul-1.c
index af0bebe..fd00388 100644
--- a/gcc/testsuite/gcc.target/arm/vnmul-1.c
+++ b/gcc/testsuite/gcc.target/arm/vnmul-1.c
@@ -1,7 +1,8 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
-/* { dg-options "-O2 -fno-rounding-math -mfpu=vfp -mfloat-abi=hard" } */
+/* { dg-options "-O2 -fno-rounding-math" } */
+/* { dg-add-options arm_fp } */
 
 double
 foo_d (double a, double b)
diff --git a/gcc/testsuite/gcc.target/arm/vnmul-2.c b/gcc/testsuite/gcc.target/arm/vnmul-2.c
index 909b2a4..c299ec1 100644
--- a/gcc/testsuite/gcc.target/arm/vnmul-2.c
+++ b/gcc/testsuite/gcc.target/arm/vnmul-2.c
@@ -1,7 +1,8 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
-/* { dg-options "-O2 -frounding-math -mfpu=vfp -mfloat-abi=hard" } */
+/* { dg-options "-O2 -frounding-math" } */
+/* { dg-add-options arm_fp } */
 
 double
 foo_d (double a, double b)
diff --git a/gcc/testsuite/gcc.target/arm/vnmul-3.c b/gcc/testsuite/gcc.target/arm/vnmul-3.c
index df02882..44c1967 100644
--- a/gcc/testsuite/gcc.target/arm/vnmul-3.c
+++ b/gcc/testsuite/gcc.target/arm/vnmul-3.c
@@ -1,7 +1,8 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
-/* { dg-options "-O2 -fno-rounding-math -mfpu=vfp -mfloat-abi=hard" } */
+/* { dg-options "-O2 -fno-rounding-math" } */
+/* { dg-add-options arm_fp } */
 
 double
 foo_d (double a, double b)
diff --git a/gcc/testsuite/gcc.target/arm/vnmul-4.c b/gcc/testsuite/gcc.target/arm/vnmul-4.c
index 670ee40..dd9cab3 100644
--- a/gcc/testsuite/gcc.target/arm/vnmul-4.c
+++ b/gcc/testsuite/gcc.target/arm/vnmul-4.c
@@ -1,7 +1,8 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
-/* { dg-options "-O2 -frounding-math -mfpu=vfp -mfloat-abi=hard" } */
+/* { dg-options "-O2 -frounding-math" } */
+/* { dg-add-options arm_fp } */
 
 double
 foo_d (double a, double b)

Re: [testsuite][ARM target attributes] Fix effective_target tests

[Christian Bruel]

Hi Christophe,

On 12/17/2015 11:17 PM, Christophe Lyon wrote:
> Hi,
>
> Here is an updated version of this patch.
> I did test it with
> -mthumb/-march=armv8-a/-mfpu=crypto-neon-fp-armv8/-mfloat-abi=hard in
> addition to my usual set of options.
>
> Compared to the previous version:
> - I added some doc in sourcebuild.texi
> - I no longer modify arm_vfp_ok...
> - I replaced all uses of arm_vfp with the new arm_fp because I found
> that the existing tests do not actually need to pass -mfpu=vfp: this
> is implicitly set as the default when using -mfloat-abi={softfp|hard}
> - I chose not to remove arm_vfp_ok because we may need it in the
> future, if a test really needs vfp (as opposed to neon for instance)
> - in gcc.target/arm/attr-crypto.c I force the initial fpu to be vfp
> via pragma instead, so that the next pragma fpu
> fpu=crypto-neon-fp-armv8 is always compatible, regardless of the
> command-line options/default fpu
> - same for attr-neon2.c and attr-neon3.c
> - I updated cmp-2.c, unsigned-float.c, vfp-1.c, vfp-ldmdbd.c,
> vfp-ldmdbs.c, vfp-ldmiad.c, vfp-ldmias.c, vfp-stmdbd.c, vfp-stmdbs.c,
> vfp-stmiad.c, vfp-stmias.c, vnmul-[1234].c to use the new arm_fp
> effective target instead of arm_vfp. This is so that they don't need
> to use -mfpu=vfp and can use the new dg-add-options arm_fp
>
> The validation results show (in addition to what I originally reported):


> - attr-crypto.c and attr-neon3.c now ICE in some cases. This is PR68895.
> - depending on the GCC configuration (e.g. --with-fpu=neon)
> attr-neon3.c may fail. This is PR68896.

Those last tests are invalid, as they use simd builtin type declaration 
in nosimd context (that was my fault originally). I'm rewriting those 
and a few others to be compliant.
I'm also working on improving the reporting for invalid uses so you 
don't get this ICE anymore.



>
> OK?
>
> Christophe
>
> 2015-12-17  Christophe Lyon  <christophe.lyon@linaro.org>
>
>      * doc/sourcebuild.texi (arm_fp_ok): Document new entry.
>      (arm_fp): Likewise.
>      * lib/target-supports.exp
>      (check_effective_target_arm_fp_ok_nocache): New.
>      (check_effective_target_arm_fp_ok): New.
>      (add_options_for_arm_fp): New.
>      (check_effective_target_arm_crypto_ok_nocache): Require
>      target_arm_v8_neon_ok instead of arm32.
>      (check_effective_target_arm_crypto_pragma_ok_nocache): New.
>      (check_effective_target_arm_crypto_pragma_ok): New.
>      (add_options_for_arm_vfp): New.
>      * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok effective
>      target. Do not force -mfloat-abi=softfp, use arm_fp_ok effective
>      target instead. Force initial fpu to vfp.
>      * gcc.target/arm/attr-neon-builtin-fail.c: Do not force
>      -mfloat-abi=softfp, use arm_fp_ok effective target instead.
>      * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove arm_neon_ok
>      dependency.
>      * gcc.target/arm/attr-neon2.c: Do not force -mfloat-abi=softfp,
>      use arm_vfp effective target instead. Force initial fpu to vfp.
>      * gcc.target/arm/attr-neon3.c: Likewise.
>      * gcc.target/arm/cmp-2.c: Use arm_fp_ok effective target instead of
>      arm_vfp_ok.
>      * gcc.target/arm/unsigned-float.c: Likewise.
>      * gcc.target/arm/vfp-1.c: Likewise.
>      * gcc.target/arm/vfp-ldmdbd.c: Likewise.
>      * gcc.target/arm/vfp-ldmdbs.c: Likewise.
>      * gcc.target/arm/vfp-ldmiad.c: Likewise.
>      * gcc.target/arm/vfp-ldmias.c: Likewise.
>      * gcc.target/arm/vfp-stmdbd.c: Likewise.
>      * gcc.target/arm/vfp-stmdbs.c: Likewise.
>      * gcc.target/arm/vfp-stmiad.c: Likewise.
>      * gcc.target/arm/vfp-stmias.c: Likewise.
>      * gcc.target/arm/vnmul-1.c: Likewise.
>      * gcc.target/arm/vnmul-2.c: Likewise.
>      * gcc.target/arm/vnmul-3.c: Likewise.
>      * gcc.target/arm/vnmul-4.c: Likewise.
>
>
>
> On 10 December 2015 at 20:52, Christophe Lyon
> <christophe.lyon@linaro.org> wrote:
>> On 10 December 2015 at 14:14, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
>>>
>>> On 10/12/15 13:04, Christophe Lyon wrote:
>>>>
>>>> On 10 December 2015 at 13:30, Kyrill Tkachov <kyrylo.tkachov@arm.com>
>>>> wrote:
>>>>>
>>>>> Hi Christophe,
>>>>>
>>>>>
>>>>> On 08/12/15 11:18, Christophe Lyon wrote:
>>>>>>
>>>>>> On 8 December 2015 at 11:50, Kyrill Tkachov <kyrylo.tkachov@arm.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> Hi Christophe,
>>>>>>>
>>>>>>>
>>>>>>> On 27/11/15 13:00, Christophe Lyon wrote:
>>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> After the recent commits from Christian adding target attributes
>>>>>>>> support for ARM FPU settings,  I've noticed that some of the tests
>>>>>>>> were failing because of incorrect assumptions wrt to the default
>>>>>>>> cpu/fpu/float-abi of the compiler.
>>>>>>>>
>>>>>>>> This patch fixes the problems I've noticed in the following way:
>>>>>>>> - do not force -mfloat-abi=softfp in dg-options, to avoid conflicts
>>>>>>>> when gcc is configured --with-float=hard
>>>>>>>>
>>>>>>>> - change arm_vfp_ok such that it tries several -mfpu/-mfloat-abi
>>>>>>>> flags, checks that __ARM_FP is defined and __ARM_NEON_FP is not
>>>>>>>> defined
>>>>>>>>
>>>>>>>> - introduce arm_fp_ok, which is similar but does not enforce fpu
>>>>>>>> setting
>>>>>>>>
>>>>>>>> - add a new effective_target: arm_crypto_pragma_ok to check that
>>>>>>>> setting this fpu via a pragma is actually supported by the current
>>>>>>>> "multilib". This is different from checking the command-line option
>>>>>>>> because the pragma might conflict with the command-line options in
>>>>>>>> use.
>>>>>>>>
>>>>>>>> The updates in the testcases are as follows:
>>>>>>>> - attr-crypto.c, we have to make sure that the defaut fpu does not
>>>>>>>> conflict with the one forced by pragma. That's why I use the arm_vfp
>>>>>>>> options/effective_target. This is needed if gcc has been configured
>>>>>>>> --with-fpu=neon-fp16, as the pragma fpu=crypto-neon-fp-armv8 would
>>>>>>>> conflict.
>>>>>>>>
>>>>>>>> - attr-neon-builtin-fail.c: use arm_fp to force the appropriate
>>>>>>>> float-abi setting. Enforcing fpu is not needed here.
>>>>>>>>
>>>>>>>> - attr-neon-fp16.c: similar, I also removed arm_neon_ok since it was
>>>>>>>> not necessary to make the test pass in my testing. On second thought,
>>>>>>>> I'm wondering whether I should leave it and make the test unsupported
>>>>>>>> in more cases (such as when forcing -march=armv5t, although it does
>>>>>>>> pass with this patch)
>>>>>>>>
>>>>>>>> - attr-neon2.c: use arm_vfp to force the appropriate float-abi
>>>>>>>> setting. Enforcing mfpu=vfp is needed to avoid conflict with the
>>>>>>>> pragma target fpu=neon (for instance if the toolchain default is
>>>>>>>> neon-fp16)
>>>>>>>>
>>>>>>>> - attr-neon3.c: similar
>>>>>>>>
>>>>>>>> Tested on a variety of configurations, see:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/230929-target-attr/report-build-info.html
>>>>>>>>
>>>>>>>> Note that the regressions reported fall into 3 categories:
>>>>>>>> - when forcing march=armv5t: tests are now unsupported because I
>>>>>>>> modified arm_crypto_ok to require arm_v8_neon_ok instead of arm32.
>>>>>>>>
>>>>>>>> - the warning reported by attr-neon-builtin-fail.c moved from line 12
>>>>>>>> to 14 and is thus seen as a regression + one improvement
>>>>>>>>
>>>>>>>> - finally, attr-neon-fp16.c causes an ICE on armeb compilers, for
>>>>>>>> which I need to post a bugzilla.
>>>>>>>>
>>>>>>>>
>>>>>>>> TBH, I'm a bit concerned by the complexity of all these multilib-like
>>>>>>>> conditions. I'm confident that I'm still missing some combinations :-)
>>>>>>>>
>>>>>>>> And with new target attributes coming, new architectures etc... all
>>>>>>>> this logic is likely to become even more complex.
>>>>>>>>
>>>>>>>> That being said, OK for trunk?
>>>>>>>
>>>>>>>
>>>>>>> I'd like us to clean up and reorganise the gcc.target/arm testsuite
>>>>>>> at some point to make it more robust with respect to the tons of
>>>>>>> multilib
>>>>>>> options and configurations we can have for arm. It's becoming very
>>>>>>> frustrating
>>>>>>> to test feature-specific stuff :(
>>>>>>>
>>>>>>> This is ok in the meantime.
>>>>>>> Sorry for the delay.
>>>>>>>
>>>>>> Thanks for the approval, glad to see I'm not the only one willing to see
>>>>>> improvements in this area :)
>>>>>>
>>>>>> Committed as r231403.
>>>>>
>>>>>
>>>>> With this patch we're seeing legitimate PASSes go to NA.
>>>>> For example:
>>>>>
>>>>> gcc.target/arm/vfp-1.c
>>>>> gcc.target/arm/vfp-ldmdbs.c
>>>>> and other vfp tests in arm.exp.
>>>>> This is, for example, for the
>>>>> variant-mthumb/-march=armv8-a/-mfpu=crypto-neon-fp-armv8/-mfloat-abi=hard
>>>>>
>>>> Hmm I'm attempting to cover such a configuration in my matrix of
>>>> validations:
>>>>
>>>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/231403/report-build-info.html
>>>>
>>>> The difference is that I use similar flags at GCC configure time, while
>>>> you
>>>> override them when running the testsuite:
>>>> --target arm-none-linux-gnueabihf --with-mode=thum --with-cpu=cortex-a57
>>>> --with-fpu=crypto-neon-fp-armv8
>>>>
>>>> I this case, I do not see the regressions.
>>>
>>>
>>> My gcc is arm-none-eabi configured with
>>> --with-float=hard --with-cpu=cortex-a15 --with-fpu=neon-vfpv4
>>> --with-float=hard
>>>
>>>>> I suspect under your new predicates they'd need to be changed to check
>>>>> for
>>>>> arm_fp_ok rather than arm_vfp_ok.
>>>>
>>>> Probably, yes.
>>>>
>>>
>>> In the test log where it checks the effective target it fails due to:
>>> arm_vfp_ok27268.c:6:4: error: #error __ARM_NEON_FP defined
>>> it's compiling the check with
>>> -mthumb -march=armv8-a -mfpu=crypto-neon-fp-armv8 -mfloat-abi=hard -o
>>> arm_vfp_ok27268.o arm_vfp_ok27268.c
>>>
>>>>> We want to avoid removing any PASSes.
>>>>> Can you please test some more to make sure we don't remove any legitimate
>>>>> passes with your patch?
>>>>
>>>> Is that the only combination in which you saw less PASSes?
>>>
>>>
>>> That's the one that was brought to my attention, but I think the issue here
>>> is just the
>>> tests that check for arm_vfp_ok rather than the new arm_fp_ok and set
>>> -mfpu=vfp explicitly.
>>> They appear unsupported if testing with an explicit neon option in mfpu, I
>>> think.
>>>
>>>>> Also, Ramana reminded me offline that any new predicates in
>>>>> target-supports.exp
>>>>> need documenting in sourcebuild.txt.
>>>>>
>>>>> In light of that, can you please revert your patch and address the issues
>>>>> above
>>>>> so that we can be confident we don't lose existing legitimate test
>>>>> coverage?
>>>>
>>>> OK.
>>>>
>>>>> Sorry for not catching these sooner.
>>>>
>>>> No problem, there are so many combinations.
>>>> I'm not sure how to define a reasonable set.
>>>
>>>
>>> So, I think the action plan here is to audit the tests that need to be
>>> changed
>>> to arm_fp_ok and add the documentation for the new predicate checks.
>>>
>>
>> I came up with a new version where I now use only the new arm_fp_ok, so
>> it might be easier just to update arm_vfp_ok and not introduce arm_fp_ok.
>>
>> However, I'm still not happy with this version because it has problems with
>> a few tests that use target attributes such as attr-crypto.c, which starts with:
>> #pragma GCC target ("fpu=crypto-neon-fp-armv8")
>>
>> This pragma fails if the compiler+options have set an incompatible
>> fpu before processing the pragma.
>>
>> So for instance, if gcc has been configured --with-fpu=neon, compiling the test
>> with no fpu option fails (neon is not compatible with crypto-neon-fp-armv8
>> because the latter has fp16)
>>
>> Now, if we use effective_target arm_vfp_ok + dg_option to force -mfpu=vfp,
>> we end up compiling with -mfpu=vfp and the pragma passes.
>>
>> But if, in addition, we force the testflags to set -mfpu=crypto-neon-fp-armv8
>> as you do, the effective_target arm_vfp_ok test now fails because it is
>> compiled with -mfpu=vfp -mfpu=crypto-neon-fp-armv8, which defines
>> __ARM_NEON_FP
>>
>> In short, the problem is how to make sure that the fpu setting is always
>> compatible with the pragma, whatever the gcc configuration and multilib
>> options used.
>>
>> Thanks,
>>
>> Christophe.
>>
>>> Thanks,
>>> Kyrill
>>>
>>>
>>>> Christophe.
>>>>
>>>>> Kyrill
>>>>>
>>>>>
>>>>>> Christophe.
>>>>>>
>>>>>>> Thanks for handling this!
>>>>>>> Kyrill
>>>>>>>
>>>>>>>
>>>>>>>> Christophe
>>>>>>>>
>>>>>>>>
>>>>>>>> 2015-11-27  Christophe Lyon  <christophe.lyon@linaro.org>
>>>>>>>>
>>>>>>>>         * lib/target-supports.exp
>>>>>>>>         (check_effective_target_arm_vfp_ok_nocache): New.
>>>>>>>>         (check_effective_target_arm_vfp_ok): Call the new
>>>>>>>>         check_effective_target_arm_vfp_ok_nocache function.
>>>>>>>>         (check_effective_target_arm_fp_ok_nocache): New.
>>>>>>>>         (check_effective_target_arm_fp_ok): New.
>>>>>>>>         (add_options_for_arm_fp): New.
>>>>>>>>         (check_effective_target_arm_crypto_ok_nocache): Require
>>>>>>>>         target_arm_v8_neon_ok instead of arm32.
>>>>>>>>         (check_effective_target_arm_crypto_pragma_ok_nocache): New.
>>>>>>>>         (check_effective_target_arm_crypto_pragma_ok): New.
>>>>>>>>         (add_options_for_arm_vfp): New.
>>>>>>>>         * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok
>>>>>>>> effective
>>>>>>>>         target. Do not force -mfloat-abi=softfp, use arm_vfp effective
>>>>>>>>         target instead.
>>>>>>>>         * gcc.target/arm/attr-neon-builtin-fail.c: Do not force
>>>>>>>>         -mfloat-abi=softfp, use arm_fp effective target instead.
>>>>>>>>         * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove arm_neon_ok
>>>>>>>>         dependency.
>>>>>>>>         * gcc.target/arm/attr-neon2.c: Do not force -mfloat-abi=softfp,
>>>>>>>>         use arm_vfp effective target instead.
>>>>>>>>         * gcc.target/arm/attr-neon3.c: Likewise.
>>>>>>>
>>>>>>>
>>>

Re: [testsuite][ARM target attributes] Fix effective_target tests

[Kyrill Tkachov]

Hi Christophe,

On 17/12/15 22:17, Christophe Lyon wrote:
> Hi,
>
> Here is an updated version of this patch.
> I did test it with
> -mthumb/-march=armv8-a/-mfpu=crypto-neon-fp-armv8/-mfloat-abi=hard in
> addition to my usual set of options.
>
> Compared to the previous version:
> - I added some doc in sourcebuild.texi
> - I no longer modify arm_vfp_ok...
> - I replaced all uses of arm_vfp with the new arm_fp because I found
> that the existing tests do not actually need to pass -mfpu=vfp: this
> is implicitly set as the default when using -mfloat-abi={softfp|hard}
> - I chose not to remove arm_vfp_ok because we may need it in the
> future, if a test really needs vfp (as opposed to neon for instance)
> - in gcc.target/arm/attr-crypto.c I force the initial fpu to be vfp
> via pragma instead, so that the next pragma fpu
> fpu=crypto-neon-fp-armv8 is always compatible, regardless of the
> command-line options/default fpu
> - same for attr-neon2.c and attr-neon3.c
> - I updated cmp-2.c, unsigned-float.c, vfp-1.c, vfp-ldmdbd.c,
> vfp-ldmdbs.c, vfp-ldmiad.c, vfp-ldmias.c, vfp-stmdbd.c, vfp-stmdbs.c,
> vfp-stmiad.c, vfp-stmias.c, vnmul-[1234].c to use the new arm_fp
> effective target instead of arm_vfp. This is so that they don't need
> to use -mfpu=vfp and can use the new dg-add-options arm_fp
>
> The validation results show (in addition to what I originally reported):
> - attr-crypto.c and attr-neon3.c now ICE in some cases. This is PR68895.
> - depending on the GCC configuration (e.g. --with-fpu=neon)
> attr-neon3.c may fail. This is PR68896.
>
> OK?

Thanks for following up on this.
I think you also need to document the new arm_crypto_pragma_ok.

Kyrill

> Christophe
>
> 2015-12-17  Christophe Lyon  <christophe.lyon@linaro.org>
>
>      * doc/sourcebuild.texi (arm_fp_ok): Document new entry.
>      (arm_fp): Likewise.
>      * lib/target-supports.exp
>      (check_effective_target_arm_fp_ok_nocache): New.
>      (check_effective_target_arm_fp_ok): New.
>      (add_options_for_arm_fp): New.
>      (check_effective_target_arm_crypto_ok_nocache): Require
>      target_arm_v8_neon_ok instead of arm32.
>      (check_effective_target_arm_crypto_pragma_ok_nocache): New.
>      (check_effective_target_arm_crypto_pragma_ok): New.
>      (add_options_for_arm_vfp): New.
>      * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok effective
>      target. Do not force -mfloat-abi=softfp, use arm_fp_ok effective
>      target instead. Force initial fpu to vfp.
>      * gcc.target/arm/attr-neon-builtin-fail.c: Do not force
>      -mfloat-abi=softfp, use arm_fp_ok effective target instead.
>      * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove arm_neon_ok
>      dependency.
>      * gcc.target/arm/attr-neon2.c: Do not force -mfloat-abi=softfp,
>      use arm_vfp effective target instead. Force initial fpu to vfp.
>      * gcc.target/arm/attr-neon3.c: Likewise.
>      * gcc.target/arm/cmp-2.c: Use arm_fp_ok effective target instead of
>      arm_vfp_ok.
>      * gcc.target/arm/unsigned-float.c: Likewise.
>      * gcc.target/arm/vfp-1.c: Likewise.
>      * gcc.target/arm/vfp-ldmdbd.c: Likewise.
>      * gcc.target/arm/vfp-ldmdbs.c: Likewise.
>      * gcc.target/arm/vfp-ldmiad.c: Likewise.
>      * gcc.target/arm/vfp-ldmias.c: Likewise.
>      * gcc.target/arm/vfp-stmdbd.c: Likewise.
>      * gcc.target/arm/vfp-stmdbs.c: Likewise.
>      * gcc.target/arm/vfp-stmiad.c: Likewise.
>      * gcc.target/arm/vfp-stmias.c: Likewise.
>      * gcc.target/arm/vnmul-1.c: Likewise.
>      * gcc.target/arm/vnmul-2.c: Likewise.
>      * gcc.target/arm/vnmul-3.c: Likewise.
>      * gcc.target/arm/vnmul-4.c: Likewise.
>
>
>
> On 10 December 2015 at 20:52, Christophe Lyon
> <christophe.lyon@linaro.org> wrote:
>> On 10 December 2015 at 14:14, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
>>> On 10/12/15 13:04, Christophe Lyon wrote:
>>>> On 10 December 2015 at 13:30, Kyrill Tkachov <kyrylo.tkachov@arm.com>
>>>> wrote:
>>>>> Hi Christophe,
>>>>>
>>>>>
>>>>> On 08/12/15 11:18, Christophe Lyon wrote:
>>>>>> On 8 December 2015 at 11:50, Kyrill Tkachov <kyrylo.tkachov@arm.com>
>>>>>> wrote:
>>>>>>> Hi Christophe,
>>>>>>>
>>>>>>>
>>>>>>> On 27/11/15 13:00, Christophe Lyon wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> After the recent commits from Christian adding target attributes
>>>>>>>> support for ARM FPU settings,  I've noticed that some of the tests
>>>>>>>> were failing because of incorrect assumptions wrt to the default
>>>>>>>> cpu/fpu/float-abi of the compiler.
>>>>>>>>
>>>>>>>> This patch fixes the problems I've noticed in the following way:
>>>>>>>> - do not force -mfloat-abi=softfp in dg-options, to avoid conflicts
>>>>>>>> when gcc is configured --with-float=hard
>>>>>>>>
>>>>>>>> - change arm_vfp_ok such that it tries several -mfpu/-mfloat-abi
>>>>>>>> flags, checks that __ARM_FP is defined and __ARM_NEON_FP is not
>>>>>>>> defined
>>>>>>>>
>>>>>>>> - introduce arm_fp_ok, which is similar but does not enforce fpu
>>>>>>>> setting
>>>>>>>>
>>>>>>>> - add a new effective_target: arm_crypto_pragma_ok to check that
>>>>>>>> setting this fpu via a pragma is actually supported by the current
>>>>>>>> "multilib". This is different from checking the command-line option
>>>>>>>> because the pragma might conflict with the command-line options in
>>>>>>>> use.
>>>>>>>>
>>>>>>>> The updates in the testcases are as follows:
>>>>>>>> - attr-crypto.c, we have to make sure that the defaut fpu does not
>>>>>>>> conflict with the one forced by pragma. That's why I use the arm_vfp
>>>>>>>> options/effective_target. This is needed if gcc has been configured
>>>>>>>> --with-fpu=neon-fp16, as the pragma fpu=crypto-neon-fp-armv8 would
>>>>>>>> conflict.
>>>>>>>>
>>>>>>>> - attr-neon-builtin-fail.c: use arm_fp to force the appropriate
>>>>>>>> float-abi setting. Enforcing fpu is not needed here.
>>>>>>>>
>>>>>>>> - attr-neon-fp16.c: similar, I also removed arm_neon_ok since it was
>>>>>>>> not necessary to make the test pass in my testing. On second thought,
>>>>>>>> I'm wondering whether I should leave it and make the test unsupported
>>>>>>>> in more cases (such as when forcing -march=armv5t, although it does
>>>>>>>> pass with this patch)
>>>>>>>>
>>>>>>>> - attr-neon2.c: use arm_vfp to force the appropriate float-abi
>>>>>>>> setting. Enforcing mfpu=vfp is needed to avoid conflict with the
>>>>>>>> pragma target fpu=neon (for instance if the toolchain default is
>>>>>>>> neon-fp16)
>>>>>>>>
>>>>>>>> - attr-neon3.c: similar
>>>>>>>>
>>>>>>>> Tested on a variety of configurations, see:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/230929-target-attr/report-build-info.html
>>>>>>>>
>>>>>>>> Note that the regressions reported fall into 3 categories:
>>>>>>>> - when forcing march=armv5t: tests are now unsupported because I
>>>>>>>> modified arm_crypto_ok to require arm_v8_neon_ok instead of arm32.
>>>>>>>>
>>>>>>>> - the warning reported by attr-neon-builtin-fail.c moved from line 12
>>>>>>>> to 14 and is thus seen as a regression + one improvement
>>>>>>>>
>>>>>>>> - finally, attr-neon-fp16.c causes an ICE on armeb compilers, for
>>>>>>>> which I need to post a bugzilla.
>>>>>>>>
>>>>>>>>
>>>>>>>> TBH, I'm a bit concerned by the complexity of all these multilib-like
>>>>>>>> conditions. I'm confident that I'm still missing some combinations :-)
>>>>>>>>
>>>>>>>> And with new target attributes coming, new architectures etc... all
>>>>>>>> this logic is likely to become even more complex.
>>>>>>>>
>>>>>>>> That being said, OK for trunk?
>>>>>>>
>>>>>>> I'd like us to clean up and reorganise the gcc.target/arm testsuite
>>>>>>> at some point to make it more robust with respect to the tons of
>>>>>>> multilib
>>>>>>> options and configurations we can have for arm. It's becoming very
>>>>>>> frustrating
>>>>>>> to test feature-specific stuff :(
>>>>>>>
>>>>>>> This is ok in the meantime.
>>>>>>> Sorry for the delay.
>>>>>>>
>>>>>> Thanks for the approval, glad to see I'm not the only one willing to see
>>>>>> improvements in this area :)
>>>>>>
>>>>>> Committed as r231403.
>>>>>
>>>>> With this patch we're seeing legitimate PASSes go to NA.
>>>>> For example:
>>>>>
>>>>> gcc.target/arm/vfp-1.c
>>>>> gcc.target/arm/vfp-ldmdbs.c
>>>>> and other vfp tests in arm.exp.
>>>>> This is, for example, for the
>>>>> variant-mthumb/-march=armv8-a/-mfpu=crypto-neon-fp-armv8/-mfloat-abi=hard
>>>>>
>>>> Hmm I'm attempting to cover such a configuration in my matrix of
>>>> validations:
>>>>
>>>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/231403/report-build-info.html
>>>>
>>>> The difference is that I use similar flags at GCC configure time, while
>>>> you
>>>> override them when running the testsuite:
>>>> --target arm-none-linux-gnueabihf --with-mode=thum --with-cpu=cortex-a57
>>>> --with-fpu=crypto-neon-fp-armv8
>>>>
>>>> I this case, I do not see the regressions.
>>>
>>> My gcc is arm-none-eabi configured with
>>> --with-float=hard --with-cpu=cortex-a15 --with-fpu=neon-vfpv4
>>> --with-float=hard
>>>
>>>>> I suspect under your new predicates they'd need to be changed to check
>>>>> for
>>>>> arm_fp_ok rather than arm_vfp_ok.
>>>> Probably, yes.
>>>>
>>> In the test log where it checks the effective target it fails due to:
>>> arm_vfp_ok27268.c:6:4: error: #error __ARM_NEON_FP defined
>>> it's compiling the check with
>>> -mthumb -march=armv8-a -mfpu=crypto-neon-fp-armv8 -mfloat-abi=hard -o
>>> arm_vfp_ok27268.o arm_vfp_ok27268.c
>>>
>>>>> We want to avoid removing any PASSes.
>>>>> Can you please test some more to make sure we don't remove any legitimate
>>>>> passes with your patch?
>>>> Is that the only combination in which you saw less PASSes?
>>>
>>> That's the one that was brought to my attention, but I think the issue here
>>> is just the
>>> tests that check for arm_vfp_ok rather than the new arm_fp_ok and set
>>> -mfpu=vfp explicitly.
>>> They appear unsupported if testing with an explicit neon option in mfpu, I
>>> think.
>>>
>>>>> Also, Ramana reminded me offline that any new predicates in
>>>>> target-supports.exp
>>>>> need documenting in sourcebuild.txt.
>>>>>
>>>>> In light of that, can you please revert your patch and address the issues
>>>>> above
>>>>> so that we can be confident we don't lose existing legitimate test
>>>>> coverage?
>>>> OK.
>>>>
>>>>> Sorry for not catching these sooner.
>>>> No problem, there are so many combinations.
>>>> I'm not sure how to define a reasonable set.
>>>
>>> So, I think the action plan here is to audit the tests that need to be
>>> changed
>>> to arm_fp_ok and add the documentation for the new predicate checks.
>>>
>> I came up with a new version where I now use only the new arm_fp_ok, so
>> it might be easier just to update arm_vfp_ok and not introduce arm_fp_ok.
>>
>> However, I'm still not happy with this version because it has problems with
>> a few tests that use target attributes such as attr-crypto.c, which starts with:
>> #pragma GCC target ("fpu=crypto-neon-fp-armv8")
>>
>> This pragma fails if the compiler+options have set an incompatible
>> fpu before processing the pragma.
>>
>> So for instance, if gcc has been configured --with-fpu=neon, compiling the test
>> with no fpu option fails (neon is not compatible with crypto-neon-fp-armv8
>> because the latter has fp16)
>>
>> Now, if we use effective_target arm_vfp_ok + dg_option to force -mfpu=vfp,
>> we end up compiling with -mfpu=vfp and the pragma passes.
>>
>> But if, in addition, we force the testflags to set -mfpu=crypto-neon-fp-armv8
>> as you do, the effective_target arm_vfp_ok test now fails because it is
>> compiled with -mfpu=vfp -mfpu=crypto-neon-fp-armv8, which defines
>> __ARM_NEON_FP
>>
>> In short, the problem is how to make sure that the fpu setting is always
>> compatible with the pragma, whatever the gcc configuration and multilib
>> options used.
>>
>> Thanks,
>>
>> Christophe.
>>
>>> Thanks,
>>> Kyrill
>>>
>>>
>>>> Christophe.
>>>>
>>>>> Kyrill
>>>>>
>>>>>
>>>>>> Christophe.
>>>>>>
>>>>>>> Thanks for handling this!
>>>>>>> Kyrill
>>>>>>>
>>>>>>>
>>>>>>>> Christophe
>>>>>>>>
>>>>>>>>
>>>>>>>> 2015-11-27  Christophe Lyon  <christophe.lyon@linaro.org>
>>>>>>>>
>>>>>>>>         * lib/target-supports.exp
>>>>>>>>         (check_effective_target_arm_vfp_ok_nocache): New.
>>>>>>>>         (check_effective_target_arm_vfp_ok): Call the new
>>>>>>>>         check_effective_target_arm_vfp_ok_nocache function.
>>>>>>>>         (check_effective_target_arm_fp_ok_nocache): New.
>>>>>>>>         (check_effective_target_arm_fp_ok): New.
>>>>>>>>         (add_options_for_arm_fp): New.
>>>>>>>>         (check_effective_target_arm_crypto_ok_nocache): Require
>>>>>>>>         target_arm_v8_neon_ok instead of arm32.
>>>>>>>>         (check_effective_target_arm_crypto_pragma_ok_nocache): New.
>>>>>>>>         (check_effective_target_arm_crypto_pragma_ok): New.
>>>>>>>>         (add_options_for_arm_vfp): New.
>>>>>>>>         * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok
>>>>>>>> effective
>>>>>>>>         target. Do not force -mfloat-abi=softfp, use arm_vfp effective
>>>>>>>>         target instead.
>>>>>>>>         * gcc.target/arm/attr-neon-builtin-fail.c: Do not force
>>>>>>>>         -mfloat-abi=softfp, use arm_fp effective target instead.
>>>>>>>>         * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove arm_neon_ok
>>>>>>>>         dependency.
>>>>>>>>         * gcc.target/arm/attr-neon2.c: Do not force -mfloat-abi=softfp,
>>>>>>>>         use arm_vfp effective target instead.
>>>>>>>>         * gcc.target/arm/attr-neon3.c: Likewise.
>>>>>>>

Re: [testsuite][ARM target attributes] Fix effective_target tests

[Christophe Lyon]

On 18 December 2015 at 15:16, Kyrill Tkachov
<kyrylo.tkachov@foss.arm.com> wrote:
> Hi Christophe,
>
>
> On 17/12/15 22:17, Christophe Lyon wrote:
>>
>> Hi,
>>
>> Here is an updated version of this patch.
>> I did test it with
>> -mthumb/-march=armv8-a/-mfpu=crypto-neon-fp-armv8/-mfloat-abi=hard in
>> addition to my usual set of options.
>>
>> Compared to the previous version:
>> - I added some doc in sourcebuild.texi
>> - I no longer modify arm_vfp_ok...
>> - I replaced all uses of arm_vfp with the new arm_fp because I found
>> that the existing tests do not actually need to pass -mfpu=vfp: this
>> is implicitly set as the default when using -mfloat-abi={softfp|hard}
>> - I chose not to remove arm_vfp_ok because we may need it in the
>> future, if a test really needs vfp (as opposed to neon for instance)
>> - in gcc.target/arm/attr-crypto.c I force the initial fpu to be vfp
>> via pragma instead, so that the next pragma fpu
>> fpu=crypto-neon-fp-armv8 is always compatible, regardless of the
>> command-line options/default fpu
>> - same for attr-neon2.c and attr-neon3.c
>> - I updated cmp-2.c, unsigned-float.c, vfp-1.c, vfp-ldmdbd.c,
>> vfp-ldmdbs.c, vfp-ldmiad.c, vfp-ldmias.c, vfp-stmdbd.c, vfp-stmdbs.c,
>> vfp-stmiad.c, vfp-stmias.c, vnmul-[1234].c to use the new arm_fp
>> effective target instead of arm_vfp. This is so that they don't need
>> to use -mfpu=vfp and can use the new dg-add-options arm_fp
>>
>> The validation results show (in addition to what I originally reported):
>> - attr-crypto.c and attr-neon3.c now ICE in some cases. This is PR68895.
>> - depending on the GCC configuration (e.g. --with-fpu=neon)
>> attr-neon3.c may fail. This is PR68896.
>>
>> OK?
>
>
> Thanks for following up on this.
> I think you also need to document the new arm_crypto_pragma_ok.
>
Indeed, I forgot it.

Here is a new version of the patch with a few words added to document
this function.
I did not modify the testcase after Christian's comments and
PR68934: my understanding is that the testscase are valid after
all and Christian is working on fixing the ICE.

2016-01-04  Christophe Lyon  <christophe.lyon@linaro.org>

    * doc/sourcebuild.texi (arm_crypto_pragma_ok): Document new entry.
    (arm_fp_ok): Likewise.
    (arm_fp): Likewise.
    (arm_crypto): Likewise.
    * lib/target-supports.exp
    (check_effective_target_arm_fp_ok_nocache): New.
    (check_effective_target_arm_fp_ok): New.
    (add_options_for_arm_fp): New.
    (check_effective_target_arm_crypto_ok_nocache): Require
    target_arm_v8_neon_ok instead of arm32.
    (check_effective_target_arm_crypto_pragma_ok_nocache): New.
    (check_effective_target_arm_crypto_pragma_ok): New.
    (add_options_for_arm_vfp): New.
    * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok effective
    target. Do not force -mfloat-abi=softfp, use arm_fp_ok effective
    target instead. Force initial fpu to vfp.
    * gcc.target/arm/attr-neon-builtin-fail.c: Do not force
    -mfloat-abi=softfp, use arm_fp_ok effective target instead.
    * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove arm_neon_ok
    dependency.
    * gcc.target/arm/attr-neon2.c: Do not force -mfloat-abi=softfp,
    use arm_vfp effective target instead. Force initial fpu to vfp.
    * gcc.target/arm/attr-neon3.c: Likewise.
    * gcc.target/arm/cmp-2.c: Use arm_fp_ok effective target instead of
    arm_vfp_ok.
    * gcc.target/arm/unsigned-float.c: Likewise.
    * gcc.target/arm/vfp-1.c: Likewise.
    * gcc.target/arm/vfp-ldmdbd.c: Likewise.
    * gcc.target/arm/vfp-ldmdbs.c: Likewise.
    * gcc.target/arm/vfp-ldmiad.c: Likewise.
    * gcc.target/arm/vfp-ldmias.c: Likewise.
    * gcc.target/arm/vfp-stmdbd.c: Likewise.
    * gcc.target/arm/vfp-stmdbs.c: Likewise.
    * gcc.target/arm/vfp-stmiad.c: Likewise.
    * gcc.target/arm/vfp-stmias.c: Likewise.
    * gcc.target/arm/vnmul-1.c: Likewise.
    * gcc.target/arm/vnmul-2.c: Likewise.
    * gcc.target/arm/vnmul-3.c: Likewise.
    * gcc.target/arm/vnmul-4.c: Likewise.

OK?

Christophe.


> Kyrill
>
>
>> Christophe
>>
>> 2015-12-17  Christophe Lyon  <christophe.lyon@linaro.org>
>>
>>      * doc/sourcebuild.texi (arm_fp_ok): Document new entry.
>>      (arm_fp): Likewise.
>>      * lib/target-supports.exp
>>      (check_effective_target_arm_fp_ok_nocache): New.
>>      (check_effective_target_arm_fp_ok): New.
>>      (add_options_for_arm_fp): New.
>>      (check_effective_target_arm_crypto_ok_nocache): Require
>>      target_arm_v8_neon_ok instead of arm32.
>>      (check_effective_target_arm_crypto_pragma_ok_nocache): New.
>>      (check_effective_target_arm_crypto_pragma_ok): New.
>>      (add_options_for_arm_vfp): New.
>>      * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok effective
>>      target. Do not force -mfloat-abi=softfp, use arm_fp_ok effective
>>      target instead. Force initial fpu to vfp.
>>      * gcc.target/arm/attr-neon-builtin-fail.c: Do not force
>>      -mfloat-abi=softfp, use arm_fp_ok effective target instead.
>>      * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove arm_neon_ok
>>      dependency.
>>      * gcc.target/arm/attr-neon2.c: Do not force -mfloat-abi=softfp,
>>      use arm_vfp effective target instead. Force initial fpu to vfp.
>>      * gcc.target/arm/attr-neon3.c: Likewise.
>>      * gcc.target/arm/cmp-2.c: Use arm_fp_ok effective target instead of
>>      arm_vfp_ok.
>>      * gcc.target/arm/unsigned-float.c: Likewise.
>>      * gcc.target/arm/vfp-1.c: Likewise.
>>      * gcc.target/arm/vfp-ldmdbd.c: Likewise.
>>      * gcc.target/arm/vfp-ldmdbs.c: Likewise.
>>      * gcc.target/arm/vfp-ldmiad.c: Likewise.
>>      * gcc.target/arm/vfp-ldmias.c: Likewise.
>>      * gcc.target/arm/vfp-stmdbd.c: Likewise.
>>      * gcc.target/arm/vfp-stmdbs.c: Likewise.
>>      * gcc.target/arm/vfp-stmiad.c: Likewise.
>>      * gcc.target/arm/vfp-stmias.c: Likewise.
>>      * gcc.target/arm/vnmul-1.c: Likewise.
>>      * gcc.target/arm/vnmul-2.c: Likewise.
>>      * gcc.target/arm/vnmul-3.c: Likewise.
>>      * gcc.target/arm/vnmul-4.c: Likewise.
>>
>>
>>
>> On 10 December 2015 at 20:52, Christophe Lyon
>> <christophe.lyon@linaro.org> wrote:
>>>
>>> On 10 December 2015 at 14:14, Kyrill Tkachov <kyrylo.tkachov@arm.com>
>>> wrote:
>>>>
>>>> On 10/12/15 13:04, Christophe Lyon wrote:
>>>>>
>>>>> On 10 December 2015 at 13:30, Kyrill Tkachov <kyrylo.tkachov@arm.com>
>>>>> wrote:
>>>>>>
>>>>>> Hi Christophe,
>>>>>>
>>>>>>
>>>>>> On 08/12/15 11:18, Christophe Lyon wrote:
>>>>>>>
>>>>>>> On 8 December 2015 at 11:50, Kyrill Tkachov <kyrylo.tkachov@arm.com>
>>>>>>> wrote:
>>>>>>>>
>>>>>>>> Hi Christophe,
>>>>>>>>
>>>>>>>>
>>>>>>>> On 27/11/15 13:00, Christophe Lyon wrote:
>>>>>>>>>
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> After the recent commits from Christian adding target attributes
>>>>>>>>> support for ARM FPU settings,  I've noticed that some of the tests
>>>>>>>>> were failing because of incorrect assumptions wrt to the default
>>>>>>>>> cpu/fpu/float-abi of the compiler.
>>>>>>>>>
>>>>>>>>> This patch fixes the problems I've noticed in the following way:
>>>>>>>>> - do not force -mfloat-abi=softfp in dg-options, to avoid conflicts
>>>>>>>>> when gcc is configured --with-float=hard
>>>>>>>>>
>>>>>>>>> - change arm_vfp_ok such that it tries several -mfpu/-mfloat-abi
>>>>>>>>> flags, checks that __ARM_FP is defined and __ARM_NEON_FP is not
>>>>>>>>> defined
>>>>>>>>>
>>>>>>>>> - introduce arm_fp_ok, which is similar but does not enforce fpu
>>>>>>>>> setting
>>>>>>>>>
>>>>>>>>> - add a new effective_target: arm_crypto_pragma_ok to check that
>>>>>>>>> setting this fpu via a pragma is actually supported by the current
>>>>>>>>> "multilib". This is different from checking the command-line option
>>>>>>>>> because the pragma might conflict with the command-line options in
>>>>>>>>> use.
>>>>>>>>>
>>>>>>>>> The updates in the testcases are as follows:
>>>>>>>>> - attr-crypto.c, we have to make sure that the defaut fpu does not
>>>>>>>>> conflict with the one forced by pragma. That's why I use the
>>>>>>>>> arm_vfp
>>>>>>>>> options/effective_target. This is needed if gcc has been configured
>>>>>>>>> --with-fpu=neon-fp16, as the pragma fpu=crypto-neon-fp-armv8 would
>>>>>>>>> conflict.
>>>>>>>>>
>>>>>>>>> - attr-neon-builtin-fail.c: use arm_fp to force the appropriate
>>>>>>>>> float-abi setting. Enforcing fpu is not needed here.
>>>>>>>>>
>>>>>>>>> - attr-neon-fp16.c: similar, I also removed arm_neon_ok since it
>>>>>>>>> was
>>>>>>>>> not necessary to make the test pass in my testing. On second
>>>>>>>>> thought,
>>>>>>>>> I'm wondering whether I should leave it and make the test
>>>>>>>>> unsupported
>>>>>>>>> in more cases (such as when forcing -march=armv5t, although it does
>>>>>>>>> pass with this patch)
>>>>>>>>>
>>>>>>>>> - attr-neon2.c: use arm_vfp to force the appropriate float-abi
>>>>>>>>> setting. Enforcing mfpu=vfp is needed to avoid conflict with the
>>>>>>>>> pragma target fpu=neon (for instance if the toolchain default is
>>>>>>>>> neon-fp16)
>>>>>>>>>
>>>>>>>>> - attr-neon3.c: similar
>>>>>>>>>
>>>>>>>>> Tested on a variety of configurations, see:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/230929-target-attr/report-build-info.html
>>>>>>>>>
>>>>>>>>> Note that the regressions reported fall into 3 categories:
>>>>>>>>> - when forcing march=armv5t: tests are now unsupported because I
>>>>>>>>> modified arm_crypto_ok to require arm_v8_neon_ok instead of arm32.
>>>>>>>>>
>>>>>>>>> - the warning reported by attr-neon-builtin-fail.c moved from line
>>>>>>>>> 12
>>>>>>>>> to 14 and is thus seen as a regression + one improvement
>>>>>>>>>
>>>>>>>>> - finally, attr-neon-fp16.c causes an ICE on armeb compilers, for
>>>>>>>>> which I need to post a bugzilla.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> TBH, I'm a bit concerned by the complexity of all these
>>>>>>>>> multilib-like
>>>>>>>>> conditions. I'm confident that I'm still missing some combinations
>>>>>>>>> :-)
>>>>>>>>>
>>>>>>>>> And with new target attributes coming, new architectures etc... all
>>>>>>>>> this logic is likely to become even more complex.
>>>>>>>>>
>>>>>>>>> That being said, OK for trunk?
>>>>>>>>
>>>>>>>>
>>>>>>>> I'd like us to clean up and reorganise the gcc.target/arm testsuite
>>>>>>>> at some point to make it more robust with respect to the tons of
>>>>>>>> multilib
>>>>>>>> options and configurations we can have for arm. It's becoming very
>>>>>>>> frustrating
>>>>>>>> to test feature-specific stuff :(
>>>>>>>>
>>>>>>>> This is ok in the meantime.
>>>>>>>> Sorry for the delay.
>>>>>>>>
>>>>>>> Thanks for the approval, glad to see I'm not the only one willing to
>>>>>>> see
>>>>>>> improvements in this area :)
>>>>>>>
>>>>>>> Committed as r231403.
>>>>>>
>>>>>>
>>>>>> With this patch we're seeing legitimate PASSes go to NA.
>>>>>> For example:
>>>>>>
>>>>>> gcc.target/arm/vfp-1.c
>>>>>> gcc.target/arm/vfp-ldmdbs.c
>>>>>> and other vfp tests in arm.exp.
>>>>>> This is, for example, for the
>>>>>>
>>>>>> variant-mthumb/-march=armv8-a/-mfpu=crypto-neon-fp-armv8/-mfloat-abi=hard
>>>>>>
>>>>> Hmm I'm attempting to cover such a configuration in my matrix of
>>>>> validations:
>>>>>
>>>>>
>>>>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/231403/report-build-info.html
>>>>>
>>>>> The difference is that I use similar flags at GCC configure time, while
>>>>> you
>>>>> override them when running the testsuite:
>>>>> --target arm-none-linux-gnueabihf --with-mode=thum
>>>>> --with-cpu=cortex-a57
>>>>> --with-fpu=crypto-neon-fp-armv8
>>>>>
>>>>> I this case, I do not see the regressions.
>>>>
>>>>
>>>> My gcc is arm-none-eabi configured with
>>>> --with-float=hard --with-cpu=cortex-a15 --with-fpu=neon-vfpv4
>>>> --with-float=hard
>>>>
>>>>>> I suspect under your new predicates they'd need to be changed to check
>>>>>> for
>>>>>> arm_fp_ok rather than arm_vfp_ok.
>>>>>
>>>>> Probably, yes.
>>>>>
>>>> In the test log where it checks the effective target it fails due to:
>>>> arm_vfp_ok27268.c:6:4: error: #error __ARM_NEON_FP defined
>>>> it's compiling the check with
>>>> -mthumb -march=armv8-a -mfpu=crypto-neon-fp-armv8 -mfloat-abi=hard -o
>>>> arm_vfp_ok27268.o arm_vfp_ok27268.c
>>>>
>>>>>> We want to avoid removing any PASSes.
>>>>>> Can you please test some more to make sure we don't remove any
>>>>>> legitimate
>>>>>> passes with your patch?
>>>>>
>>>>> Is that the only combination in which you saw less PASSes?
>>>>
>>>>
>>>> That's the one that was brought to my attention, but I think the issue
>>>> here
>>>> is just the
>>>> tests that check for arm_vfp_ok rather than the new arm_fp_ok and set
>>>> -mfpu=vfp explicitly.
>>>> They appear unsupported if testing with an explicit neon option in mfpu,
>>>> I
>>>> think.
>>>>
>>>>>> Also, Ramana reminded me offline that any new predicates in
>>>>>> target-supports.exp
>>>>>> need documenting in sourcebuild.txt.
>>>>>>
>>>>>> In light of that, can you please revert your patch and address the
>>>>>> issues
>>>>>> above
>>>>>> so that we can be confident we don't lose existing legitimate test
>>>>>> coverage?
>>>>>
>>>>> OK.
>>>>>
>>>>>> Sorry for not catching these sooner.
>>>>>
>>>>> No problem, there are so many combinations.
>>>>> I'm not sure how to define a reasonable set.
>>>>
>>>>
>>>> So, I think the action plan here is to audit the tests that need to be
>>>> changed
>>>> to arm_fp_ok and add the documentation for the new predicate checks.
>>>>
>>> I came up with a new version where I now use only the new arm_fp_ok, so
>>> it might be easier just to update arm_vfp_ok and not introduce arm_fp_ok.
>>>
>>> However, I'm still not happy with this version because it has problems
>>> with
>>> a few tests that use target attributes such as attr-crypto.c, which
>>> starts with:
>>> #pragma GCC target ("fpu=crypto-neon-fp-armv8")
>>>
>>> This pragma fails if the compiler+options have set an incompatible
>>> fpu before processing the pragma.
>>>
>>> So for instance, if gcc has been configured --with-fpu=neon, compiling
>>> the test
>>> with no fpu option fails (neon is not compatible with
>>> crypto-neon-fp-armv8
>>> because the latter has fp16)
>>>
>>> Now, if we use effective_target arm_vfp_ok + dg_option to force
>>> -mfpu=vfp,
>>> we end up compiling with -mfpu=vfp and the pragma passes.
>>>
>>> But if, in addition, we force the testflags to set
>>> -mfpu=crypto-neon-fp-armv8
>>> as you do, the effective_target arm_vfp_ok test now fails because it is
>>> compiled with -mfpu=vfp -mfpu=crypto-neon-fp-armv8, which defines
>>> __ARM_NEON_FP
>>>
>>> In short, the problem is how to make sure that the fpu setting is always
>>> compatible with the pragma, whatever the gcc configuration and multilib
>>> options used.
>>>
>>> Thanks,
>>>
>>> Christophe.
>>>
>>>> Thanks,
>>>> Kyrill
>>>>
>>>>
>>>>> Christophe.
>>>>>
>>>>>> Kyrill
>>>>>>
>>>>>>
>>>>>>> Christophe.
>>>>>>>
>>>>>>>> Thanks for handling this!
>>>>>>>> Kyrill
>>>>>>>>
>>>>>>>>
>>>>>>>>> Christophe
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> 2015-11-27  Christophe Lyon  <christophe.lyon@linaro.org>
>>>>>>>>>
>>>>>>>>>         * lib/target-supports.exp
>>>>>>>>>         (check_effective_target_arm_vfp_ok_nocache): New.
>>>>>>>>>         (check_effective_target_arm_vfp_ok): Call the new
>>>>>>>>>         check_effective_target_arm_vfp_ok_nocache function.
>>>>>>>>>         (check_effective_target_arm_fp_ok_nocache): New.
>>>>>>>>>         (check_effective_target_arm_fp_ok): New.
>>>>>>>>>         (add_options_for_arm_fp): New.
>>>>>>>>>         (check_effective_target_arm_crypto_ok_nocache): Require
>>>>>>>>>         target_arm_v8_neon_ok instead of arm32.
>>>>>>>>>         (check_effective_target_arm_crypto_pragma_ok_nocache): New.
>>>>>>>>>         (check_effective_target_arm_crypto_pragma_ok): New.
>>>>>>>>>         (add_options_for_arm_vfp): New.
>>>>>>>>>         * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok
>>>>>>>>> effective
>>>>>>>>>         target. Do not force -mfloat-abi=softfp, use arm_vfp
>>>>>>>>> effective
>>>>>>>>>         target instead.
>>>>>>>>>         * gcc.target/arm/attr-neon-builtin-fail.c: Do not force
>>>>>>>>>         -mfloat-abi=softfp, use arm_fp effective target instead.
>>>>>>>>>         * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove
>>>>>>>>> arm_neon_ok
>>>>>>>>>         dependency.
>>>>>>>>>         * gcc.target/arm/attr-neon2.c: Do not force
>>>>>>>>> -mfloat-abi=softfp,
>>>>>>>>>         use arm_vfp effective target instead.
>>>>>>>>>         * gcc.target/arm/attr-neon3.c: Likewise.
>>>>>>>>
>>>>>>>>
>

Re: [testsuite][ARM target attributes] Fix effective_target tests

[Christophe Lyon]

[-- Attachment #1: Type: text/plain, Size: 17195 bytes --]

On 4 January 2016 at 15:20, Christophe Lyon <christophe.lyon@linaro.org> wrote:
> On 18 December 2015 at 15:16, Kyrill Tkachov
> <kyrylo.tkachov@foss.arm.com> wrote:
>> Hi Christophe,
>>
>>
>> On 17/12/15 22:17, Christophe Lyon wrote:
>>>
>>> Hi,
>>>
>>> Here is an updated version of this patch.
>>> I did test it with
>>> -mthumb/-march=armv8-a/-mfpu=crypto-neon-fp-armv8/-mfloat-abi=hard in
>>> addition to my usual set of options.
>>>
>>> Compared to the previous version:
>>> - I added some doc in sourcebuild.texi
>>> - I no longer modify arm_vfp_ok...
>>> - I replaced all uses of arm_vfp with the new arm_fp because I found
>>> that the existing tests do not actually need to pass -mfpu=vfp: this
>>> is implicitly set as the default when using -mfloat-abi={softfp|hard}
>>> - I chose not to remove arm_vfp_ok because we may need it in the
>>> future, if a test really needs vfp (as opposed to neon for instance)
>>> - in gcc.target/arm/attr-crypto.c I force the initial fpu to be vfp
>>> via pragma instead, so that the next pragma fpu
>>> fpu=crypto-neon-fp-armv8 is always compatible, regardless of the
>>> command-line options/default fpu
>>> - same for attr-neon2.c and attr-neon3.c
>>> - I updated cmp-2.c, unsigned-float.c, vfp-1.c, vfp-ldmdbd.c,
>>> vfp-ldmdbs.c, vfp-ldmiad.c, vfp-ldmias.c, vfp-stmdbd.c, vfp-stmdbs.c,
>>> vfp-stmiad.c, vfp-stmias.c, vnmul-[1234].c to use the new arm_fp
>>> effective target instead of arm_vfp. This is so that they don't need
>>> to use -mfpu=vfp and can use the new dg-add-options arm_fp
>>>
>>> The validation results show (in addition to what I originally reported):
>>> - attr-crypto.c and attr-neon3.c now ICE in some cases. This is PR68895.
>>> - depending on the GCC configuration (e.g. --with-fpu=neon)
>>> attr-neon3.c may fail. This is PR68896.
>>>
>>> OK?
>>
>>
>> Thanks for following up on this.
>> I think you also need to document the new arm_crypto_pragma_ok.
>>
> Indeed, I forgot it.
>
> Here is a new version of the patch with a few words added to document
> this function.
> I did not modify the testcase after Christian's comments and
> PR68934: my understanding is that the testscase are valid after
> all and Christian is working on fixing the ICE.
>
With the attachment, this time...


> 2016-01-04  Christophe Lyon  <christophe.lyon@linaro.org>
>
>     * doc/sourcebuild.texi (arm_crypto_pragma_ok): Document new entry.
>     (arm_fp_ok): Likewise.
>     (arm_fp): Likewise.
>     (arm_crypto): Likewise.
>     * lib/target-supports.exp
>     (check_effective_target_arm_fp_ok_nocache): New.
>     (check_effective_target_arm_fp_ok): New.
>     (add_options_for_arm_fp): New.
>     (check_effective_target_arm_crypto_ok_nocache): Require
>     target_arm_v8_neon_ok instead of arm32.
>     (check_effective_target_arm_crypto_pragma_ok_nocache): New.
>     (check_effective_target_arm_crypto_pragma_ok): New.
>     (add_options_for_arm_vfp): New.
>     * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok effective
>     target. Do not force -mfloat-abi=softfp, use arm_fp_ok effective
>     target instead. Force initial fpu to vfp.
>     * gcc.target/arm/attr-neon-builtin-fail.c: Do not force
>     -mfloat-abi=softfp, use arm_fp_ok effective target instead.
>     * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove arm_neon_ok
>     dependency.
>     * gcc.target/arm/attr-neon2.c: Do not force -mfloat-abi=softfp,
>     use arm_vfp effective target instead. Force initial fpu to vfp.
>     * gcc.target/arm/attr-neon3.c: Likewise.
>     * gcc.target/arm/cmp-2.c: Use arm_fp_ok effective target instead of
>     arm_vfp_ok.
>     * gcc.target/arm/unsigned-float.c: Likewise.
>     * gcc.target/arm/vfp-1.c: Likewise.
>     * gcc.target/arm/vfp-ldmdbd.c: Likewise.
>     * gcc.target/arm/vfp-ldmdbs.c: Likewise.
>     * gcc.target/arm/vfp-ldmiad.c: Likewise.
>     * gcc.target/arm/vfp-ldmias.c: Likewise.
>     * gcc.target/arm/vfp-stmdbd.c: Likewise.
>     * gcc.target/arm/vfp-stmdbs.c: Likewise.
>     * gcc.target/arm/vfp-stmiad.c: Likewise.
>     * gcc.target/arm/vfp-stmias.c: Likewise.
>     * gcc.target/arm/vnmul-1.c: Likewise.
>     * gcc.target/arm/vnmul-2.c: Likewise.
>     * gcc.target/arm/vnmul-3.c: Likewise.
>     * gcc.target/arm/vnmul-4.c: Likewise.
>
> OK?
>
> Christophe.
>
>
>> Kyrill
>>
>>
>>> Christophe
>>>
>>> 2015-12-17  Christophe Lyon  <christophe.lyon@linaro.org>
>>>
>>>      * doc/sourcebuild.texi (arm_fp_ok): Document new entry.
>>>      (arm_fp): Likewise.
>>>      * lib/target-supports.exp
>>>      (check_effective_target_arm_fp_ok_nocache): New.
>>>      (check_effective_target_arm_fp_ok): New.
>>>      (add_options_for_arm_fp): New.
>>>      (check_effective_target_arm_crypto_ok_nocache): Require
>>>      target_arm_v8_neon_ok instead of arm32.
>>>      (check_effective_target_arm_crypto_pragma_ok_nocache): New.
>>>      (check_effective_target_arm_crypto_pragma_ok): New.
>>>      (add_options_for_arm_vfp): New.
>>>      * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok effective
>>>      target. Do not force -mfloat-abi=softfp, use arm_fp_ok effective
>>>      target instead. Force initial fpu to vfp.
>>>      * gcc.target/arm/attr-neon-builtin-fail.c: Do not force
>>>      -mfloat-abi=softfp, use arm_fp_ok effective target instead.
>>>      * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove arm_neon_ok
>>>      dependency.
>>>      * gcc.target/arm/attr-neon2.c: Do not force -mfloat-abi=softfp,
>>>      use arm_vfp effective target instead. Force initial fpu to vfp.
>>>      * gcc.target/arm/attr-neon3.c: Likewise.
>>>      * gcc.target/arm/cmp-2.c: Use arm_fp_ok effective target instead of
>>>      arm_vfp_ok.
>>>      * gcc.target/arm/unsigned-float.c: Likewise.
>>>      * gcc.target/arm/vfp-1.c: Likewise.
>>>      * gcc.target/arm/vfp-ldmdbd.c: Likewise.
>>>      * gcc.target/arm/vfp-ldmdbs.c: Likewise.
>>>      * gcc.target/arm/vfp-ldmiad.c: Likewise.
>>>      * gcc.target/arm/vfp-ldmias.c: Likewise.
>>>      * gcc.target/arm/vfp-stmdbd.c: Likewise.
>>>      * gcc.target/arm/vfp-stmdbs.c: Likewise.
>>>      * gcc.target/arm/vfp-stmiad.c: Likewise.
>>>      * gcc.target/arm/vfp-stmias.c: Likewise.
>>>      * gcc.target/arm/vnmul-1.c: Likewise.
>>>      * gcc.target/arm/vnmul-2.c: Likewise.
>>>      * gcc.target/arm/vnmul-3.c: Likewise.
>>>      * gcc.target/arm/vnmul-4.c: Likewise.
>>>
>>>
>>>
>>> On 10 December 2015 at 20:52, Christophe Lyon
>>> <christophe.lyon@linaro.org> wrote:
>>>>
>>>> On 10 December 2015 at 14:14, Kyrill Tkachov <kyrylo.tkachov@arm.com>
>>>> wrote:
>>>>>
>>>>> On 10/12/15 13:04, Christophe Lyon wrote:
>>>>>>
>>>>>> On 10 December 2015 at 13:30, Kyrill Tkachov <kyrylo.tkachov@arm.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> Hi Christophe,
>>>>>>>
>>>>>>>
>>>>>>> On 08/12/15 11:18, Christophe Lyon wrote:
>>>>>>>>
>>>>>>>> On 8 December 2015 at 11:50, Kyrill Tkachov <kyrylo.tkachov@arm.com>
>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> Hi Christophe,
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 27/11/15 13:00, Christophe Lyon wrote:
>>>>>>>>>>
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> After the recent commits from Christian adding target attributes
>>>>>>>>>> support for ARM FPU settings,  I've noticed that some of the tests
>>>>>>>>>> were failing because of incorrect assumptions wrt to the default
>>>>>>>>>> cpu/fpu/float-abi of the compiler.
>>>>>>>>>>
>>>>>>>>>> This patch fixes the problems I've noticed in the following way:
>>>>>>>>>> - do not force -mfloat-abi=softfp in dg-options, to avoid conflicts
>>>>>>>>>> when gcc is configured --with-float=hard
>>>>>>>>>>
>>>>>>>>>> - change arm_vfp_ok such that it tries several -mfpu/-mfloat-abi
>>>>>>>>>> flags, checks that __ARM_FP is defined and __ARM_NEON_FP is not
>>>>>>>>>> defined
>>>>>>>>>>
>>>>>>>>>> - introduce arm_fp_ok, which is similar but does not enforce fpu
>>>>>>>>>> setting
>>>>>>>>>>
>>>>>>>>>> - add a new effective_target: arm_crypto_pragma_ok to check that
>>>>>>>>>> setting this fpu via a pragma is actually supported by the current
>>>>>>>>>> "multilib". This is different from checking the command-line option
>>>>>>>>>> because the pragma might conflict with the command-line options in
>>>>>>>>>> use.
>>>>>>>>>>
>>>>>>>>>> The updates in the testcases are as follows:
>>>>>>>>>> - attr-crypto.c, we have to make sure that the defaut fpu does not
>>>>>>>>>> conflict with the one forced by pragma. That's why I use the
>>>>>>>>>> arm_vfp
>>>>>>>>>> options/effective_target. This is needed if gcc has been configured
>>>>>>>>>> --with-fpu=neon-fp16, as the pragma fpu=crypto-neon-fp-armv8 would
>>>>>>>>>> conflict.
>>>>>>>>>>
>>>>>>>>>> - attr-neon-builtin-fail.c: use arm_fp to force the appropriate
>>>>>>>>>> float-abi setting. Enforcing fpu is not needed here.
>>>>>>>>>>
>>>>>>>>>> - attr-neon-fp16.c: similar, I also removed arm_neon_ok since it
>>>>>>>>>> was
>>>>>>>>>> not necessary to make the test pass in my testing. On second
>>>>>>>>>> thought,
>>>>>>>>>> I'm wondering whether I should leave it and make the test
>>>>>>>>>> unsupported
>>>>>>>>>> in more cases (such as when forcing -march=armv5t, although it does
>>>>>>>>>> pass with this patch)
>>>>>>>>>>
>>>>>>>>>> - attr-neon2.c: use arm_vfp to force the appropriate float-abi
>>>>>>>>>> setting. Enforcing mfpu=vfp is needed to avoid conflict with the
>>>>>>>>>> pragma target fpu=neon (for instance if the toolchain default is
>>>>>>>>>> neon-fp16)
>>>>>>>>>>
>>>>>>>>>> - attr-neon3.c: similar
>>>>>>>>>>
>>>>>>>>>> Tested on a variety of configurations, see:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/230929-target-attr/report-build-info.html
>>>>>>>>>>
>>>>>>>>>> Note that the regressions reported fall into 3 categories:
>>>>>>>>>> - when forcing march=armv5t: tests are now unsupported because I
>>>>>>>>>> modified arm_crypto_ok to require arm_v8_neon_ok instead of arm32.
>>>>>>>>>>
>>>>>>>>>> - the warning reported by attr-neon-builtin-fail.c moved from line
>>>>>>>>>> 12
>>>>>>>>>> to 14 and is thus seen as a regression + one improvement
>>>>>>>>>>
>>>>>>>>>> - finally, attr-neon-fp16.c causes an ICE on armeb compilers, for
>>>>>>>>>> which I need to post a bugzilla.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> TBH, I'm a bit concerned by the complexity of all these
>>>>>>>>>> multilib-like
>>>>>>>>>> conditions. I'm confident that I'm still missing some combinations
>>>>>>>>>> :-)
>>>>>>>>>>
>>>>>>>>>> And with new target attributes coming, new architectures etc... all
>>>>>>>>>> this logic is likely to become even more complex.
>>>>>>>>>>
>>>>>>>>>> That being said, OK for trunk?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I'd like us to clean up and reorganise the gcc.target/arm testsuite
>>>>>>>>> at some point to make it more robust with respect to the tons of
>>>>>>>>> multilib
>>>>>>>>> options and configurations we can have for arm. It's becoming very
>>>>>>>>> frustrating
>>>>>>>>> to test feature-specific stuff :(
>>>>>>>>>
>>>>>>>>> This is ok in the meantime.
>>>>>>>>> Sorry for the delay.
>>>>>>>>>
>>>>>>>> Thanks for the approval, glad to see I'm not the only one willing to
>>>>>>>> see
>>>>>>>> improvements in this area :)
>>>>>>>>
>>>>>>>> Committed as r231403.
>>>>>>>
>>>>>>>
>>>>>>> With this patch we're seeing legitimate PASSes go to NA.
>>>>>>> For example:
>>>>>>>
>>>>>>> gcc.target/arm/vfp-1.c
>>>>>>> gcc.target/arm/vfp-ldmdbs.c
>>>>>>> and other vfp tests in arm.exp.
>>>>>>> This is, for example, for the
>>>>>>>
>>>>>>> variant-mthumb/-march=armv8-a/-mfpu=crypto-neon-fp-armv8/-mfloat-abi=hard
>>>>>>>
>>>>>> Hmm I'm attempting to cover such a configuration in my matrix of
>>>>>> validations:
>>>>>>
>>>>>>
>>>>>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/231403/report-build-info.html
>>>>>>
>>>>>> The difference is that I use similar flags at GCC configure time, while
>>>>>> you
>>>>>> override them when running the testsuite:
>>>>>> --target arm-none-linux-gnueabihf --with-mode=thum
>>>>>> --with-cpu=cortex-a57
>>>>>> --with-fpu=crypto-neon-fp-armv8
>>>>>>
>>>>>> I this case, I do not see the regressions.
>>>>>
>>>>>
>>>>> My gcc is arm-none-eabi configured with
>>>>> --with-float=hard --with-cpu=cortex-a15 --with-fpu=neon-vfpv4
>>>>> --with-float=hard
>>>>>
>>>>>>> I suspect under your new predicates they'd need to be changed to check
>>>>>>> for
>>>>>>> arm_fp_ok rather than arm_vfp_ok.
>>>>>>
>>>>>> Probably, yes.
>>>>>>
>>>>> In the test log where it checks the effective target it fails due to:
>>>>> arm_vfp_ok27268.c:6:4: error: #error __ARM_NEON_FP defined
>>>>> it's compiling the check with
>>>>> -mthumb -march=armv8-a -mfpu=crypto-neon-fp-armv8 -mfloat-abi=hard -o
>>>>> arm_vfp_ok27268.o arm_vfp_ok27268.c
>>>>>
>>>>>>> We want to avoid removing any PASSes.
>>>>>>> Can you please test some more to make sure we don't remove any
>>>>>>> legitimate
>>>>>>> passes with your patch?
>>>>>>
>>>>>> Is that the only combination in which you saw less PASSes?
>>>>>
>>>>>
>>>>> That's the one that was brought to my attention, but I think the issue
>>>>> here
>>>>> is just the
>>>>> tests that check for arm_vfp_ok rather than the new arm_fp_ok and set
>>>>> -mfpu=vfp explicitly.
>>>>> They appear unsupported if testing with an explicit neon option in mfpu,
>>>>> I
>>>>> think.
>>>>>
>>>>>>> Also, Ramana reminded me offline that any new predicates in
>>>>>>> target-supports.exp
>>>>>>> need documenting in sourcebuild.txt.
>>>>>>>
>>>>>>> In light of that, can you please revert your patch and address the
>>>>>>> issues
>>>>>>> above
>>>>>>> so that we can be confident we don't lose existing legitimate test
>>>>>>> coverage?
>>>>>>
>>>>>> OK.
>>>>>>
>>>>>>> Sorry for not catching these sooner.
>>>>>>
>>>>>> No problem, there are so many combinations.
>>>>>> I'm not sure how to define a reasonable set.
>>>>>
>>>>>
>>>>> So, I think the action plan here is to audit the tests that need to be
>>>>> changed
>>>>> to arm_fp_ok and add the documentation for the new predicate checks.
>>>>>
>>>> I came up with a new version where I now use only the new arm_fp_ok, so
>>>> it might be easier just to update arm_vfp_ok and not introduce arm_fp_ok.
>>>>
>>>> However, I'm still not happy with this version because it has problems
>>>> with
>>>> a few tests that use target attributes such as attr-crypto.c, which
>>>> starts with:
>>>> #pragma GCC target ("fpu=crypto-neon-fp-armv8")
>>>>
>>>> This pragma fails if the compiler+options have set an incompatible
>>>> fpu before processing the pragma.
>>>>
>>>> So for instance, if gcc has been configured --with-fpu=neon, compiling
>>>> the test
>>>> with no fpu option fails (neon is not compatible with
>>>> crypto-neon-fp-armv8
>>>> because the latter has fp16)
>>>>
>>>> Now, if we use effective_target arm_vfp_ok + dg_option to force
>>>> -mfpu=vfp,
>>>> we end up compiling with -mfpu=vfp and the pragma passes.
>>>>
>>>> But if, in addition, we force the testflags to set
>>>> -mfpu=crypto-neon-fp-armv8
>>>> as you do, the effective_target arm_vfp_ok test now fails because it is
>>>> compiled with -mfpu=vfp -mfpu=crypto-neon-fp-armv8, which defines
>>>> __ARM_NEON_FP
>>>>
>>>> In short, the problem is how to make sure that the fpu setting is always
>>>> compatible with the pragma, whatever the gcc configuration and multilib
>>>> options used.
>>>>
>>>> Thanks,
>>>>
>>>> Christophe.
>>>>
>>>>> Thanks,
>>>>> Kyrill
>>>>>
>>>>>
>>>>>> Christophe.
>>>>>>
>>>>>>> Kyrill
>>>>>>>
>>>>>>>
>>>>>>>> Christophe.
>>>>>>>>
>>>>>>>>> Thanks for handling this!
>>>>>>>>> Kyrill
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> Christophe
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> 2015-11-27  Christophe Lyon  <christophe.lyon@linaro.org>
>>>>>>>>>>
>>>>>>>>>>         * lib/target-supports.exp
>>>>>>>>>>         (check_effective_target_arm_vfp_ok_nocache): New.
>>>>>>>>>>         (check_effective_target_arm_vfp_ok): Call the new
>>>>>>>>>>         check_effective_target_arm_vfp_ok_nocache function.
>>>>>>>>>>         (check_effective_target_arm_fp_ok_nocache): New.
>>>>>>>>>>         (check_effective_target_arm_fp_ok): New.
>>>>>>>>>>         (add_options_for_arm_fp): New.
>>>>>>>>>>         (check_effective_target_arm_crypto_ok_nocache): Require
>>>>>>>>>>         target_arm_v8_neon_ok instead of arm32.
>>>>>>>>>>         (check_effective_target_arm_crypto_pragma_ok_nocache): New.
>>>>>>>>>>         (check_effective_target_arm_crypto_pragma_ok): New.
>>>>>>>>>>         (add_options_for_arm_vfp): New.
>>>>>>>>>>         * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok
>>>>>>>>>> effective
>>>>>>>>>>         target. Do not force -mfloat-abi=softfp, use arm_vfp
>>>>>>>>>> effective
>>>>>>>>>>         target instead.
>>>>>>>>>>         * gcc.target/arm/attr-neon-builtin-fail.c: Do not force
>>>>>>>>>>         -mfloat-abi=softfp, use arm_fp effective target instead.
>>>>>>>>>>         * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove
>>>>>>>>>> arm_neon_ok
>>>>>>>>>>         dependency.
>>>>>>>>>>         * gcc.target/arm/attr-neon2.c: Do not force
>>>>>>>>>> -mfloat-abi=softfp,
>>>>>>>>>>         use arm_vfp effective target instead.
>>>>>>>>>>         * gcc.target/arm/attr-neon3.c: Likewise.
>>>>>>>>>
>>>>>>>>>
>>

[-- Attachment #2: target-attr-v6.patch --]
[-- Type: text/x-diff, Size: 19372 bytes --]

diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi
index 61de4a5..3b4352d 100644
--- a/gcc/doc/sourcebuild.texi
+++ b/gcc/doc/sourcebuild.texi
@@ -1511,9 +1511,21 @@ Target generates decimal floating point instructions with current options.
 @item arm32
 ARM target generates 32-bit code.
 
+@item arm_crypto_pragma_ok
+@anchor{arm_crypto_pragma_ok}
+ARM target supports pragma target @code{fpu=crypto-neon-fp-armv8}.
+This may need @code{-mfpu=crypto-neon-fp-armv8} or equivalent options.
+Some multilibs may be incompatible with these options.
+
 @item arm_eabi
 ARM target adheres to the ABI for the ARM Architecture.
 
+@item arm_fp_ok
+@anchor{arm_fp_ok}
+ARM target defines @code{__ARM_FP} using @code{-mfloat-abi=softfp} or
+equivalent options.  Some multilibs may be incompatible with these
+options.
+
 @item arm_hf_eabi
 ARM target adheres to the VFP and Advanced SIMD Register Arguments
 variant of the ABI for the ARM Architecture (as selected with
@@ -2035,6 +2047,17 @@ The supported values of @var{feature} for directive @code{dg-add-options}
 are:
 
 @table @code
+@item arm_crypto_pragma
+Crypto extension pragma support.  Only ARM targets support this
+feature, and only then in certain modes; see the
+@ref{arm_crypto_pragma_ok,,arm_crypto_pragma_ok effective target
+keyword}.
+
+@item arm_fp
+@code{__ARM_FP} definition.  Only ARM targets support this feature, and only then
+in certain modes; see the @ref{arm_fp_ok,,arm_fp_ok effective target
+keyword}.
+
 @item arm_neon
 NEON support.  Only ARM targets support this feature, and only then
 in certain modes; see the @ref{arm_neon_ok,,arm_neon_ok effective target
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 4e349e9..6272073 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -2721,6 +2721,47 @@ proc check_effective_target_arm_hard_vfp_ok { } {
     }
 }
 
+# Return 1 if this is an ARM target defining __ARM_FP. We may need
+# -mfloat-abi=softfp or equivalent options.  Some multilibs may be
+# incompatible with these options.  Also set et_arm_fp_flags to the
+# best options to add.
+
+proc check_effective_target_arm_fp_ok_nocache { } {
+    global et_arm_fp_flags
+    set et_arm_fp_flags ""
+    if { [check_effective_target_arm32] } {
+	foreach flags {"" "-mfloat-abi=softfp" "-mfloat-abi=hard"} {
+	    if { [check_no_compiler_messages_nocache arm_fp_ok object {
+		#ifndef __ARM_FP
+		#error __ARM_FP not defined
+		#endif
+	    } "$flags"] } {
+		set et_arm_fp_flags $flags
+		return 1
+	    }
+	}
+    }
+
+    return 0
+}
+
+proc check_effective_target_arm_fp_ok { } {
+    return [check_cached_effective_target arm_fp_ok \
+		check_effective_target_arm_fp_ok_nocache]
+}
+
+# Add the options needed to define __ARM_FP.  We need either
+# -mfloat-abi=softfp or -mfloat-abi=hard, but if one is already
+# specified by the multilib, use it.
+
+proc add_options_for_arm_fp { flags } {
+    if { ! [check_effective_target_arm_fp_ok] } {
+	return "$flags"
+    }
+    global et_arm_fp_flags
+    return "$flags $et_arm_fp_flags"
+}
+
 # Return 1 if this is an ARM target that supports DSP multiply with
 # current multilib flags.
 
@@ -2753,7 +2794,7 @@ proc check_effective_target_arm_unaligned { } {
 proc check_effective_target_arm_crypto_ok_nocache { } {
     global et_arm_crypto_flags
     set et_arm_crypto_flags ""
-    if { [check_effective_target_arm32] } {
+    if { [check_effective_target_arm_v8_neon_ok] } {
 	foreach flags {"" "-mfloat-abi=softfp" "-mfpu=crypto-neon-fp-armv8" "-mfpu=crypto-neon-fp-armv8 -mfloat-abi=softfp"} {
 	    if { [check_no_compiler_messages_nocache arm_crypto_ok object {
 		#include "arm_neon.h"
@@ -2788,6 +2829,52 @@ proc add_options_for_arm_crypto { flags } {
     return "$flags $et_arm_crypto_flags"
 }
 
+# Return 1 if this is an ARM target supporting pragma target
+# fpu=crypto-neon-fp-armv8, possibly needing -mfloat-abi=softfp or
+# equivalent options.  Some multilibs may be incompatible with these
+# options.  Also set et_arm_crypto_pragma_flags to the best options to
+# add.
+
+proc check_effective_target_arm_crypto_pragma_ok_nocache { } {
+    global et_arm_crypto_pragma_flags
+    set et_arm_crypto_pragma_flags ""
+    if { [check_effective_target_arm_v8_neon_ok] } {
+	foreach flags {"" "-mfloat-abi=softfp" "-mfpu=crypto-neon-fp-armv8" "-mfpu=crypto-neon-fp-armv8 -mfloat-abi=softfp"} {
+	    if { [check_no_compiler_messages_nocache arm_crypto_pragmaok object {
+		#pragma GCC target ("fpu=crypto-neon-fp-armv8")
+		#include "arm_neon.h"
+		uint8x16_t
+		foo (uint8x16_t a, uint8x16_t b)
+		{
+	          return vaeseq_u8 (a, b);
+		}
+	    } "[add_options_for_arm_v8_neon ""] $flags"] } {
+		set et_arm_crypto_pragma_flags "[add_options_for_arm_v8_neon ""] $flags"
+		return 1
+	    }
+	}
+    }
+
+    return 0
+}
+
+# Return 1 if this is an ARM target supporting pragma target
+# -mfpu=crypto-neon-fp-armv8.
+
+proc check_effective_target_arm_crypto_pragma_ok { } {
+    return [check_cached_effective_target arm_crypto_pragma_ok \
+		check_effective_target_arm_crypto_pragma_ok_nocache]
+}
+
+# Add options for crypto extensions pragma.
+proc add_options_for_arm_crypto_pragma { flags } {
+    if { ! [check_effective_target_arm_crypto_pragma_ok] } {
+        return "$flags"
+    }
+    global et_arm_crypto_pragma_flags
+    return "$flags $et_arm_crypto_pragma_flags"
+}
+
 # Add the options needed for NEON.  We need either -mfloat-abi=softfp
 # or -mfloat-abi=hard, but if one is already specified by the
 # multilib, use it.  Similarly, if a -mfpu option already enables
@@ -2907,8 +2994,8 @@ proc check_effective_target_arm_crc_ok { } {
 
 # Return 1 if this is an ARM target supporting -mfpu=neon-fp16
 # -mfloat-abi=softfp or equivalent options.  Some multilibs may be
-# incompatible with these options.  Also set et_arm_neon_flags to the
-# best options to add.
+# incompatible with these options.  Also set et_arm_neon_fp16_flags to
+# the best options to add.
 
 proc check_effective_target_arm_neon_fp16_ok_nocache { } {
     global et_arm_neon_fp16_flags
diff --git a/gcc/testsuite/gcc.target/arm/attr-crypto.c b/gcc/testsuite/gcc.target/arm/attr-crypto.c
index 1db5984..3b4b7c5 100644
--- a/gcc/testsuite/gcc.target/arm/attr-crypto.c
+++ b/gcc/testsuite/gcc.target/arm/attr-crypto.c
@@ -1,6 +1,14 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_crypto_ok } */
-/* { dg-options "-O2 -mfloat-abi=softfp" } */
+/* Make sure we can force fpu=vfp before switching using the
+   pragma.  */
+/* { dg-require-effective-target arm_fp_ok } */
+/* { dg-require-effective-target arm_crypto_pragma_ok } */
+/* { dg-options "-O2 -march=armv8-a" } */
+/* { dg-add-options arm_fp } */
+
+/* Reset fpu to a value compatible with the next pragmas.  */
+#pragma GCC target ("fpu=vfp")
+#pragma GCC push_options
 
 #pragma GCC target ("fpu=crypto-neon-fp-armv8")
 
@@ -28,7 +36,7 @@ foo (void)
   return res[0];
 }
 
-#pragma GCC reset_options
+#pragma GCC pop_options
 
 /* Check that the FP version is correctly reset.  */
 
diff --git a/gcc/testsuite/gcc.target/arm/attr-neon-builtin-fail.c b/gcc/testsuite/gcc.target/arm/attr-neon-builtin-fail.c
index 6ac32fc..05dc579 100644
--- a/gcc/testsuite/gcc.target/arm/attr-neon-builtin-fail.c
+++ b/gcc/testsuite/gcc.target/arm/attr-neon-builtin-fail.c
@@ -1,7 +1,9 @@
 /* Check that calling a neon builtin from a function compiled with vfp fails.  */
 /* { dg-do compile } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-require-effective-target arm_neon_ok } */
-/* { dg-options "-O2 -mfloat-abi=softfp" } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_fp } */
 
 #include <arm_neon.h>
 
@@ -12,6 +14,5 @@ foo (uint8x16_t *p)
   *p = vmovq_n_u8 (3); /* { dg-message "called from here" } */
 }
 
-/* { dg-error "inlining failed in call to always_inline" "" { target *-*-* } 0 }
- */
+/* { dg-error "inlining failed in call to always_inline" "" { target *-*-* } 0 } */
 
diff --git a/gcc/testsuite/gcc.target/arm/attr-neon-fp16.c b/gcc/testsuite/gcc.target/arm/attr-neon-fp16.c
index 3cf8918..984992f 100644
--- a/gcc/testsuite/gcc.target/arm/attr-neon-fp16.c
+++ b/gcc/testsuite/gcc.target/arm/attr-neon-fp16.c
@@ -1,6 +1,7 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_neon_ok } */
-/* { dg-options "-mfp16-format=ieee -mfloat-abi=softfp" } */
+/* { dg-require-effective-target arm_fp_ok } */
+/* { dg-options "-mfp16-format=ieee" } */
+/* { dg-add-options arm_fp } */
 
 #include "arm_neon.h"
 
diff --git a/gcc/testsuite/gcc.target/arm/attr-neon2.c b/gcc/testsuite/gcc.target/arm/attr-neon2.c
index 819fad4..2966825 100644
--- a/gcc/testsuite/gcc.target/arm/attr-neon2.c
+++ b/gcc/testsuite/gcc.target/arm/attr-neon2.c
@@ -1,6 +1,12 @@
 /* { dg-do compile } */
 /* { dg-require-effective-target arm_neon_ok } */
-/* { dg-options "-O2 -mfloat-abi=softfp -mfpu=vfp" } */
+/* { dg-require-effective-target arm_fp_ok } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_fp } */
+
+/* Reset fpu to a value compatible with the next pragmas.  */
+#pragma GCC target ("fpu=vfp")
+#pragma GCC push_options
 
 #pragma GCC target ("fpu=neon")
 #include <arm_neon.h>
@@ -12,7 +18,7 @@ my (int8x8_t __a, int8x8_t __b)
   return __a + __b;
 }
 
-#pragma GCC reset_options
+#pragma GCC pop_options
 
 /* Check that command line option is restored.  */
 int8x8_t 
diff --git a/gcc/testsuite/gcc.target/arm/attr-neon3.c b/gcc/testsuite/gcc.target/arm/attr-neon3.c
index 30a1479..17e429a 100644
--- a/gcc/testsuite/gcc.target/arm/attr-neon3.c
+++ b/gcc/testsuite/gcc.target/arm/attr-neon3.c
@@ -1,6 +1,12 @@
 /* { dg-do compile } */
 /* { dg-require-effective-target arm_crypto_ok } */
-/* { dg-options "-O2 -mfloat-abi=softfp -mfpu=vfp" } */
+/* { dg-require-effective-target arm_fp_ok } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_fp } */
+
+/* Reset fpu to a value compatible with the next pragmas.  */
+#pragma GCC target ("fpu=vfp")
+#pragma GCC push_options
 
 #include <arm_neon.h>
 
diff --git a/gcc/testsuite/gcc.target/arm/cmp-2.c b/gcc/testsuite/gcc.target/arm/cmp-2.c
index ed6b609..70e4509 100644
--- a/gcc/testsuite/gcc.target/arm/cmp-2.c
+++ b/gcc/testsuite/gcc.target/arm/cmp-2.c
@@ -1,7 +1,8 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
-/* { dg-options "-O -mfpu=vfp -mfloat-abi=softfp" } */
+/* { dg-options "-O" } */
+/* { dg-add-options arm_fp } */
 /* { dg-final { scan-assembler-not "\tbl\t" } } */
 /* { dg-final { scan-assembler-not "__aeabi" } } */
 int x, y;
diff --git a/gcc/testsuite/gcc.target/arm/unsigned-float.c b/gcc/testsuite/gcc.target/arm/unsigned-float.c
index b9ed681..e1cda0c 100644
--- a/gcc/testsuite/gcc.target/arm/unsigned-float.c
+++ b/gcc/testsuite/gcc.target/arm/unsigned-float.c
@@ -1,8 +1,9 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
 /* { dg-options "-march=armv7-a -O1" } */
-/* { dg-additional-options "-mfloat-abi=softfp" { target { ! { arm_hf_eabi } } } } */
+/* { dg-add-options arm_fp } */
+
 
 #include <stdint.h>
 
diff --git a/gcc/testsuite/gcc.target/arm/vfp-1.c b/gcc/testsuite/gcc.target/arm/vfp-1.c
index 9aa5302..7add1b8 100644
--- a/gcc/testsuite/gcc.target/arm/vfp-1.c
+++ b/gcc/testsuite/gcc.target/arm/vfp-1.c
@@ -1,6 +1,7 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -mfpu=vfp -mfloat-abi=softfp -ffp-contract=off" } */
-/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
+/* { dg-options "-O2 -ffp-contract=off" } */
+/* { dg-add-options arm_fp } */
 /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
 
 extern float fabsf (float);
diff --git a/gcc/testsuite/gcc.target/arm/vfp-ldmdbd.c b/gcc/testsuite/gcc.target/arm/vfp-ldmdbd.c
index 7041579..3489a2a 100644
--- a/gcc/testsuite/gcc.target/arm/vfp-ldmdbd.c
+++ b/gcc/testsuite/gcc.target/arm/vfp-ldmdbd.c
@@ -1,7 +1,8 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
-/* { dg-options "-O2 -mfpu=vfp -mfloat-abi=softfp" } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_fp } */
 
 extern void bar (double);
 
diff --git a/gcc/testsuite/gcc.target/arm/vfp-ldmdbs.c b/gcc/testsuite/gcc.target/arm/vfp-ldmdbs.c
index 0187c01..8fda405 100644
--- a/gcc/testsuite/gcc.target/arm/vfp-ldmdbs.c
+++ b/gcc/testsuite/gcc.target/arm/vfp-ldmdbs.c
@@ -1,7 +1,8 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
-/* { dg-options "-O2 -mfpu=vfp -mfloat-abi=softfp" } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_fp } */
 
 extern void bar (float);
 
diff --git a/gcc/testsuite/gcc.target/arm/vfp-ldmiad.c b/gcc/testsuite/gcc.target/arm/vfp-ldmiad.c
index 9c22f1f..422e3ed 100644
--- a/gcc/testsuite/gcc.target/arm/vfp-ldmiad.c
+++ b/gcc/testsuite/gcc.target/arm/vfp-ldmiad.c
@@ -1,7 +1,8 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
-/* { dg-options "-O2 -mfpu=vfp -mfloat-abi=softfp" } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_fp } */
 
 extern void bar (double);
 
diff --git a/gcc/testsuite/gcc.target/arm/vfp-ldmias.c b/gcc/testsuite/gcc.target/arm/vfp-ldmias.c
index 92051fd..31d2ee1 100644
--- a/gcc/testsuite/gcc.target/arm/vfp-ldmias.c
+++ b/gcc/testsuite/gcc.target/arm/vfp-ldmias.c
@@ -1,7 +1,8 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
-/* { dg-options "-O2 -mfpu=vfp -mfloat-abi=softfp" } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_fp } */
 
 extern void bar (float);
 
diff --git a/gcc/testsuite/gcc.target/arm/vfp-stmdbd.c b/gcc/testsuite/gcc.target/arm/vfp-stmdbd.c
index 53383b5..686fe86 100644
--- a/gcc/testsuite/gcc.target/arm/vfp-stmdbd.c
+++ b/gcc/testsuite/gcc.target/arm/vfp-stmdbd.c
@@ -1,7 +1,8 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
-/* { dg-options "-O2 -mfpu=vfp -mfloat-abi=softfp" } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_fp } */
 
 void
 foo (double *p, double a, double b, int n)
diff --git a/gcc/testsuite/gcc.target/arm/vfp-stmdbs.c b/gcc/testsuite/gcc.target/arm/vfp-stmdbs.c
index 6570def..dbb30ec 100644
--- a/gcc/testsuite/gcc.target/arm/vfp-stmdbs.c
+++ b/gcc/testsuite/gcc.target/arm/vfp-stmdbs.c
@@ -1,7 +1,8 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
-/* { dg-options "-O2 -mfpu=vfp -mfloat-abi=softfp" } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_fp } */
 
 void
 foo (float *p, float a, float b, int n)
diff --git a/gcc/testsuite/gcc.target/arm/vfp-stmiad.c b/gcc/testsuite/gcc.target/arm/vfp-stmiad.c
index 28e9d73..665fa7a 100644
--- a/gcc/testsuite/gcc.target/arm/vfp-stmiad.c
+++ b/gcc/testsuite/gcc.target/arm/vfp-stmiad.c
@@ -1,7 +1,8 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
-/* { dg-options "-O2 -mfpu=vfp -mfloat-abi=softfp" } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_fp } */
 
 void
 foo (double *p, double a, double b, int n)
diff --git a/gcc/testsuite/gcc.target/arm/vfp-stmias.c b/gcc/testsuite/gcc.target/arm/vfp-stmias.c
index efa5fbe..90940e5 100644
--- a/gcc/testsuite/gcc.target/arm/vfp-stmias.c
+++ b/gcc/testsuite/gcc.target/arm/vfp-stmias.c
@@ -1,7 +1,8 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
-/* { dg-options "-O2 -mfpu=vfp -mfloat-abi=softfp" } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_fp } */
 
 void
 foo (float *p, float a, float b, int n)
diff --git a/gcc/testsuite/gcc.target/arm/vnmul-1.c b/gcc/testsuite/gcc.target/arm/vnmul-1.c
index af0bebe..fd00388 100644
--- a/gcc/testsuite/gcc.target/arm/vnmul-1.c
+++ b/gcc/testsuite/gcc.target/arm/vnmul-1.c
@@ -1,7 +1,8 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
-/* { dg-options "-O2 -fno-rounding-math -mfpu=vfp -mfloat-abi=hard" } */
+/* { dg-options "-O2 -fno-rounding-math" } */
+/* { dg-add-options arm_fp } */
 
 double
 foo_d (double a, double b)
diff --git a/gcc/testsuite/gcc.target/arm/vnmul-2.c b/gcc/testsuite/gcc.target/arm/vnmul-2.c
index 909b2a4..c299ec1 100644
--- a/gcc/testsuite/gcc.target/arm/vnmul-2.c
+++ b/gcc/testsuite/gcc.target/arm/vnmul-2.c
@@ -1,7 +1,8 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
-/* { dg-options "-O2 -frounding-math -mfpu=vfp -mfloat-abi=hard" } */
+/* { dg-options "-O2 -frounding-math" } */
+/* { dg-add-options arm_fp } */
 
 double
 foo_d (double a, double b)
diff --git a/gcc/testsuite/gcc.target/arm/vnmul-3.c b/gcc/testsuite/gcc.target/arm/vnmul-3.c
index df02882..44c1967 100644
--- a/gcc/testsuite/gcc.target/arm/vnmul-3.c
+++ b/gcc/testsuite/gcc.target/arm/vnmul-3.c
@@ -1,7 +1,8 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
-/* { dg-options "-O2 -fno-rounding-math -mfpu=vfp -mfloat-abi=hard" } */
+/* { dg-options "-O2 -fno-rounding-math" } */
+/* { dg-add-options arm_fp } */
 
 double
 foo_d (double a, double b)
diff --git a/gcc/testsuite/gcc.target/arm/vnmul-4.c b/gcc/testsuite/gcc.target/arm/vnmul-4.c
index 670ee40..dd9cab3 100644
--- a/gcc/testsuite/gcc.target/arm/vnmul-4.c
+++ b/gcc/testsuite/gcc.target/arm/vnmul-4.c
@@ -1,7 +1,8 @@
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_vfp_ok } */
+/* { dg-require-effective-target arm_fp_ok } */
 /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */
-/* { dg-options "-O2 -frounding-math -mfpu=vfp -mfloat-abi=hard" } */
+/* { dg-options "-O2 -frounding-math" } */
+/* { dg-add-options arm_fp } */
 
 double
 foo_d (double a, double b)

Re: [testsuite][ARM target attributes] Fix effective_target tests

[Kyrill Tkachov]

Hi Christophe,

On 04/01/16 14:21, Christophe Lyon wrote:
> On 4 January 2016 at 15:20, Christophe Lyon <christophe.lyon@linaro.org> wrote:
>> On 18 December 2015 at 15:16, Kyrill Tkachov
>> <kyrylo.tkachov@foss.arm.com> wrote:
>>> Hi Christophe,
>>>
>>>
>>> On 17/12/15 22:17, Christophe Lyon wrote:
>>>> Hi,
>>>>
>>>> Here is an updated version of this patch.
>>>> I did test it with
>>>> -mthumb/-march=armv8-a/-mfpu=crypto-neon-fp-armv8/-mfloat-abi=hard in
>>>> addition to my usual set of options.
>>>>
>>>> Compared to the previous version:
>>>> - I added some doc in sourcebuild.texi
>>>> - I no longer modify arm_vfp_ok...
>>>> - I replaced all uses of arm_vfp with the new arm_fp because I found
>>>> that the existing tests do not actually need to pass -mfpu=vfp: this
>>>> is implicitly set as the default when using -mfloat-abi={softfp|hard}
>>>> - I chose not to remove arm_vfp_ok because we may need it in the
>>>> future, if a test really needs vfp (as opposed to neon for instance)
>>>> - in gcc.target/arm/attr-crypto.c I force the initial fpu to be vfp
>>>> via pragma instead, so that the next pragma fpu
>>>> fpu=crypto-neon-fp-armv8 is always compatible, regardless of the
>>>> command-line options/default fpu
>>>> - same for attr-neon2.c and attr-neon3.c
>>>> - I updated cmp-2.c, unsigned-float.c, vfp-1.c, vfp-ldmdbd.c,
>>>> vfp-ldmdbs.c, vfp-ldmiad.c, vfp-ldmias.c, vfp-stmdbd.c, vfp-stmdbs.c,
>>>> vfp-stmiad.c, vfp-stmias.c, vnmul-[1234].c to use the new arm_fp
>>>> effective target instead of arm_vfp. This is so that they don't need
>>>> to use -mfpu=vfp and can use the new dg-add-options arm_fp
>>>>
>>>> The validation results show (in addition to what I originally reported):
>>>> - attr-crypto.c and attr-neon3.c now ICE in some cases. This is PR68895.
>>>> - depending on the GCC configuration (e.g. --with-fpu=neon)
>>>> attr-neon3.c may fail. This is PR68896.
>>>>
>>>> OK?
>>>
>>> Thanks for following up on this.
>>> I think you also need to document the new arm_crypto_pragma_ok.
>>>
>> Indeed, I forgot it.
>>
>> Here is a new version of the patch with a few words added to document
>> this function.
>> I did not modify the testcase after Christian's comments and
>> PR68934: my understanding is that the testscase are valid after
>> all and Christian is working on fixing the ICE.
>>
> With the attachment, this time...

This is ok now.
Sorry for the delay.
I believe Christian is working on the ICEs exposed by this, so we should be ok.

Thanks,
Kyrill

>
>> 2016-01-04  Christophe Lyon  <christophe.lyon@linaro.org>
>>
>>      * doc/sourcebuild.texi (arm_crypto_pragma_ok): Document new entry.
>>      (arm_fp_ok): Likewise.
>>      (arm_fp): Likewise.
>>      (arm_crypto): Likewise.
>>      * lib/target-supports.exp
>>      (check_effective_target_arm_fp_ok_nocache): New.
>>      (check_effective_target_arm_fp_ok): New.
>>      (add_options_for_arm_fp): New.
>>      (check_effective_target_arm_crypto_ok_nocache): Require
>>      target_arm_v8_neon_ok instead of arm32.
>>      (check_effective_target_arm_crypto_pragma_ok_nocache): New.
>>      (check_effective_target_arm_crypto_pragma_ok): New.
>>      (add_options_for_arm_vfp): New.
>>      * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok effective
>>      target. Do not force -mfloat-abi=softfp, use arm_fp_ok effective
>>      target instead. Force initial fpu to vfp.
>>      * gcc.target/arm/attr-neon-builtin-fail.c: Do not force
>>      -mfloat-abi=softfp, use arm_fp_ok effective target instead.
>>      * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove arm_neon_ok
>>      dependency.
>>      * gcc.target/arm/attr-neon2.c: Do not force -mfloat-abi=softfp,
>>      use arm_vfp effective target instead. Force initial fpu to vfp.
>>      * gcc.target/arm/attr-neon3.c: Likewise.
>>      * gcc.target/arm/cmp-2.c: Use arm_fp_ok effective target instead of
>>      arm_vfp_ok.
>>      * gcc.target/arm/unsigned-float.c: Likewise.
>>      * gcc.target/arm/vfp-1.c: Likewise.
>>      * gcc.target/arm/vfp-ldmdbd.c: Likewise.
>>      * gcc.target/arm/vfp-ldmdbs.c: Likewise.
>>      * gcc.target/arm/vfp-ldmiad.c: Likewise.
>>      * gcc.target/arm/vfp-ldmias.c: Likewise.
>>      * gcc.target/arm/vfp-stmdbd.c: Likewise.
>>      * gcc.target/arm/vfp-stmdbs.c: Likewise.
>>      * gcc.target/arm/vfp-stmiad.c: Likewise.
>>      * gcc.target/arm/vfp-stmias.c: Likewise.
>>      * gcc.target/arm/vnmul-1.c: Likewise.
>>      * gcc.target/arm/vnmul-2.c: Likewise.
>>      * gcc.target/arm/vnmul-3.c: Likewise.
>>      * gcc.target/arm/vnmul-4.c: Likewise.
>>
>> OK?
>>
>> Christophe.
>>
>>
>>> Kyrill
>>>
>>>
>>>> Christophe
>>>>
>>>> 2015-12-17  Christophe Lyon  <christophe.lyon@linaro.org>
>>>>
>>>>       * doc/sourcebuild.texi (arm_fp_ok): Document new entry.
>>>>       (arm_fp): Likewise.
>>>>       * lib/target-supports.exp
>>>>       (check_effective_target_arm_fp_ok_nocache): New.
>>>>       (check_effective_target_arm_fp_ok): New.
>>>>       (add_options_for_arm_fp): New.
>>>>       (check_effective_target_arm_crypto_ok_nocache): Require
>>>>       target_arm_v8_neon_ok instead of arm32.
>>>>       (check_effective_target_arm_crypto_pragma_ok_nocache): New.
>>>>       (check_effective_target_arm_crypto_pragma_ok): New.
>>>>       (add_options_for_arm_vfp): New.
>>>>       * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok effective
>>>>       target. Do not force -mfloat-abi=softfp, use arm_fp_ok effective
>>>>       target instead. Force initial fpu to vfp.
>>>>       * gcc.target/arm/attr-neon-builtin-fail.c: Do not force
>>>>       -mfloat-abi=softfp, use arm_fp_ok effective target instead.
>>>>       * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove arm_neon_ok
>>>>       dependency.
>>>>       * gcc.target/arm/attr-neon2.c: Do not force -mfloat-abi=softfp,
>>>>       use arm_vfp effective target instead. Force initial fpu to vfp.
>>>>       * gcc.target/arm/attr-neon3.c: Likewise.
>>>>       * gcc.target/arm/cmp-2.c: Use arm_fp_ok effective target instead of
>>>>       arm_vfp_ok.
>>>>       * gcc.target/arm/unsigned-float.c: Likewise.
>>>>       * gcc.target/arm/vfp-1.c: Likewise.
>>>>       * gcc.target/arm/vfp-ldmdbd.c: Likewise.
>>>>       * gcc.target/arm/vfp-ldmdbs.c: Likewise.
>>>>       * gcc.target/arm/vfp-ldmiad.c: Likewise.
>>>>       * gcc.target/arm/vfp-ldmias.c: Likewise.
>>>>       * gcc.target/arm/vfp-stmdbd.c: Likewise.
>>>>       * gcc.target/arm/vfp-stmdbs.c: Likewise.
>>>>       * gcc.target/arm/vfp-stmiad.c: Likewise.
>>>>       * gcc.target/arm/vfp-stmias.c: Likewise.
>>>>       * gcc.target/arm/vnmul-1.c: Likewise.
>>>>       * gcc.target/arm/vnmul-2.c: Likewise.
>>>>       * gcc.target/arm/vnmul-3.c: Likewise.
>>>>       * gcc.target/arm/vnmul-4.c: Likewise.
>>>>
>>>>
>>>>
>>>> On 10 December 2015 at 20:52, Christophe Lyon
>>>> <christophe.lyon@linaro.org> wrote:
>>>>> On 10 December 2015 at 14:14, Kyrill Tkachov <kyrylo.tkachov@arm.com>
>>>>> wrote:
>>>>>> On 10/12/15 13:04, Christophe Lyon wrote:
>>>>>>> On 10 December 2015 at 13:30, Kyrill Tkachov <kyrylo.tkachov@arm.com>
>>>>>>> wrote:
>>>>>>>> Hi Christophe,
>>>>>>>>
>>>>>>>>
>>>>>>>> On 08/12/15 11:18, Christophe Lyon wrote:
>>>>>>>>> On 8 December 2015 at 11:50, Kyrill Tkachov <kyrylo.tkachov@arm.com>
>>>>>>>>> wrote:
>>>>>>>>>> Hi Christophe,
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 27/11/15 13:00, Christophe Lyon wrote:
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> After the recent commits from Christian adding target attributes
>>>>>>>>>>> support for ARM FPU settings,  I've noticed that some of the tests
>>>>>>>>>>> were failing because of incorrect assumptions wrt to the default
>>>>>>>>>>> cpu/fpu/float-abi of the compiler.
>>>>>>>>>>>
>>>>>>>>>>> This patch fixes the problems I've noticed in the following way:
>>>>>>>>>>> - do not force -mfloat-abi=softfp in dg-options, to avoid conflicts
>>>>>>>>>>> when gcc is configured --with-float=hard
>>>>>>>>>>>
>>>>>>>>>>> - change arm_vfp_ok such that it tries several -mfpu/-mfloat-abi
>>>>>>>>>>> flags, checks that __ARM_FP is defined and __ARM_NEON_FP is not
>>>>>>>>>>> defined
>>>>>>>>>>>
>>>>>>>>>>> - introduce arm_fp_ok, which is similar but does not enforce fpu
>>>>>>>>>>> setting
>>>>>>>>>>>
>>>>>>>>>>> - add a new effective_target: arm_crypto_pragma_ok to check that
>>>>>>>>>>> setting this fpu via a pragma is actually supported by the current
>>>>>>>>>>> "multilib". This is different from checking the command-line option
>>>>>>>>>>> because the pragma might conflict with the command-line options in
>>>>>>>>>>> use.
>>>>>>>>>>>
>>>>>>>>>>> The updates in the testcases are as follows:
>>>>>>>>>>> - attr-crypto.c, we have to make sure that the defaut fpu does not
>>>>>>>>>>> conflict with the one forced by pragma. That's why I use the
>>>>>>>>>>> arm_vfp
>>>>>>>>>>> options/effective_target. This is needed if gcc has been configured
>>>>>>>>>>> --with-fpu=neon-fp16, as the pragma fpu=crypto-neon-fp-armv8 would
>>>>>>>>>>> conflict.
>>>>>>>>>>>
>>>>>>>>>>> - attr-neon-builtin-fail.c: use arm_fp to force the appropriate
>>>>>>>>>>> float-abi setting. Enforcing fpu is not needed here.
>>>>>>>>>>>
>>>>>>>>>>> - attr-neon-fp16.c: similar, I also removed arm_neon_ok since it
>>>>>>>>>>> was
>>>>>>>>>>> not necessary to make the test pass in my testing. On second
>>>>>>>>>>> thought,
>>>>>>>>>>> I'm wondering whether I should leave it and make the test
>>>>>>>>>>> unsupported
>>>>>>>>>>> in more cases (such as when forcing -march=armv5t, although it does
>>>>>>>>>>> pass with this patch)
>>>>>>>>>>>
>>>>>>>>>>> - attr-neon2.c: use arm_vfp to force the appropriate float-abi
>>>>>>>>>>> setting. Enforcing mfpu=vfp is needed to avoid conflict with the
>>>>>>>>>>> pragma target fpu=neon (for instance if the toolchain default is
>>>>>>>>>>> neon-fp16)
>>>>>>>>>>>
>>>>>>>>>>> - attr-neon3.c: similar
>>>>>>>>>>>
>>>>>>>>>>> Tested on a variety of configurations, see:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/230929-target-attr/report-build-info.html
>>>>>>>>>>>
>>>>>>>>>>> Note that the regressions reported fall into 3 categories:
>>>>>>>>>>> - when forcing march=armv5t: tests are now unsupported because I
>>>>>>>>>>> modified arm_crypto_ok to require arm_v8_neon_ok instead of arm32.
>>>>>>>>>>>
>>>>>>>>>>> - the warning reported by attr-neon-builtin-fail.c moved from line
>>>>>>>>>>> 12
>>>>>>>>>>> to 14 and is thus seen as a regression + one improvement
>>>>>>>>>>>
>>>>>>>>>>> - finally, attr-neon-fp16.c causes an ICE on armeb compilers, for
>>>>>>>>>>> which I need to post a bugzilla.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> TBH, I'm a bit concerned by the complexity of all these
>>>>>>>>>>> multilib-like
>>>>>>>>>>> conditions. I'm confident that I'm still missing some combinations
>>>>>>>>>>> :-)
>>>>>>>>>>>
>>>>>>>>>>> And with new target attributes coming, new architectures etc... all
>>>>>>>>>>> this logic is likely to become even more complex.
>>>>>>>>>>>
>>>>>>>>>>> That being said, OK for trunk?
>>>>>>>>>>
>>>>>>>>>> I'd like us to clean up and reorganise the gcc.target/arm testsuite
>>>>>>>>>> at some point to make it more robust with respect to the tons of
>>>>>>>>>> multilib
>>>>>>>>>> options and configurations we can have for arm. It's becoming very
>>>>>>>>>> frustrating
>>>>>>>>>> to test feature-specific stuff :(
>>>>>>>>>>
>>>>>>>>>> This is ok in the meantime.
>>>>>>>>>> Sorry for the delay.
>>>>>>>>>>
>>>>>>>>> Thanks for the approval, glad to see I'm not the only one willing to
>>>>>>>>> see
>>>>>>>>> improvements in this area :)
>>>>>>>>>
>>>>>>>>> Committed as r231403.
>>>>>>>>
>>>>>>>> With this patch we're seeing legitimate PASSes go to NA.
>>>>>>>> For example:
>>>>>>>>
>>>>>>>> gcc.target/arm/vfp-1.c
>>>>>>>> gcc.target/arm/vfp-ldmdbs.c
>>>>>>>> and other vfp tests in arm.exp.
>>>>>>>> This is, for example, for the
>>>>>>>>
>>>>>>>> variant-mthumb/-march=armv8-a/-mfpu=crypto-neon-fp-armv8/-mfloat-abi=hard
>>>>>>>>
>>>>>>> Hmm I'm attempting to cover such a configuration in my matrix of
>>>>>>> validations:
>>>>>>>
>>>>>>>
>>>>>>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/231403/report-build-info.html
>>>>>>>
>>>>>>> The difference is that I use similar flags at GCC configure time, while
>>>>>>> you
>>>>>>> override them when running the testsuite:
>>>>>>> --target arm-none-linux-gnueabihf --with-mode=thum
>>>>>>> --with-cpu=cortex-a57
>>>>>>> --with-fpu=crypto-neon-fp-armv8
>>>>>>>
>>>>>>> I this case, I do not see the regressions.
>>>>>>
>>>>>> My gcc is arm-none-eabi configured with
>>>>>> --with-float=hard --with-cpu=cortex-a15 --with-fpu=neon-vfpv4
>>>>>> --with-float=hard
>>>>>>
>>>>>>>> I suspect under your new predicates they'd need to be changed to check
>>>>>>>> for
>>>>>>>> arm_fp_ok rather than arm_vfp_ok.
>>>>>>> Probably, yes.
>>>>>>>
>>>>>> In the test log where it checks the effective target it fails due to:
>>>>>> arm_vfp_ok27268.c:6:4: error: #error __ARM_NEON_FP defined
>>>>>> it's compiling the check with
>>>>>> -mthumb -march=armv8-a -mfpu=crypto-neon-fp-armv8 -mfloat-abi=hard -o
>>>>>> arm_vfp_ok27268.o arm_vfp_ok27268.c
>>>>>>
>>>>>>>> We want to avoid removing any PASSes.
>>>>>>>> Can you please test some more to make sure we don't remove any
>>>>>>>> legitimate
>>>>>>>> passes with your patch?
>>>>>>> Is that the only combination in which you saw less PASSes?
>>>>>>
>>>>>> That's the one that was brought to my attention, but I think the issue
>>>>>> here
>>>>>> is just the
>>>>>> tests that check for arm_vfp_ok rather than the new arm_fp_ok and set
>>>>>> -mfpu=vfp explicitly.
>>>>>> They appear unsupported if testing with an explicit neon option in mfpu,
>>>>>> I
>>>>>> think.
>>>>>>
>>>>>>>> Also, Ramana reminded me offline that any new predicates in
>>>>>>>> target-supports.exp
>>>>>>>> need documenting in sourcebuild.txt.
>>>>>>>>
>>>>>>>> In light of that, can you please revert your patch and address the
>>>>>>>> issues
>>>>>>>> above
>>>>>>>> so that we can be confident we don't lose existing legitimate test
>>>>>>>> coverage?
>>>>>>> OK.
>>>>>>>
>>>>>>>> Sorry for not catching these sooner.
>>>>>>> No problem, there are so many combinations.
>>>>>>> I'm not sure how to define a reasonable set.
>>>>>>
>>>>>> So, I think the action plan here is to audit the tests that need to be
>>>>>> changed
>>>>>> to arm_fp_ok and add the documentation for the new predicate checks.
>>>>>>
>>>>> I came up with a new version where I now use only the new arm_fp_ok, so
>>>>> it might be easier just to update arm_vfp_ok and not introduce arm_fp_ok.
>>>>>
>>>>> However, I'm still not happy with this version because it has problems
>>>>> with
>>>>> a few tests that use target attributes such as attr-crypto.c, which
>>>>> starts with:
>>>>> #pragma GCC target ("fpu=crypto-neon-fp-armv8")
>>>>>
>>>>> This pragma fails if the compiler+options have set an incompatible
>>>>> fpu before processing the pragma.
>>>>>
>>>>> So for instance, if gcc has been configured --with-fpu=neon, compiling
>>>>> the test
>>>>> with no fpu option fails (neon is not compatible with
>>>>> crypto-neon-fp-armv8
>>>>> because the latter has fp16)
>>>>>
>>>>> Now, if we use effective_target arm_vfp_ok + dg_option to force
>>>>> -mfpu=vfp,
>>>>> we end up compiling with -mfpu=vfp and the pragma passes.
>>>>>
>>>>> But if, in addition, we force the testflags to set
>>>>> -mfpu=crypto-neon-fp-armv8
>>>>> as you do, the effective_target arm_vfp_ok test now fails because it is
>>>>> compiled with -mfpu=vfp -mfpu=crypto-neon-fp-armv8, which defines
>>>>> __ARM_NEON_FP
>>>>>
>>>>> In short, the problem is how to make sure that the fpu setting is always
>>>>> compatible with the pragma, whatever the gcc configuration and multilib
>>>>> options used.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Christophe.
>>>>>
>>>>>> Thanks,
>>>>>> Kyrill
>>>>>>
>>>>>>
>>>>>>> Christophe.
>>>>>>>
>>>>>>>> Kyrill
>>>>>>>>
>>>>>>>>
>>>>>>>>> Christophe.
>>>>>>>>>
>>>>>>>>>> Thanks for handling this!
>>>>>>>>>> Kyrill
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> Christophe
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> 2015-11-27  Christophe Lyon  <christophe.lyon@linaro.org>
>>>>>>>>>>>
>>>>>>>>>>>          * lib/target-supports.exp
>>>>>>>>>>>          (check_effective_target_arm_vfp_ok_nocache): New.
>>>>>>>>>>>          (check_effective_target_arm_vfp_ok): Call the new
>>>>>>>>>>>          check_effective_target_arm_vfp_ok_nocache function.
>>>>>>>>>>>          (check_effective_target_arm_fp_ok_nocache): New.
>>>>>>>>>>>          (check_effective_target_arm_fp_ok): New.
>>>>>>>>>>>          (add_options_for_arm_fp): New.
>>>>>>>>>>>          (check_effective_target_arm_crypto_ok_nocache): Require
>>>>>>>>>>>          target_arm_v8_neon_ok instead of arm32.
>>>>>>>>>>>          (check_effective_target_arm_crypto_pragma_ok_nocache): New.
>>>>>>>>>>>          (check_effective_target_arm_crypto_pragma_ok): New.
>>>>>>>>>>>          (add_options_for_arm_vfp): New.
>>>>>>>>>>>          * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok
>>>>>>>>>>> effective
>>>>>>>>>>>          target. Do not force -mfloat-abi=softfp, use arm_vfp
>>>>>>>>>>> effective
>>>>>>>>>>>          target instead.
>>>>>>>>>>>          * gcc.target/arm/attr-neon-builtin-fail.c: Do not force
>>>>>>>>>>>          -mfloat-abi=softfp, use arm_fp effective target instead.
>>>>>>>>>>>          * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove
>>>>>>>>>>> arm_neon_ok
>>>>>>>>>>>          dependency.
>>>>>>>>>>>          * gcc.target/arm/attr-neon2.c: Do not force
>>>>>>>>>>> -mfloat-abi=softfp,
>>>>>>>>>>>          use arm_vfp effective target instead.
>>>>>>>>>>>          * gcc.target/arm/attr-neon3.c: Likewise.
>>>>>>>>>>