diff mbox

[PR,79905] ICE with vector_type

Message ID 7cd553e8-623a-d2d4-0ace-331f3df14701@acm.org
State New
Headers show

Commit Message

Nathan Sidwell April 4, 2017, 1:49 p.m. UTC
On 04/04/2017 09:00 AM, Richard Biener wrote:

> tree
> add_builtin_type (const char *name, tree type)
> {
>   tree   id = get_identifier (name);
>   tree decl = build_decl (BUILTINS_LOCATION, TYPE_DECL, id, type);
>   return lang_hooks.decls.pushdecl (decl);
> }
>
> this seems to miss setting TYPE_NAME (type) = decl - oh, that may be
> what set_underlying_type does via the langhook.

Correct, via the langhook.  I wonder if the smacking of the 
incoming-type's TYPE_NAME is so that the global tree node references the 
now-named builtin type.  If we were to make a clone here, (things like) 
VS4SI_type_node would remain an unnamed type.  And I guess debug would 
be unhappy?

> At this point I'd rather restrict fiddling in this fragile area in
> rs6000.c only ;)

me too.

> Does removing the TYPE_NAME setting fix things?

no,  those TYPE_NAME assignments are redundant -- set_underlying_type 
has already initialized it.

The attached PoC fixes 79905 (no idea what ppc test it might break)

nathan

Comments

Bill Schmidt April 4, 2017, 1:57 p.m. UTC | #1
I'll try the POC patch in a bit (kind of ugly as we have to replicate this for a whole bunch of types, I guess).

Just FYI, I noticed this similar bug report came in today, not sure about the types:  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80309

-- Bill

Bill Schmidt, Ph.D.
GCC for Linux on Power
Linux on Power Toolchain
IBM Linux Technology Center
wschmidt@linux.vnet.ibm.com

> On Apr 4, 2017, at 8:49 AM, Nathan Sidwell <nathan@acm.org> wrote:
> 
> On 04/04/2017 09:00 AM, Richard Biener wrote:
> 
>> tree
>> add_builtin_type (const char *name, tree type)
>> {
>>  tree   id = get_identifier (name);
>>  tree decl = build_decl (BUILTINS_LOCATION, TYPE_DECL, id, type);
>>  return lang_hooks.decls.pushdecl (decl);
>> }
>> 
>> this seems to miss setting TYPE_NAME (type) = decl - oh, that may be
>> what set_underlying_type does via the langhook.
> 
> Correct, via the langhook.  I wonder if the smacking of the incoming-type's TYPE_NAME is so that the global tree node references the now-named builtin type.  If we were to make a clone here, (things like) VS4SI_type_node would remain an unnamed type.  And I guess debug would be unhappy?
> 
>> At this point I'd rather restrict fiddling in this fragile area in
>> rs6000.c only ;)
> 
> me too.
> 
>> Does removing the TYPE_NAME setting fix things?
> 
> no,  those TYPE_NAME assignments are redundant -- set_underlying_type has already initialized it.
> 
> The attached PoC fixes 79905 (no idea what ppc test it might break)
> 
> nathan
> 
> -- 
> Nathan Sidwell
> <79905-2.diff>
Nathan Sidwell April 4, 2017, 2:02 p.m. UTC | #2
On 04/04/2017 09:57 AM, Bill Schmidt wrote:
> I'll try the POC patch in a bit (kind of ugly as we have to replicate this for a whole bunch of types, I guess).

thanks.  Yup, all rs6000's builtins.  Wrapping it all in a
tree my_builtin_vector (char const *name, tree elt, unsigned num);
helper would probably make rs6000.c's initing cleaner?


> Just FYI, I noticed this similar bug report came in today, not sure about the types:  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80309

thanks.  Probably not the same, but let's wait for a reduced testcase?

nathan
Bill Schmidt April 4, 2017, 5:40 p.m. UTC | #3
At first blush, the POC patch breaks every test that uses -flto.  I'll see if I can dig deeper after a bit,
in case this doesn't make the problem obvious.

Thanks,
Bill

> On Apr 4, 2017, at 9:02 AM, Nathan Sidwell <nathan@acm.org> wrote:
> 
> On 04/04/2017 09:57 AM, Bill Schmidt wrote:
>> I'll try the POC patch in a bit (kind of ugly as we have to replicate this for a whole bunch of types, I guess).
> 
> thanks.  Yup, all rs6000's builtins.  Wrapping it all in a
> tree my_builtin_vector (char const *name, tree elt, unsigned num);
> helper would probably make rs6000.c's initing cleaner?
> 
> 
>> Just FYI, I noticed this similar bug report came in today, not sure about the types:  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80309
> 
> thanks.  Probably not the same, but let's wait for a reduced testcase?
> 
> nathan
> 
> -- 
> Nathan Sidwell
>
diff mbox

Patch

Index: config/rs6000/rs6000.c
===================================================================
--- config/rs6000/rs6000.c	(revision 246647)
+++ config/rs6000/rs6000.c	(working copy)
@@ -17459,9 +17459,23 @@  rs6000_init_builtins (void)
   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;
+  { // Ugly POC hack
+    TYPE_NAME (V4SI_type_node) = error_mark_node; // placeholder
+    tdecl = add_builtin_type ("__vector signed int", V4SI_type_node);
+    TYPE_NAME (V4SI_type_node) = NULL_TREE; // restore
 
+    // Ew, change the underlying type
+    DECL_ORIGINAL_TYPE (tdecl) = V4SI_type_node;
+    tree clone = build_variant_type_copy (V4SI_type_node);
+    TYPE_STUB_DECL (clone) = TYPE_STUB_DECL (V4SI_type_node);
+    TYPE_NAME (clone) = tdecl;
+
+    TREE_USED (clone) = 1;
+    TREE_TYPE (tdecl) = clone;
+
+    V4SI_type_node = TREE_TYPE (tdecl);
+  }
+  
   tdecl = add_builtin_type ("__vector __bool int", bool_V4SI_type_node);
   TYPE_NAME (bool_V4SI_type_node) = tdecl;