diff mbox

, PR 71806, Fix -mfloat128/-mfloat128-hardware defaults on power9

Message ID 20160708133133.GA31893@ibm-tiger.the-meissners.org
State New
Headers show

Commit Message

Michael Meissner July 8, 2016, 1:31 p.m. UTC 
If you configure either GCC 6.x or trunk with the --with-cpu=power9 option, it
will enable __float128 support, since power9 has the ISA 3.0 hardware IEEE
128-bit floating point instructions.  However, the libquadmath and libstdc++
libraries have not been fixed to enable the PowerPC support, and the build
fails.  Similarly, users who use -mcpu=power9 will get __float128 enabled, and
they will run into problems if they use features that are not yet present.

This patch, changes the behavior so that IEEE 128-bit floating point
instructions are enabled if you use ISA 3.0 (-mcpu=power9), provding you use
the -mfloat128 option.  Or if you use the -mfloat128-hardware option, it will
enable the base -mfloat128 support.

It is expected that when glibc, libstdc++, and libquadmath are enhanced to add
__float128 support, that the -mfloat128 option will be enabled automatically
for power7/power8 systems, and -mfloat128-hardware will be enabled for power9
systems.

Included in this post are two attachments, one for trunk, and the other for the
GCC 6.x branch.  I have tested both patches against their respected bases, and
there are no regressions (once the one test that assumed -mcpu=power9 enabled
IEEE 128-bit floating point is fixed with this patch).  Are these ok to install
in their respective trees?

The only difference between the GCC 6 patch and the trunk patch is the trunk
adds a check for -mupper-regs-di, which has not been backported to the GCC 6
branch.

[gcc]
2016-07-08  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/71806
	* config/rs6000/rs6000-cpus.def (ISA_3_0_MASKS_SERVER): Do not
	enable -mfloat128-hardware by default.
	(ISA_3_0_MASKS_IEEE): New macro to give all of the VSX options
	that IEEE 128-bit hardware support needs.
	* config/rs6000/rs6000.c (rs6000_option_override_internal): If
	-mcpu=power9 -mfloat128, enable -mfloat128-hardware by default.
	Use ISA_3_0_MASKS_IEEE as the set of options that IEEE 128-bit
	floating point requires.
	* doc/invoke.texi (RS/6000 and PowerPC Options): Document
	-mfloat128 and -mfloat128-hardware changes.

[gcc/testsuite]
2016-07-08  Michael Meissner  <meissner@linux.vnet.ibm.com>

	* gcc.target/powerpc/p9-lxvx-stxvx-3.c: Add -mfloat128 option.

Comments

Segher Boessenkool July 8, 2016, 2:13 p.m. UTC | #1 
On Fri, Jul 08, 2016 at 09:31:33AM -0400, Michael Meissner wrote:
> 	* gcc.target/powerpc/p9-lxvx-stxvx-3.c: Add -mfloat128 option.

Is that the only testcase that needs updating?

> --- gcc/config/rs6000/rs6000-cpus.def	(revision 238127)
> +++ gcc/config/rs6000/rs6000-cpus.def	(working copy)
> @@ -63,7 +63,6 @@
>  /* Add ISEL back into ISA 3.0, since it is supposed to be a win.  Do not add
>     P9_MINMAX until the hardware that supports it is available.  */
>  #define ISA_3_0_MASKS_SERVER	(ISA_2_7_MASKS_SERVER			\
> -				 | OPTION_MASK_FLOAT128_HW		\

Please add a comment for this as well?

