Re: [PATCH] PR rtl-optimization/32219: optimizer causees wrong code in pic/hidden/weak symbol checking

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Thu, Feb 12, 2015 at 4:04 PM, Richard Henderson <rth@redhat.com> wrote:
> On 02/12/2015 03:05 PM, H.J. Lu wrote:
>> @@ -6830,9 +6830,15 @@ default_binds_local_p_2 (const_tree exp, bool shlib, bool weak_dominate)
>>    bool resolved_locally = false;
>>    if (symtab_node *node = symtab_node::get (exp))
>>      {
>> -      /* When not building shared library and weak_dominate is true:
>> -         weak, common or initialized symbols are resolved locally.  */
>> -      if ((weak_dominate && !shlib && node->definition)
>> +      /* When weak_dominate is true and not building shared library or
>> +      non-default visibility is specified by user: weak, common or
>> +      initialized symbols are resolved locally.
>> +      */
>> +      if (((!shlib
>> +         || (DECL_VISIBILITY_SPECIFIED (exp)
>> +             && DECL_VISIBILITY (exp) != VISIBILITY_DEFAULT))
>> +        && weak_dominate
>> +        && node->definition)
>>         || node->in_other_partition
>>         || resolution_local_p (node->resolution))
>>       resolved_locally = true;
>
> Hum.  I don't find that particularly easy to reason with either.
>
> How about this?  I'm about half-way through regression testing on it.
>
> I re-instated the use of resolution_to_local_definition_p, and attempt to infer
> a proper value for that when lto isn't in use.  I use this to eliminate only
> undef-weak early, rather than non-dominate weak.
>
> I re-instated the use of the existence of the local definition in the
> DECL_VISIBILITY test.  But unlike before, I reason that this allows us to
> eliminate the second visibility check.  We either have an assertion from the
> user (SPECIFIED), or we know we have a definition.  We no longer rely on the
> DECL_EXTERNAL test in the middle eliminating symbols without a definition.
>
> I shuffled some of the "return false" tests around among themselves, attempting
> to put the simplest test first.  No change in behavior there.
>
> (First patch is delta from the 5-patch bundle; second patch is the
> composite from trunk, to avoid confusion.)
>
>

I tried the second patch.  Results look good on Linux/x86-64.

Thanks.


-- 
H.J.