On Tue, Apr 4, 2017 at 1:31 PM, Nathan Sidwell <nat...@acm.org> wrote:
> On 04/04/2017 04:27 AM, Richard Biener wrote:
>>
>> On Mon, Apr 3, 2017 at 9:03 PM, Nathan Sidwell <nat...@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
