Trust TREE_ADDRESSABLE

> > On Fri, 13 Jun 2014, Jan Hubicka wrote:
> > 
> > > > 
> > > > When you extract the address and use it.  For example when you
> > > > do auto-parallelization and outline a part of your function it
> > > > passes arrays as addresses.
> > > > 
> > > > Or if you start to introduce address induction variables like
> > > > the vectorizer or IVOPTs does.
> > > 
> > > I see, nothing really done by current early/IPA optimizers and in those cases
> > > we also want to set TREE_ADDRESSABLE bit, too I suppose.
> > > Do you think I should make patch for setting the NOVOPS bits in ipa code?
> > 
> > No, please don't introduce new users of NOVOPS (it's a quite broken
> > hack - it's sth like a 'const' function with side-effects so we should
> > have instead used 'const' and some kind of volatile flag).  We're
> > not using NOVOPS much and that's good (I think handling of such
> > function calls are somewhat broken).
> 
> I meant DECL_NONALIASED.  I will test the patch and lets see.

Hi,
this patch adds the discussed code to set DECL_NOALIASED so we get better AA
with partitioning.  We probably also can sed DECL_NOALIASED for variables
whose address is passed only to external calls that do not capture the
parameters (i.e. memset).

I suppose I can teach ipa-ref code about this, but will need a bit of
extra infrastructure for that, since currently REF_ADDR does not associate
any information about these.

Martin, this is related to your controlled uses.  What do you think about adding
stable UIDs into ipa_ref datastructure and then adding a vector into cgraph edges
that describe what REFs are directly used as parameters of a given callsite?
It will take some work to maintain these, but we will be able to remove them when
call site or parameter was eliminated in a more general way.

I suppose we could also use these to associate REFs with given use in the
satement or constructor (i.e. have pointer to statement as well as pointer to
specific use within the statement). With this we will be able to redirect
references same way as we redirect callgraph edges now.  This is something I
need to for the ipa-visibility optimizations.

Honza

Bootstrapped/regtested and lto-bootstrapped x86_64-linux, will commit it shortly.

	* ipa.c (clear_addressable_bit): Set also DECL_NONALIASED.
	(ipa_discover_readonly_nonaddressable_var): Compute also NONALIASED.

On Mon, 23 Jun 2014, Jan Hubicka wrote:

> > > On Fri, 13 Jun 2014, Jan Hubicka wrote:
> > > 
> > > > > 
> > > > > When you extract the address and use it.  For example when you
> > > > > do auto-parallelization and outline a part of your function it
> > > > > passes arrays as addresses.
> > > > > 
> > > > > Or if you start to introduce address induction variables like
> > > > > the vectorizer or IVOPTs does.
> > > > 
> > > > I see, nothing really done by current early/IPA optimizers and in those cases
> > > > we also want to set TREE_ADDRESSABLE bit, too I suppose.
> > > > Do you think I should make patch for setting the NOVOPS bits in ipa code?
> > > 
> > > No, please don't introduce new users of NOVOPS (it's a quite broken
> > > hack - it's sth like a 'const' function with side-effects so we should
> > > have instead used 'const' and some kind of volatile flag).  We're
> > > not using NOVOPS much and that's good (I think handling of such
> > > function calls are somewhat broken).
> > 
> > I meant DECL_NONALIASED.  I will test the patch and lets see.
> 
> Hi,
> this patch adds the discussed code to set DECL_NOALIASED so we get better AA
> with partitioning.  We probably also can sed DECL_NOALIASED for variables
> whose address is passed only to external calls that do not capture the
> parameters (i.e. memset).
> 
> I suppose I can teach ipa-ref code about this, but will need a bit of
> extra infrastructure for that, since currently REF_ADDR does not associate
> any information about these.
> 
> Martin, this is related to your controlled uses.  What do you think about adding
> stable UIDs into ipa_ref datastructure and then adding a vector into cgraph edges
> that describe what REFs are directly used as parameters of a given callsite?
> It will take some work to maintain these, but we will be able to remove them when
> call site or parameter was eliminated in a more general way.
> 
> I suppose we could also use these to associate REFs with given use in the
> satement or constructor (i.e. have pointer to statement as well as pointer to
> specific use within the statement). With this we will be able to redirect
> references same way as we redirect callgraph edges now.  This is something I
> need to for the ipa-visibility optimizations.

I don't like this very much.  It's fragile and it will be very hard to
detect bugs caused by it.

Please don't spread uses of the DECL_NONALIASED "hack".

If we are only concerned about LTO I'd rather have a in_lto_p check
in may_be_aliased and trust TREE_ADDRESSABLE there.

Richard.

