Message ID | 20120301125307.GA9390@virgil.arch.suse.de |
---|---|
State | New |
Headers | show |
On 2/03/2012, at 1:53 AM, Martin Jambor wrote: > Hi, > > the following patch backports 4.5 behavior to 4.4 IPA-CP > initialization when it decides which nodes need to be duplicated (we > often say cloned, but cloning is an overloaded term in gcc) in order > to modify them. In 4.5, this is decided by a new predicate called > cgraph_only_called_directly_p which checks both node->needed and > node->local.externally_visible, whereas 4.4 looks only at the needed > flag. > > As described in the PR, this wrecks havoc for ipcp_update_callgraph > which is the stage of the old IPA-CP when it undoes its decisions > which turned out not to be safe. > > Bootstrapped and tested on x86_64-linux, OK for the branch? > > ... > Index: gcc/ipa-cp.c > =================================================================== > --- gcc/ipa-cp.c (revision 184662) > +++ gcc/ipa-cp.c (working copy) > @@ -508,7 +508,7 @@ ipcp_initialize_node_lattices (struct cg > > if (ipa_is_called_with_var_arguments (info)) > type = IPA_BOTTOM; > - else if (!node->needed) > + else if (!node->needed && !node->local.externally_visible) > type = IPA_TOP; > /* When cloning is allowed, we can assume that externally visible functions > are not called. We will compensate this by cloning later. */ Because this is for a branch, I would be more conservative and avoid any new instances of IPA_TOP -- those that can be obtained from subsequent else-if clauses. I.e., else if (!node->needed) type = !node->local.externally_visible ? IPA_TOP : IPA_BOTTOM; Thanks, -- Maxim Kuvyrkov CodeSourcery / Mentor Graphics
On Fri, Mar 02, 2012 at 06:54:13PM +1300, Maxim Kuvyrkov wrote: > > --- gcc/ipa-cp.c (revision 184662) > > +++ gcc/ipa-cp.c (working copy) > > @@ -508,7 +508,7 @@ ipcp_initialize_node_lattices (struct cg > > > > if (ipa_is_called_with_var_arguments (info)) > > type = IPA_BOTTOM; > > - else if (!node->needed) > > + else if (!node->needed && !node->local.externally_visible) > > type = IPA_TOP; > > /* When cloning is allowed, we can assume that externally visible functions > > are not called. We will compensate this by cloning later. */ > > Because this is for a branch, I would be more conservative and avoid any > new instances of IPA_TOP -- those that can be obtained from subsequent > else-if clauses. I.e., > > else if (!node->needed) > type = !node->local.externally_visible ? IPA_TOP : IPA_BOTTOM; The above doesn't add any new instances of IPA_TOP, the node->local.externally_visible && !node->needed will be either IPA_BOTTOM, if not desirable for cloning, or IPA_TOP with n_cloning_candidates++ (previously they would all be IPA_TOP). I think Martin's patch as is is fine. Jakub
Hi, On Fri, Mar 02, 2012 at 06:54:13PM +1300, Maxim Kuvyrkov wrote: > On 2/03/2012, at 1:53 AM, Martin Jambor wrote: > > > Hi, > > > > the following patch backports 4.5 behavior to 4.4 IPA-CP > > initialization when it decides which nodes need to be duplicated (we > > often say cloned, but cloning is an overloaded term in gcc) in order > > to modify them. In 4.5, this is decided by a new predicate called > > cgraph_only_called_directly_p which checks both node->needed and > > node->local.externally_visible, whereas 4.4 looks only at the needed > > flag. > > > > As described in the PR, this wrecks havoc for ipcp_update_callgraph > > which is the stage of the old IPA-CP when it undoes its decisions > > which turned out not to be safe. > > > > Bootstrapped and tested on x86_64-linux, OK for the branch? > > > > > ... > > Index: gcc/ipa-cp.c > > =================================================================== > > --- gcc/ipa-cp.c (revision 184662) > > +++ gcc/ipa-cp.c (working copy) > > @@ -508,7 +508,7 @@ ipcp_initialize_node_lattices (struct cg > > > > if (ipa_is_called_with_var_arguments (info)) > > type = IPA_BOTTOM; > > - else if (!node->needed) > > + else if (!node->needed && !node->local.externally_visible) > > type = IPA_TOP; > > /* When cloning is allowed, we can assume that externally visible functions > > are not called. We will compensate this by cloning later. */ > > Because this is for a branch, I would be more conservative and avoid any new instances of IPA_TOP -- those that can be obtained from subsequent else-if clauses. I.e., > > else if (!node->needed) > type = !node->local.externally_visible ? IPA_TOP : IPA_BOTTOM; > That would be too conservative because it would mean we would never duplicate a node and thus never propagate constants into externally visible functions, even not at -O3 and we do want to keep doing that. I have committed my patch after it has been approved on IRC by Richi and Jakub. Thanks, Martin
Index: gcc/ipa-cp.c =================================================================== --- gcc/ipa-cp.c (revision 184662) +++ gcc/ipa-cp.c (working copy) @@ -508,7 +508,7 @@ ipcp_initialize_node_lattices (struct cg if (ipa_is_called_with_var_arguments (info)) type = IPA_BOTTOM; - else if (!node->needed) + else if (!node->needed && !node->local.externally_visible) type = IPA_TOP; /* When cloning is allowed, we can assume that externally visible functions are not called. We will compensate this by cloning later. */