On 11/8/18 7: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.
Agreed. The existence of the bitmaps to me indicates these objects
aren't really good candidates for operator overloads.
>
>> 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.
Most likely yes based on my experiences last year with teasing bits of
this stuff apart.
jeff