> Honza
> 
> Bootstrapped/regtested and lto-bootstrapped x86_64-linux, will commit it shortly.
> 
> 	* ipa.c (clear_addressable_bit): Set also DECL_NONALIASED.
> 	(ipa_discover_readonly_nonaddressable_var): Compute also NONALIASED.
> Index: ipa.c
> ===================================================================
> --- ipa.c	(revision 211881)
> +++ ipa.c	(working copy)
> @@ -669,6 +669,10 @@ clear_addressable_bit (varpool_node *vno
>  {
>    vnode->address_taken = false;
>    TREE_ADDRESSABLE (vnode->decl) = 0;
> +  /* Set also non-aliased bit.  In LTO, when program is partitioned, we no longer
> +     trust TREE_ADDRESSABLE for TREE_PUBLIC variables and then DECL_NONALIASED is
> +     useful to improve code.  */
> +  DECL_NONALIASED (vnode->decl) = 1;
>    return false;
>  }
>  
> @@ -690,6 +694,7 @@ ipa_discover_readonly_nonaddressable_var
>    FOR_EACH_VARIABLE (vnode)
>      if (!vnode->alias
>  	&& (TREE_ADDRESSABLE (vnode->decl)
> +	    || !DECL_NONALIASED (vnode->decl)
>  	    || !vnode->writeonly
>  	    || !TREE_READONLY (vnode->decl)))
>        {
> @@ -703,8 +708,8 @@ ipa_discover_readonly_nonaddressable_var
>  	  continue;
>  	if (!address_taken)
>  	  {
> -	    if (TREE_ADDRESSABLE (vnode->decl) && dump_file)
> -	      fprintf (dump_file, " %s (non-addressable)", vnode->name ());
> +	    if ((TREE_ADDRESSABLE (vnode->decl) || !DECL_NONALIASED (vnode->decl)) && dump_file)
> +	      fprintf (dump_file, " %s (non-addressable non-aliased)", vnode->name ());
>  	    varpool_for_node_and_aliases (vnode, clear_addressable_bit, NULL, true);
>  	  }
>  	if (!address_taken && !written
> 
>

Hi,

On Mon, Jun 23, 2014 at 04:55:36AM +0200, Jan Hubicka wrote:
> > > On Fri, 13 Jun 2014, Jan Hubicka wrote:
> > > 
> > > > > 
> > > > > When you extract the address and use it.  For example when you
> > > > > do auto-parallelization and outline a part of your function it
> > > > > passes arrays as addresses.
> > > > > 
> > > > > Or if you start to introduce address induction variables like
> > > > > the vectorizer or IVOPTs does.
> > > > 
> > > > I see, nothing really done by current early/IPA optimizers and in those cases
> > > > we also want to set TREE_ADDRESSABLE bit, too I suppose.
> > > > Do you think I should make patch for setting the NOVOPS bits in ipa code?
> > > 
> > > No, please don't introduce new users of NOVOPS (it's a quite broken
> > > hack - it's sth like a 'const' function with side-effects so we should
> > > have instead used 'const' and some kind of volatile flag).  We're
> > > not using NOVOPS much and that's good (I think handling of such
> > > function calls are somewhat broken).
> > 
> > I meant DECL_NONALIASED.  I will test the patch and lets see.
> 
> Hi,
> this patch adds the discussed code to set DECL_NOALIASED so we get better AA
> with partitioning.  We probably also can sed DECL_NOALIASED for variables
> whose address is passed only to external calls that do not capture the
> parameters (i.e. memset).
> 
> I suppose I can teach ipa-ref code about this, but will need a bit of
> extra infrastructure for that, since currently REF_ADDR does not associate
> any information about these.
> 
> Martin, this is related to your controlled uses.  What do you think about adding
> stable UIDs into ipa_ref datastructure and then adding a vector into cgraph edges
> that describe what REFs are directly used as parameters of a given callsite?
> It will take some work to maintain these, but we will be able to remove them when
> call site or parameter was eliminated in a more general way.

I'm still recovering from getting up at six in the morning today so I
may be a bit slow but: the big patch already assigns (per-function)
UIDs to "interesting DECLs and then maintains this information in jump
functions.  The only advantage of reference UIDs I can think of now is
that we would stop treating all references from and to same things as
equal (because currently we delete the first one we find).  Is that
what you want to achieve?

And by the way, if we add support for nocapture calls like memeset
that you described above, the big ipa-prop "noescape" patch will
actually directly calculate the nonaliased flag.  Perhaps it should be
even called that and not noescape.


> I suppose we could also use these to associate REFs with given use in the
> satement or constructor (i.e. have pointer to statement as well as pointer to
> specific use within the statement). With this we will be able to redirect
> references same way as we redirect callgraph edges now.  This is something I
> need to for the ipa-visibility optimizations.

I see.  I will think about this some more (and will be happy to chat on IRC).
Thanks,

Martin


> 
> Honza
> 
> Bootstrapped/regtested and lto-bootstrapped x86_64-linux, will commit it shortly.
> 
> 	* ipa.c (clear_addressable_bit): Set also DECL_NONALIASED.
> 	(ipa_discover_readonly_nonaddressable_var): Compute also NONALIASED.
> Index: ipa.c
> ===================================================================
> --- ipa.c	(revision 211881)
> +++ ipa.c	(working copy)
> @@ -669,6 +669,10 @@ clear_addressable_bit (varpool_node *vno
>  {
>    vnode->address_taken = false;
>    TREE_ADDRESSABLE (vnode->decl) = 0;
> +  /* Set also non-aliased bit.  In LTO, when program is partitioned, we no longer
> +     trust TREE_ADDRESSABLE for TREE_PUBLIC variables and then DECL_NONALIASED is
> +     useful to improve code.  */
> +  DECL_NONALIASED (vnode->decl) = 1;
>    return false;
>  }
>  
> @@ -690,6 +694,7 @@ ipa_discover_readonly_nonaddressable_var
>    FOR_EACH_VARIABLE (vnode)
>      if (!vnode->alias
>  	&& (TREE_ADDRESSABLE (vnode->decl)
> +	    || !DECL_NONALIASED (vnode->decl)
>  	    || !vnode->writeonly
>  	    || !TREE_READONLY (vnode->decl)))
>        {
> @@ -703,8 +708,8 @@ ipa_discover_readonly_nonaddressable_var
>  	  continue;
>  	if (!address_taken)
>  	  {
> -	    if (TREE_ADDRESSABLE (vnode->decl) && dump_file)
> -	      fprintf (dump_file, " %s (non-addressable)", vnode->name ());
> +	    if ((TREE_ADDRESSABLE (vnode->decl) || !DECL_NONALIASED (vnode->decl)) && dump_file)
> +	      fprintf (dump_file, " %s (non-addressable non-aliased)", vnode->name ());
>  	    varpool_for_node_and_aliases (vnode, clear_addressable_bit, NULL, true);
>  	  }
>  	if (!address_taken && !written

> I don't like this very much.  It's fragile and it will be very hard to
> detect bugs caused by it.
> 
> Please don't spread uses of the DECL_NONALIASED "hack".
> 
> If we are only concerned about LTO I'd rather have a in_lto_p check
> in may_be_aliased and trust TREE_ADDRESSABLE there.

I do not like it ether, but I tought it was outcome of the discussion to use
it.

I do not see how in_lto_p helps here, but we probably want to go for the
altnerative where ipa-visibility sets TREE_ADDRESSABLE for all external
variables and then we trust it unconditonally?

Honza

On June 23, 2014 6:15:10 PM CEST, Jan Hubicka <hubicka@ucw.cz> wrote:
>> I don't like this very much.  It's fragile and it will be very hard
>to
>> detect bugs caused by it.
>> 
>> Please don't spread uses of the DECL_NONALIASED "hack".
>> 
>> If we are only concerned about LTO I'd rather have a in_lto_p check
>> in may_be_aliased and trust TREE_ADDRESSABLE there.
>
>I do not like it ether, but I tought it was outcome of the discussion
>to use
>it.
>
>I do not see how in_lto_p helps here, but we probably want to go for
>the

If we are sure it is correctly set in lto1 it helps.

>altnerative where ipa-visibility sets TREE_ADDRESSABLE for all external
>variables and then we trust it unconditonally?

That works for me, too.  But at least add a checking assert that may-be-aliased is not invoked before that.

Richard.


>
>Honza

> On June 23, 2014 6:15:10 PM CEST, Jan Hubicka <hubicka@ucw.cz> wrote:
> >> I don't like this very much.  It's fragile and it will be very hard
> >to
> >> detect bugs caused by it.
> >> 
> >> Please don't spread uses of the DECL_NONALIASED "hack".
> >> 
> >> If we are only concerned about LTO I'd rather have a in_lto_p check
> >> in may_be_aliased and trust TREE_ADDRESSABLE there.
> >
> >I do not like it ether, but I tought it was outcome of the discussion
> >to use
> >it.
> >
> >I do not see how in_lto_p helps here, but we probably want to go for
> >the
> 
> If we are sure it is correctly set in lto1 it helps.
> 
> >altnerative where ipa-visibility sets TREE_ADDRESSABLE for all external
> >variables and then we trust it unconditonally?
> 
> That works for me, too.  But at least add a checking assert that may-be-aliased is not invoked before that.

OK, I suppose can check cgraph_state for that (after construction it will be
all set).  Will prepapre patch tonight.

Honza
> 
> Richard.
> 
> 
> >
> >Honza
>