ok. The patch is fine.
On Thu, Aug 28, 2014 at 1:46 PM, Teresa Johnson <tejohn...@google.com> wrote:
> On Thu, Aug 28, 2014 at 1:40 PM, Xinliang David Li <davi...@google.com> wrote:
>>
>> Do you know why the previous check is not enough ?
>>
>> cgraph_can_remove_if_no_direct_calls_and_refs_p (struct cgraph_node *node)
>
> This will return true for the external node, but it also returns true
> for the non-external node. The non-external node is a COMDAT, as as
> the comments in that routine indicate, COMDATs can be removed even if
> they are externally_visible.
>
> Teresa
>
>>
>> David
>>
>>
>> On Thu, Aug 28, 2014 at 1:29 PM, Teresa Johnson <tejohn...@google.com>
>> wrote:
>>>
>>> This patch fixes up varpool nodes after LIPO linking as we were doing
>>> for cgraph node references. While, here I made some fixes to the
>>> cgraph fixup as well (mark address taken as appropriate) and removed
>>> old references. The latter exposed an issue with resolved cgraph nodes
>>> getting eliminated when they were only referenced from vtables and the
>>> LIPO linking selected an external copy as the resolved node. Addressed
>>> this by forcing the LIPO linking to prefer the non-external copy.
>>>
>>> Passes regression testing, internal benchmark testing in progress. Ok
>>> for google/4_9 if that succeeds?
>>>
>>> Teresa
>>>
>>> 2014-08-28 Teresa Johnson <tejohn...@google.com>
>>>
>>> Google ref b/17038802.
>>> * l-ipo.c (resolve_cgraph_node): Pick non-external node.
>>> (fixup_reference_list): Fixup varpool references, remove old
>>> references, mark cgraph nodes as address taken as needed.
>>>
>>> Index: l-ipo.c
>>> ===================================================================
>>> --- l-ipo.c (revision 213975)
>>> +++ l-ipo.c (working copy)
>>> @@ -1564,6 +1564,15 @@ resolve_cgraph_node (struct cgraph_sym **slot, str
>>> (*slot)->rep_decl = decl2;
>>> return;
>>> }
>>> + /* Similarly, pick the non-external symbol, since external
>>> + symbols may be eliminated by symtab_remove_unreachable_nodes
>>> + after ipa inlining (see process_references). */
>>> + if (DECL_EXTERNAL (decl1) && !DECL_EXTERNAL (decl2))
>>> + {
>>> + (*slot)->rep_node = node;
>>> + (*slot)->rep_decl = decl2;
>>> + return;
>>> + }
>>>
>>> has_prof1 = has_profile_info (decl1);
>>> bool is_aux1 = cgraph_is_auxiliary (decl1);
>>> @@ -2304,31 +2313,44 @@ fixup_reference_list (struct varpool_node *node)
>>> int i;
>>> struct ipa_ref *ref;
>>> struct ipa_ref_list *list = &node->ref_list;
>>> - vec<cgraph_node_ptr> new_refered;
>>> + vec<symtab_node *> new_refered;
>>> vec<int> new_refered_type;
>>> - struct cgraph_node *c;
>>> + struct symtab_node *sym_node;
>>> enum ipa_ref_use use_type = IPA_REF_LOAD;
>>>
>>> new_refered.create (10);
>>> new_refered_type.create (10);
>>> for (i = 0; ipa_ref_list_reference_iterate (list, i, ref); i++)
>>> {
>>> - if (!is_a <cgraph_node> (ref->referred))
>>> - continue;
>>> -
>>> - struct cgraph_node *cnode = ipa_ref_node (ref);
>>> - struct cgraph_node *r_cnode
>>> - = cgraph_lipo_get_resolved_node (cnode->decl);
>>> - if (r_cnode != cnode)
>>> + if (is_a <cgraph_node> (ref->referred))
>>> {
>>> + struct cgraph_node *cnode = ipa_ref_node (ref);
>>> + struct cgraph_node *r_cnode
>>> + = cgraph_lipo_get_resolved_node (cnode->decl);
>>> new_refered.safe_push (r_cnode);
>>> use_type = ref->use;
>>> new_refered_type.safe_push ((int) use_type);
>>> + gcc_assert (use_type != IPA_REF_ADDR
>>> + || cnode->global.inlined_to
>>> + || cnode->address_taken);
>>> + if (use_type == IPA_REF_ADDR)
>>> + cgraph_mark_address_taken_node (r_cnode);
>>> }
>>> + else if (is_a <varpool_node> (ref->referred))
>>> + {
>>> + struct varpool_node *var = ipa_ref_varpool_node (ref);
>>> + struct varpool_node *r_var = real_varpool_node (var->decl);
>>> + new_refered.safe_push (r_var);
>>> + use_type = ref->use;
>>> + new_refered_type.safe_push ((int) use_type);
>>> + }
>>> + else
>>> + gcc_assert (false);
>>> }
>>> - for (i = 0; new_refered.iterate (i, &c); ++i)
>>> + ipa_remove_all_references (&node->ref_list);
>>> + for (i = 0; new_refered.iterate (i, &sym_node); ++i)
>>> {
>>> - ipa_record_reference (node, c,
>>> + ipa_record_reference (node, sym_node,
>>> (enum ipa_ref_use) new_refered_type[i],
>>> NULL);
>>> }
>>> }
>>>
>>>
>>> --
>>> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
>>
>>
>
>
>
> --
> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
