This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: VRP: make range_includes_zero_p handle value_ranges


On Wed, Aug 22, 2018 at 11:31 AM Aldy Hernandez <aldyh@redhat.com> wrote:
>
>
>
> On 08/21/2018 05:46 AM, Richard Biener wrote:
> > On Wed, Aug 15, 2018 at 3:33 AM Aldy Hernandez <aldyh@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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]