[rs6000,C/C++] Fix PR target/86324: divkc3-1.c FAILs when compiling with -mabi=ieeelongdouble

Message ID bcca27f4-571b-025a-bc8f-fec4944f3ba4@linux.ibm.com
State New
Headers show
Series
  • [rs6000,C/C++] Fix PR target/86324: divkc3-1.c FAILs when compiling with -mabi=ieeelongdouble
Related show

Commit Message

Peter Bergner June 27, 2018, 8:26 p.m.
The test case in PR86324 exposes a problem when long double is the same as
__ieee128 and we attempt to use the KC mode attribute.  A complimentary
problem exists when long double is the same as __ibm128 and we try to use
the IC mode attribute.  The problem is that for the type that long double
is set to, we always use internally the long double type and TFmode/TCmode
rather than __ieee128/K[FC]mode or __ibm128/I[FC]mode, so we have problems
when looking up the unused modes during attribute processing.

Talking with Joseph offline, he suggested to add a hook that allows the
target to translate mode attributes like the above into the mode that
should be used.  The patch below implements that suggestion and it fixes
the problem test cases.

This passed bootstrap and regtesting with no regressions on powerpc64le-linux.
Ok for trunk?

This is also broken in GCC 8, is this ok to backport once it has has baked
on trunk for a few days and assuming testing comes out clean?

The pr86324-2.c test case is also broken in GCC 7, but it looks like that
has been broken forever, so I'm not sure I'm inclined to want to backport
this that far.  Thoughts?

Peter


gcc/
	PR target/86324
	* target.def (translate_mode_attribute): New hook.
	* targhooks.h (default_translate_mode_attribute): Declare.
	* targhooks.c (default_translate_mode_attribute): New function.
	* doc/tm.texi.in (TARGET_TRANSLATE_MODE_ATTRIBUTE): New hook.
	* doc/tm.texi: Regenerate.
	* config/rs6000/rs6000.c (TARGET_TRANSLATE_MODE_ATTRIBUTE): Define.
	(rs6000_translate_mode_attribute): New function.

gcc/c-family/
	PR target/86324
	* c-attribs.c (handle_mode_attribute): Call new translate_mode_attribute
	target hook.

Comments

Peter Bergner June 27, 2018, 9:02 p.m. | #1
On 6/27/18 3:26 PM, Peter Bergner wrote:
> gcc/

> 	PR target/86324

> 	* target.def (translate_mode_attribute): New hook.

> 	* targhooks.h (default_translate_mode_attribute): Declare.

> 	* targhooks.c (default_translate_mode_attribute): New function.

> 	* doc/tm.texi.in (TARGET_TRANSLATE_MODE_ATTRIBUTE): New hook.

> 	* doc/tm.texi: Regenerate.

> 	* config/rs6000/rs6000.c (TARGET_TRANSLATE_MODE_ATTRIBUTE): Define.

> 	(rs6000_translate_mode_attribute): New function.

> 

> gcc/c-family/

> 	PR target/86324

> 	* c-attribs.c (handle_mode_attribute): Call new translate_mode_attribute

> 	target hook.


Oops, I forgot the test cases.  Here they are.

Peter


gcc/testsuite/
	PR target/86324
	gcc.target/powerpc/pr86324-1.c: New test.
	gcc.target/powerpc/pr86324-2.c: Likewise.

Index: gcc/testsuite/gcc.target/powerpc/pr86324-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr86324-1.c	(nonexistent)
+++ gcc/testsuite/gcc.target/powerpc/pr86324-1.c	(working copy)
@@ -0,0 +1,10 @@
+/* { dg-do compile { target { powerpc*-*-linux* } } } */
+/* { dg-options "-mlong-double-128 -mabi=ieeelongdouble -Wno-psabi" } */
+
+typedef __complex float cflt_t __attribute__((mode(KC)));
+
+cflt_t
+divide (cflt_t *ptr)
+{
+  return *ptr;
+}
Index: gcc/testsuite/gcc.target/powerpc/pr86324-2.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr86324-2.c	(nonexistent)
+++ gcc/testsuite/gcc.target/powerpc/pr86324-2.c	(working copy)
@@ -0,0 +1,10 @@
+/* { dg-do compile { target { powerpc*-*-linux* } } } */
+/* { dg-options "-mlong-double-128 -mabi=ibmlongdouble -Wno-psabi" } */
+
+typedef __complex float cflt_t __attribute__((mode(IC)));
+
+cflt_t
+divide (cflt_t *ptr)
+{
+  return *ptr;
+}
Segher Boessenkool June 27, 2018, 9:35 p.m. | #2
On Wed, Jun 27, 2018 at 03:26:42PM -0500, Peter Bergner wrote:
> The test case in PR86324 exposes a problem when long double is the same as

> __ieee128 and we attempt to use the KC mode attribute.  A complimentary

