diff mbox

[PR,79905] ICE with vector_type

Message ID f334e863-5702-f13d-5a04-90cd88459b21@acm.org
State New
Headers show

Commit Message

Nathan Sidwell April 6, 2017, 6:34 p.m. UTC 
On 04/06/2017 11:13 AM, Bill Schmidt wrote:

>>> Nathan's patch regstraps cleanly.  I'll try Richard's variant (dropping the if test below) now.
>
> As expected, this version passes regstrap as well.

Thanks for testing.

Segher, this fixes a C++ ICE where TYPE_CANONICALs didn't match, but the 
types were the same (and non-structural comparison).  The underlying 
cause is that types with different TYPE_NAME are considered different 
canonical types.  add_builtin_type smacked TYPE_NAME of the canonical 
type, therefore guaranteeing that any subsequent vector types would be 
thought of as different.

ok?

Bill's been testing these patches.

nathan

Comments

Segher Boessenkool April 6, 2017, 8:17 p.m. UTC | #1 
Hi!

On Thu, Apr 06, 2017 at 02:34:03PM -0400, Nathan Sidwell wrote:
> Segher, this fixes a C++ ICE where TYPE_CANONICALs didn't match, but the 
> types were the same (and non-structural comparison).  The underlying 
> cause is that types with different TYPE_NAME are considered different 
> canonical types.  add_builtin_type smacked TYPE_NAME of the canonical 
> type, therefore guaranteeing that any subsequent vector types would be 
> thought of as different.

> Index: config/rs6000/rs6000.c
> ===================================================================
> --- config/rs6000/rs6000.c	(revision 246647)
> +++ config/rs6000/rs6000.c	(working copy)
> @@ -17257,6 +17257,22 @@ rs6000_expand_builtin (tree exp, rtx tar
>    gcc_unreachable ();
>  }
>  
> +/* Create a builtin vector type with a name.  Taking care not to give
> +   the canonical type a name.  */
> +
> +static tree
> +rs6000_vt (const char *name, tree elt_type, unsigned num_elts)

I don't like this cryptic name very much.  Maybe you could just use a
longer name and indent differently (break at the "=" for example), or
do a macro around where it is used a lot?

But, okay for trunk whatever you decide on this.  Thanks!


Segher
Nathan Sidwell April 10, 2017, 11:24 a.m. UTC | #2 
On 04/06/2017 04:17 PM, Segher Boessenkool wrote:

> I don't like this cryptic name very much.  Maybe you could just use a
> longer name and indent differently (break at the "=" for example), or
> do a macro around where it is used a lot?

I went with rs6000_vector_type
diff mbox

Patch

2017-04-05  Nathan Sidwell  <nathan@acm.org>
	    Richard Biener  <rguenther@suse.de>

	PR target/79905
	* config/rs6000/rs6000.c (rs6000_vt): New.
	(rs6000_init_builtins): Use it.

	testsuite/
	* g++.dg/torture/pr79905.C: New.

Index: config/rs6000/rs6000.c
===================================================================
--- config/rs6000/rs6000.c	(revision 246647)
+++ config/rs6000/rs6000.c	(working copy)
@@ -17257,6 +17257,22 @@  rs6000_expand_builtin (tree exp, rtx tar
   gcc_unreachable ();
 }
 
