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: record_ranges_from_incoming_edge: use value_range API for creating new range


On Thu, Nov 8, 2018 at 3:54 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>
>
>
> On 11/8/18 9:49 AM, Richard Biener wrote:
> > On Thu, Nov 8, 2018 at 3:31 PM Aldy Hernandez <aldyh@redhat.com> wrote:
> >>
> >>
> >>
> >> On 11/8/18 9:24 AM, Richard Biener wrote:
> >>> On Thu, Nov 8, 2018 at 1:17 PM Aldy Hernandez <aldyh@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


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