diff mbox

, Fix _Complex when there are multiple FP types the same size

Message ID 20160428210613.GA29745@ibm-tiger.the-meissners.org
State New
Headers show

Commit Message

Michael Meissner April 28, 2016, 9:06 p.m. UTC 
In GCC 6.x, I was not able to get complex __float128 to work before the cut off
period for stage1 submissions. This patch enables complex support for PowerPC
__float128. Note, it does not address adding complex support in libgcc.

Note, similar to the x86_64, you cannot say:

	typedef _Complex __float128 f128_complex_type;

	f128_complex_type a, b, c;

instead you must use an alternate for using __attributes__:

	typedef _Complex float __attribute__((mode(KC))) f128_complex_type;

The problem is when layout_type created the complex type, mode_for_size is
used to find the appropriate complex type, using a size of 2 times the base
type. Unfortunately, in the PowerPC, we have two 128-bit floating point types
and mode_for_size returns the wrong one.

I solved this by having genmodes.c build a table that give a particular mode,
gives the mode that is the complex type for that mode, and having tree.c and
stor-layout.c use this type instead of using mode_for_size.

I did a boostrap and regression test and it did not show any regressions.

Are these patches acceptable to check into the trunk? After they go into the
trunk, and if it doesn't destablize the other ports, I would like to check
these changes into the GCC 6.x branch. Is this ok?

There are 3 attachments to this mail:

   1)	The first attachment (gcc-stage7.patch001b) are the machine independent
	changes.

   2)	The second attachment (gcc-stage7.patch001c) are the changes to the
	PowerPC compiler.

   3)	The third attachment (gcc-stage7.patch001d) is a new test to make sure
	complex float128 continues to work.

I or somebody else will submit a patch later to add support for libgcc.

[gcc]
2016-04-28  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 instead of trying to figure out the
	appropriate mode based on the size.

	* 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): Add support for
	__float128 and __ibm128 complex.
	(FLOAT128_IBM_P): Likewise.
	(ALTIVEC_COMPLEX): 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-28  Michael Meissner  <meissner@linux.vnet.ibm.com>

	* gcc.target/powerpc/float128-complex-1.c: New test for complex
	__float128.

Comments

Segher Boessenkool April 28, 2016, 10:10 p.m. UTC | #1 
On Thu, Apr 28, 2016 at 05:06:14PM -0400, Michael Meissner wrote:
Hi Mike,

> 	* 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): Add support for
> 	__float128 and __ibm128 complex.
> 	(FLOAT128_IBM_P): Likewise.
> 	(ALTIVEC_COMPLEX): 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-28  Michael Meissner  <meissner@linux.vnet.ibm.com>
> 
> 	* gcc.target/powerpc/float128-complex-1.c: New test for complex
> 	__float128.

It is more normal to not have blank lines between files in the changelog.

>  #define FLOAT128_IEEE_P(MODE)						\
> -  (((MODE) == TFmode && TARGET_IEEEQUAD)				\
> -   || ((MODE) == KFmode))
> +  ((TARGET_IEEEQUAD && ((MODE) == TFmode || (MODE) == TCmode))		\
> +   || ((MODE) == KFmode) || ((MODE) == KCmode))
>  
>  #define FLOAT128_IBM_P(MODE)						\
> -  (((MODE) == TFmode && !TARGET_IEEEQUAD)				\
> -   || ((MODE) == IFmode))
> +  ((!TARGET_IEEEQUAD && ((MODE) == TFmode || (MODE) == TCmode))		\
> +   || ((MODE) == IFmode) || ((MODE) == ICmode))

Maybe these should be inline functions now?

> +/* _Complex __float128 needs two registers.  */
> +#define ALTIVEC_COMPLEX	((TARGET_FLOAT128) ? 1 : 0)

This name is pretty confusing, way too generic too.  It is only used
right below, you can just code it there?

>  /* Return registers */
>  #define GP_ARG_RETURN GP_ARG_MIN_REG
>  #define FP_ARG_RETURN FP_ARG_MIN_REG
>  #define ALTIVEC_ARG_RETURN (FIRST_ALTIVEC_REGNO + 2)
>  #define FP_ARG_MAX_RETURN (DEFAULT_ABI != ABI_ELFv2 ? FP_ARG_RETURN	\
>  			   : (FP_ARG_RETURN + AGGR_ARG_NUM_REG - 1))
> -#define ALTIVEC_ARG_MAX_RETURN (DEFAULT_ABI != ABI_ELFv2 ? ALTIVEC_ARG_RETURN \
> +#define ALTIVEC_ARG_MAX_RETURN (DEFAULT_ABI != ABI_ELFv2		 \
> +				? (ALTIVEC_ARG_RETURN + ALTIVEC_COMPLEX) \
>  			        : (ALTIVEC_ARG_RETURN + AGGR_ARG_NUM_REG - 1))

