diff mbox

[testsuite] skip ARM neon-fp16 tests for other -mcpu values

Message ID 4DF954EE.30802@codesourcery.com
State New
Headers show

Commit Message

Janis Johnson June 16, 2011, 12:57 a.m. UTC 
On 06/15/2011 05:53 AM, Richard Earnshaw wrote:
> On 10/06/11 00:43, Janis Johnson wrote:
>> On 06/08/2011 03:17 AM, Joseph S. Myers wrote:
>>> On Tue, 7 Jun 2011, Janis Johnson wrote:
>>>
>>>> These tests fail when multilib options use -mfpu=xxxx and override the
>>>> -mfpu=neon-fp16 used for the test:
>>>>
>>>>   g++.dg/ext/arm-fp16/arm-fp16-ops-5.C
>>>>   g++.dg/ext/arm-fp16/arm-fp16-ops-6.C
>>>>   gcc.dg/torture/arm-fp16-ops-5.c
>>>>   gcc.dg/torture/arm-fp16-ops-6.c
>>>>   gcc.target/arm/fp16-compile-vcvt.c
>>>>
>>>> The option -mfpu-neon-fp16 is added via "dg-add-options arm_neon_fp16"
>>>> after an earlier "dg-require-effective-target arm_neon_fp16_ok".
>>>> This patch modifies check_effective_target_arm_neon_fp16_ok_nocache to
>>>> return 0 (causing the test to be skipped) if multilib flags include
>>>> -mfpu= with a value other than neon-fp16.
>>>
>>> But I'd think they ought to work with any -mfpu= option supporting 
>>> half-precision instructions - that is, vfpv3-fp16, vfpv3-d16-fp16, 
>>> vfpv3xd-fp16, neon-fp16, vfpv4, vfpv4-d16, fpv4-sp-d16, neon-vfpv4 
>>> (anything with "true" in the last field in arm-fpus.def; for the 
>>> testsuite, you can probably suppose anything -mfpu=*fp16*, 
>>> -mfpu=*fpv[4-9]*, -mfpu=*fpv[1-9][0-9]*).
>>>
>>
>> These tests look for specific instructions being generated and fail when
>> the check is hacked up to allow other fp16 variants and not require neon.
>> I'd like to use the original patch for this.  OK?
>>
>> Janis
> 
> That sounds like you might have uncovered a bug.  Can you provide some
> more detail?
> 
> Running (manually) gcc.target/arm/fp16-compile-vcvt.c with "-O
> -mfpu=vfpv4 -mfp16-format=ieee" gives the instructions required by the
> scanner.
> 
> R.

The bug was in my attempt to run the tests with other -mfpu values, so
I'm very glad you caught that.  I tried again, getting rid of the neon
requirement along the way, and found a way to run the VFP fp16 tests
with any of the fp16 values that Joseph listed.

This patch renames *arm_neon_fp16* to *arm_fp16* and skips tests if the
multilib does not support arm32, includes -mfpu that is not fp16, or
includes -mfloat-abi=soft.  If the multilib uses -mfpu= with an fp16
value then that is used, otherwise -mfpu=vfpv4 is used.  Added flags
include -mfloat-abi=softfp in case the default is "soft".

Tested on arm-none-linux-gnueabi with multilib test flags (not actual
runtime support) for each -march value and each fp16 -mfpu value.

OK for trunk, and for 4.6 a few days later?

Janis
2011-06-15  Janis Johnson  <janisjo@codesourcery.com>

	* lib/target-supports.exp (add_options_for_arm_fp16): Renamed
	from add_options_for_arm_neon_fp16.
	(check_effective_target_arm_fp16_ok_nocache): Renamed from
	check_effective_target_arm_neon_fp16_ok_nocache.
	Check -mfpu and -mfloat-abi options from current multilib.
	Do not require neon support.
	(check_effective_target_arm_fp16_ok): Renamed from
	check_effecitve_target_arm_neon_fp16_ok.
	* g++.dg/ext/arm-fp16/arm-fp16-ops-5.C: Use new names for
	arm_neon_fp16_ok and arm_fp16.
	* g++.dg/ext/arm-fp16/arm-fp16-ops-6.C: Likewise.
	* gcc.dg/torture/arm-fp16-ops-5.c: Likewise.
	* gcc.dg/torture/arm-fp16-ops-6.c: Likewise.
	* gcc.target/arm/fp16-compile-vcvt.c: Likewise.

Comments

Mike Stump June 25, 2011, 11:56 p.m. UTC | #1 
On Jun 15, 2011, at 5:57 PM, Janis Johnson wrote:
> The bug was in my attempt to run the tests with other -mfpu values, so
> I'm very glad you caught that.  I tried again, getting rid of the neon
> requirement along the way, and found a way to run the VFP fp16 tests
> with any of the fp16 values that Joseph listed.
> 
> This patch renames *arm_neon_fp16* to *arm_fp16* and skips tests if the
> multilib does not support arm32, includes -mfpu that is not fp16, or
> includes -mfloat-abi=soft.  If the multilib uses -mfpu= with an fp16
> value then that is used, otherwise -mfpu=vfpv4 is used.  Added flags
> include -mfloat-abi=softfp in case the default is "soft".

