On Thu, Nov 8, 2018 at 3:54 PM Aldy Hernandez <al...@redhat.com> wrote:
>
>
>
> On 11/8/18 9:49 AM, Richard Biener wrote:
> > On Thu, Nov 8, 2018 at 3:31 PM Aldy Hernandez <al...@redhat.com> wrote:
> >>
> >>
> >>
> >> On 11/8/18 9:24 AM, Richard Biener wrote:
> >>> On Thu, Nov 8, 2018 at 1:17 PM Aldy Hernandez <al...@redhat.com> wrote:
> >>>>
> >>>> This one's rather obvious and does not depend on any get_range_info API
> >>>> change.
> >>>>
> >>>> OK for trunk?
> >>>
> >>> Hmm, no - that's broken. IIRC m_equiv are shared bitmaps if you
> >>> do tem = *old_vr so you modify it in place with equiv_clear().
> >>
> >> Good point.
> >>
> >>>
> >>> Thus, operator= should be really deleted or mapped to value_range::set()
> >>> in which case tem = *old_vr would do useless bitmap allocation and
> >>> copying that you then clear.
> >>
> >> Actually, I was thinking that perhaps the assignment and equality
> >> operators should disregard the equivalence bitmap. In this case, copy
> >> everything EXCEPT the bitmap and set the resulting equivalence bitmap to
> >> NULL.
> >
> > I think that's equally confusing.
>
> I don't think so. When you ask whether range A and range B are equal,
> you're almost always interested in the range itself, not the equivalence
> table behind it.
>
> We could also get rid of the == operator and just provide:
>
> bool equal_p(bool ignore_equivs);
>
> How does this sound?
Good.
> >
> >> It's also annoying to use ::ignore_equivs_equal_p(). Since that seems
> >> to be the predominant way of comparing ranges, perhaps it should be the
> >> default.
> >
> > I think a good approach would be to isolate m_equiv more because it is
> > really an implementation detail of the propagator. Thus, make
> >
> > class value_range_with_equiv : public value_range
> > {
> > ... all the equiv stuff..
> > }
> >
> > make the lattice of type value_range_with_equiv and see what tickles
> > down.
> >
> > value_range_with_equiv wouldn't implement copy and assignment
> > (too expensive) and value_range can do with the trivial implementation.
> >
> > And most consumers/workers can just work on the equiv-less variants.
>
> I like this. Unfortunately, not feasible for this cycle (for me
> anyhow-- patches welcome though :)). How about equal_p() as described
> above?
Works for me but you still need to sort out the copying, so if you think
splitting is not feasible (I'll give it a try) then please disable assingment
and copy operators and fixup code.
Richard.
>
> Aldy
