Re: [COMMITTED] Do not call range_on_path_entry for SSAs defined within the path

[Aldy Hernandez <aldyh@redhat.com>]

It's been fixed on trunk.

Aldy

On Fri, Oct 15, 2021, 09:52 Christophe LYON via Gcc-patches <
gcc-patches@gcc.gnu.org> wrote:

>
> On 14/10/2021 14:21, Aldy Hernandez via Gcc-patches wrote:
> > In the path solver, when requesting the range of an SSA for which we
> > know nothing, we ask the ranger for the range incoming to the path.
> > We do this by asking for all the incoming ranges to the path entry
> > block and unioning them.
> >
> > The problem here is that we're asking for a range on path entry for an
> > SSA which *is* defined in the path, but for which we know nothing
> > about:
> >
> >       some_global.1_2 = some_global;
> >       _3 = (char) some_global.1_2;
> >
> > This request is causing us to ask for range_on_edge of _3 on the
> > incoming edges to the path.  This is a bit of nonsensical request
> > because _3 isn't live on entry to the path, so ranger correctly
> > returns UNDEFINED.  The proper thing is to avoid asking this in the
> > first place.
> >
> > I have added a relevant assert, since it doesn't make sense to call
> > range_on_path_entry for SSAs defined within the path.
> >
> > Tested on x86-64 Linux.
> >
> >       PR 102736
> >
> > gcc/ChangeLog:
> >
> >       PR tree/optimization/102736
> >       * gimple-range-path.cc (path_range_query::range_on_path_entry):
> >       Assert that the requested range is defined outside the path.
> >       (path_range_query::ssa_range_in_phi): Do not call
> >       range_on_path_entry for SSA names that are defined within the
> >       path.
> >
> > gcc/testsuite/ChangeLog:
> >
> >       * gcc.dg/tree-ssa/pr102736.c: New test.
> > ---
> >   gcc/gimple-range-path.cc                 |  6 +++++-
> >   gcc/testsuite/gcc.dg/tree-ssa/pr102736.c | 21 +++++++++++++++++++++
> >   2 files changed, 26 insertions(+), 1 deletion(-)
> >   create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr102736.c
> >
> > diff --git a/gcc/gimple-range-path.cc b/gcc/gimple-range-path.cc
> > index 422abfddb8f..694271306a7 100644
> > --- a/gcc/gimple-range-path.cc
> > +++ b/gcc/gimple-range-path.cc
> > @@ -134,6 +134,7 @@ path_range_query::defined_outside_path (tree name)
> >   void
> >   path_range_query::range_on_path_entry (irange &r, tree name)
> >   {
> > +  gcc_checking_assert (defined_outside_path (name));
> >     int_range_max tmp;
> >     basic_block entry = entry_bb ();
> >     bool changed = false;
> > @@ -258,7 +259,10 @@ path_range_query::ssa_range_in_phi (irange &r, gphi
> *phi)
> >               // Using both the range on entry to the path, and the
> >               // range on this edge yields significantly better
> >               // results.
> > -             range_on_path_entry (r, arg);
> > +             if (defined_outside_path (arg))
> > +               range_on_path_entry (r, arg);
> > +             else
> > +               r.set_varying (TREE_TYPE (name));
> >               m_ranger.range_on_edge (tmp, e_in, arg);
> >               r.intersect (tmp);
> >               return;
> > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr102736.c
> b/gcc/testsuite/gcc.dg/tree-ssa/pr102736.c
> > new file mode 100644
> > index 00000000000..7e556f01a86
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr102736.c
> > @@ -0,0 +1,21 @@
> > +// { dg-do run }
> > +// { dg-options "-O1 -ftree-vrp" }
> > +
> > +int a, b = -1, c;
> > +int d = 1;
> > +static inline char e(char f, int g) { return g ? f : 0; }
> > +static inline char h(char f) { return f < a ? f : f < a; }
> > +static inline unsigned char i(unsigned char f, int g) { return g ? f :
> f > g; }
> > +void j() {
> > +L:
> > +  c = e(1, i(h(b), d));
> > +  if (b)
> > +    return;
> > +  goto L;
> > +}
> > +int main() {
> > +  j();
> > +  if (c != 1)
> > +    __builtin_abort ();
> > +  return 0;
> > +}
>
> Hi,
>
>
> The new test fails at execution on arm / aarch64, not sure if you are
> aware of that already?
>
>
> Thanks,
>
> Christophe
>
>
>