[4.4,PR,52430] IPA-CP has to clone or leave alone externally_visible nodes

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?

Thanks,

Martin


2012-02-29  Martin Jambor  <mjambor@suse.cz>

	PR tree-optimization/52430
	* ipa-cp.c (ipcp_initialize_node_lattices): Also consider
	node->local.externally_visible as needed.

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