On Wed, Jun 2, 2021 at 3:28 PM Richard Biener via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On Tue, Jun 1, 2021 at 4:00 PM bin.cheng via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > Hi,
> > As described in patch summary, this fixes the wrong code issue by adding overflow-ness
> > check for iv1.step - iv2.step.
> >
> > Bootstrap and test on x86_64.  Any comments?
>
> +         bool wrap_p = TYPE_OVERFLOW_WRAPS (step_type);
> +         if (wrap_p)
> +           {
> +             tree t = fold_binary_to_constant (GE_EXPR, step_type,
> +                                               iv0->step, iv1->step);
> +             wrap_p = integer_zerop (t);
> +           }
>
> I think we can't use TYPE_OVERFLOW_WRAPS/TYPE_OVERFLOW_UNDEFINED since
> that's only relevant for expressions written by the user - we're
> computing iv0.step - iv1.step
> which can even overflow when TYPE_OVERFLOW_UNDEFINED (in fact we may not
> even generate this expression then!).  So I think we have to do sth like
>
>    /* If the iv0->step - iv1->step wraps, fail.  */
>    if (!operand_equal_p (iv0->step, iv1->step)
>        && (TREE_CODE (iv0->step) != INTEGER_CST || TREE_CODE
> (iv1->step) != INTEGER_CST)
>        && !wi::gt (wi::to_widest (iv0->step), wi::to_widest (iv1->step))
>      return false;
>
> which only handles equality and all integer constant steps. You could
Thanks for the suggestion.  I realized that we have LE/LT/NE
conditions here, and for LE/LT what we need to check is iv0/iv1
converge to each other, rather than diverge.  Also steps here can only
be constants, so there is no need to use range information.

> also use ranges
> like
>
>  wide_int min0, max0, min1, max1;
>   if (!operand_equal_p (iv->step, iv1->step)
>       && (determine_value_range (iv0->step, &min0, &max0) != VR_RANGE
>              || determine_value_range (iv1->step, &min1, &max1) != VR_RANGE
>              || !wi::ge (min0, max1)))
>    return false;
>
> Note I'm not sure why
>
>        iv0->step = step;
>        if (!POINTER_TYPE_P (type))
>         iv0->no_overflow = false;
I don't exactly remember, this was added sometime when no_overflow was
introduced.  Note we only do various checks for non NE_EXPR so the
step isn't always less in absolute value?  I will check if we should
reset it in all cases.

Patch updated.  test ongoing.

Thanks,
bin
>
> here the no_overflow reset does not happen for pointer types?  Or
> rather why does
> it happen at all?  Don't we strictly make the step less in absolute value?
>
> > Thanks,
> > bin