On Mon, 12 Feb 2024 at 09:00, Richard Biener <richard.guent...@gmail.com> wrote:
>
> On Sat, Feb 10, 2024 at 10:21 PM Toon Moene <t...@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.09c543858...@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: t...@moene.org - phone: +31 346 214290
> > Saturnushof 14, 3738 XG  Maartensdijk, The Netherlands
> >

Reply via email to