> problem exists when long double is the same as __ibm128 and we try to use

> the IC mode attribute.  The problem is that for the type that long double

> is set to, we always use internally the long double type and TFmode/TCmode

> rather than __ieee128/K[FC]mode or __ibm128/I[FC]mode, so we have problems

> when looking up the unused modes during attribute processing.

> 

> Talking with Joseph offline, he suggested to add a hook that allows the

> target to translate mode attributes like the above into the mode that

> should be used.  The patch below implements that suggestion and it fixes

> the problem test cases.

> 

> This passed bootstrap and regtesting with no regressions on powerpc64le-linux.

> Ok for trunk?

> 

> This is also broken in GCC 8, is this ok to backport once it has has baked

> on trunk for a few days and assuming testing comes out clean?

> 

> The pr86324-2.c test case is also broken in GCC 7, but it looks like that

> has been broken forever, so I'm not sure I'm inclined to want to backport

> this that far.  Thoughts?


This is all okay with me, if someone approves the generic parts.

One thing:

> --- gcc/target.def	(revision 262159)

> +++ gcc/target.def	(working copy)

> @@ -3310,6 +3310,16 @@ constants can be done inline.  The funct

>   HOST_WIDE_INT, (const_tree constant, HOST_WIDE_INT basic_align),

>   default_constant_alignment)

>  

> +DEFHOOK

> +(translate_mode_attribute,

> + "Define this hook if the port should translate machine_mode @var{mode}\n\

> +to another mode.  For example, rs6000's @code{KFmode}, when it is the same\n\

> +as @code{TFmode}.\n\

> +\n\

> +The default version of the hook returns that mode that was passed in.",

> + machine_mode, (machine_mode mode),

> + default_translate_mode_attribute)


It isn't clear here when this hook is called.  Is the idea to use it
everywhere where modes are created, or only where it is used now (the arg
to a "mode" attribute)?  Probably the latter, but it's not really clear
from the text.

Thanks,


Segher
Peter Bergner June 27, 2018, 11:33 p.m. | #3
On 6/27/18 4:35 PM, Segher Boessenkool wrote:
>> +DEFHOOK

>> +(translate_mode_attribute,

>> + "Define this hook if the port should translate machine_mode @var{mode}\n\

>> +to another mode.  For example, rs6000's @code{KFmode}, when it is the same\n\

>> +as @code{TFmode}.\n\

>> +\n\

>> +The default version of the hook returns that mode that was passed in.",

>> + machine_mode, (machine_mode mode),

>> + default_translate_mode_attribute)

> 

> It isn't clear here when this hook is called.  Is the idea to use it

> everywhere where modes are created, or only where it is used now (the arg

> to a "mode" attribute)?  Probably the latter, but it's not really clear

> from the text.


The latter and good idea.  How about this wording instead?

Peter

Index: gcc/target.def
===================================================================
--- gcc/target.def	(revision 262159)
+++ gcc/target.def	(working copy)
@@ -3310,6 +3310,16 @@ constants can be done inline.  The funct
  HOST_WIDE_INT, (const_tree constant, HOST_WIDE_INT basic_align),
  default_constant_alignment)

+DEFHOOK
+(translate_mode_attribute,
+ "Define this hook if during mode attribute processing, the port should\n\
+translate machine_mode @var{mode} to another mode.  For example, rs6000's\n\
+@code{KFmode}, when it is the same as @code{TFmode}.\n\
+\n\
+The default version of the hook returns that mode that was passed in.",
+ machine_mode, (machine_mode mode),
+ default_translate_mode_attribute)
+
 /* True if MODE is valid for the target.  By "valid", we mean able to
    be manipulated in non-trivial ways.  In particular, this means all
    the arithmetic is supported.  */

Patch

Index: gcc/target.def
===================================================================
--- gcc/target.def	(revision 262159)
+++ gcc/target.def	(working copy)
@@ -3310,6 +3310,16 @@  constants can be done inline.  The funct
  HOST_WIDE_INT, (const_tree constant, HOST_WIDE_INT basic_align),
  default_constant_alignment)
 
+DEFHOOK
+(translate_mode_attribute,
+ "Define this hook if the port should translate machine_mode @var{mode}\n\
+to another mode.  For example, rs6000's @code{KFmode}, when it is the same\n\
+as @code{TFmode}.\n\
+\n\
+The default version of the hook returns that mode that was passed in.",
+ machine_mode, (machine_mode mode),
+ default_translate_mode_attribute)
+
 /* True if MODE is valid for the target.  By "valid", we mean able to
    be manipulated in non-trivial ways.  In particular, this means all
    the arithmetic is supported.  */
Index: gcc/targhooks.c
===================================================================
--- gcc/targhooks.c	(revision 262159)
+++ gcc/targhooks.c	(working copy)
@@ -393,6 +393,14 @@  default_mangle_assembler_name (const cha
   return get_identifier (stripped);
 }
 
