public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Re: [PATCH] rtl-optimization/113597 - recover base term for argument pointers
       [not found] <20240209102637.6BB1D3858438@sourceware.org>
@ 2024-02-10 21:20 ` Toon Moene
  2024-02-12  8:00   ` Richard Biener
  0 siblings, 1 reply; 6+ messages in thread
From: Toon Moene @ 2024-02-10 21:20 UTC (permalink / raw)
  To: Richard Biener, gcc-patches; +Cc: jeffreyalaw

I managed to try this patch on aarch64-linux-gnu:

This is the test run without your patch:

https://gcc.gnu.org/pipermail/gcc-testresults/2024-February/807637.html

And this is the "result" with your patch:

https://gcc.gnu.org/pipermail/gcc-testresults/2024-February/807645.html

For me, as for you, it works for x86_64-linux-gnu:

https://gcc.gnu.org/pipermail/gcc-testresults/2024-February/807609.html

I hope this helps.

Kind regards,
Toon Moene.

On 2/9/24 11:26, Richard Biener wrote:
> The following allows a base term to be derived from an existing
> MEM_EXPR, notably the points-to set of a MEM_REF base.  For the
> testcase in the PR this helps RTL DSE elide stores to a stack
> temporary.  This covers pointers to NONLOCAL which can be mapped
> to arg_base_value, helping to disambiguate against other special
> bases (ADDRESS) as well as PARM_DECL accesses.
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
> 
> This is an attempt to recover some of the losses from dumbing down
> find_base_{term,value}.  I did give my ideas how to properly do
> this during stage1 a start, I will post a short incomplete RFC series
> later today.
> 
> OK for trunk?
> 
> I've included all languages in testing and also tested with -m32 but
> details of RTL alias analysis might escape me ...
> 
> Thanks,
> Richard.
> 
> 	PR rtl-optimization/113597
> 	* alias.cc (find_base_term): Add argument for the whole mem
> 	and derive a base term from its MEM_EXPR.
> 	(true_dependence_1): Pass down the MEMs to find_base_term.
> 	(write_dependence_p): Likewise.
> 	(may_alias_p): Likewise.
> ---
>   gcc/alias.cc | 43 ++++++++++++++++++++++++++++++++++++-------
>   1 file changed, 36 insertions(+), 7 deletions(-)
> 
> diff --git a/gcc/alias.cc b/gcc/alias.cc
> index 6fad4b29d31..e33c56b0e80 100644
> --- a/gcc/alias.cc
> +++ b/gcc/alias.cc
> @@ -40,6 +40,9 @@ along with GCC; see the file COPYING3.  If not see
>   #include "rtl-iter.h"
>   #include "cgraph.h"
>   #include "ipa-utils.h"
> +#include "stringpool.h"
> +#include "value-range.h"
> +#include "tree-ssanames.h"
>   
>   /* The aliasing API provided here solves related but different problems:
>   
> @@ -190,6 +193,10 @@ static struct {
>   	arguments, since we do not know at this level whether accesses
>   	based on different arguments can alias.  The ADDRESS has id 0.
>   
> +	This is solely useful to disambiguate against other ADDRESS
> +	bases as we know incoming pointers cannot point to local
> +	stack, frame or argument space.
> +
>        2. stack_pointer_rtx, frame_pointer_rtx, hard_frame_pointer_rtx
>   	(if distinct from frame_pointer_rtx) and arg_pointer_rtx.
>   	Each of these rtxes has a separate ADDRESS associated with it,
> @@ -2113,12 +2120,34 @@ find_base_term (rtx x, vec<std::pair<cselib_val *,
>      to avoid visiting them multiple times.  We unwind that changes here.  */
>   
>   static rtx
> -find_base_term (rtx x)
> +find_base_term (rtx x, const_rtx mem = NULL_RTX)
>   {
>     auto_vec<std::pair<cselib_val *, struct elt_loc_list *>, 32> visited_vals;
>     rtx res = find_base_term (x, visited_vals);
>     for (unsigned i = 0; i < visited_vals.length (); ++i)
>       visited_vals[i].first->locs = visited_vals[i].second;
> +  if (!res && mem && MEM_EXPR (mem))
> +    {
> +      tree base = get_base_address (MEM_EXPR (mem));
> +      if (TREE_CODE (base) == PARM_DECL
> +	  && DECL_RTL_SET_P (base))
> +	/* We need to look at how we expanded a PARM_DECL.  It might be in
> +	   the argument space (UNIQUE_BASE_VALUE_ARGP) or it might
> +	   be spilled (UNIQUE_BASE_VALUE_FP/UNIQUE_BASE_VALUE_HFP).  */
> +	res = find_base_term (DECL_RTL (base));
> +      else if (TREE_CODE (base) == MEM_REF
> +	       && TREE_CODE (TREE_OPERAND (base, 0)) == SSA_NAME
> +	       && SSA_NAME_PTR_INFO (TREE_OPERAND (base, 0)))
> +	{
> +	  auto pt = &SSA_NAME_PTR_INFO (TREE_OPERAND (base, 0))->pt;
> +	  if (pt->nonlocal
> +	      && !pt->anything
> +	      && !pt->escaped
> +	      && !pt->ipa_escaped
> +	      && bitmap_empty_p (pt->vars))
> +	    res = arg_base_value;
> +	}
> +    }
>     return res;
>   }
>   
> @@ -3035,13 +3064,13 @@ true_dependence_1 (const_rtx mem, machine_mode mem_mode, rtx mem_addr,
>     if (MEM_ADDR_SPACE (mem) != MEM_ADDR_SPACE (x))
>       return true;
>   
> -  base = find_base_term (x_addr);
> +  base = find_base_term (x_addr, x);
>     if (base && (GET_CODE (base) == LABEL_REF
>   	       || (GET_CODE (base) == SYMBOL_REF
>   		   && CONSTANT_POOL_ADDRESS_P (base))))
>       return false;
>   
> -  rtx mem_base = find_base_term (true_mem_addr);
> +  rtx mem_base = find_base_term (true_mem_addr, mem);
>     if (! base_alias_check (x_addr, base, true_mem_addr, mem_base,
>   			  GET_MODE (x), mem_mode))
>       return false;
> @@ -3142,7 +3171,7 @@ write_dependence_p (const_rtx mem,
>     if (MEM_ADDR_SPACE (mem) != MEM_ADDR_SPACE (x))
>       return true;
>   
> -  base = find_base_term (true_mem_addr);
> +  base = find_base_term (true_mem_addr, mem);
>     if (! writep
>         && base
>         && (GET_CODE (base) == LABEL_REF
> @@ -3150,7 +3179,7 @@ write_dependence_p (const_rtx mem,
>   	      && CONSTANT_POOL_ADDRESS_P (base))))
>       return false;
>   
> -  rtx x_base = find_base_term (true_x_addr);
> +  rtx x_base = find_base_term (true_x_addr, x);
>     if (! base_alias_check (true_x_addr, x_base, true_mem_addr, base,
>   			  GET_MODE (x), GET_MODE (mem)))
>       return false;
> @@ -3265,8 +3294,8 @@ may_alias_p (const_rtx mem, const_rtx x)
>     if (MEM_ADDR_SPACE (mem) != MEM_ADDR_SPACE (x))
>       return true;
>   
> -  rtx x_base = find_base_term (x_addr);
> -  rtx mem_base = find_base_term (mem_addr);
> +  rtx x_base = find_base_term (x_addr, x);
> +  rtx mem_base = find_base_term (mem_addr, mem);
>     if (! base_alias_check (x_addr, x_base, mem_addr, mem_base,
>   			  GET_MODE (x), GET_MODE (mem_addr)))
>       return false;

-- 
Toon Moene - e-mail: toon@moene.org - phone: +31 346 214290
Saturnushof 14, 3738 XG  Maartensdijk, The Netherlands


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] rtl-optimization/113597 - recover base term for argument pointers
  2024-02-10 21:20 ` [PATCH] rtl-optimization/113597 - recover base term for argument pointers Toon Moene
@ 2024-02-12  8:00   ` Richard Biener
  2024-02-12 15:26     ` Christophe Lyon
  0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2024-02-12  8:00 UTC (permalink / raw)
  To: Toon Moene; +Cc: Richard Biener, gcc-patches, jeffreyalaw

On Sat, Feb 10, 2024 at 10:21 PM Toon Moene <toon@moene.org> wrote:
>
> I managed to try this patch on aarch64-linux-gnu:
>
> This is the test run without your patch:
>
> https://gcc.gnu.org/pipermail/gcc-testresults/2024-February/807637.html
>
> And this is the "result" with your patch:
>
> https://gcc.gnu.org/pipermail/gcc-testresults/2024-February/807645.html

Interesting .. this looks like a miscompile of stage2.  Note that the Linaro
CI was happy:

https://patchwork.sourceware.org/project/gcc/patch/20240209102649.09C543858404@sourceware.org/

Richard.

> For me, as for you, it works for x86_64-linux-gnu:
>
> https://gcc.gnu.org/pipermail/gcc-testresults/2024-February/807609.html
>
> I hope this helps.
>
> Kind regards,
> Toon Moene.
>
> On 2/9/24 11:26, Richard Biener wrote:
> > The following allows a base term to be derived from an existing
> > MEM_EXPR, notably the points-to set of a MEM_REF base.  For the
> > testcase in the PR this helps RTL DSE elide stores to a stack
> > temporary.  This covers pointers to NONLOCAL which can be mapped
> > to arg_base_value, helping to disambiguate against other special
> > bases (ADDRESS) as well as PARM_DECL accesses.
> >
> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
> >
> > This is an attempt to recover some of the losses from dumbing down
> > find_base_{term,value}.  I did give my ideas how to properly do
> > this during stage1 a start, I will post a short incomplete RFC series
> > later today.
> >
> > OK for trunk?
> >
> > I've included all languages in testing and also tested with -m32 but
> > details of RTL alias analysis might escape me ...
> >
> > Thanks,
> > Richard.
> >
> >       PR rtl-optimization/113597
> >       * alias.cc (find_base_term): Add argument for the whole mem
> >       and derive a base term from its MEM_EXPR.
> >       (true_dependence_1): Pass down the MEMs to find_base_term.
> >       (write_dependence_p): Likewise.
> >       (may_alias_p): Likewise.
> > ---
> >   gcc/alias.cc | 43 ++++++++++++++++++++++++++++++++++++-------
> >   1 file changed, 36 insertions(+), 7 deletions(-)
> >
> > diff --git a/gcc/alias.cc b/gcc/alias.cc
> > index 6fad4b29d31..e33c56b0e80 100644
> > --- a/gcc/alias.cc
> > +++ b/gcc/alias.cc
> > @@ -40,6 +40,9 @@ along with GCC; see the file COPYING3.  If not see
> >   #include "rtl-iter.h"
> >   #include "cgraph.h"
> >   #include "ipa-utils.h"
> > +#include "stringpool.h"
> > +#include "value-range.h"
> > +#include "tree-ssanames.h"
> >
> >   /* The aliasing API provided here solves related but different problems:
> >
> > @@ -190,6 +193,10 @@ static struct {
> >       arguments, since we do not know at this level whether accesses
> >       based on different arguments can alias.  The ADDRESS has id 0.
> >
> > +     This is solely useful to disambiguate against other ADDRESS
> > +     bases as we know incoming pointers cannot point to local
> > +     stack, frame or argument space.
> > +
> >        2. stack_pointer_rtx, frame_pointer_rtx, hard_frame_pointer_rtx
> >       (if distinct from frame_pointer_rtx) and arg_pointer_rtx.
> >       Each of these rtxes has a separate ADDRESS associated with it,
> > @@ -2113,12 +2120,34 @@ find_base_term (rtx x, vec<std::pair<cselib_val *,
> >      to avoid visiting them multiple times.  We unwind that changes here.  */
> >
> >   static rtx
> > -find_base_term (rtx x)
> > +find_base_term (rtx x, const_rtx mem = NULL_RTX)
> >   {
> >     auto_vec<std::pair<cselib_val *, struct elt_loc_list *>, 32> visited_vals;
> >     rtx res = find_base_term (x, visited_vals);
> >     for (unsigned i = 0; i < visited_vals.length (); ++i)
> >       visited_vals[i].first->locs = visited_vals[i].second;
> > +  if (!res && mem && MEM_EXPR (mem))
> > +    {
> > +      tree base = get_base_address (MEM_EXPR (mem));
> > +      if (TREE_CODE (base) == PARM_DECL
> > +       && DECL_RTL_SET_P (base))
> > +     /* We need to look at how we expanded a PARM_DECL.  It might be in
> > +        the argument space (UNIQUE_BASE_VALUE_ARGP) or it might
> > +        be spilled (UNIQUE_BASE_VALUE_FP/UNIQUE_BASE_VALUE_HFP).  */
> > +     res = find_base_term (DECL_RTL (base));
> > +      else if (TREE_CODE (base) == MEM_REF
> > +            && TREE_CODE (TREE_OPERAND (base, 0)) == SSA_NAME
> > +            && SSA_NAME_PTR_INFO (TREE_OPERAND (base, 0)))
> > +     {
> > +       auto pt = &SSA_NAME_PTR_INFO (TREE_OPERAND (base, 0))->pt;
> > +       if (pt->nonlocal
> > +           && !pt->anything
> > +           && !pt->escaped
> > +           && !pt->ipa_escaped
> > +           && bitmap_empty_p (pt->vars))
> > +         res = arg_base_value;
> > +     }
> > +    }
> >     return res;
> >   }
> >
> > @@ -3035,13 +3064,13 @@ true_dependence_1 (const_rtx mem, machine_mode mem_mode, rtx mem_addr,
> >     if (MEM_ADDR_SPACE (mem) != MEM_ADDR_SPACE (x))
> >       return true;
> >
> > -  base = find_base_term (x_addr);
> > +  base = find_base_term (x_addr, x);
> >     if (base && (GET_CODE (base) == LABEL_REF
> >              || (GET_CODE (base) == SYMBOL_REF
> >                  && CONSTANT_POOL_ADDRESS_P (base))))
> >       return false;
> >
> > -  rtx mem_base = find_base_term (true_mem_addr);
> > +  rtx mem_base = find_base_term (true_mem_addr, mem);
> >     if (! base_alias_check (x_addr, base, true_mem_addr, mem_base,
> >                         GET_MODE (x), mem_mode))
> >       return false;
> > @@ -3142,7 +3171,7 @@ write_dependence_p (const_rtx mem,
> >     if (MEM_ADDR_SPACE (mem) != MEM_ADDR_SPACE (x))
> >       return true;
> >
> > -  base = find_base_term (true_mem_addr);
> > +  base = find_base_term (true_mem_addr, mem);
> >     if (! writep
> >         && base
> >         && (GET_CODE (base) == LABEL_REF
> > @@ -3150,7 +3179,7 @@ write_dependence_p (const_rtx mem,
> >             && CONSTANT_POOL_ADDRESS_P (base))))
> >       return false;
> >
> > -  rtx x_base = find_base_term (true_x_addr);
> > +  rtx x_base = find_base_term (true_x_addr, x);
> >     if (! base_alias_check (true_x_addr, x_base, true_mem_addr, base,
> >                         GET_MODE (x), GET_MODE (mem)))
> >       return false;
> > @@ -3265,8 +3294,8 @@ may_alias_p (const_rtx mem, const_rtx x)
> >     if (MEM_ADDR_SPACE (mem) != MEM_ADDR_SPACE (x))
> >       return true;
> >
> > -  rtx x_base = find_base_term (x_addr);
> > -  rtx mem_base = find_base_term (mem_addr);
> > +  rtx x_base = find_base_term (x_addr, x);
> > +  rtx mem_base = find_base_term (mem_addr, mem);
> >     if (! base_alias_check (x_addr, x_base, mem_addr, mem_base,
> >                         GET_MODE (x), GET_MODE (mem_addr)))
> >       return false;
>
> --
> Toon Moene - e-mail: toon@moene.org - phone: +31 346 214290
> Saturnushof 14, 3738 XG  Maartensdijk, The Netherlands
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] rtl-optimization/113597 - recover base term for argument pointers
  2024-02-12  8:00   ` Richard Biener
@ 2024-02-12 15:26     ` Christophe Lyon
  0 siblings, 0 replies; 6+ messages in thread
From: Christophe Lyon @ 2024-02-12 15:26 UTC (permalink / raw)
  To: Richard Biener; +Cc: Toon Moene, Richard Biener, gcc-patches, jeffreyalaw

On Mon, 12 Feb 2024 at 09:00, Richard Biener <richard.guenther@gmail.com> wrote:
>
> On Sat, Feb 10, 2024 at 10:21 PM Toon Moene <toon@moene.org> wrote:
> >
> > I managed to try this patch on aarch64-linux-gnu:
> >
> > This is the test run without your patch:
> >
> > https://gcc.gnu.org/pipermail/gcc-testresults/2024-February/807637.html
> >
> > And this is the "result" with your patch:
> >
> > https://gcc.gnu.org/pipermail/gcc-testresults/2024-February/807645.html
>
> Interesting .. this looks like a miscompile of stage2.  Note that the Linaro
> CI was happy:
>
> https://patchwork.sourceware.org/project/gcc/patch/20240209102649.09C543858404@sourceware.org/
>
Hi!

Indeed this is NOT a bootstrap build, we only have such checks in postcommit CI.

Thanks,

Christophe

> Richard.
>
> > For me, as for you, it works for x86_64-linux-gnu:
> >
> > https://gcc.gnu.org/pipermail/gcc-testresults/2024-February/807609.html
> >
> > I hope this helps.
> >
> > Kind regards,
> > Toon Moene.
> >
> > On 2/9/24 11:26, Richard Biener wrote:
> > > The following allows a base term to be derived from an existing
> > > MEM_EXPR, notably the points-to set of a MEM_REF base.  For the
> > > testcase in the PR this helps RTL DSE elide stores to a stack
> > > temporary.  This covers pointers to NONLOCAL which can be mapped
> > > to arg_base_value, helping to disambiguate against other special
> > > bases (ADDRESS) as well as PARM_DECL accesses.
> > >
> > > Bootstrapped and tested on x86_64-unknown-linux-gnu.
> > >
> > > This is an attempt to recover some of the losses from dumbing down
> > > find_base_{term,value}.  I did give my ideas how to properly do
> > > this during stage1 a start, I will post a short incomplete RFC series
> > > later today.
> > >
> > > OK for trunk?
> > >
> > > I've included all languages in testing and also tested with -m32 but
> > > details of RTL alias analysis might escape me ...
> > >
> > > Thanks,
> > > Richard.
> > >
> > >       PR rtl-optimization/113597
> > >       * alias.cc (find_base_term): Add argument for the whole mem
> > >       and derive a base term from its MEM_EXPR.
> > >       (true_dependence_1): Pass down the MEMs to find_base_term.
> > >       (write_dependence_p): Likewise.
> > >       (may_alias_p): Likewise.
> > > ---
> > >   gcc/alias.cc | 43 ++++++++++++++++++++++++++++++++++++-------
> > >   1 file changed, 36 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/gcc/alias.cc b/gcc/alias.cc
> > > index 6fad4b29d31..e33c56b0e80 100644
> > > --- a/gcc/alias.cc
> > > +++ b/gcc/alias.cc
> > > @@ -40,6 +40,9 @@ along with GCC; see the file COPYING3.  If not see
> > >   #include "rtl-iter.h"
> > >   #include "cgraph.h"
> > >   #include "ipa-utils.h"
> > > +#include "stringpool.h"
> > > +#include "value-range.h"
> > > +#include "tree-ssanames.h"
> > >
> > >   /* The aliasing API provided here solves related but different problems:
> > >
> > > @@ -190,6 +193,10 @@ static struct {
> > >       arguments, since we do not know at this level whether accesses
> > >       based on different arguments can alias.  The ADDRESS has id 0.
> > >
> > > +     This is solely useful to disambiguate against other ADDRESS
> > > +     bases as we know incoming pointers cannot point to local
> > > +     stack, frame or argument space.
> > > +
> > >        2. stack_pointer_rtx, frame_pointer_rtx, hard_frame_pointer_rtx
> > >       (if distinct from frame_pointer_rtx) and arg_pointer_rtx.
> > >       Each of these rtxes has a separate ADDRESS associated with it,
> > > @@ -2113,12 +2120,34 @@ find_base_term (rtx x, vec<std::pair<cselib_val *,
> > >      to avoid visiting them multiple times.  We unwind that changes here.  */
> > >
> > >   static rtx
> > > -find_base_term (rtx x)
> > > +find_base_term (rtx x, const_rtx mem = NULL_RTX)
> > >   {
> > >     auto_vec<std::pair<cselib_val *, struct elt_loc_list *>, 32> visited_vals;
> > >     rtx res = find_base_term (x, visited_vals);
> > >     for (unsigned i = 0; i < visited_vals.length (); ++i)
> > >       visited_vals[i].first->locs = visited_vals[i].second;
> > > +  if (!res && mem && MEM_EXPR (mem))
> > > +    {
> > > +      tree base = get_base_address (MEM_EXPR (mem));
> > > +      if (TREE_CODE (base) == PARM_DECL
> > > +       && DECL_RTL_SET_P (base))
> > > +     /* We need to look at how we expanded a PARM_DECL.  It might be in
> > > +        the argument space (UNIQUE_BASE_VALUE_ARGP) or it might
> > > +        be spilled (UNIQUE_BASE_VALUE_FP/UNIQUE_BASE_VALUE_HFP).  */
> > > +     res = find_base_term (DECL_RTL (base));
> > > +      else if (TREE_CODE (base) == MEM_REF
> > > +            && TREE_CODE (TREE_OPERAND (base, 0)) == SSA_NAME
> > > +            && SSA_NAME_PTR_INFO (TREE_OPERAND (base, 0)))
> > > +     {
> > > +       auto pt = &SSA_NAME_PTR_INFO (TREE_OPERAND (base, 0))->pt;
> > > +       if (pt->nonlocal
> > > +           && !pt->anything
> > > +           && !pt->escaped
> > > +           && !pt->ipa_escaped
> > > +           && bitmap_empty_p (pt->vars))
> > > +         res = arg_base_value;
> > > +     }
> > > +    }
> > >     return res;
> > >   }
> > >
> > > @@ -3035,13 +3064,13 @@ true_dependence_1 (const_rtx mem, machine_mode mem_mode, rtx mem_addr,
> > >     if (MEM_ADDR_SPACE (mem) != MEM_ADDR_SPACE (x))
> > >       return true;
> > >
> > > -  base = find_base_term (x_addr);
> > > +  base = find_base_term (x_addr, x);
> > >     if (base && (GET_CODE (base) == LABEL_REF
> > >              || (GET_CODE (base) == SYMBOL_REF
> > >                  && CONSTANT_POOL_ADDRESS_P (base))))
> > >       return false;
> > >
> > > -  rtx mem_base = find_base_term (true_mem_addr);
> > > +  rtx mem_base = find_base_term (true_mem_addr, mem);
> > >     if (! base_alias_check (x_addr, base, true_mem_addr, mem_base,
> > >                         GET_MODE (x), mem_mode))
> > >       return false;
> > > @@ -3142,7 +3171,7 @@ write_dependence_p (const_rtx mem,
> > >     if (MEM_ADDR_SPACE (mem) != MEM_ADDR_SPACE (x))
> > >       return true;
> > >
> > > -  base = find_base_term (true_mem_addr);
> > > +  base = find_base_term (true_mem_addr, mem);
> > >     if (! writep
> > >         && base
> > >         && (GET_CODE (base) == LABEL_REF
> > > @@ -3150,7 +3179,7 @@ write_dependence_p (const_rtx mem,
> > >             && CONSTANT_POOL_ADDRESS_P (base))))
> > >       return false;
> > >
> > > -  rtx x_base = find_base_term (true_x_addr);
> > > +  rtx x_base = find_base_term (true_x_addr, x);
> > >     if (! base_alias_check (true_x_addr, x_base, true_mem_addr, base,
> > >                         GET_MODE (x), GET_MODE (mem)))
> > >       return false;
> > > @@ -3265,8 +3294,8 @@ may_alias_p (const_rtx mem, const_rtx x)
> > >     if (MEM_ADDR_SPACE (mem) != MEM_ADDR_SPACE (x))
> > >       return true;
> > >
> > > -  rtx x_base = find_base_term (x_addr);
> > > -  rtx mem_base = find_base_term (mem_addr);
> > > +  rtx x_base = find_base_term (x_addr, x);
> > > +  rtx mem_base = find_base_term (mem_addr, mem);
> > >     if (! base_alias_check (x_addr, x_base, mem_addr, mem_base,
> > >                         GET_MODE (x), GET_MODE (mem_addr)))
> > >       return false;
> >
> > --
> > Toon Moene - e-mail: toon@moene.org - phone: +31 346 214290
> > Saturnushof 14, 3738 XG  Maartensdijk, The Netherlands
> >

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] rtl-optimization/113597 - recover base term for argument pointers
  2024-03-03 23:58 ` Jeff Law
@ 2024-03-04  8:05   ` Richard Biener
  0 siblings, 0 replies; 6+ messages in thread
From: Richard Biener @ 2024-03-04  8:05 UTC (permalink / raw)
  To: Jeff Law; +Cc: gcc-patches

On Sun, 3 Mar 2024, Jeff Law wrote:

> 
> 
> On 2/9/24 03:26, Richard Biener wrote:
> > The following allows a base term to be derived from an existing
> > MEM_EXPR, notably the points-to set of a MEM_REF base.  For the
> > testcase in the PR this helps RTL DSE elide stores to a stack
> > temporary.  This covers pointers to NONLOCAL which can be mapped
> > to arg_base_value, helping to disambiguate against other special
> > bases (ADDRESS) as well as PARM_DECL accesses.
> I like it and as you note later, it's extendable.
> 
> > 
> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
> > 
> > This is an attempt to recover some of the losses from dumbing down
> > find_base_{term,value}.  I did give my ideas how to properly do
> > this during stage1 a start, I will post a short incomplete RFC series
> > later today.
> I saw those, but set them aside for gcc-15.
> 
> > 
> > OK for trunk?
> > 
> > I've included all languages in testing and also tested with -m32 but
> > details of RTL alias analysis might escape me ...
> > 
> > Thanks,
> > Richard.
> > 
> >  PR rtl-optimization/113597
> >  * alias.cc (find_base_term): Add argument for the whole mem
> >  and derive a base term from its MEM_EXPR.
> >  (true_dependence_1): Pass down the MEMs to find_base_term.
> >  (write_dependence_p): Likewise.
> >  (may_alias_p): Likewise.
> I'd lean ever so slightly against including this.  Not because I see anything
> wrong, more so because we don't have a lot of time for this to shake out if
> there are any problems.  But I wouldn't go as far as to say I object to
> including it.
> 
> So OK for the trunk if you want to go forward now.  Or defer if you want to
> take the somewhat safer route of waiting to gcc-15 to tackle this.

There was fallout (arm bootstrap fail) reported, so I defer it to 15
for which I posted another RFC series.  I do admit that I can't promise
to finish anything here.  The reported fallout was not too bad
luckily, or maybe just nobody noticed yet.

Richard.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] rtl-optimization/113597 - recover base term for argument pointers
       [not found] <65c5fdce.5d0a0220.b4fa7.11a6SMTPIN_ADDED_MISSING@mx.google.com>
@ 2024-03-03 23:58 ` Jeff Law
  2024-03-04  8:05   ` Richard Biener
  0 siblings, 1 reply; 6+ messages in thread
From: Jeff Law @ 2024-03-03 23:58 UTC (permalink / raw)
  To: Richard Biener, gcc-patches



On 2/9/24 03:26, Richard Biener wrote:
> The following allows a base term to be derived from an existing
> MEM_EXPR, notably the points-to set of a MEM_REF base.  For the
> testcase in the PR this helps RTL DSE elide stores to a stack
> temporary.  This covers pointers to NONLOCAL which can be mapped
> to arg_base_value, helping to disambiguate against other special
> bases (ADDRESS) as well as PARM_DECL accesses.
I like it and as you note later, it's extendable.

> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
> 
> This is an attempt to recover some of the losses from dumbing down
> find_base_{term,value}.  I did give my ideas how to properly do
> this during stage1 a start, I will post a short incomplete RFC series
> later today.
I saw those, but set them aside for gcc-15.

> 
> OK for trunk?
> 
> I've included all languages in testing and also tested with -m32 but
> details of RTL alias analysis might escape me ...
> 
> Thanks,
> Richard.
> 
> 	PR rtl-optimization/113597
> 	* alias.cc (find_base_term): Add argument for the whole mem
> 	and derive a base term from its MEM_EXPR.
> 	(true_dependence_1): Pass down the MEMs to find_base_term.
> 	(write_dependence_p): Likewise.
> 	(may_alias_p): Likewise.
I'd lean ever so slightly against including this.  Not because I see 
anything wrong, more so because we don't have a lot of time for this to 
shake out if there are any problems.  But I wouldn't go as far as to say 
I object to including it.

So OK for the trunk if you want to go forward now.  Or defer if you want 
to take the somewhat safer route of waiting to gcc-15 to tackle this.

jeff


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] rtl-optimization/113597 - recover base term for argument pointers
@ 2024-02-09 10:26 Richard Biener
  0 siblings, 0 replies; 6+ messages in thread
From: Richard Biener @ 2024-02-09 10:26 UTC (permalink / raw)
  To: gcc-patches; +Cc: jeffreyalaw

The following allows a base term to be derived from an existing
MEM_EXPR, notably the points-to set of a MEM_REF base.  For the
testcase in the PR this helps RTL DSE elide stores to a stack
temporary.  This covers pointers to NONLOCAL which can be mapped
to arg_base_value, helping to disambiguate against other special
bases (ADDRESS) as well as PARM_DECL accesses.

Bootstrapped and tested on x86_64-unknown-linux-gnu.

This is an attempt to recover some of the losses from dumbing down
find_base_{term,value}.  I did give my ideas how to properly do
this during stage1 a start, I will post a short incomplete RFC series
later today.

OK for trunk?

I've included all languages in testing and also tested with -m32 but
details of RTL alias analysis might escape me ...

Thanks,
Richard.

	PR rtl-optimization/113597
	* alias.cc (find_base_term): Add argument for the whole mem
	and derive a base term from its MEM_EXPR.
	(true_dependence_1): Pass down the MEMs to find_base_term.
	(write_dependence_p): Likewise.
	(may_alias_p): Likewise.
---
 gcc/alias.cc | 43 ++++++++++++++++++++++++++++++++++++-------
 1 file changed, 36 insertions(+), 7 deletions(-)

diff --git a/gcc/alias.cc b/gcc/alias.cc
index 6fad4b29d31..e33c56b0e80 100644
--- a/gcc/alias.cc
+++ b/gcc/alias.cc
@@ -40,6 +40,9 @@ along with GCC; see the file COPYING3.  If not see
 #include "rtl-iter.h"
 #include "cgraph.h"
 #include "ipa-utils.h"
+#include "stringpool.h"
+#include "value-range.h"
+#include "tree-ssanames.h"
 
 /* The aliasing API provided here solves related but different problems:
 
@@ -190,6 +193,10 @@ static struct {
 	arguments, since we do not know at this level whether accesses
 	based on different arguments can alias.  The ADDRESS has id 0.
 
+	This is solely useful to disambiguate against other ADDRESS
+	bases as we know incoming pointers cannot point to local
+	stack, frame or argument space.
+
      2. stack_pointer_rtx, frame_pointer_rtx, hard_frame_pointer_rtx
 	(if distinct from frame_pointer_rtx) and arg_pointer_rtx.
 	Each of these rtxes has a separate ADDRESS associated with it,
@@ -2113,12 +2120,34 @@ find_base_term (rtx x, vec<std::pair<cselib_val *,
    to avoid visiting them multiple times.  We unwind that changes here.  */
 
 static rtx
-find_base_term (rtx x)
+find_base_term (rtx x, const_rtx mem = NULL_RTX)
 {
   auto_vec<std::pair<cselib_val *, struct elt_loc_list *>, 32> visited_vals;
   rtx res = find_base_term (x, visited_vals);
   for (unsigned i = 0; i < visited_vals.length (); ++i)
     visited_vals[i].first->locs = visited_vals[i].second;
+  if (!res && mem && MEM_EXPR (mem))
+    {
+      tree base = get_base_address (MEM_EXPR (mem));
+      if (TREE_CODE (base) == PARM_DECL
+	  && DECL_RTL_SET_P (base))
+	/* We need to look at how we expanded a PARM_DECL.  It might be in
+	   the argument space (UNIQUE_BASE_VALUE_ARGP) or it might
+	   be spilled (UNIQUE_BASE_VALUE_FP/UNIQUE_BASE_VALUE_HFP).  */
+	res = find_base_term (DECL_RTL (base));
+      else if (TREE_CODE (base) == MEM_REF
+	       && TREE_CODE (TREE_OPERAND (base, 0)) == SSA_NAME
+	       && SSA_NAME_PTR_INFO (TREE_OPERAND (base, 0)))
+	{
+	  auto pt = &SSA_NAME_PTR_INFO (TREE_OPERAND (base, 0))->pt;
+	  if (pt->nonlocal
+	      && !pt->anything
+	      && !pt->escaped
+	      && !pt->ipa_escaped
+	      && bitmap_empty_p (pt->vars))
+	    res = arg_base_value;
+	}
+    }
   return res;
 }
 
@@ -3035,13 +3064,13 @@ true_dependence_1 (const_rtx mem, machine_mode mem_mode, rtx mem_addr,
   if (MEM_ADDR_SPACE (mem) != MEM_ADDR_SPACE (x))
     return true;
 
-  base = find_base_term (x_addr);
+  base = find_base_term (x_addr, x);
   if (base && (GET_CODE (base) == LABEL_REF
 	       || (GET_CODE (base) == SYMBOL_REF
 		   && CONSTANT_POOL_ADDRESS_P (base))))
     return false;
 
-  rtx mem_base = find_base_term (true_mem_addr);
+  rtx mem_base = find_base_term (true_mem_addr, mem);
   if (! base_alias_check (x_addr, base, true_mem_addr, mem_base,
 			  GET_MODE (x), mem_mode))
     return false;
@@ -3142,7 +3171,7 @@ write_dependence_p (const_rtx mem,
   if (MEM_ADDR_SPACE (mem) != MEM_ADDR_SPACE (x))
     return true;
 
-  base = find_base_term (true_mem_addr);
+  base = find_base_term (true_mem_addr, mem);
   if (! writep
       && base
       && (GET_CODE (base) == LABEL_REF
@@ -3150,7 +3179,7 @@ write_dependence_p (const_rtx mem,
 	      && CONSTANT_POOL_ADDRESS_P (base))))
     return false;
 
-  rtx x_base = find_base_term (true_x_addr);
+  rtx x_base = find_base_term (true_x_addr, x);
   if (! base_alias_check (true_x_addr, x_base, true_mem_addr, base,
 			  GET_MODE (x), GET_MODE (mem)))
     return false;
@@ -3265,8 +3294,8 @@ may_alias_p (const_rtx mem, const_rtx x)
   if (MEM_ADDR_SPACE (mem) != MEM_ADDR_SPACE (x))
     return true;
 
-  rtx x_base = find_base_term (x_addr);
-  rtx mem_base = find_base_term (mem_addr);
+  rtx x_base = find_base_term (x_addr, x);
+  rtx mem_base = find_base_term (mem_addr, mem);
   if (! base_alias_check (x_addr, x_base, mem_addr, mem_base,
 			  GET_MODE (x), GET_MODE (mem_addr)))
     return false;
-- 
2.35.3

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-03-04  8:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20240209102637.6BB1D3858438@sourceware.org>
2024-02-10 21:20 ` [PATCH] rtl-optimization/113597 - recover base term for argument pointers Toon Moene
2024-02-12  8:00   ` Richard Biener
2024-02-12 15:26     ` Christophe Lyon
     [not found] <65c5fdce.5d0a0220.b4fa7.11a6SMTPIN_ADDED_MISSING@mx.google.com>
2024-03-03 23:58 ` Jeff Law
2024-03-04  8:05   ` Richard Biener
2024-02-09 10:26 Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).