So you are changing the ABI here (for non-v2).  Are there any complications
to that, is it compatible in all cases?

> -#define PRINT_OPERAND_PUNCT_VALID_P(CODE)  ((CODE) == '&')
> +#define PRINT_OPERAND_PUNCT_VALID_P(CODE)  ((CODE) == '&' || (CODE) == '@')

I think you mixed this in from another change?


Segher
Michael Meissner April 28, 2016, 11:40 p.m. UTC | #2 
On Thu, Apr 28, 2016 at 05:10:07PM -0500, Segher Boessenkool wrote:
> On Thu, Apr 28, 2016 at 05:06:14PM -0400, Michael Meissner wrote:
> Hi Mike,
> 
> > 	* 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): Add support for
> > 	__float128 and __ibm128 complex.
> > 	(FLOAT128_IBM_P): Likewise.
> > 	(ALTIVEC_COMPLEX): 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-28  Michael Meissner  <meissner@linux.vnet.ibm.com>
> > 
> > 	* gcc.target/powerpc/float128-complex-1.c: New test for complex
> > 	__float128.
> 
> It is more normal to not have blank lines between files in the changelog.

Ok.

> >  #define FLOAT128_IEEE_P(MODE)						\
> > -  (((MODE) == TFmode && TARGET_IEEEQUAD)				\
> > -   || ((MODE) == KFmode))
> > +  ((TARGET_IEEEQUAD && ((MODE) == TFmode || (MODE) == TCmode))		\
> > +   || ((MODE) == KFmode) || ((MODE) == KCmode))
> >  
> >  #define FLOAT128_IBM_P(MODE)						\
> > -  (((MODE) == TFmode && !TARGET_IEEEQUAD)				\
> > -   || ((MODE) == IFmode))
> > +  ((!TARGET_IEEEQUAD && ((MODE) == TFmode || (MODE) == TCmode))		\
> > +   || ((MODE) == IFmode) || ((MODE) == ICmode))
> 
> Maybe these should be inline functions now?

No they can't be without moving them somewhere else. The problem is if you
declare them as functions machine_mode must be defined, and it isn't in a lot
of cases, such as compiling the gen* functions. Even if machine_mode might
eventually be defined, rs6000.h can be included for machmode.h is defined.

> > +/* _Complex __float128 needs two registers.  */
> > +#define ALTIVEC_COMPLEX	((TARGET_FLOAT128) ? 1 : 0)

I wrote it that way to make ALTIVEC_ARG_MAX_RETURN fit in 79 columns. If you
want, I can probably rework ALTIVEC_ARG_MAX_RETURN.

> 
> This name is pretty confusing, way too generic too.  It is only used
> right below, you can just code it there?
> 
> >  /* Return registers */
> >  #define GP_ARG_RETURN GP_ARG_MIN_REG
> >  #define FP_ARG_RETURN FP_ARG_MIN_REG
> >  #define ALTIVEC_ARG_RETURN (FIRST_ALTIVEC_REGNO + 2)
> >  #define FP_ARG_MAX_RETURN (DEFAULT_ABI != ABI_ELFv2 ? FP_ARG_RETURN	\
> >  			   : (FP_ARG_RETURN + AGGR_ARG_NUM_REG - 1))
> > -#define ALTIVEC_ARG_MAX_RETURN (DEFAULT_ABI != ABI_ELFv2 ? ALTIVEC_ARG_RETURN \
> > +#define ALTIVEC_ARG_MAX_RETURN (DEFAULT_ABI != ABI_ELFv2		 \
> > +				? (ALTIVEC_ARG_RETURN + ALTIVEC_COMPLEX) \
> >  			        : (ALTIVEC_ARG_RETURN + AGGR_ARG_NUM_REG - 1))
> 
> So you are changing the ABI here (for non-v2).  Are there any complications
> to that, is it compatible in all cases?

I don't think so, all it is saying is we return complex float128 in 2 altivec
registers. Given you could not access complex float128 before, I don't think it
is an issue. Note, presently you must use -mfloat128 to enable it at all.

> 
> > -#define PRINT_OPERAND_PUNCT_VALID_P(CODE)  ((CODE) == '&')
> > +#define PRINT_OPERAND_PUNCT_VALID_P(CODE)  ((CODE) == '&' || (CODE) == '@')
> 
> I think you mixed this in from another change?

Yes, this is from another change.

I will issue another patch tomorrow.
diff mbox

Patch

Index: gcc/machmode.h
===================================================================
--- gcc/machmode.h	(revision 235529)
+++ 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	(revision 235529)
+++ gcc/stor-layout.c	(working copy)
@@ -2146,11 +2146,19 @@  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 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));
+	}
+
       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	(revision 235529)
+++ 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	(revision 235529)
+++ gcc/tree.c	(working copy)
@@ -8767,6 +8767,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));