On Wed, Aug 22, 2018 at 11:31 AM Aldy Hernandez <al...@redhat.com> wrote:
>
>
>
> On 08/21/2018 05:46 AM, Richard Biener wrote:
> > On Wed, Aug 15, 2018 at 3:33 AM Aldy Hernandez <al...@redhat.com> wrote:
>
> >> Finally, my apologies for including a tiny change to the
> >> POINTER_PLUS_EXPR handling code as well. It came about the same set of
> >> auditing tests.
> >
> > Bah, please split up things here ;) I've done a related change there
> > yesterday...
> >
> >>
> >> It turns out we can handle POINTER_PLUS_EXPR(~[0,0], [X,Y]) without
> >> bailing as VR_VARYING in extract_range_from_binary_expr_1. In doing so,
> >> I also noticed that ~[0,0] is not the only non-null. We could also have
> >> ~[0,2] and still know that the pointer is not zero. I have adjusted
> >> range_is_nonnull accordingly.
> >
> > But there are other consumers and it would have been better to
> > change range_includes_zero_p to do the natural thing (get a VR) and
> > then remove range_is_nonnull as redundant if possible.
>
> Indeed. Cleaning up range_includes_zero_p makes VRP and friends a lot
> cleaner. Thanks for the suggestion.
>
> I lazily avoided cleaning up the division code affected in this patch
> too much, since it's going to be superseded by my division changes in
> the other patch.
>
> OK pending tests?
-/* Return 1 if [MIN, MAX] includes the value zero, 0 if it does not
- include the value zero, -2 if we cannot tell. */
+/* Return 1 if *VR includes the value zero, 0 if it does not include
+ the value zero, or -2 if we cannot tell. */
int
-range_includes_zero_p (tree min, tree max)
+range_includes_zero_p (const value_range *vr)
{
- tree zero = build_int_cst (TREE_TYPE (min), 0);
- return value_inside_range (zero, min, max);
+ if (vr->type == VR_UNDEFINED || vr->type == VR_VARYING)
+ return -2;
+
+ tree zero = build_int_cst (TREE_TYPE (vr->min), 0);
+ if (vr->type == VR_RANGE)
+ return value_inside_range (zero, vr->min, vr->max);
+ else
+ return !value_inside_range (zero, vr->min, vr->max);
}
please make it return a bool. VR_VARYING means the range does
include zero. For VR_UNDEFINED we could say it doesn't or we choose
to not possibly optimize and thus return true (just do that for now).
That's because VR_VARYING is [-INF, INF] and VR_UNDEFINED is
an empty range.
I suppose the -2 for we cannot tell was for the case of symbolic
ranges where again we can conservatively return true
(and your return !value_inside_range for VR_ANTI_RANGE botched
the tri-state return value anyways...).
So, can you please rework that?
Thanks,
Richard.
> Aldy