+/* Create a builtin vector type with a name.  Taking care not to give
+   the canonical type a name.  */
+
+static tree
+rs6000_vt (const char *name, tree elt_type, unsigned num_elts)
+{
+  tree result = build_vector_type (elt_type, num_elts);
+
+  /* Copy so we don't give the canonical type a name.  */
+  result = build_variant_type_copy (result);
+
+  add_builtin_type (name, result);
+
+  return result;
+}
+
 static void
 rs6000_init_builtins (void)
 {
@@ -17273,18 +17289,25 @@  rs6000_init_builtins (void)
 
   V2SI_type_node = build_vector_type (intSI_type_node, 2);
   V2SF_type_node = build_vector_type (float_type_node, 2);
-  V2DI_type_node = build_vector_type (intDI_type_node, 2);
-  V2DF_type_node = build_vector_type (double_type_node, 2);
+  V2DI_type_node = rs6000_vt (TARGET_POWERPC64 ? "__vector long"
+			      : "__vector long long", intDI_type_node, 2);
+  V2DF_type_node = rs6000_vt ("__vector double", double_type_node, 2);
   V4HI_type_node = build_vector_type (intHI_type_node, 4);
-  V4SI_type_node = build_vector_type (intSI_type_node, 4);
-  V4SF_type_node = build_vector_type (float_type_node, 4);
-  V8HI_type_node = build_vector_type (intHI_type_node, 8);
-  V16QI_type_node = build_vector_type (intQI_type_node, 16);
-
-  unsigned_V16QI_type_node = build_vector_type (unsigned_intQI_type_node, 16);
-  unsigned_V8HI_type_node = build_vector_type (unsigned_intHI_type_node, 8);
-  unsigned_V4SI_type_node = build_vector_type (unsigned_intSI_type_node, 4);
-  unsigned_V2DI_type_node = build_vector_type (unsigned_intDI_type_node, 2);
+  V4SI_type_node = rs6000_vt ("__vector signed int", intSI_type_node, 4);
+  V4SF_type_node = rs6000_vt ("__vector float", float_type_node, 4);
+  V8HI_type_node = rs6000_vt ("__vector signed short", intHI_type_node, 8);
+  V16QI_type_node = rs6000_vt ("__vector signed char", intQI_type_node, 16);
+
+  unsigned_V16QI_type_node = rs6000_vt ("__vector unsigned char",
+					unsigned_intQI_type_node, 16);
+  unsigned_V8HI_type_node = rs6000_vt ("__vector unsigned short",
+				       unsigned_intHI_type_node, 8);
+  unsigned_V4SI_type_node = rs6000_vt ("__vector unsigned int",
+				       unsigned_intSI_type_node, 4);
+  unsigned_V2DI_type_node = rs6000_vt (TARGET_POWERPC64
+				       ? "__vector unsigned long"
+				       : "__vector unsigned long long",
+				       unsigned_intDI_type_node, 2);
 
   opaque_V2SF_type_node = build_opaque_vector_type (float_type_node, 2);
   opaque_V2SI_type_node = build_opaque_vector_type (intSI_type_node, 2);
@@ -17299,8 +17322,9 @@  rs6000_init_builtins (void)
      must live in VSX registers.  */
   if (intTI_type_node)
     {
-      V1TI_type_node = build_vector_type (intTI_type_node, 1);
-      unsigned_V1TI_type_node = build_vector_type (unsigned_intTI_type_node, 1);
+      V1TI_type_node = rs6000_vt ("__vector __int128", intTI_type_node, 1);
+      unsigned_V1TI_type_node = rs6000_vt ("__vector unsigned __int128",
+					   unsigned_intTI_type_node, 1);
     }
 
   /* The 'vector bool ...' types must be kept distinct from 'vector unsigned ...'
@@ -17432,83 +17456,16 @@  rs6000_init_builtins (void)
   tdecl = add_builtin_type ("__pixel", pixel_type_node);
   TYPE_NAME (pixel_type_node) = tdecl;
 
-  bool_V16QI_type_node = build_vector_type (bool_char_type_node, 16);
-  bool_V8HI_type_node = build_vector_type (bool_short_type_node, 8);
-  bool_V4SI_type_node = build_vector_type (bool_int_type_node, 4);
-  bool_V2DI_type_node = build_vector_type (bool_long_type_node, 2);
-  pixel_V8HI_type_node = build_vector_type (pixel_type_node, 8);
-
-  tdecl = add_builtin_type ("__vector unsigned char", unsigned_V16QI_type_node);
-  TYPE_NAME (unsigned_V16QI_type_node) = tdecl;
-
-  tdecl = add_builtin_type ("__vector signed char", V16QI_type_node);
-  TYPE_NAME (V16QI_type_node) = tdecl;
-
-  tdecl = add_builtin_type ("__vector __bool char", bool_V16QI_type_node);
-  TYPE_NAME (bool_V16QI_type_node) = tdecl;
-
-  tdecl = add_builtin_type ("__vector unsigned short", unsigned_V8HI_type_node);
-  TYPE_NAME (unsigned_V8HI_type_node) = tdecl;
-
-  tdecl = add_builtin_type ("__vector signed short", V8HI_type_node);
-  TYPE_NAME (V8HI_type_node) = tdecl;
-
-  tdecl = add_builtin_type ("__vector __bool short", bool_V8HI_type_node);
-  TYPE_NAME (bool_V8HI_type_node) = tdecl;
-
-  tdecl = add_builtin_type ("__vector unsigned int", unsigned_V4SI_type_node);
-  TYPE_NAME (unsigned_V4SI_type_node) = tdecl;
-
-  tdecl = add_builtin_type ("__vector signed int", V4SI_type_node);
-  TYPE_NAME (V4SI_type_node) = tdecl;
-
-  tdecl = add_builtin_type ("__vector __bool int", bool_V4SI_type_node);
-  TYPE_NAME (bool_V4SI_type_node) = tdecl;
-
-  tdecl = add_builtin_type ("__vector float", V4SF_type_node);
-  TYPE_NAME (V4SF_type_node) = tdecl;
-
-  tdecl = add_builtin_type ("__vector __pixel", pixel_V8HI_type_node);
-  TYPE_NAME (pixel_V8HI_type_node) = tdecl;
-
-  tdecl = add_builtin_type ("__vector double", V2DF_type_node);
-  TYPE_NAME (V2DF_type_node) = tdecl;
-
-  if (TARGET_POWERPC64)
-    {
-      tdecl = add_builtin_type ("__vector long", V2DI_type_node);
-      TYPE_NAME (V2DI_type_node) = tdecl;
-
-      tdecl = add_builtin_type ("__vector unsigned long",
-				unsigned_V2DI_type_node);
-      TYPE_NAME (unsigned_V2DI_type_node) = tdecl;
-
-      tdecl = add_builtin_type ("__vector __bool long", bool_V2DI_type_node);
-      TYPE_NAME (bool_V2DI_type_node) = tdecl;
-    }
-  else
-    {
-      tdecl = add_builtin_type ("__vector long long", V2DI_type_node);
-      TYPE_NAME (V2DI_type_node) = tdecl;
-
-      tdecl = add_builtin_type ("__vector unsigned long long",
-				unsigned_V2DI_type_node);
-      TYPE_NAME (unsigned_V2DI_type_node) = tdecl;
-
-      tdecl = add_builtin_type ("__vector __bool long long",
-				bool_V2DI_type_node);
-      TYPE_NAME (bool_V2DI_type_node) = tdecl;
-    }
-
-  if (V1TI_type_node)
-    {
-      tdecl = add_builtin_type ("__vector __int128", V1TI_type_node);
-      TYPE_NAME (V1TI_type_node) = tdecl;
-
-      tdecl = add_builtin_type ("__vector unsigned __int128",
-				unsigned_V1TI_type_node);
-      TYPE_NAME (unsigned_V1TI_type_node) = tdecl;
-    }
+  bool_V16QI_type_node = rs6000_vt ("__vector __bool char",
+				    bool_char_type_node, 16);
+  bool_V8HI_type_node = rs6000_vt ("__vector __bool short",
+				   bool_short_type_node, 8);
+  bool_V4SI_type_node = rs6000_vt ("__vector __bool int",
+				   bool_int_type_node, 4);
+  bool_V2DI_type_node = rs6000_vt (TARGET_POWERPC64 ? "__vector __bool long"
+				   : "__vector __bool long long",
+				   bool_long_type_node, 2);
+  pixel_V8HI_type_node = rs6000_vt ("__vector __pixel", pixel_type_node, 8);
 
   /* Paired and SPE builtins are only available if you build a compiler with
      the appropriate options, so only create those builtins with the
Index: testsuite/g++.dg/torture/pr79905.C
===================================================================
--- testsuite/g++.dg/torture/pr79905.C	(revision 0)
+++ testsuite/g++.dg/torture/pr79905.C	(working copy)
@@ -0,0 +1,9 @@ 
+// PR target/79905
+// { dg-do compile { target { powerpc*-*-* } } }
+// { dg-require-effective-target powerpc_altivec_ok } 
+
+typedef int V4i __attribute__((vector_size(16)));
+void a (V4i) {
+  vector int b;
+  a (b);
+}