>    /* IEEE 128-bit floating point hardware instructions imply enabling
>       __float128.  */
>    if (TARGET_FLOAT128_HW
> -      && (rs6000_isa_flags & (OPTION_MASK_P9_VECTOR
> -			      | OPTION_MASK_DIRECT_MOVE
> -			      | OPTION_MASK_UPPER_REGS_DI
> -			      | OPTION_MASK_UPPER_REGS_DF
> -			      | OPTION_MASK_UPPER_REGS_SF)) == 0)
> +      && (rs6000_isa_flags & ISA_3_0_MASKS_IEEE) != ISA_3_0_MASKS_IEEE)
>      {
>        if ((rs6000_isa_flags_explicit & OPTION_MASK_FLOAT128_HW) != 0)
>  	error ("-mfloat128-hardware requires full ISA 3.0 support");

That is not the same thing...  New one looks better, is this a bugfix?
The changelog doesn't say.

Okay for trunk and 6 with those nits fixed.  Thanks,


Segher
Michael Meissner July 8, 2016, 2:17 p.m. UTC | #2 
On Fri, Jul 08, 2016 at 09:13:50AM -0500, Segher Boessenkool wrote:
> On Fri, Jul 08, 2016 at 09:31:33AM -0400, Michael Meissner wrote:
> > 	* gcc.target/powerpc/p9-lxvx-stxvx-3.c: Add -mfloat128 option.
> 
> Is that the only testcase that needs updating?
> 
> > --- gcc/config/rs6000/rs6000-cpus.def	(revision 238127)
> > +++ gcc/config/rs6000/rs6000-cpus.def	(working copy)
> > @@ -63,7 +63,6 @@
> >  /* Add ISEL back into ISA 3.0, since it is supposed to be a win.  Do not add
> >     P9_MINMAX until the hardware that supports it is available.  */
> >  #define ISA_3_0_MASKS_SERVER	(ISA_2_7_MASKS_SERVER			\
> > -				 | OPTION_MASK_FLOAT128_HW		\
> 
> Please add a comment for this as well?

Ok.

> >    /* IEEE 128-bit floating point hardware instructions imply enabling
> >       __float128.  */
> >    if (TARGET_FLOAT128_HW
> > -      && (rs6000_isa_flags & (OPTION_MASK_P9_VECTOR
> > -			      | OPTION_MASK_DIRECT_MOVE
> > -			      | OPTION_MASK_UPPER_REGS_DI
> > -			      | OPTION_MASK_UPPER_REGS_DF
> > -			      | OPTION_MASK_UPPER_REGS_SF)) == 0)
> > +      && (rs6000_isa_flags & ISA_3_0_MASKS_IEEE) != ISA_3_0_MASKS_IEEE)
> >      {
> >        if ((rs6000_isa_flags_explicit & OPTION_MASK_FLOAT128_HW) != 0)
> >  	error ("-mfloat128-hardware requires full ISA 3.0 support");
> 
> That is not the same thing...  New one looks better, is this a bugfix?
> The changelog doesn't say.

I just moved the OPTIONS_MASKS_* used here to a common macro that is checked
earlier to enable hardware support if -mfloat128.

> Okay for trunk and 6 with those nits fixed.  Thanks,

Thanks.
Segher Boessenkool July 8, 2016, 2:23 p.m. UTC | #3 
On Fri, Jul 08, 2016 at 10:17:14AM -0400, Michael Meissner wrote:
> > >    /* IEEE 128-bit floating point hardware instructions imply enabling
> > >       __float128.  */
> > >    if (TARGET_FLOAT128_HW
> > > -      && (rs6000_isa_flags & (OPTION_MASK_P9_VECTOR
> > > -			      | OPTION_MASK_DIRECT_MOVE
> > > -			      | OPTION_MASK_UPPER_REGS_DI
> > > -			      | OPTION_MASK_UPPER_REGS_DF
> > > -			      | OPTION_MASK_UPPER_REGS_SF)) == 0)
> > > +      && (rs6000_isa_flags & ISA_3_0_MASKS_IEEE) != ISA_3_0_MASKS_IEEE)
> > >      {
> > >        if ((rs6000_isa_flags_explicit & OPTION_MASK_FLOAT128_HW) != 0)
> > >  	error ("-mfloat128-hardware requires full ISA 3.0 support");
> > 
> > That is not the same thing...  New one looks better, is this a bugfix?
> > The changelog doesn't say.
> 
> I just moved the OPTIONS_MASKS_* used here to a common macro that is checked
> earlier to enable hardware support if -mfloat128.

The old code tests if all options are off; the new if any are off.


Segher
diff mbox

Patch

Index: gcc/config/rs6000/rs6000-cpus.def
===================================================================
--- gcc/config/rs6000/rs6000-cpus.def	(revision 238127)
+++ gcc/config/rs6000/rs6000-cpus.def	(working copy)
@@ -63,7 +63,6 @@ 
 /* Add ISEL back into ISA 3.0, since it is supposed to be a win.  Do not add
    P9_MINMAX until the hardware that supports it is available.  */
 #define ISA_3_0_MASKS_SERVER	(ISA_2_7_MASKS_SERVER			\
-				 | OPTION_MASK_FLOAT128_HW		\
 				 | OPTION_MASK_ISEL			\
 				 | OPTION_MASK_MODULO			\
 				 | OPTION_MASK_P9_FUSION		\
@@ -72,6 +71,16 @@ 
 				 | OPTION_MASK_P9_MISC			\
 				 | OPTION_MASK_P9_VECTOR)
 
+/* Support for the IEEE 128-bit floating point hardware requires a lot of the
+   VSX instructions that are part of ISA 3.0.  */
+#define ISA_3_0_MASKS_IEEE	(OPTION_MASK_VSX			\
+				 | OPTION_MASK_P8_VECTOR		\
+				 | OPTION_MASK_P9_VECTOR		\
+				 | OPTION_MASK_DIRECT_MOVE		\
+				 | OPTION_MASK_UPPER_REGS_DI		\
+				 | OPTION_MASK_UPPER_REGS_DF		\
+				 | OPTION_MASK_UPPER_REGS_SF)
+
 #define POWERPC_7400_MASK	(OPTION_MASK_PPC_GFXOPT | OPTION_MASK_ALTIVEC)
 
 /* Deal with ports that do not have -mstrict-align.  */
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 238130)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -4381,14 +4381,21 @@  rs6000_option_override_internal (bool gl
       rs6000_isa_flags &= ~(OPTION_MASK_FLOAT128 | OPTION_MASK_FLOAT128_HW);
     }
 
