Re: [patch] Fix type merging deficiency during WPA

[Richard Biener <richard dot guenther at gmail dot com>]

On Wed, Jul 6, 2016 at 12:21 PM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Wed, Jul 6, 2016 at 11:33 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>>> I see.  I think the solution is to perform cgraph/varpool merging
>>> before attempting to read in
>>> the global decl stream.  IIRC Micha had (old) patches for this.
>>
>> How can you merge varpool nodes if you haven't merged types?

You merge them just in the way the linker instructs you via the
resolution table.

>>> But I wonder why we don't tree-merge 'n' here (from my C example) and
>>> thus figure
>>> that the type domain of x is equal?  Or is it that 'n' and 'x' are in
>>> the same SCC (they
>>> referece each other in some way)?  In this case the bug would be that we
>>> fail to treat them equal optimistically.  That said, I don't see how
>>> TYPE_CANONICAL computation is relevant - what is relevant is the failure to
>>> merge the two types.
>>> In debugging this I'd start to see if the hashes are not equal or if
>>> they are equal
>>> at which node we consider them to differ.
>>
>> We just have 2 different DECLs with different DECL_UIDs, the definition from a
>> compilation unit and a reference from another compilation unit, so the hashes
>> naturally differ too.  What's supposed to have them reconciled at this point?
>
> I am talking about tree/SCC merging which happily merges global decls as well.
> It uses custom hashing (see lto-streamer-out.c:hash_tree) which doesn't hash
> DECL_UID (obviously).  This merging process should be optimistic for all nodes
> in the same SCC as well.
>
> That said, I expect the types to be tree merged and wonder why they are not.
>
> If they were merged they'd obviously share TYPE_CANONICAL because there
> would be only one type to compute TYPE_CANONICAL for.

It probably boils down to one unit refering to the DECL with DECL_EXTERNAL set
and one to the DECL with TREE_STATIC set?  This would mean that
hashing/comparing
should be set up to "merge" those but the merging ultimatively rejected by some
toplevel logic (so we get users merged).  But as said, early symtab
merging would
fix this as well.

Richard.

> Richard.
>
>> The TYPE_CANONICAL computation is relevant because, with GCC 6, the criterion
>> for compatibility of pointer types is the alias set, which is based on the
>> TYPE_CANONICAL of the pointed-to type, so we fail to merge pointer types
>> because warn_type_compatibility_p returns non-zero if TYPE_CANONICAL differs.
>>
>> --
>> Eric Botcazou