On Thu, Nov 8, 2018 at 3:50 PM Aldy Hernandez <al...@redhat.com> wrote:
>
>
>
> On 11/8/18 9:43 AM, Richard Biener wrote:
> > On Thu, Nov 8, 2018 at 3:27 PM Aldy Hernandez <al...@redhat.com> wrote:
> >>
> >>
> >>
> >> On 11/8/18 9:21 AM, Richard Biener wrote:
> >>> On Thu, Nov 8, 2018 at 1:09 PM Aldy Hernandez <al...@redhat.com> wrote:
> >>>>
> >>>> All this nonsense:
> >>>>
> >>>> - rtype = get_range_info (t, &min, &max);
> >>>> - if (rtype == VR_RANGE)
> >>>> - {
> >>>> - if (wi::lt_p (max, w, TYPE_SIGN (TREE_TYPE (t))))
> >>>> - return true;
> >>>> - if (wi::lt_p (w, min, TYPE_SIGN (TREE_TYPE (t))))
> >>>> - return true;
> >>>> - }
> >>>> - else if (rtype == VR_ANTI_RANGE
> >>>> - && wi::le_p (min, w, TYPE_SIGN (TREE_TYPE (t)))
> >>>> - && wi::le_p (w, max, TYPE_SIGN (TREE_TYPE (t))))
> >>>>
> >>>> Replaced by an API like Kutulu intended.
> >>>>
> >>>> + get_range_info (t, vr);
> >>>> + if (!vr.may_contain_p (wide_int_to_tree (TREE_TYPE (t), w)))
> >>>>
> >>>> Ain't it grand?
> >>>
> >>> Well. The not-so-grand thing is that you possibly ggc-allocate
> >>> three INTEGER_CST nodes here.
> >>
> >> Hmmm... I'd really prefer to use a simple API call, instead of having to
> >> twiddle with the extremes manually. Ideally no one should be looking
> >> inside of a value_range.
> >>
> >> Do recommend another way of implementing may_contain_p ?
> >
> > I think many places dealing with get_range_info () should instead
> > work on the (to be created...) 1:1 copy of value_range ontop of
> > wide-int-range instead.
>
> I'd prefer to not expose that we're going to use wide_int or any other
> implementation to the users of get_range_info().
But it's exposed at the moment. And I don't see it change. And you
should not allocate memory for no good reason.
> >
> > So - can you add a wide_int_range class to wide-int-range.h
> > that implements the same (but with wide-ints rather than trees
> > obviously) API as value-range?
>
> Hmmm, I don't have time for this release cycle. Perhaps something to be
> entertained for GCC+1?
>
> Again, I prefer my patch as is. I cleans up the code, and keeps us from
> introducing problematic bugs. Anything dealing with anti ranges is
> fraught with peril, as my cleanups to tree-vrp revealed.
>
> If using these INTEGER_CST's causes any measurable performance
> difference, I'd be happy to look into it.
Just leave the code unchanged then in this release?
Richard.
> Aldy
