[PATCH], Add check ppc_cpu_supports_hw to testsuite

[PATCH], Add check ppc_cpu_supports_hw to testsuite

[Michael Meissner]

The PowerPC __builtin_cpu_supports and __builtin_cpu_is built-in functions
require GLIBC 2.23, since they use fixed words at the end of thread control
area to store the HWCAP and HWCAP2 bits.  If the compiler was not configured
with the appropriate GLIBC, the compiler will generate a 0 as the result of the
built-in function call.

I've been adding the target_clone attribute support to GCC, and the resolver
function uses __builtin_cpu_supports to detect which hardware ISA is being
used.  On systems with an older GLIBC, only the default clone function will get
called because __builtin_cpu_supports returns 0.

This adds a target supports option in dejagnu so that future tests can use this
to determine whether or not to test target_clones.

I have verified that this patch works with the patches I plan to submit
tomorrow for enhancing the PowerPC target_clone support.

Can I install this into the trunk?

Given that GCC 7 supports __builtin_cpu_is and __builtin_cpu_supports, I would
ask if I could backport this to GCC 7.x as well to allow future tests to be
back ported.

2017-06-27  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/81193
	* lib/target-supports.exp
	(check_ppc_cpu_supports_hw_available): New test to make sure
	__builtin_cpu_supports works on power7 and newer.

Index: gcc/testsuite/lib/target-supports.exp
===================================================================
--- gcc/testsuite/lib/target-supports.exp	(revision 249606)
+++ gcc/testsuite/lib/target-supports.exp	(working copy)
@@ -1930,6 +1930,37 @@ proc check_effective_target_powerpc64_no
 	} {-O2}]
 }
 
+# Return 1 if the target supports the __builtin_cpu_supports built-in,
+# including having a new enough library to support the test.  Cache the result.
+# Require at least a power7 to run on.
+
+proc check_ppc_cpu_supports_hw_available { } {
+    return [check_cached_effective_target ppc_cpu_supports_hw_available {
+	# Some simulators are known to not support VSX/power8 instructions.
+	# For now, disable on Darwin
+	if { [istarget powerpc-*-eabi]
+	     || [istarget powerpc*-*-eabispe]
+	     || [istarget *-*-darwin*]} {
+	    expr 0
+	} else {
+	    set options "-mvsx"
+	    check_runtime_nocache ppc_cpu_supports_hw_available {
+		int main()
+		{
+		#ifdef __MACH__
+		  asm volatile ("xxlor vs0,vs0,vs0");
+		#else
+		  asm volatile ("xxlor 0,0,0");
+	        #endif
+		  if (!__builtin_cpu_supports ("vsx"))
+		    return 1;
+		  return 0;
+		}
+	    } $options
+	}
+    }]
+}
+
 # Return 1 if the target supports executing power8 vector instructions, 0
 # otherwise.  Cache the result.
 