+/* The default implementation of TARGET_TRANSLATE_MODE_ATTRIBUTE.  */
+
+machine_mode
+default_translate_mode_attribute (machine_mode mode)
+{
+  return mode;
+}
+
 /* True if MODE is valid for the target.  By "valid", we mean able to
    be manipulated in non-trivial ways.  In particular, this means all
    the arithmetic is supported.
Index: gcc/targhooks.h
===================================================================
--- gcc/targhooks.h	(revision 262159)
+++ gcc/targhooks.h	(working copy)
@@ -72,6 +72,7 @@  extern void default_print_operand_addres
 extern bool default_print_operand_punct_valid_p (unsigned char);
 extern tree default_mangle_assembler_name (const char *);
 
+extern machine_mode default_translate_mode_attribute (machine_mode);
 extern bool default_scalar_mode_supported_p (scalar_mode);
 extern bool default_libgcc_floating_mode_supported_p (scalar_float_mode);
 extern opt_scalar_float_mode default_floatn_mode (int, bool);
Index: gcc/doc/tm.texi
===================================================================
--- gcc/doc/tm.texi	(revision 262159)
+++ gcc/doc/tm.texi	(working copy)
@@ -4255,6 +4255,14 @@  hook returns true for both @code{ptr_mod
 Define this to return nonzero if the memory reference @var{ref}  may alias with the system C library errno location.  The default  version of this hook assumes the system C library errno location  is either a declaration of type int or accessed by dereferencing  a pointer to int.
 @end deftypefn
 
+@deftypefn {Target Hook} machine_mode TARGET_TRANSLATE_MODE_ATTRIBUTE (machine_mode @var{mode})
+Define this hook if the port should translate machine_mode @var{mode}
+to another mode.  For example, rs6000's @code{KFmode}, when it is the same
+as @code{TFmode}.
+
+The default version of the hook returns that mode that was passed in.
+@end deftypefn
+
 @deftypefn {Target Hook} bool TARGET_SCALAR_MODE_SUPPORTED_P (scalar_mode @var{mode})
 Define this to return nonzero if the port is prepared to handle
 insns involving scalar mode @var{mode}.  For a scalar mode to be
Index: gcc/doc/tm.texi.in
===================================================================
--- gcc/doc/tm.texi.in	(revision 262159)
+++ gcc/doc/tm.texi.in	(working copy)
@@ -3336,6 +3336,8 @@  stack.
 
 @hook TARGET_REF_MAY_ALIAS_ERRNO
 
+@hook TARGET_TRANSLATE_MODE_ATTRIBUTE
+
 @hook TARGET_SCALAR_MODE_SUPPORTED_P
 
 @hook TARGET_VECTOR_MODE_SUPPORTED_P
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 262159)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -1802,6 +1802,9 @@  static const struct attribute_spec rs600
 #undef TARGET_EH_RETURN_FILTER_MODE
 #define TARGET_EH_RETURN_FILTER_MODE rs6000_eh_return_filter_mode
 
+#undef TARGET_TRANSLATE_MODE_ATTRIBUTE
+#define TARGET_TRANSLATE_MODE_ATTRIBUTE rs6000_translate_mode_attribute
+
 #undef TARGET_SCALAR_MODE_SUPPORTED_P
 #define TARGET_SCALAR_MODE_SUPPORTED_P rs6000_scalar_mode_supported_p
 
@@ -35677,6 +35680,18 @@  rs6000_eh_return_filter_mode (void)
   return TARGET_32BIT ? SImode : word_mode;
 }
 
+/* Target hook for translate_mode_attribute.  */
+static machine_mode
+rs6000_translate_mode_attribute (machine_mode mode)
+{
+  if ((FLOAT128_IEEE_P (mode)
+       && ieee128_float_type_node == long_double_type_node)
+      || (FLOAT128_IBM_P (mode)
+	  && ibm128_float_type_node == long_double_type_node))
+    return COMPLEX_MODE_P (mode) ? E_TCmode : E_TFmode;
+  return mode;
+}
+
 /* Target hook for scalar_mode_supported_p.  */
 static bool
 rs6000_scalar_mode_supported_p (scalar_mode mode)
Index: gcc/c-family/c-attribs.c
===================================================================
--- gcc/c-family/c-attribs.c	(revision 262159)
+++ gcc/c-family/c-attribs.c	(working copy)
@@ -1517,6 +1517,10 @@  handle_mode_attribute (tree *node, tree
 	  return NULL_TREE;
 	}
 
+      /* Allow the target a chance to translate MODE into something supported.
+	 See PR86324.  */
+      mode = targetm.translate_mode_attribute (mode);
+
       valid_mode = false;
       switch (GET_MODE_CLASS (mode))
 	{