* [PR 79905] ICE with vector_type
@ 2017-04-03 19:03 Nathan Sidwell
2017-04-04 8:28 ` Richard Biener
0 siblings, 1 reply; 23+ messages in thread
From: Nathan Sidwell @ 2017-04-03 19:03 UTC (permalink / raw)
To: wschmidt; +Cc: GCC Patches
[-- Attachment #1: Type: text/plain, Size: 812 bytes --]
Bill,
can you give this patch a spin please? I've smoke tested it on a ppc64le
x-compiler, but don't have one to run executables. regression testing
on an x86_64-linux system is ok.
The DEPENDENT_TYPE_VALID_P thing is a red herring.
It is the canonical type table's equal function considering most types
with different TYPE_NAMEs to be different. Now, you might think that
only applies to things like RECORD_TYPE, but no, wchar_t is different
from plain int, for example.
However, as you can see there's already a get-out for COMPLEX_TYPE, and
I think the same is needed for VECTOR_TYPE.
Both set_underlying_type and rs6000 set the builtin vector type's
TYPE_NAME, and so one constructed via applying
__attribute__((vector_size(16))) will never match. And it should.
nathan
--
Nathan Sidwell
[-- Attachment #2: 79905.diff --]
[-- Type: text/x-patch, Size: 816 bytes --]
Index: tree.c
===================================================================
--- tree.c (revision 246647)
+++ tree.c (working copy)
@@ -7005,10 +7005,11 @@ type_cache_hasher::equal (type_hash *a,
if (a->hash != b->hash
|| TREE_CODE (a->type) != TREE_CODE (b->type)
|| TREE_TYPE (a->type) != TREE_TYPE (b->type)
- || !attribute_list_equal (TYPE_ATTRIBUTES (a->type),
- TYPE_ATTRIBUTES (b->type))
|| (TREE_CODE (a->type) != COMPLEX_TYPE
- && TYPE_NAME (a->type) != TYPE_NAME (b->type)))
+ && TREE_CODE (a->type) != VECTOR_TYPE
+ && TYPE_NAME (a->type) != TYPE_NAME (b->type))
+ || !attribute_list_equal (TYPE_ATTRIBUTES (a->type),
+ TYPE_ATTRIBUTES (b->type)))
return 0;
/* Be careful about comparing arrays before and after the element type
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PR 79905] ICE with vector_type
2017-04-03 19:03 [PR 79905] ICE with vector_type Nathan Sidwell
@ 2017-04-04 8:28 ` Richard Biener
2017-04-04 11:31 ` Nathan Sidwell
0 siblings, 1 reply; 23+ messages in thread
From: Richard Biener @ 2017-04-04 8:28 UTC (permalink / raw)
To: Nathan Sidwell; +Cc: William J. Schmidt, GCC Patches
On Mon, Apr 3, 2017 at 9:03 PM, Nathan Sidwell <nathan@acm.org> wrote:
> Bill,
> can you give this patch a spin please? I've smoke tested it on a ppc64le
> x-compiler, but don't have one to run executables. regression testing on an
> x86_64-linux system is ok.
>
> The DEPENDENT_TYPE_VALID_P thing is a red herring.
>
> It is the canonical type table's equal function considering most types with
> different TYPE_NAMEs to be different. Now, you might think that only
> applies to things like RECORD_TYPE, but no, wchar_t is different from plain
> int, for example.
>
> However, as you can see there's already a get-out for COMPLEX_TYPE, and I
> think the same is needed for VECTOR_TYPE.
Hmm, but that is essentially a hack given that build_complex_type does things
it shouldn't (assign a name to the type). make_vector_type doesn't do that so
it shouldn't get such special-handing.
> Both set_underlying_type and rs6000 set the builtin vector type's TYPE_NAME,
> and so one constructed via applying __attribute__((vector_size(16))) will
> never match. And it should.
why? They only need to share the canonical type not the type node itself
(unless their name is the same, of course).
Now -- that name comparing is somewhat odd. We hash type "variants"
with different names the same (so setting the name after inserting sth into
the hash is allowed, but it will overwrite the old entry so any unnamed uses
up to now get a type with a name...). I guess we'd be better off leaving
only unnamed types in the hash and building a type variant whenever we
add a name to such type.
Richard.
> nathan
> --
> Nathan Sidwell
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PR 79905] ICE with vector_type
2017-04-04 8:28 ` Richard Biener
@ 2017-04-04 11:31 ` Nathan Sidwell
2017-04-04 13:00 ` Richard Biener
0 siblings, 1 reply; 23+ messages in thread
From: Nathan Sidwell @ 2017-04-04 11:31 UTC (permalink / raw)
To: Richard Biener; +Cc: William J. Schmidt, GCC Patches
On 04/04/2017 04:27 AM, Richard Biener wrote:
> On Mon, Apr 3, 2017 at 9:03 PM, Nathan Sidwell <nathan@acm.org> wrote:
>> However, as you can see there's already a get-out for COMPLEX_TYPE, and I
>> think the same is needed for VECTOR_TYPE.
>
> Hmm, but that is essentially a hack given that build_complex_type does things
> it shouldn't (assign a name to the type).
Oh, didn't know that.
>> Both set_underlying_type and rs6000 set the builtin vector type's TYPE_NAME,
>> and so one constructed via applying __attribute__((vector_size(16))) will
>> never match. And it should.
>
> why? They only need to share the canonical type not the type node itself
> (unless their name is the same, of course).
Correct, that's what I meant. the canonical type equal function must
match a just-consed vector with the already-hashed builtin.
> Now -- that name comparing is somewhat odd. We hash type "variants"
> with different names the same (so setting the name after inserting sth into
> the hash is allowed, but it will overwrite the old entry so any unnamed uses
> up to now get a type with a name...). I guess we'd be better off leaving
> only unnamed types in the hash and building a type variant whenever we
> add a name to such type.
Right, the name matching surprised me, and my first attempt was to
remove it. but that breaks wchar_t. wchar_t is the same representation
as int (pedantically, some integral type), but is a distinct type (in
C++ at least, I don't think C cares). The canonical type system records
language-level distinctness, not representation distinctness.
The difference between wchar_t and vector types is the only way to get a
type the same as wchar_t is to use 'wchar_t' (so we have to start with a
type at least as canonical as what we want). Whereas vector types are
constructed via applying attributes to some underlying scalar type (so
we have to go find the canonical type).
It would therefore seem that this code in set_underlying_type
(c-common.c) is wrong:
if (DECL_IS_BUILTIN (x) && TREE_CODE (TREE_TYPE (x)) != ARRAY_TYPE)
{
if (TYPE_NAME (TREE_TYPE (x)) == 0)
TYPE_NAME (TREE_TYPE (x)) = x;
}
And this rs6000.c code is totally bogus:
tdecl = add_builtin_type ("__vector unsigned int",
unsigned_V4SI_type_node);
TYPE_NAME (unsigned_V4SI_type_node) = tdecl; <-- this line
(note that at this point, add_builtin_type has already set the TYPE_NAME
via set_underlying type)
For C++'s creation of the wchar_t node, we'd need to special case
setting the node's name in some way?
[char16_t and char32_t have the same distinctness property as wchar_t.
I think that's the complete set]
nathan
--
Nathan Sidwell
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PR 79905] ICE with vector_type
2017-04-04 11:31 ` Nathan Sidwell
@ 2017-04-04 13:00 ` Richard Biener
2017-04-04 13:49 ` Nathan Sidwell
0 siblings, 1 reply; 23+ messages in thread
From: Richard Biener @ 2017-04-04 13:00 UTC (permalink / raw)
To: Nathan Sidwell; +Cc: William J. Schmidt, GCC Patches
On Tue, Apr 4, 2017 at 1:31 PM, Nathan Sidwell <nathan@acm.org> wrote:
> On 04/04/2017 04:27 AM, Richard Biener wrote:
>>
>> On Mon, Apr 3, 2017 at 9:03 PM, Nathan Sidwell <nathan@acm.org> wrote:
>
>
>>> However, as you can see there's already a get-out for COMPLEX_TYPE, and I
>>> think the same is needed for VECTOR_TYPE.
>>
>>
>> Hmm, but that is essentially a hack given that build_complex_type does
>> things
>> it shouldn't (assign a name to the type).
>
>
> Oh, didn't know that.
>
>>> Both set_underlying_type and rs6000 set the builtin vector type's
>>> TYPE_NAME,
>>> and so one constructed via applying __attribute__((vector_size(16))) will
>>> never match. And it should.
>>
>>
>> why? They only need to share the canonical type not the type node itself
>> (unless their name is the same, of course).
>
>
> Correct, that's what I meant. the canonical type equal function must match
> a just-consed vector with the already-hashed builtin.
>
>> Now -- that name comparing is somewhat odd. We hash type "variants"
>> with different names the same (so setting the name after inserting sth
>> into
>> the hash is allowed, but it will overwrite the old entry so any unnamed
>> uses
>> up to now get a type with a name...). I guess we'd be better off leaving
>> only unnamed types in the hash and building a type variant whenever we
>> add a name to such type.
>
>
> Right, the name matching surprised me, and my first attempt was to remove
> it. but that breaks wchar_t. wchar_t is the same representation as int
> (pedantically, some integral type), but is a distinct type (in C++ at least,
> I don't think C cares). The canonical type system records language-level
> distinctness, not representation distinctness.
>
> The difference between wchar_t and vector types is the only way to get a
> type the same as wchar_t is to use 'wchar_t' (so we have to start with a
> type at least as canonical as what we want). Whereas vector types are
> constructed via applying attributes to some underlying scalar type (so we
> have to go find the canonical type).
>
> It would therefore seem that this code in set_underlying_type (c-common.c)
> is wrong:
> if (DECL_IS_BUILTIN (x) && TREE_CODE (TREE_TYPE (x)) != ARRAY_TYPE)
> {
> if (TYPE_NAME (TREE_TYPE (x)) == 0)
> TYPE_NAME (TREE_TYPE (x)) = x;
> }
This is quite a fragile area -- you also have to watch dwarf2out.c. Most of the
TYPE_NAME "hacks" are to make debuginfo happy.
> And this rs6000.c code is totally bogus:
> tdecl = add_builtin_type ("__vector unsigned int",
> unsigned_V4SI_type_node);
> TYPE_NAME (unsigned_V4SI_type_node) = tdecl; <-- this line
Yeah, not sure what that like is for -- add_builtin_type properly adds a name.
Note it may be again to make debuginfo happy ;)
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.
> (note that at this point, add_builtin_type has already set the TYPE_NAME via
> set_underlying type)
>
> For C++'s creation of the wchar_t node, we'd need to special case setting
> the node's name in some way?
>
> [char16_t and char32_t have the same distinctness property as wchar_t. I
> think that's the complete set]
At this point I'd rather restrict fiddling in this fragile area in
rs6000.c only ;)
Does removing the TYPE_NAME setting fix things?
Richard.
> nathan
>
> --
> Nathan Sidwell
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PR 79905] ICE with vector_type
2017-04-04 13:00 ` Richard Biener
@ 2017-04-04 13:49 ` Nathan Sidwell
2017-04-04 13:57 ` Bill Schmidt
0 siblings, 1 reply; 23+ messages in thread
From: Nathan Sidwell @ 2017-04-04 13:49 UTC (permalink / raw)
To: Richard Biener; +Cc: William J. Schmidt, GCC Patches
[-- Attachment #1: Type: text/plain, Size: 1026 bytes --]
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
[-- Attachment #2: 79905-2.diff --]
[-- Type: text/x-patch, Size: 1171 bytes --]
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;
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PR 79905] ICE with vector_type
2017-04-04 13:49 ` Nathan Sidwell
@ 2017-04-04 13:57 ` Bill Schmidt
2017-04-04 14:02 ` Nathan Sidwell
0 siblings, 1 reply; 23+ messages in thread
From: Bill Schmidt @ 2017-04-04 13:57 UTC (permalink / raw)
To: Nathan Sidwell; +Cc: Richard Biener, GCC Patches
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>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PR 79905] ICE with vector_type
2017-04-04 13:57 ` Bill Schmidt
@ 2017-04-04 14:02 ` Nathan Sidwell
2017-04-04 17:40 ` Bill Schmidt
0 siblings, 1 reply; 23+ messages in thread
From: Nathan Sidwell @ 2017-04-04 14:02 UTC (permalink / raw)
To: Bill Schmidt; +Cc: Richard Biener, GCC Patches
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PR 79905] ICE with vector_type
2017-04-04 14:02 ` Nathan Sidwell
@ 2017-04-04 17:40 ` Bill Schmidt
2017-04-04 18:02 ` Nathan Sidwell
0 siblings, 1 reply; 23+ messages in thread
From: Bill Schmidt @ 2017-04-04 17:40 UTC (permalink / raw)
To: Nathan Sidwell; +Cc: Richard Biener, GCC Patches
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
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PR 79905] ICE with vector_type
2017-04-04 17:40 ` Bill Schmidt
@ 2017-04-04 18:02 ` Nathan Sidwell
2017-04-04 20:46 ` Bill Schmidt
0 siblings, 1 reply; 23+ messages in thread
From: Nathan Sidwell @ 2017-04-04 18:02 UTC (permalink / raw)
To: Bill Schmidt; +Cc: Richard Biener, GCC Patches
[-- Attachment #1: Type: text/plain, Size: 242 bytes --]
On 04/04/2017 01:40 PM, Bill Schmidt wrote:
> 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.
Try this.
nathan
--
Nathan Sidwell
[-- Attachment #2: 79905-3.diff --]
[-- Type: text/x-patch, Size: 1787 bytes --]
Index: c-family/c-common.c
===================================================================
--- c-family/c-common.c (revision 246647)
+++ c-family/c-common.c (working copy)
@@ -7465,7 +7465,8 @@ set_underlying_type (tree x)
{
if (x == error_mark_node)
return;
- if (DECL_IS_BUILTIN (x) && TREE_CODE (TREE_TYPE (x)) != ARRAY_TYPE)
+ if (DECL_IS_BUILTIN (x) && TYPE_NAME (TREE_TYPE (x)) != error_mark_node
+ && TREE_CODE (TREE_TYPE (x)) != ARRAY_TYPE)
{
if (TYPE_NAME (TREE_TYPE (x)) == 0)
TYPE_NAME (TREE_TYPE (x)) = x;
Index: config/rs6000/rs6000.c
===================================================================
--- config/rs6000/rs6000.c (revision 246647)
+++ config/rs6000/rs6000.c (working copy)
@@ -17459,9 +17459,28 @@ 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
+ if (tdecl)
+ {
+ // Ew, change the underlying type
+#if 0
+ 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;
+#endif
+ TREE_USED (TREE_TYPE (tdecl)) = true;
+ V4SI_type_node = TREE_TYPE (tdecl);
+ }
+ }
+
tdecl = add_builtin_type ("__vector __bool int", bool_V4SI_type_node);
TYPE_NAME (bool_V4SI_type_node) = tdecl;
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PR 79905] ICE with vector_type
2017-04-04 18:02 ` Nathan Sidwell
@ 2017-04-04 20:46 ` Bill Schmidt
2017-04-05 13:18 ` Nathan Sidwell
0 siblings, 1 reply; 23+ messages in thread
From: Bill Schmidt @ 2017-04-04 20:46 UTC (permalink / raw)
To: Nathan Sidwell; +Cc: Richard Biener, GCC Patches
> On Apr 4, 2017, at 1:02 PM, Nathan Sidwell <nathan@acm.org> wrote:
>
> On 04/04/2017 01:40 PM, Bill Schmidt wrote:
>> 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.
>
> Try this.
Thanks, Nathan, that regstraps cleanly on powerpc64le and fixes the test case in the PR.
Bill
>
> nathan
>
> --
> Nathan Sidwell
> <79905-3.diff>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PR 79905] ICE with vector_type
2017-04-04 20:46 ` Bill Schmidt
@ 2017-04-05 13:18 ` Nathan Sidwell
2017-04-05 13:35 ` Bill Schmidt
0 siblings, 1 reply; 23+ messages in thread
From: Nathan Sidwell @ 2017-04-05 13:18 UTC (permalink / raw)
To: Bill Schmidt; +Cc: Richard Biener, GCC Patches
[-- Attachment #1: Type: text/plain, Size: 373 bytes --]
Here's a completed patch. Bill, if you could regression test it, that'd
be great! (feel free to augment the testcase)
Richard, rather than copy set_underlying_type's type cloning I added a
check in it for error_mark_node as the existing name. If you'd prefer
that remain unchanged, I can duplicate the type cloning into rs6000_vt
itself.
nathan
--
Nathan Sidwell
[-- Attachment #2: ppc-79905.diff --]
[-- Type: text/x-patch, Size: 8733 bytes --]
2017-04-05 Nathan Sidwell <nathan@acm.org>
PR target/79905
* config/rs6000/rs6000.c (rs6000_vt): New.
(rs6000_init_builtins): Use it.
c-family/
* c-common.c (set_underlying_type): Make builtin type-copy when
name is error_mark.
testsuite/
* g++.dg/torture/pr79905.C: New.
Index: c-family/c-common.c
===================================================================
--- c-family/c-common.c (revision 246647)
+++ c-family/c-common.c (working copy)
@@ -7465,7 +7465,12 @@ set_underlying_type (tree x)
{
if (x == error_mark_node)
return;
- if (DECL_IS_BUILTIN (x) && TREE_CODE (TREE_TYPE (x)) != ARRAY_TYPE)
+ if (DECL_IS_BUILTIN (x)
+ && TREE_CODE (TREE_TYPE (x)) != ARRAY_TYPE
+ /* Callers set name to error_mark_node to force the builtin to
+ have a type copy. That ensures the canonical type doesn't
+ get a name. */
+ && TYPE_NAME (TREE_TYPE (x)) != error_mark_node)
{
if (TYPE_NAME (TREE_TYPE (x)) == 0)
TYPE_NAME (TREE_TYPE (x)) = x;
Index: config/rs6000/rs6000.c
===================================================================
--- config/rs6000/rs6000.c (revision 246647)
+++ config/rs6000/rs6000.c (working copy)
@@ -17257,6 +17257,29 @@ 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);
+ tree orig_name = TYPE_NAME (canonical);
+
+ /* Tell set_underlying_type to create a clone. */
+ TYPE_NAME (result) = error_mark_node;
+ tree named_type = add_builtin_type (name, canonical);
+ TYPE_NAME (result) = orig_name;
+ if (named_type)
+ {
+ gcc_assert (TREE_TYPE (named_type) != result);
+ result = TREE_TYPE (named_type);
+ TREE_USED (result) = true;
+ }
+
+ return resut;
+}
+
static void
rs6000_init_builtins (void)
{
@@ -17273,18 +17296,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 +17329,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 +17463,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*-*-* } } }
+
+
+typedef int V4i __attribute__((vector_size(16)));
+void a (V4i) {
+ vector int b;
+ a (b);
+}
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PR 79905] ICE with vector_type
2017-04-05 13:18 ` Nathan Sidwell
@ 2017-04-05 13:35 ` Bill Schmidt
2017-04-05 14:14 ` Nathan Sidwell
0 siblings, 1 reply; 23+ messages in thread
From: Bill Schmidt @ 2017-04-05 13:35 UTC (permalink / raw)
To: Nathan Sidwell; +Cc: Richard Biener, GCC Patches
Hi Nathan,
> On Apr 5, 2017, at 8:18 AM, Nathan Sidwell <nathan@acm.org> wrote:
>
> Here's a completed patch. Bill, if you could regression test it, that'd be great! (feel free to augment the testcase)
Thanks for the patch! Looks like there are some compile problems. I can fix "resut", but not sure what the intent is for "canonical":
In file included from /home/wschmidt/gcc/gcc-mainline-test/gcc/config/rs6000/rs6
000.c:26:0:
/home/wschmidt/gcc/gcc-mainline-test/gcc/config/rs6000/rs6000.c: In function 'tr
ee_node* rs6000_vt(const char*, tree, unsigned int)':
/home/wschmidt/gcc/gcc-mainline-test/gcc/config/rs6000/rs6000.c:17267:31: error:
'canonical' was not declared in this scope
tree orig_name = TYPE_NAME (canonical);
^
/home/wschmidt/gcc/gcc-mainline-test/gcc/tree.h:296:21: note: in definition of m
acro 'TREE_CLASS_CHECK'
(tree_class_check ((T), (CLASS), __FILE__, __LINE__, __FUNCTION__))
^
/home/wschmidt/gcc/gcc-mainline-test/gcc/tree.h:1840:26: note: in expansion of m
acro 'TYPE_CHECK'
#define TYPE_NAME(NODE) (TYPE_CHECK (NODE)->type_common.name)
^~~~~~~~~~
/home/wschmidt/gcc/gcc-mainline-test/gcc/config/rs6000/rs6000.c:17267:20: note:
in expansion of macro 'TYPE_NAME'
tree orig_name = TYPE_NAME (canonical);
^~~~~~~~~
/home/wschmidt/gcc/gcc-mainline-test/gcc/config/rs6000/rs6000.c:17280:10: error:
'resut' was not declared in this scope
return resut;
^~~~~
Bill
>
> Richard, rather than copy set_underlying_type's type cloning I added a check in it for error_mark_node as the existing name. If you'd prefer that remain unchanged, I can duplicate the type cloning into rs6000_vt itself.
>
> nathan
>
> --
> Nathan Sidwell
> <ppc-79905.diff>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PR 79905] ICE with vector_type
2017-04-05 13:35 ` Bill Schmidt
@ 2017-04-05 14:14 ` Nathan Sidwell
2017-04-05 20:33 ` Bill Schmidt
0 siblings, 1 reply; 23+ messages in thread
From: Nathan Sidwell @ 2017-04-05 14:14 UTC (permalink / raw)
To: Bill Schmidt; +Cc: Richard Biener, GCC Patches
[-- Attachment #1: Type: text/plain, Size: 224 bytes --]
> Thanks for the patch! Looks like there are some compile problems. I can fix "resut", but not sure what the intent is for "canonical":
I'm a dumbass. I built the x86_64 compiler :(
try this.
nathan
--
Nathan Sidwell
[-- Attachment #2: ppc-79905.diff --]
[-- Type: text/x-patch, Size: 8728 bytes --]
2017-04-05 Nathan Sidwell <nathan@acm.org>
PR target/79905
* config/rs6000/rs6000.c (rs6000_vt): New.
(rs6000_init_builtins): Use it.
c-family/
* c-common.c (set_underlying_type): Make builtin type-copy when
name is error_mark.
testsuite/
* g++.dg/torture/pr79905.C: New.
Index: c-family/c-common.c
===================================================================
--- c-family/c-common.c (revision 246647)
+++ c-family/c-common.c (working copy)
@@ -7465,7 +7465,12 @@ set_underlying_type (tree x)
{
if (x == error_mark_node)
return;
- if (DECL_IS_BUILTIN (x) && TREE_CODE (TREE_TYPE (x)) != ARRAY_TYPE)
+ if (DECL_IS_BUILTIN (x)
+ && TREE_CODE (TREE_TYPE (x)) != ARRAY_TYPE
+ /* Callers set name to error_mark_node to force the builtin to
+ have a type copy. That ensures the canonical type doesn't
+ get a name. */
+ && TYPE_NAME (TREE_TYPE (x)) != error_mark_node)
{
if (TYPE_NAME (TREE_TYPE (x)) == 0)
TYPE_NAME (TREE_TYPE (x)) = x;
Index: config/rs6000/rs6000.c
===================================================================
--- config/rs6000/rs6000.c (revision 246647)
+++ config/rs6000/rs6000.c (working copy)
@@ -17257,6 +17257,29 @@ 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);
+ tree orig_name = TYPE_NAME (result);
+
+ /* Tell set_underlying_type to create a clone. */
+ TYPE_NAME (result) = error_mark_node;
+ tree named_type = add_builtin_type (name, result);
+ TYPE_NAME (result) = orig_name;
+ if (named_type)
+ {
+ gcc_assert (TREE_TYPE (named_type) != result);
+ result = TREE_TYPE (named_type);
+ TREE_USED (result) = true;
+ }
+
+ return result;
+}
+
static void
rs6000_init_builtins (void)
{
@@ -17273,18 +17296,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 +17329,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 +17463,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*-*-* } } }
+
+
+typedef int V4i __attribute__((vector_size(16)));
+void a (V4i) {
+ vector int b;
+ a (b);
+}
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PR 79905] ICE with vector_type
2017-04-05 14:14 ` Nathan Sidwell
@ 2017-04-05 20:33 ` Bill Schmidt
2017-04-06 10:29 ` Richard Biener
0 siblings, 1 reply; 23+ messages in thread
From: Bill Schmidt @ 2017-04-05 20:33 UTC (permalink / raw)
To: Nathan Sidwell; +Cc: Richard Biener, GCC Patches
[-- Attachment #1: Type: text/plain, Size: 462 bytes --]
On 4/5/17 9:14 AM, Nathan Sidwell wrote:
>
>> Thanks for the patch! Looks like there are some compile problems. I
>> can fix "resut", but not sure what the intent is for "canonical":
>
> I'm a dumbass. I built the x86_64 compiler :(
> try this.
>
> nathan
Thanks! Regtest showed that this blows up on the gcc_assert for Fortran
and Go. If I change the assert to an explicit test, everything is
fine. Corrected patch attached; needs a change log ofc.
Bill
[-- Attachment #2: pr79905-2017-04-05.patch --]
[-- Type: text/plain, Size: 8135 bytes --]
Index: gcc/c-family/c-common.c
===================================================================
--- gcc/c-family/c-common.c (revision 246681)
+++ gcc/c-family/c-common.c (working copy)
@@ -7465,7 +7465,12 @@ set_underlying_type (tree x)
{
if (x == error_mark_node)
return;
- if (DECL_IS_BUILTIN (x) && TREE_CODE (TREE_TYPE (x)) != ARRAY_TYPE)
+ if (DECL_IS_BUILTIN (x)
+ && TREE_CODE (TREE_TYPE (x)) != ARRAY_TYPE
+ /* Callers set name to error_mark_node to force the builtin to
+ have a type copy. That ensures the canonical type doesn't
+ get a name. */
+ && TYPE_NAME (TREE_TYPE (x)) != error_mark_node)
{
if (TYPE_NAME (TREE_TYPE (x)) == 0)
TYPE_NAME (TREE_TYPE (x)) = x;
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c (revision 246681)
+++ gcc/config/rs6000/rs6000.c (working copy)
@@ -17257,6 +17257,29 @@ rs6000_expand_builtin (tree exp, rtx target, rtx s
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);
+ tree orig_name = TYPE_NAME (result);
+
+ /* Tell set_underlying_type to create a clone. */
+ TYPE_NAME (result) = error_mark_node;
+ tree named_type = add_builtin_type (name, result);
+ TYPE_NAME (result) = orig_name;
+
+ if (named_type && TREE_TYPE (named_type) != result)
+ {
+ result = TREE_TYPE (named_type);
+ TREE_USED (result) = true;
+ }
+
+ return result;
+}
+
static void
rs6000_init_builtins (void)
{
@@ -17273,18 +17296,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);
+ 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 = 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);
+ 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 +17329,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,84 +17463,17 @@ 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);
+ 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);
- 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;
- }
-
/* Paired and SPE builtins are only available if you build a compiler with
the appropriate options, so only create those builtins with the
appropriate compiler option. Create Altivec and VSX builtins on machines
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PR 79905] ICE with vector_type
2017-04-05 20:33 ` Bill Schmidt
@ 2017-04-06 10:29 ` Richard Biener
2017-04-06 11:28 ` Nathan Sidwell
0 siblings, 1 reply; 23+ messages in thread
From: Richard Biener @ 2017-04-06 10:29 UTC (permalink / raw)
To: Bill Schmidt; +Cc: Nathan Sidwell, GCC Patches
On Wed, Apr 5, 2017 at 10:33 PM, Bill Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:
> On 4/5/17 9:14 AM, Nathan Sidwell wrote:
>>
>>> Thanks for the patch! Looks like there are some compile problems. I
>>> can fix "resut", but not sure what the intent is for "canonical":
>>
>> I'm a dumbass. I built the x86_64 compiler :(
>> try this.
>>
>> nathan
> Thanks! Regtest showed that this blows up on the gcc_assert for Fortran
> and Go. If I change the assert to an explicit test, everything is
> fine. Corrected patch attached; needs a change log ofc.
Ick.
As said, why not do
+static tree
+rs6000_vt (const char *name, tree elt_type, unsigned num_elts)
+{
+ tree result = build_vector_type (elt_type, num_elts);
+ tree orig_name = TYPE_NAME (result);
+
+ /* Tell set_underlying_type to create a clone. */
+ TYPE_NAME (result) = error_mark_node;
result = build_variant_type_copy (result);
+ tree named_type = add_builtin_type (name, result);
+ TYPE_NAME (result) = orig_name;
+
+ if (named_type && TREE_TYPE (named_type) != result)
+ {
+ result = TREE_TYPE (named_type);
+ TREE_USED (result) = true;
why's this needed?
+ }
I don't like that error_mark_node hack at all.
Richard.
> Bill
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PR 79905] ICE with vector_type
2017-04-06 10:29 ` Richard Biener
@ 2017-04-06 11:28 ` Nathan Sidwell
2017-04-06 14:04 ` Richard Biener
0 siblings, 1 reply; 23+ messages in thread
From: Nathan Sidwell @ 2017-04-06 11:28 UTC (permalink / raw)
To: Richard Biener, Bill Schmidt; +Cc: GCC Patches
[-- Attachment #1: Type: text/plain, Size: 53 bytes --]
Let's try this one then.
nathan
--
Nathan Sidwell
[-- Attachment #2: ppc-79905-2.diff --]
[-- Type: text/x-patch, Size: 7721 bytes --]
2017-04-05 Nathan Sidwell <nathan@acm.org>
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,23 @@ 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);
+
+ if (result == TYPE_CANONICAL (result))
+ /* 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 +17290,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 +17323,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 +17457,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*-*-* } } }
+
+
+typedef int V4i __attribute__((vector_size(16)));
+void a (V4i) {
+ vector int b;
+ a (b);
+}
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PR 79905] ICE with vector_type
2017-04-06 11:28 ` Nathan Sidwell
@ 2017-04-06 14:04 ` Richard Biener
2017-04-06 14:20 ` Bill Schmidt
0 siblings, 1 reply; 23+ messages in thread
From: Richard Biener @ 2017-04-06 14:04 UTC (permalink / raw)
To: Nathan Sidwell; +Cc: Bill Schmidt, GCC Patches
On Thu, Apr 6, 2017 at 1:28 PM, Nathan Sidwell <nathan@acm.org> wrote:
> Let's try this one then.
I'd call this
+ if (result == TYPE_CANONICAL (result))
+ /* Copy so we don't give the canonical type a name. */
+ result = build_variant_type_copy (result);
premature optimization -- I wonder if anything breaks if you always copy?
(that is, I expect result is always the canonical type here?)
Richard.
> nathan
>
> --
> Nathan Sidwell
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PR 79905] ICE with vector_type
2017-04-06 14:04 ` Richard Biener
@ 2017-04-06 14:20 ` Bill Schmidt
2017-04-06 14:26 ` Bill Schmidt
0 siblings, 1 reply; 23+ messages in thread
From: Bill Schmidt @ 2017-04-06 14:20 UTC (permalink / raw)
To: Richard Biener; +Cc: Nathan Sidwell, GCC Patches
> On Apr 6, 2017, at 9:04 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>
> On Thu, Apr 6, 2017 at 1:28 PM, Nathan Sidwell <nathan@acm.org> wrote:
>> Let's try this one then.
Nathan's patch regstraps cleanly. I'll try Richard's variant (dropping the if test below) now.
Bill
>
> I'd call this
>
> + if (result == TYPE_CANONICAL (result))
> + /* Copy so we don't give the canonical type a name. */
> + result = build_variant_type_copy (result);
>
> premature optimization -- I wonder if anything breaks if you always copy?
> (that is, I expect result is always the canonical type here?)
>
> Richard.
>
>> nathan
>>
>> --
>> Nathan Sidwell
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PR 79905] ICE with vector_type
2017-04-06 14:20 ` Bill Schmidt
@ 2017-04-06 14:26 ` Bill Schmidt
2017-04-06 15:13 ` Bill Schmidt
0 siblings, 1 reply; 23+ messages in thread
From: Bill Schmidt @ 2017-04-06 14:26 UTC (permalink / raw)
To: Richard Biener; +Cc: Nathan Sidwell, GCC Patches
> On Apr 6, 2017, at 9:19 AM, Bill Schmidt <wschmidt@linux.vnet.ibm.com> wrote:
>
>
>> On Apr 6, 2017, at 9:04 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>>
>> On Thu, Apr 6, 2017 at 1:28 PM, Nathan Sidwell <nathan@acm.org> wrote:
>>> Let's try this one then.
>
> Nathan's patch regstraps cleanly. I'll try Richard's variant (dropping the if test below) now.
FYI, the test case should also include:
// { dg-require-effective-target powerpc_altivec_ok }
to avoid problems on targets without AltiVec support.
Bill
>
> Bill
>>
>> I'd call this
>>
>> + if (result == TYPE_CANONICAL (result))
>> + /* Copy so we don't give the canonical type a name. */
>> + result = build_variant_type_copy (result);
>>
>> premature optimization -- I wonder if anything breaks if you always copy?
>> (that is, I expect result is always the canonical type here?)
>>
>> Richard.
>>
>>> nathan
>>>
>>> --
>>> Nathan Sidwell
>>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PR 79905] ICE with vector_type
2017-04-06 14:26 ` Bill Schmidt
@ 2017-04-06 15:13 ` Bill Schmidt
2017-04-06 18:34 ` Nathan Sidwell
0 siblings, 1 reply; 23+ messages in thread
From: Bill Schmidt @ 2017-04-06 15:13 UTC (permalink / raw)
To: Richard Biener; +Cc: Nathan Sidwell, GCC Patches
> On Apr 6, 2017, at 9:26 AM, Bill Schmidt <wschmidt@linux.vnet.ibm.com> wrote:
>
>
>> On Apr 6, 2017, at 9:19 AM, Bill Schmidt <wschmidt@linux.vnet.ibm.com> wrote:
>>
>>
>>> On Apr 6, 2017, at 9:04 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>>>
>>> On Thu, Apr 6, 2017 at 1:28 PM, Nathan Sidwell <nathan@acm.org> wrote:
>>>> Let's try this one then.
>>
>> 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.
Bill
>
> FYI, the test case should also include:
>
> // { dg-require-effective-target powerpc_altivec_ok }
>
> to avoid problems on targets without AltiVec support.
>
> Bill
>>
>> Bill
>>>
>>> I'd call this
>>>
>>> + if (result == TYPE_CANONICAL (result))
>>> + /* Copy so we don't give the canonical type a name. */
>>> + result = build_variant_type_copy (result);
>>>
>>> premature optimization -- I wonder if anything breaks if you always copy?
>>> (that is, I expect result is always the canonical type here?)
>>>
>>> Richard.
>>>
>>>> nathan
>>>>
>>>> --
>>>> Nathan Sidwell
>>>
>>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PR 79905] ICE with vector_type
2017-04-06 15:13 ` Bill Schmidt
@ 2017-04-06 18:34 ` Nathan Sidwell
2017-04-06 20:18 ` Segher Boessenkool
0 siblings, 1 reply; 23+ messages in thread
From: Nathan Sidwell @ 2017-04-06 18:34 UTC (permalink / raw)
To: Bill Schmidt, segher; +Cc: Richard Biener, GCC Patches
[-- Attachment #1: Type: text/plain, Size: 674 bytes --]
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
--
Nathan Sidwell
[-- Attachment #2: ppc-79905-3.diff --]
[-- Type: text/x-patch, Size: 7770 bytes --]
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);
+}
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PR 79905] ICE with vector_type
2017-04-06 18:34 ` Nathan Sidwell
@ 2017-04-06 20:18 ` Segher Boessenkool
2017-04-10 11:24 ` Nathan Sidwell
0 siblings, 1 reply; 23+ messages in thread
From: Segher Boessenkool @ 2017-04-06 20:18 UTC (permalink / raw)
To: Nathan Sidwell; +Cc: Bill Schmidt, Richard Biener, GCC Patches
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PR 79905] ICE with vector_type
2017-04-06 20:18 ` Segher Boessenkool
@ 2017-04-10 11:24 ` Nathan Sidwell
0 siblings, 0 replies; 23+ messages in thread
From: Nathan Sidwell @ 2017-04-10 11:24 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: Bill Schmidt, Richard Biener, GCC Patches
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
--
Nathan Sidwell
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2017-04-10 11:24 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-03 19:03 [PR 79905] ICE with vector_type Nathan Sidwell
2017-04-04 8:28 ` Richard Biener
2017-04-04 11:31 ` Nathan Sidwell
2017-04-04 13:00 ` Richard Biener
2017-04-04 13:49 ` Nathan Sidwell
2017-04-04 13:57 ` Bill Schmidt
2017-04-04 14:02 ` Nathan Sidwell
2017-04-04 17:40 ` Bill Schmidt
2017-04-04 18:02 ` Nathan Sidwell
2017-04-04 20:46 ` Bill Schmidt
2017-04-05 13:18 ` Nathan Sidwell
2017-04-05 13:35 ` Bill Schmidt
2017-04-05 14:14 ` Nathan Sidwell
2017-04-05 20:33 ` Bill Schmidt
2017-04-06 10:29 ` Richard Biener
2017-04-06 11:28 ` Nathan Sidwell
2017-04-06 14:04 ` Richard Biener
2017-04-06 14:20 ` Bill Schmidt
2017-04-06 14:26 ` Bill Schmidt
2017-04-06 15:13 ` Bill Schmidt
2017-04-06 18:34 ` Nathan Sidwell
2017-04-06 20:18 ` Segher Boessenkool
2017-04-10 11:24 ` Nathan Sidwell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).