Re: [PATCH, 4.4, PR 52430] IPA-CP has to clone or leave alone externally_visible nodes
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
Re: [PATCH, 4.4, PR 52430] IPA-CP has to clone or leave alone externally_visible nodes
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
Re: [PATCH, 4.4, PR 52430] IPA-CP has to clone or leave alone externally_visible nodes
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
