diff mbox

[#3] , Fix _Complex when there are multiple FP types the same size

Message ID 20160502211024.GA17046@ibm-tiger.the-meissners.org
State New
Headers show

Commit Message

Michael Meissner May 2, 2016, 9:10 p.m. UTC 
On Mon, May 02, 2016 at 12:54:43PM +0200, Bernd Schmidt wrote:
> On 04/30/2016 06:00 PM, Segher Boessenkool wrote:
> >On Fri, Apr 29, 2016 at 04:51:27PM -0400, Michael Meissner wrote:
> >>2016-04-29  Michael Meissner  <meissner@linux.vnet.ibm.com>
> >>
> >>	* config/rs6000/rs6000.c (rs6000_hard_regno_nregs_internal): Add
> >>	support for __float128 complex datatypes.
> >>	(rs6000_hard_regno_mode_ok): Likewise.
> >>	(rs6000_setup_reg_addr_masks): Likewise.
> >>	(rs6000_complex_function_value): Likewise.
> >>	* config/rs6000/rs6000.h (FLOAT128_IEEE_P): Likewise.
> >>	__float128 and __ibm128 complex.
> >>	(FLOAT128_IBM_P): Likewise.
> >>	(ALTIVEC_ARG_MAX_RETURN): Likewise.
> >>	* doc/extend.texi (Additional Floating Types): Document that
> >>	-mfloat128 must be used to enable __float128.  Document complex
> >>	__float128 and __ibm128 support.
> >>
> >>[gcc/testsuite]
> >>2016-04-29  Michael Meissner  <meissner@linux.vnet.ibm.com>
> >>
> >>	* gcc.target/powerpc/float128-complex-1.c: New tests for complex
> >>	__float128.
> >>	* gcc.target/powerpc/float128-complex-2.c: Likewise.
> >
> >The powerpc parts are okay for trunk.  Thank you!
> 
> Ok for the other parts as well. Although I wonder:
> 
> >+      /* build_complex_type has set the expected mode to allow having multiple
> >+	 complex types for multiple floating point types that have the same
> >+	 size such as the PowerPC with __ibm128 and __float128.  If this was
> >+	 not set, figure out the mode manually.  */
> >+      if (TYPE_MODE (type) == VOIDmode)
> >+	{
> >+	  unsigned int precision = TYPE_PRECISION (TREE_TYPE (type));
> >+	  enum mode_class mclass = (TREE_CODE (TREE_TYPE (type)) == REAL_TYPE
> >+				    ? MODE_COMPLEX_FLOAT : MODE_COMPLEX_INT);
> >+	  SET_TYPE_MODE (type, mode_for_size (2 * precision, mclass, 0));
> >+	}
> 
> What happens if you assert that it isn't VOIDmode?

On the PowerPC, I did a full boostrap with the code changed to a gcc_assert,
and it didn't trigger.

However, in looking at it further, there are only two places that layout_type
is called after making a complex node.

The first is build_complex_type that I provided a patch for. The other is a
similar function in the Fortran front end (gfc_build_complex_type). Now,
assuming you only use the 3 standard floating point modes (float_type_node,
double_type_node, and long_double_type_node) you wouldn't notice any
problem. But if somehow Fortran can create a __float128 type, it would trigger
the assertion (in the new patches) or generate the wrong type (in the previous
patches).

So I would like to commit the following changes (assuming they bootstrap and
have no regressions) instead. Are these patches ok for the trunk, and
eventually the gcc 6.2 branch if they don't break other back ends?

Fortran people, I'm adding you to the To: list, because of the Fortran
change. What the problem is the current layout_type uses 2*size of the base
type to create the complex type. However, in the current PowerPC, we are
transitioning from using IBM extended double format (pair of doubles to give
more mantissa bits, but no extra exponent range) to IEEE 128-bit format, and we
have two 128-bit complex types. You get all sorts of errors if you want to use
the REAL or IMAGINARY parts and you get the wrong type.

The patch builds a table that maps a given base mode to the complex mode that
uses the base mode as the two elements (i.e. GET_MODE_COMPLEX_MODE (DFmode)
would return DCmode.

The machine independent changes are in the gcc-stage7.patch003b, and the
PowerPC specific changes are in gcc-stage7.patch003c.

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

	* machmode.h (mode_complex): Add support to give the complex mode
	for a given mode.
	(GET_MODE_COMPLEX_MODE): Likewise.
	* stor-layout.c (layout_type): For COMPLEX_TYPE, use the mode
	stored by build_complex_type and gfc_build_complex_type instead of
	trying to figure out the appropriate mode based on the size. Raise
	an assertion error, if the type was not set.
	* genmodes.c (struct mode_data): Add field for the complex type of
	the given type.
	(blank_mode): Likewise.
	(make_complex_modes): Remember the complex mode created in the
	base type.
	(emit_mode_complex): Write out the mode_complex array to map a
	type mode to the complex version.
	(emit_insn_modes_c): Likewise.
	* tree.c (build_complex_type): Set the complex type to use before
	calling layout_type.
	* config/rs6000/rs6000.c (rs6000_hard_regno_nregs_internal): Add
	support for __float128 complex datatypes.
	(rs6000_hard_regno_mode_ok): Likewise.
	(rs6000_setup_reg_addr_masks): Likewise.
	(rs6000_complex_function_value): Likewise.
	* config/rs6000/rs6000.h (FLOAT128_IEEE_P): Likewise.
	__float128 and __ibm128 complex.
	(FLOAT128_IBM_P): Likewise.
	(ALTIVEC_ARG_MAX_RETURN): Likewise.
	* doc/extend.texi (Additional Floating Types): Document that
	-mfloat128 must be used to enable __float128.  Document complex
	__float128 and __ibm128 support.

[gcc/fortran]
2016-05-02  Michael Meissner  <meissner@linux.vnet.ibm.com>

	* trans-types.c (gfc_build_complex_type):

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

	* gcc.target/powerpc/float128-complex-1.c: New tests for complex
	__float128.
	* gcc.target/powerpc/float128-complex-2.c: Likewise.

Comments

FX Coudert May 2, 2016, 9:24 p.m. UTC | #1 
Hi Michael,

Thanks for letting us know.

Right now, the Fortran front-end uses the following real modes:
  - those corresponding to {float,double,long_double}_type_node
  - TFmode (if libquadmath support is enabled)
and then uses the corresponding complex modes.

So, I guess the question in your case is: in each compiler settings, is there a TFmode? If so, that would not play nice: the front-end current does not handle several real modes with equal precision.

In case you want to test, simple Fortran code to create a 128-bit real x and complex y is:

  real(kind=16) :: x
  complex(kind=16) :: y

I’m guessing if that emits the correct code in both settings, the rest should be fine.
Michael Meissner May 2, 2016, 9:46 p.m. UTC | #2 
On Mon, May 02, 2016 at 11:24:25PM +0200, FX wrote:
> Hi Michael,
> 
> Thanks for letting us know.
> 
> Right now, the Fortran front-end uses the following real modes:
>   - those corresponding to {float,double,long_double}_type_node
>   - TFmode (if libquadmath support is enabled)
> and then uses the corresponding complex modes.
> 
> So, I guess the question in your case is: in each compiler settings, is there a TFmode? If so, that would not play nice: the front-end current does not handle several real modes with equal precision.

There is a TFmode. Right now, the default for TFmode is IBM extended double,
and __float128 is a keyword added to C/C++ to get the IEEE 128-bit floating
point type.

At the moment, libquadmath is not enabled (complex support is one of the
issues, and having all of the built-ins registers, etc. is another), but I'm
hoping in the GCC 7 time frame to get it supported.

There is an option to switch the default from IBM extended double to IEEE
128-bit, but we need to enhance the libraries before we can let users use
it. Ideally by the time GCC 7 is released, users will be able to use IEEE
128-bit floating point as their default long double format. Our motivation is
that the upcoming power9 hardware has IEEE 128-bit hardware support (and the
fact that there are a lot of accuracy issues with the current IBM extended
double format).

So, I'm trying to do these stepps in a piecemeal fashion, rather than having
one giant flag day.

> In case you want to test, simple Fortran code to create a 128-bit real x and complex y is:
> 
>   real(kind=16) :: x
>   complex(kind=16) :: y
> 
> I’m guessing if that emits the correct code in both settings, the rest should be fine.

Yes. I suspect right now, there isn't an issue, since you are using the default
type nodes.
Michael Meissner May 2, 2016, 10:29 p.m. UTC | #3 
The bootstrap and make check succeeded without problems.
Bernd Schmidt May 2, 2016, 11:06 p.m. UTC | #4 
On 05/02/2016 11:10 PM, Michael Meissner wrote:
>
> So I would like to commit the following changes (assuming they bootstrap and
> have no regressions) instead. Are these patches ok for the trunk, and
> eventually the gcc 6.2 branch if they don't break other back ends?

Ok for the non-rs6000 changes.


Bernd
Jakub Jelinek May 5, 2016, 1:35 p.m. UTC | #5 
On Mon, May 02, 2016 at 05:10:24PM -0400, Michael Meissner wrote:
> [gcc/fortran]
> 2016-05-02  Michael Meissner  <meissner@linux.vnet.ibm.com>
> 
> 	* trans-types.c (gfc_build_complex_type):

Something missing above...

	Jakub
diff mbox

Patch

Index: gcc/machmode.h
===================================================================
--- gcc/machmode.h	(.../svn+ssh://meissner@gcc.gnu.org/svn/gcc/trunk/gcc/machmode.h)	(revision 235776)
+++ gcc/machmode.h	(.../gcc/machmode.h)	(working copy)
@@ -269,6 +269,10 @@  extern const unsigned char mode_wider[NU
 extern const unsigned char mode_2xwider[NUM_MACHINE_MODES];
 #define GET_MODE_2XWIDER_MODE(MODE) ((machine_mode) mode_2xwider[MODE])
 
+/* Get the complex mode from the component mode.  */
+extern const unsigned char mode_complex[NUM_MACHINE_MODES];
+#define GET_MODE_COMPLEX_MODE(MODE) ((machine_mode) mode_complex[MODE])
+
 /* Return the mode for data of a given size SIZE and mode class CLASS.
    If LIMIT is nonzero, then don't use modes bigger than MAX_FIXED_MODE_SIZE.
    The value is BLKmode if no other mode is found.  */
Index: gcc/stor-layout.c
===================================================================
--- gcc/stor-layout.c	(.../svn+ssh://meissner@gcc.gnu.org/svn/gcc/trunk/gcc/stor-layout.c)	(revision 235776)
+++ gcc/stor-layout.c	(.../gcc/stor-layout.c)	(working copy)
@@ -2146,11 +2146,13 @@  layout_type (tree type)
 
     case COMPLEX_TYPE:
       TYPE_UNSIGNED (type) = TYPE_UNSIGNED (TREE_TYPE (type));
-      SET_TYPE_MODE (type,
-		     mode_for_size (2 * TYPE_PRECISION (TREE_TYPE (type)),
-				    (TREE_CODE (TREE_TYPE (type)) == REAL_TYPE
-				     ? MODE_COMPLEX_FLOAT : MODE_COMPLEX_INT),
-				     0));
+
+      /* build_complex_type and fortran's gfc_build_complex_type have set the
+	 expected mode to allow having multiple complex types for multiple
+	 floating point types that have the same size such as the PowerPC with
+	 __ibm128 and __float128.  */
+      gcc_assert (TYPE_MODE (type) != VOIDmode);
+
       TYPE_SIZE (type) = bitsize_int (GET_MODE_BITSIZE (TYPE_MODE (type)));
       TYPE_SIZE_UNIT (type) = size_int (GET_MODE_SIZE (TYPE_MODE (type)));
       break;
Index: gcc/genmodes.c
===================================================================
--- gcc/genmodes.c	(.../svn+ssh://meissner@gcc.gnu.org/svn/gcc/trunk/gcc/genmodes.c)	(revision 235776)
+++ gcc/genmodes.c	(.../gcc/genmodes.c)	(working copy)
@@ -66,6 +66,7 @@  struct mode_data
 				   this mode as a component.  */
   struct mode_data *next_cont;  /* Next mode in that list.  */
 
+  struct mode_data *complex;	/* complex type with mode as component.  */
   const char *file;		/* file and line of definition, */
   unsigned int line;		/* for error reporting */
   unsigned int counter;		/* Rank ordering of modes */
@@ -83,7 +84,7 @@  static struct mode_data *void_mode;
 static const struct mode_data blank_mode = {
   0, "<unknown>", MAX_MODE_CLASS,
   -1U, -1U, -1U, -1U,
-  0, 0, 0, 0, 0,
+  0, 0, 0, 0, 0, 0,
   "<unknown>", 0, 0, 0, 0, false, 0
 };
 
@@ -472,6 +473,7 @@  make_complex_modes (enum mode_class cl,
 
       c = new_mode (cclass, buf, file, line);
       c->component = m;
+      m->complex = c;
     }
 }
 
@@ -1381,6 +1383,22 @@  emit_mode_wider (void)
 }
 
 static void
+emit_mode_complex (void)
+{
+  int c;
+  struct mode_data *m;
+
+  print_decl ("unsigned char", "mode_complex", "NUM_MACHINE_MODES");
+
+  for_all_modes (c, m)
+    tagged_printf ("%smode",
+		   m->complex ? m->complex->name : void_mode->name,
+		   m->name);
+
+  print_closer ();
+}
+
+static void
 emit_mode_mask (void)
 {
   int c;
@@ -1745,6 +1763,7 @@  emit_insn_modes_c (void)
   emit_mode_size ();
   emit_mode_nunits ();
   emit_mode_wider ();
+  emit_mode_complex ();
   emit_mode_mask ();
   emit_mode_inner ();
   emit_mode_unit_size ();
Index: gcc/tree.c
===================================================================
--- gcc/tree.c	(.../svn+ssh://meissner@gcc.gnu.org/svn/gcc/trunk/gcc/tree.c)	(revision 235776)
+++ gcc/tree.c	(.../gcc/tree.c)	(working copy)
@@ -8774,6 +8774,7 @@  build_complex_type (tree component_type)
   t = make_node (COMPLEX_TYPE);
 
   TREE_TYPE (t) = TYPE_MAIN_VARIANT (component_type);
+  SET_TYPE_MODE (t, GET_MODE_COMPLEX_MODE (TYPE_MODE (component_type)));
 
   /* If we already have such a type, use the old one.  */
   hstate.add_object (TYPE_HASH (component_type));
Index: gcc/fortran/trans-types.c
===================================================================
--- gcc/fortran/trans-types.c	(.../svn+ssh://meissner@gcc.gnu.org/svn/gcc/trunk/gcc/fortran/trans-types.c)	(revision 235776)
+++ gcc/fortran/trans-types.c	(.../gcc/fortran/trans-types.c)	(working copy)
@@ -828,6 +828,7 @@  gfc_build_complex_type (tree scalar_type
 
   new_type = make_node (COMPLEX_TYPE);
   TREE_TYPE (new_type) = scalar_type;
+  SET_TYPE_MODE (new_type, GET_MODE_COMPLEX_MODE (TYPE_MODE (scalar_type)));
   layout_type (new_type);
   return new_type;
 }