diff mbox series

[2/3,V2] Fix IEEE 128-bit min/max test.

Message ID 20210617225609.GA4816@ibm-toto.the-meissners.org
State New
Headers show
Series None | expand

Commit Message

Michael Meissner June 17, 2021, 10:56 p.m. UTC 
Here is a replacement patch.  Can I check this into the master branch, and
eventually backport it to GCC 11?

[PATCH] Fix IEEE 128-bit min/max test.

This patch fixes the float128-minmax.c test so that it can accommodate the
generation of xsmincqp and xsmaxcqp instructions on power10.  I changed
the effective target from 'float128' to 'ppc_float128_hw', since this
needs the IEEE 128-bit float hardware support.  Changing to use
'ppc_float128_hw' allows the 'lp64' test to be dropped.  The 'lp64' test
was needed because big endian 32-bit code cannot enable the IEEE 128-bit
floating point instructions.

gcc/testsuite/
2021-06-17  Michael Meissner  <meissner@linux.ibm.com>

	* gcc.target/powerpc/float128-minmax.c: Adjust expected code for
	power10.
	* lib/target-supports.exp (check_effective_target_has_arch_pwr10):
	New target support.
---
 gcc/testsuite/gcc.target/powerpc/float128-minmax.c | 10 ++++++----
 gcc/testsuite/lib/target-supports.exp              | 10 ++++++++++
 2 files changed, 16 insertions(+), 4 deletions(-)

Comments

Michael Meissner June 23, 2021, 7:08 p.m. UTC | #1 
Ping this patch:

| Date: Thu, 17 Jun 2021 18:56:09 -0400
| Subject: [PATCH 2/3 V2] Fix IEEE 128-bit min/max test.
| Message-ID: <20210617225609.GA4816@ibm-toto.the-meissners.org>
Michael Meissner June 23, 2021, 7:20 p.m. UTC | #2 
Note, this patch should go into GCC 11 after baking on the master branch.
Segher Boessenkool June 30, 2021, 12:06 a.m. UTC | #3 
On Thu, Jun 17, 2021 at 06:56:09PM -0400, Michael Meissner wrote:
> The 'lp64' test
> was needed because big endian 32-bit code cannot enable the IEEE 128-bit
> floating point instructions.

No, *does not* enable them.  After

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 2c249e186e1e..d4aac4164cfe 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -4281,7 +4281,7 @@ rs6000_option_override_internal (bool global_init_p)
       rs6000_isa_flags &= ~OPTION_MASK_FLOAT128_HW;
     }
 