+  /* If we have -mfloat128 and full ISA 3.0 support, enable -mfloat128-hardware
+     by default.  */
+  if (TARGET_FLOAT128 && !TARGET_FLOAT128_HW
+      && (rs6000_isa_flags & ISA_3_0_MASKS_IEEE) == ISA_3_0_MASKS_IEEE
+      && !(rs6000_isa_flags_explicit & OPTION_MASK_FLOAT128_HW))
+    {
+      rs6000_isa_flags |= OPTION_MASK_FLOAT128_HW;
+      if ((rs6000_isa_flags & OPTION_MASK_FLOAT128) != 0)
+	rs6000_isa_flags_explicit |= OPTION_MASK_FLOAT128_HW;
+    }
+
   /* IEEE 128-bit floating point hardware instructions imply enabling
      __float128.  */
   if (TARGET_FLOAT128_HW
-      && (rs6000_isa_flags & (OPTION_MASK_P9_VECTOR
-			      | OPTION_MASK_DIRECT_MOVE
-			      | OPTION_MASK_UPPER_REGS_DI
-			      | OPTION_MASK_UPPER_REGS_DF
-			      | OPTION_MASK_UPPER_REGS_SF)) == 0)
+      && (rs6000_isa_flags & ISA_3_0_MASKS_IEEE) != ISA_3_0_MASKS_IEEE)
     {
       if ((rs6000_isa_flags_explicit & OPTION_MASK_FLOAT128_HW) != 0)
 	error ("-mfloat128-hardware requires full ISA 3.0 support");
@@ -4396,10 +4403,6 @@  rs6000_option_override_internal (bool gl
       rs6000_isa_flags &= ~OPTION_MASK_FLOAT128_HW;
     }
 
-  else if (TARGET_P9_VECTOR && !TARGET_FLOAT128_HW
-	   && (rs6000_isa_flags_explicit & OPTION_MASK_FLOAT128_HW) == 0)
-    rs6000_isa_flags |= OPTION_MASK_FLOAT128_HW;
-
   if (TARGET_FLOAT128_HW
       && (rs6000_isa_flags_explicit & OPTION_MASK_FLOAT128) == 0)
     rs6000_isa_flags |= OPTION_MASK_FLOAT128;
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 238150)
+++ gcc/doc/invoke.texi	(working copy)
@@ -20355,9 +20355,14 @@  hardware instructions.
 
 The VSX instruction set (@option{-mvsx}, @option{-mcpu=power7}, or
 @option{-mcpu=power8}) must be enabled to use the @option{-mfloat128}
-option.  The @code{-mfloat128} option only works on PowerPC 64-bit
+option.  The @option{-mfloat128} option only works on PowerPC 64-bit
 Linux systems.
 
+If you use the ISA 3.0 instruction set (@option{-mcpu=power9}), the
+@option{-mfloat128} option will also enable the generation of ISA 3.0
+IEEE 128-bit floating point instructions.  Otherwise, IEEE 128-bit
+floating point will be done with software emulation.
+
 @item -mfloat128-hardware
 @itemx -mno-float128-hardware
 @opindex mfloat128-hardware
@@ -20365,6 +20370,13 @@  Linux systems.
 Enable/disable using ISA 3.0 hardware instructions to support the
 @var{__float128} data type.
 
+If you use @option{-mfloat128-hardware}, it will enable the option
+@option{-mfloat128} as well.
+
+If you select ISA 3.0 instructions with @option{-mcpu=power9}, but do
+not use either @option{-mfloat128} or @option{-mfloat128-hardware},
+the IEEE 128-bit floating point support will not be enabled.
+
 @item -mmodulo
 @itemx -mno-modulo
 @opindex mmodulo
Index: gcc/testsuite/gcc.target/powerpc/p9-lxvx-stxvx-3.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/p9-lxvx-stxvx-3.c	(revision 238150)
+++ gcc/testsuite/gcc.target/powerpc/p9-lxvx-stxvx-3.c	(working copy)
@@ -1,6 +1,6 @@ 
 /* { dg-do compile { target { powerpc64le-*-* } } } */
 /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power9" } } */
-/* { dg-options "-mcpu=power9 -O3" } */
+/* { dg-options "-mcpu=power9 -O3 -mfloat128" } */
 /* { dg-require-effective-target powerpc_p9vector_ok } */
 /* { dg-final { scan-assembler "lxvx" } } */
 /* { dg-final { scan-assembler "stxvx" } } */