On Mon, Sep 4, 2023 at 11:06 PM Jeff Law via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
>
>
> On 9/1/23 11:30, Andrew Pinski via Gcc-patches wrote:
> > So it turns out there was a simplier way of starting to
> > improve VRP to start to fix PR 110131, PR 108360, and PR 108397.
> > That was rewrite test_for_singularity to use range_op_handler
> > and Value_Range.
> >
> > This patch implements that and
> >
> > OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
> >
> > gcc/ChangeLog:
> >
> > * vr-values.cc (test_for_singularity): Add edge argument
> > and rewrite using range_op_handler.
> > (simplify_compare_using_range_pairs): Use Value_Range
> > instead of value_range and update test_for_singularity call.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * gcc.dg/tree-ssa/vrp124.c: New test.
> > * gcc.dg/tree-ssa/vrp125.c: New test.
> > ---
>
> > diff --git a/gcc/vr-values.cc b/gcc/vr-values.cc
> > index 52ab4fe6109..2474e57ee90 100644
> > --- a/gcc/vr-values.cc
> > +++ b/gcc/vr-values.cc
> > @@ -904,69 +904,33 @@ simplify_using_ranges::simplify_bit_ops_using_ranges
> > }
> >
> > /* We are comparing trees OP1 and OP2 using COND_CODE. OP1 has
> > - a known value range VR.
> > + a known value range OP1_RANGE.
> >
> > If there is one and only one value which will satisfy the
> > - conditional, then return that value. Else return NULL.
> > -
> > - If signed overflow must be undefined for the value to satisfy
> > - the conditional, then set *STRICT_OVERFLOW_P to true. */
> > + conditional on the EDGE, then return that value.
> > + Else return NULL. */
> >
> > static tree
> > test_for_singularity (enum tree_code cond_code, tree op1,
> > - tree op2, const value_range *vr)
> > + tree op2, const int_range_max &op1_range, bool edge)
> > {
> > - tree min = NULL;
> > - tree max = NULL;
> > -
> > - /* Extract minimum/maximum values which satisfy the conditional as it was
> > - written. */
> > - if (cond_code == LE_EXPR || cond_code == LT_EXPR)
> > + /* This is already a singularity. */
> > + if (cond_code == NE_EXPR || cond_code == EQ_EXPR)
> > + return NULL;
> I don't think this is necessarily the right thing to do for NE.
>
> Consider if op1 has the range [0,1] and op2 has the value 1. If the
> code is NE, then we should be able to return a singularity of 0 since
> that's the only value for x where x ne 1 is true given the range for x.
The "false" edge singularity is already known when NE is supplied. I
don't think changing it to the "true" edge singularity will be helpful
all of the time; preferring the value of 0 is a different story.
But that is a different patch and for a different location rather than
inside VRP; it should be in either isel or expand (more likely isel).
Thanks,
Andrew
>
>
>
> I like what you're trying to do, it just needs a bit of refinement I think.
>
> jeff