> OK for trunk, and for 4.6 a few days later?

Ok.  Ok for 4.6.  For 4.6, as also please ensure that the RMs don't have the branch locked down.

General comment, I'm happy to have the front-end, target and library maintainers review and approve the normal additions to the .exp files to support testing their bits.
Janis Johnson June 27, 2011, 2:15 a.m. UTC | #2 
On 06/25/2011 04:56 PM, Mike Stump wrote:
> On Jun 15, 2011, at 5:57 PM, Janis Johnson wrote:
>> The bug was in my attempt to run the tests with other -mfpu values, so
>> I'm very glad you caught that.  I tried again, getting rid of the neon
>> requirement along the way, and found a way to run the VFP fp16 tests
>> with any of the fp16 values that Joseph listed.
>>
>> This patch renames *arm_neon_fp16* to *arm_fp16* and skips tests if the
>> multilib does not support arm32, includes -mfpu that is not fp16, or
>> includes -mfloat-abi=soft.  If the multilib uses -mfpu= with an fp16
>> value then that is used, otherwise -mfpu=vfpv4 is used.  Added flags
>> include -mfloat-abi=softfp in case the default is "soft".
> 
>> OK for trunk, and for 4.6 a few days later?
> 
> Ok.  Ok for 4.6.  For 4.6, as also please ensure that the RMs don't have the branch locked down.

I'm waiting until after the 4.6.1 release to move anything more to that branch.

> General comment, I'm happy to have the front-end, target and library
> maintainers review and approve the normal additions to the .exp files
> to support testing their bits.

Yep!

Janis
diff mbox

Patch

Index: lib/target-supports.exp
===================================================================
--- lib/target-supports.exp	(revision 175083)
+++ lib/target-supports.exp	(working copy)
@@ -1947,45 +1947,53 @@ 
 # or -mfloat-abi=hard, but if one is already specified by the
 # multilib, use it.
 
