On Mon, 9 Apr 2012, Jan Hubicka wrote:
> Hi,
> this patch fixes several different ICEs related to handling aliases in WHOPR
> partitioning. It took me over week debug this, but when variable alias
> is added to a boundary and its destination is not added, we get queue of
> unforutnate events where the destinatoin gets analyzed and added at ltrans
> time
> resulting in interesting miscompilation seen at Mozilla with some vtables.
> The problem is that constructor won't get streamed when the declaration is
> not in varpool at partitioning time and thus once the variable is re-added
> it has zero constructor.
> Of course the problem manifests itself in various weird ways depending
> on ordering of linker command maing it very difficult to reduce anything.
>
> While working on this I also noticed that PR 52634 is about related problem
> where aliases are incorectly partitioned into multiple partitions.
> The patch also fixes the varpool ICEs mentioned in the other two PRs.
> I failed to produce testcase version of PR52722 testcase, since it does not
> link now either, but it won't ICE.
>
> I will commit the patch and wait for some time, but I would like to backport
> it to 4.7, since it solves quite nasty misoptimization problem.
Yeah, it looks fine to me.
> At mainline after this patch i would like to follow with series of cleanups
> and API changes I have in queue for symtab work.
Thanks,
Richard.
> Honza
>
> PR lto/52722
> PR lto/51765
> PR lto/52634
> * lto-cgraph.c (compute_ltrans_boundary): When alias is in the boundary,
> add its target too.
> * lto.c (add_references_to_partition): Add also aliased nodes.
> (add_cgraph_node_to_partition,
> add_varpool_node_to_partition): Work on nodes, not functions/variables;
> when adding alias, add also the aliased object.
> * gcc.dg/lto/pr52634_1.c: New testcase.
> * gcc.dg/lto/pr52634_0.c: New testcase.
> Index: lto-cgraph.c
> ===================================================================
> *** lto-cgraph.c (revision 185767)
> --- lto-cgraph.c (working copy)
> *************** compute_ltrans_boundary (struct lto_out_
> *** 799,804 ****
> --- 799,806 ----
> lto_set_varpool_encoder_encode_initializer (varpool_encoder, vnode);
> add_references (encoder, varpool_encoder, &vnode->ref_list);
> }
> + else if (vnode->alias || vnode->alias_of)
> + add_references (encoder, varpool_encoder, &vnode->ref_list);
> }
>
> /* Go over all the nodes again to include callees that are not in
> Index: lto/lto.c
> ===================================================================
> *** lto/lto.c (revision 185767)
> --- lto/lto.c (working copy)
> *************** free_ltrans_partitions (void)
> *** 1444,1450 ****
> VEC_free (ltrans_partition, heap, ltrans_partitions);
> }
>
> ! /* See all references that go to comdat objects and bring them into
> partition too. */
> static void
> add_references_to_partition (ltrans_partition part, struct ipa_ref_list
> *refs)
> {
> --- 1444,1451 ----
> VEC_free (ltrans_partition, heap, ltrans_partitions);
> }
>
> ! /* See all references that go to comdat objects and bring them into
> partition too.
> ! Also see all aliases of the newly added entry and bring them, too. */
> static void
> add_references_to_partition (ltrans_partition part, struct ipa_ref_list
> *refs)
> {
> *************** add_references_to_partition (ltrans_part
> *** 1453,1467 ****
> for (i = 0; ipa_ref_list_reference_iterate (refs, i, ref); i++)
> {
> if (ref->refered_type == IPA_REF_CGRAPH
> ! && DECL_COMDAT (cgraph_function_node (ipa_ref_node (ref), NULL)->decl)
> && !cgraph_node_in_set_p (ipa_ref_node (ref), part->cgraph_set))
> add_cgraph_node_to_partition (part, ipa_ref_node (ref));
> else
> if (ref->refered_type == IPA_REF_VARPOOL
> ! && DECL_COMDAT (ipa_ref_varpool_node (ref)->decl)
> ! && !varpool_node_in_set_p (ipa_ref_varpool_node (ref),
> part->varpool_set))
> add_varpool_node_to_partition (part, ipa_ref_varpool_node (ref));
> }
> }
>
> /* Worker for add_cgraph_node_to_partition. */
> --- 1454,1498 ----
> for (i = 0; ipa_ref_list_reference_iterate (refs, i, ref); i++)
> {
> if (ref->refered_type == IPA_REF_CGRAPH
> ! && (DECL_COMDAT (cgraph_function_node (ipa_ref_node (ref),
> ! NULL)->decl)
> ! || (ref->use == IPA_REF_ALIAS
> ! && lookup_attribute
> ! ("weakref", DECL_ATTRIBUTES (ipa_ref_node (ref)->decl))))
> && !cgraph_node_in_set_p (ipa_ref_node (ref), part->cgraph_set))
> add_cgraph_node_to_partition (part, ipa_ref_node (ref));
> else
> if (ref->refered_type == IPA_REF_VARPOOL
> ! && (DECL_COMDAT (ipa_ref_varpool_node (ref)->decl)
> ! || (ref->use == IPA_REF_ALIAS
> ! && lookup_attribute
> ! ("weakref",
> ! DECL_ATTRIBUTES (ipa_ref_varpool_node (ref)->decl))))
> ! && !varpool_node_in_set_p (ipa_ref_varpool_node (ref),
> ! part->varpool_set))
> add_varpool_node_to_partition (part, ipa_ref_varpool_node (ref));
> }
> + for (i = 0; ipa_ref_list_refering_iterate (refs, i, ref); i++)
> + {
> + if (ref->refering_type == IPA_REF_CGRAPH
> + && ref->use == IPA_REF_ALIAS
> + && !cgraph_node_in_set_p (ipa_ref_refering_node (ref),
> + part->cgraph_set)
> + && !lookup_attribute ("weakref",
> + DECL_ATTRIBUTES
> + (ipa_ref_refering_node (ref)->decl)))
> + add_cgraph_node_to_partition (part, ipa_ref_refering_node (ref));
> + else
> + if (ref->refering_type == IPA_REF_VARPOOL
> + && ref->use == IPA_REF_ALIAS
> + && !varpool_node_in_set_p (ipa_ref_refering_varpool_node (ref),
> + part->varpool_set)
> + && !lookup_attribute ("weakref",
> + DECL_ATTRIBUTES
> + (ipa_ref_refering_varpool_node
> (ref)->decl)))
> + add_varpool_node_to_partition (part,
> + ipa_ref_refering_varpool_node (ref));
> + }
> }
>
> /* Worker for add_cgraph_node_to_partition. */
> *************** add_cgraph_node_to_partition (ltrans_par
> *** 1501,1509 ****
> cgraph_node_set_iterator csi;
> struct cgraph_node *n;
>
> - /* We always decide on functions, not associated thunks and aliases. */
> - node = cgraph_function_node (node, NULL);
> -
> /* If NODE is already there, we have nothing to do. */
> csi = cgraph_node_set_find (part->cgraph_set, node);
> if (!csi_end_p (csi))
> --- 1532,1537 ----
> *************** add_cgraph_node_to_partition (ltrans_par
> *** 1522,1528 ****
> --- 1550,1563 ----
> && !cgraph_node_in_set_p (e->callee, part->cgraph_set))
> add_cgraph_node_to_partition (part, e->callee);
>
> + /* The only way to assemble non-weakref alias is to add the aliased
> object into
> + the unit. */
> add_references_to_partition (part, &node->ref_list);
> + n = cgraph_function_node (node, NULL);
> + if (n != node
> + && !lookup_attribute ("weakref",
> + DECL_ATTRIBUTES (node->decl)))
> + add_cgraph_node_to_partition (part, n);
>
> if (node->same_comdat_group)
> for (n = node->same_comdat_group; n != node; n = n->same_comdat_group)
> *************** static void
> *** 1535,1542 ****
> add_varpool_node_to_partition (ltrans_partition part, struct varpool_node
> *vnode)
> {
> varpool_node_set_iterator vsi;
> !
> ! vnode = varpool_variable_node (vnode, NULL);
>
> /* If NODE is already there, we have nothing to do. */
> vsi = varpool_node_set_find (part->varpool_set, vnode);
> --- 1570,1576 ----
> add_varpool_node_to_partition (ltrans_partition part, struct varpool_node
> *vnode)
> {
> varpool_node_set_iterator vsi;
> ! struct varpool_node *v;
>
> /* If NODE is already there, we have nothing to do. */
> vsi = varpool_node_set_find (part->varpool_set, vnode);
> *************** add_varpool_node_to_partition (ltrans_pa
> *** 1554,1559 ****
> --- 1588,1601 ----
> }
> vnode->aux = (void *)((size_t)vnode->aux + 1);
>
> + /* The only way to assemble non-weakref alias is to add the aliased
> object into
> + the unit. */
> + v = varpool_variable_node (vnode, NULL);
> + if (v != vnode
> + && !lookup_attribute ("weakref",
> + DECL_ATTRIBUTES (vnode->decl)))
> + add_varpool_node_to_partition (part, v);
> +
> add_references_to_partition (part, &vnode->ref_list);
>
> if (vnode->same_comdat_group
> Index: testsuite/gcc.dg/lto/pr52634_1.c
> ===================================================================
> --- testsuite/gcc.dg/lto/pr52634_1.c (revision 0)
> +++ testsuite/gcc.dg/lto/pr52634_1.c (revision 0)
> @@ -0,0 +1,2 @@
> +int cfliteKeyCallBacks = 5;
> +extern int cfliteValueCallBacks __attribute__((alias("cfliteKeyCallBacks")));
> Index: testsuite/gcc.dg/lto/pr52634_0.c
> ===================================================================
> --- testsuite/gcc.dg/lto/pr52634_0.c (revision 0)
> +++ testsuite/gcc.dg/lto/pr52634_0.c (revision 0)
> @@ -0,0 +1,5 @@
> +/* { dg-lto-do link } */
> +/* { dg-lto-options {{-flto -r -nostdlib -flto-partition=1to1}} */
> +extern int cfliteValueCallBacks;
> +void baz (int *);
> +int main () { baz(&cfliteValueCallBacks); }
>
>
--
Richard Guenther <rguent...@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