@@ -6922,6 +6953,7 @@ proc is-effective-target { arg } {
 	  "ppc_float128_sw" { set selected [check_ppc_float128_sw_available] }
 	  "ppc_float128_hw" { set selected [check_ppc_float128_hw_available] }
 	  "ppc_recip_hw"   { set selected [check_ppc_recip_hw_available] }
+	  "ppc_cpu_supports_hw" { set selected [check_ppc_cpu_supports_hw_available] }
 	  "dfp_hw"         { set selected [check_dfp_hw_available] }
 	  "htm_hw"         { set selected [check_htm_hw_available] }
 	  "named_sections" { set selected [check_named_sections_available] }

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.vnet.ibm.com, phone: +1 (978) 899-4797

Re: [PATCH], Add check ppc_cpu_supports_hw to testsuite

[Jeff Law]

On 06/27/2017 05:53 PM, Michael Meissner wrote:
> The PowerPC __builtin_cpu_supports and __builtin_cpu_is built-in functions
> require GLIBC 2.23, since they use fixed words at the end of thread control
> area to store the HWCAP and HWCAP2 bits.  If the compiler was not configured
> with the appropriate GLIBC, the compiler will generate a 0 as the result of the
> built-in function call.
> 
> I've been adding the target_clone attribute support to GCC, and the resolver
> function uses __builtin_cpu_supports to detect which hardware ISA is being
> used.  On systems with an older GLIBC, only the default clone function will get
> called because __builtin_cpu_supports returns 0.
> 
> This adds a target supports option in dejagnu so that future tests can use this
> to determine whether or not to test target_clones.
> 
> I have verified that this patch works with the patches I plan to submit
> tomorrow for enhancing the PowerPC target_clone support.
> 
> Can I install this into the trunk?
> 
> Given that GCC 7 supports __builtin_cpu_is and __builtin_cpu_supports, I would
> ask if I could backport this to GCC 7.x as well to allow future tests to be
> back ported.
> 
> 2017-06-27  Michael Meissner  <meissner@linux.vnet.ibm.com>
> 
> 	PR target/81193
> 	* lib/target-supports.exp
> 	(check_ppc_cpu_supports_hw_available): New test to make sure
> 	__builtin_cpu_supports works on power7 and newer.
OK for the trunk.  It's not my call on the release branches though.

jeff

Re: [PATCH], Add check ppc_cpu_supports_hw to testsuite

[Peter Bergner]

On 6/27/17 6:53 PM, Michael Meissner wrote:
> This adds a target supports option in dejagnu so that future tests can use this
> to determine whether or not to test target_clones.

I like the idea, but some comments...



> +		#ifdef __MACH__
> +		  asm volatile ("xxlor vs0,vs0,vs0");
> +		#else
> +		  asm volatile ("xxlor 0,0,0");
> +	        #endif

What is this hunk for?  We're only interested in the return value from
the builtin below, correct?



> +		  if (!__builtin_cpu_supports ("vsx"))
> +		    return 1;
> +		  return 0;

...and more importantly, why limit us to testing "vsx"?  Why not test
for "ppc32", which should be true for *all* kernels we'd ever run on,
including ppc32, ppc64 and ppc64le?

Peter

Re: [PATCH], Add check ppc_cpu_supports_hw to testsuite

[Michael Meissner]

On Wed, Jun 28, 2017 at 09:58:40AM -0500, Peter Bergner wrote:
> On 6/27/17 6:53 PM, Michael Meissner wrote:
> > This adds a target supports option in dejagnu so that future tests can use this
> > to determine whether or not to test target_clones.
> 
> I like the idea, but some comments...
> 
> 
> 
> > +		#ifdef __MACH__
> > +		  asm volatile ("xxlor vs0,vs0,vs0");
> > +		#else
> > +		  asm volatile ("xxlor 0,0,0");
> > +	        #endif
> 
> What is this hunk for?  We're only interested in the return value from
> the builtin below, correct?
>
> 
> 
> > +		  if (!__builtin_cpu_supports ("vsx"))
> > +		    return 1;
> > +		  return 0;
> 
> ...and more importantly, why limit us to testing "vsx"?  Why not test
> for "ppc32", which should be true for *all* kernels we'd ever run on,
> including ppc32, ppc64 and ppc64le?

I wanted to make sure the test actually returned true in the correct test.
I.e. guarantee that the machine had VSX instructions (i.e. the asm) and that
__builtin_cpu_supports returned true.

But if you wanted to use a previous ISA flag, feel free to submit patches.  I
just wanted a quick way to make sure the clone tests were run properly when GCC
was configured with a 2.23 GLIBC or newer.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.vnet.ibm.com, phone: +1 (978) 899-4797

Re: [PATCH], Add check ppc_cpu_supports_hw to testsuite

[Segher Boessenkool]

Hi!

On Tue, Jun 27, 2017 at 07:53:21PM -0400, Michael Meissner wrote:
> --- gcc/testsuite/lib/target-supports.exp	(revision 249606)
> +++ gcc/testsuite/lib/target-supports.exp	(working copy)
> @@ -1930,6 +1930,37 @@ proc check_effective_target_powerpc64_no
>  	} {-O2}]
>  }
>  
> +# Return 1 if the target supports the __builtin_cpu_supports built-in,
> +# including having a new enough library to support the test.  Cache the result.
> +# Require at least a power7 to run on.
> +
> +proc check_ppc_cpu_supports_hw_available { } {

This probably shouldn't have "ppc" in the name?  Or "hw"?

> +    return [check_cached_effective_target ppc_cpu_supports_hw_available {
> +	# Some simulators are known to not support VSX/power8 instructions.
> +	# For now, disable on Darwin
> +	if { [istarget powerpc-*-eabi]
> +	     || [istarget powerpc*-*-eabispe]
> +	     || [istarget *-*-darwin*]} {
> +	    expr 0
> +	} else {
> +	    set options "-mvsx"
> +	    check_runtime_nocache ppc_cpu_supports_hw_available {
> +		int main()
> +		{
> +		#ifdef __MACH__
> +		  asm volatile ("xxlor vs0,vs0,vs0");
> +		#else
> +		  asm volatile ("xxlor 0,0,0");
> +	        #endif
> +		  if (!__builtin_cpu_supports ("vsx"))
> +		    return 1;
> +		  return 0;
> +		}
> +	    } $options
> +	}
> +    }]
> +}

As Peter said, I'd rather test for "ppc32", so this works anywhere.

That would give

proc check_cpu_supports_available { } {
    if { [istarget powerpc*-*-*] } {
        return [check_runtime cpu_supports_available {
          int main() { return !__builtin_cpu_supports ("ppc32"); }
        }]
    }

    return 0
}

(and other archs can add their stuff then).

Why did you use check_runtime_nocache btw?  Is that just copy-paste?


Segher

Re: [PATCH], Add check ppc_cpu_supports_hw to testsuite

[Michael Meissner]

On Wed, Jun 28, 2017 at 03:48:50PM -0500, Segher Boessenkool wrote:
> As Peter said, I'd rather test for "ppc32", so this works anywhere.

Fair enough.

> That would give
> 
> proc check_cpu_supports_available { } {
>     if { [istarget powerpc*-*-*] } {
>         return [check_runtime cpu_supports_available {
>           int main() { return !__builtin_cpu_supports ("ppc32"); }
>         }]
>     }
> 
>     return 0
> }
> 
> (and other archs can add their stuff then).
> 
> Why did you use check_runtime_nocache btw?  Is that just copy-paste?

Just copy-paste.

Like the target_clones stuff, right now, it is only x86 and PowerPC that
supports __builtin_cpu*.

I don't really see the point of having a machine independent test for
__builtin_cpu_*, but if you feel strongly about it go for it.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.vnet.ibm.com, phone: +1 (978) 899-4797

Re: [PATCH], Add check ppc_cpu_supports_hw to testsuite

[Segher Boessenkool]

On Wed, Jun 28, 2017 at 05:04:37PM -0400, Michael Meissner wrote:
> > Why did you use check_runtime_nocache btw?  Is that just copy-paste?
> 
> Just copy-paste.
> 
> Like the target_clones stuff, right now, it is only x86 and PowerPC that
> supports __builtin_cpu*.
> 
> I don't really see the point of having a machine independent test for
> __builtin_cpu_*, but if you feel strongly about it go for it.

Why have divergent names for things, if all targets could use the same?
Yes I realise almost all uses will be in target-specific testcases.

Getting the interface right up front is more important than the
implementation, it's a pain to change the interface later (have to
modify a lot of testcases, etc.)


Segher