-proc add_options_for_arm_neon_fp16 { flags } {
-    if { ! [check_effective_target_arm_neon_fp16_ok] } {
+proc add_options_for_arm_fp16 { flags } {
+    if { ! [check_effective_target_arm_fp16_ok] } {
 	return "$flags"
     }
-    global et_arm_neon_fp16_flags
-    return "$flags $et_arm_neon_fp16_flags"
+    global et_arm_fp16_flags
+    return "$flags $et_arm_fp16_flags"
 }
 
-# Return 1 if this is an ARM target supporting -mfpu=neon-fp16
-# -mfloat-abi=softfp or equivalent options.  Skip multilibs that are
-# incompatible with these options.  Also set et_arm_neon_flags to the
-# best options to add.
+# Return 1 if this is an ARM target that can support a VFP fp16 variant.
+# Skip multilibs that are incompatible with these options and set
+# et_arm_fp16_flags to the best options to add.
 
-proc check_effective_target_arm_neon_fp16_ok_nocache { } {
-    global et_arm_neon_fp16_flags
-    set et_arm_neon_fp16_flags ""
-    if { [check_effective_target_arm32] } {
-	if [check-flags [list "" { *-*-* } { "-mfpu=*" } { "-mfpu=neon-fp16*" } ]] {
-	    return 0
-	}
-	# Always add -mfpu=neon-fp16, since there is no preprocessor
-	# macro for FP16 support.
-	foreach flags {"-mfpu=neon-fp16" "-mfpu=neon-fp16 -mfloat-abi=softfp"} {
-	    if { [check_no_compiler_messages_nocache arm_neon_fp16_ok object {
-		#include "arm_neon.h"
-		int dummy;
-	    } "$flags"] } {
-		set et_arm_neon_fp16_flags $flags
-		return 1
-	    }
-	}
+proc check_effective_target_arm_fp16_ok_nocache { } {
+    global et_arm_fp16_flags
+    set et_arm_fp16_flags ""
+    if { ! [check_effective_target_arm32] } {
+	return 0;
     }
+    if [check-flags [list "" { *-*-* } { "-mfpu=*" } { "-mfpu=*fp16*" "-mfpu=*fpv[4-9]*" "-mfpu=*fpv[1-9][0-9]*" } ]] {
+	# Multilib flags would override -mfpu.
+	return 0
+    }
+    if [check-flags [list "" { *-*-* } { "-mfloat-abi=soft" } { "" } ]] {
+	# Must generate floating-point instructions.
+	return 0
+    }
+    if [check-flags [list "" { *-*-* } { "-mfpu=*" } { "" } ]] {
+        # The existing -mfpu value is OK; use it, but add softfp.
+	set et_arm_fp16_flags "-mfloat-abi=softfp"
+	return 1;
+    }
+    # Add -mfpu for a VFP fp16 variant since there is no preprocessor
+    # macro to check for this support.
+    set flags "-mfpu=vfpv4 -mfloat-abi=softfp"
+    if { [check_no_compiler_messages_nocache arm_fp16_ok assembly {
+	int dummy;
+    } "$flags"] } {
+	set et_arm_fp16_flags "$flags"
+	return 1
+    }
 
     return 0
 }
 
-proc check_effective_target_arm_neon_fp16_ok { } {
-    return [check_cached_effective_target arm_neon_fp16_ok \
-		check_effective_target_arm_neon_fp16_ok_nocache]
+proc check_effective_target_arm_fp16_ok { } {
+    return [check_cached_effective_target arm_fp16_ok \
+		check_effective_target_arm_fp16_ok_nocache]
 }
 
 # Return 1 is this is an ARM target where -mthumb causes Thumb-1 to be
Index: g++.dg/ext/arm-fp16/arm-fp16-ops-5.C
===================================================================
--- g++.dg/ext/arm-fp16/arm-fp16-ops-5.C	(revision 175083)
+++ g++.dg/ext/arm-fp16/arm-fp16-ops-5.C	(working copy)
@@ -1,8 +1,8 @@ 
 /* Test various operators on __fp16 and mixed __fp16/float operands.  */
 /* { dg-do compile { target arm*-*-* } } */
-/* { dg-require-effective-target arm_neon_fp16_ok } */
+/* { dg-require-effective-target arm_fp16_ok } */
 /* { dg-options "-mfp16-format=ieee" } */
-/* { dg-add-options arm_neon_fp16 } */
+/* { dg-add-options arm_fp16 } */
 
 #include "arm-fp16-ops.h"
 
Index: g++.dg/ext/arm-fp16/arm-fp16-ops-6.C
===================================================================
--- g++.dg/ext/arm-fp16/arm-fp16-ops-6.C	(revision 175083)
+++ g++.dg/ext/arm-fp16/arm-fp16-ops-6.C	(working copy)
@@ -1,8 +1,8 @@ 
 /* Test various operators on __fp16 and mixed __fp16/float operands.  */
 /* { dg-do compile { target arm*-*-* } } */
-/* { dg-require-effective-target arm_neon_fp16_ok } */
+/* { dg-require-effective-target arm_fp16_ok } */
 /* { dg-options "-mfp16-format=ieee -ffast-math" } */
-/* { dg-add-options arm_neon_fp16 } */
+/* { dg-add-options arm_fp16 } */
 
 #include "arm-fp16-ops.h"
 
Index: gcc.dg/torture/arm-fp16-ops-5.c
===================================================================
--- gcc.dg/torture/arm-fp16-ops-5.c	(revision 175083)
+++ gcc.dg/torture/arm-fp16-ops-5.c	(working copy)
@@ -1,8 +1,8 @@ 
 /* Test various operators on __fp16 and mixed __fp16/float operands.  */
 /* { dg-do compile { target arm*-*-* } } */
-/* { dg-require-effective-target arm_neon_fp16_ok } */
+/* { dg-require-effective-target arm_fp16_ok } */
 /* { dg-options "-mfp16-format=ieee" } */
-/* { dg-add-options arm_neon_fp16 } */
+/* { dg-add-options arm_fp16 } */
 
 #include "arm-fp16-ops.h"
 
Index: gcc.dg/torture/arm-fp16-ops-6.c
===================================================================
--- gcc.dg/torture/arm-fp16-ops-6.c	(revision 175083)
+++ gcc.dg/torture/arm-fp16-ops-6.c	(working copy)
@@ -1,8 +1,8 @@ 
 /* Test various operators on __fp16 and mixed __fp16/float operands.  */
 /* { dg-do compile { target arm*-*-* } } */
-/* { dg-require-effective-target arm_neon_fp16_ok } */
+/* { dg-require-effective-target arm_fp16_ok } */
 /* { dg-options "-mfp16-format=ieee -ffast-math" } */
-/* { dg-add-options arm_neon_fp16 } */
+/* { dg-add-options arm_fp16 } */
 
 #include "arm-fp16-ops.h"
 
Index: gcc.target/arm/fp16-compile-vcvt.c
===================================================================
--- gcc.target/arm/fp16-compile-vcvt.c	(revision 175083)
+++ gcc.target/arm/fp16-compile-vcvt.c	(working copy)
@@ -1,7 +1,7 @@ 
 /* { dg-do compile } */
-/* { dg-require-effective-target arm_neon_fp16_ok } */
+/* { dg-require-effective-target arm_fp16_ok } */
 /* { dg-options "-mfp16-format=ieee" } */
-/* { dg-add-options arm_neon_fp16 } */
+/* { dg-add-options arm_fp16 } */
 
 /* Test generation of VFP __fp16 instructions.  */