-  if (TARGET_FLOAT128_HW && !TARGET_64BIT)
+  if (0&& TARGET_FLOAT128_HW && !TARGET_64BIT)
     {
       if ((rs6000_isa_flags_explicit & OPTION_MASK_FLOAT128_HW) != 0)
        error ("%qs requires %qs", "%<-mfloat128-hardware%>", "-m64");

you can compile fine with -m32 if you add -mfloat128-hardware as well
(it is disabled for BE as well, that should be fixed as well a few lines
up from there).

Can you show any code that will not work please?  Not allowing QP float
with -m32 causes many more problems than just allowing it would.

> 	* gcc.target/powerpc/float128-minmax.c: Adjust expected code for
> 	power10.
> 	* lib/target-supports.exp (check_effective_target_has_arch_pwr10):
> 	New target support.

Just "New." please.

>  /* { dg-require-effective-target powerpc_p9vector_ok } */

Please try whether you can lose that line as well.

Okay for trunk, and for 11 after the usual soak.  Thanks!


Segher
Michael Meissner June 30, 2021, 4:25 p.m. UTC | #4 
On Tue, Jun 29, 2021 at 07:06:14PM -0500, Segher Boessenkool wrote:
> On Thu, Jun 17, 2021 at 06:56:09PM -0400, Michael Meissner wrote:
> > The 'lp64' test
> > was needed because big endian 32-bit code cannot enable the IEEE 128-bit
> > floating point instructions.
> 
> No, *does not* enable them.  After
> 
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index 2c249e186e1e..d4aac4164cfe 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -4281,7 +4281,7 @@ rs6000_option_override_internal (bool global_init_p)
>        rs6000_isa_flags &= ~OPTION_MASK_FLOAT128_HW;
>      }
>  
> -  if (TARGET_FLOAT128_HW && !TARGET_64BIT)
> +  if (0&& TARGET_FLOAT128_HW && !TARGET_64BIT)
>      {
>        if ((rs6000_isa_flags_explicit & OPTION_MASK_FLOAT128_HW) != 0)
>         error ("%qs requires %qs", "%<-mfloat128-hardware%>", "-m64");
> 
> you can compile fine with -m32 if you add -mfloat128-hardware as well
> (it is disabled for BE as well, that should be fixed as well a few lines
> up from there).
> 
> Can you show any code that will not work please?  Not allowing QP float
> with -m32 causes many more problems than just allowing it would.

I will in a bit, but it isn't that simple.  Right now, all of the optimizations
for converting char/short/int -> __float128 (and presumably the other way)
don't work on 32-bit, because they want to split:

	(parallel [(set (reg:KF <n>)
			(float:KF (reg:SI <m>)))
		   (clobber (reg:DI <tmp>))])

Into:

	(set (reg:DI <tmp>)
	     (sign_extend:DI (reg:SI <m>)))

	(set (reg:KF <n>)
	     (float:KF (reg:DI <tmp>)))

And the sign_extend SI->DI doesn't exist in 32-bit.
	

> > 	* gcc.target/powerpc/float128-minmax.c: Adjust expected code for
> > 	power10.
> > 	* lib/target-supports.exp (check_effective_target_has_arch_pwr10):
> > 	New target support.
> 
> Just "New." please.
> 
> >  /* { dg-require-effective-target powerpc_p9vector_ok } */
> 
> Please try whether you can lose that line as well.
> 
> Okay for trunk, and for 11 after the usual soak.  Thanks!
> 
> 
> Segher
diff mbox series

Patch

diff --git a/gcc/testsuite/gcc.target/powerpc/float128-minmax.c b/gcc/testsuite/gcc.target/powerpc/float128-minmax.c
index fe397518f2f..b0e6bd39873 100644
--- a/gcc/testsuite/gcc.target/powerpc/float128-minmax.c
+++ b/gcc/testsuite/gcc.target/powerpc/float128-minmax.c
@@ -1,6 +1,5 @@ 
-/* { dg-do compile { target lp64 } } */
 /* { dg-require-effective-target powerpc_p9vector_ok } */
-/* { dg-require-effective-target float128 } */
+/* { dg-require-effective-target ppc_float128_hw } */
 /* { dg-options "-mpower9-vector -O2 -ffast-math" } */
 
 #ifndef TYPE
@@ -12,5 +11,8 @@ 
 TYPE f128_min (TYPE a, TYPE b) { return __builtin_fminf128 (a, b); }
 TYPE f128_max (TYPE a, TYPE b) { return __builtin_fmaxf128 (a, b); }
 
-/* { dg-final { scan-assembler     {\mxscmpuqp\M} } } */
-/* { dg-final { scan-assembler-not {\mbl\M}       } } */
+/* Adjust code power10 which has native min/max instructions.  */
+/* { dg-final { scan-assembler-times {\mxscmpuqp\M} 2 { target { ! has_arch_pwr10 } } } } */
+/* { dg-final { scan-assembler-times {\mxsmincqp\M} 1 { target has_arch_pwr10 } } } */
+/* { dg-final { scan-assembler-times {\mxsmaxcqp\M} 1 { target has_arch_pwr10 } } } */
+/* { dg-final { scan-assembler-not {\mbl\M} } } */
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 7f78c5593ac..789723fb287 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -6127,6 +6127,16 @@  proc check_effective_target_has_arch_pwr9 { } {
 	}]
 }
 
+proc check_effective_target_has_arch_pwr10 { } {
+	return [check_no_compiler_messages arch_pwr10 assembly {
+		#ifndef _ARCH_PWR10
+		#error does not have power10 support.
+		#else
+		/* "has power10 support" */
+		#endif
+	}]
+}
+
 # Return 1 if this is a PowerPC target supporting -mcpu=power10.
 # Limit this to 64-bit linux systems for now until other targets support
 # power10.