On 5/29/19 7:24 AM, Richard Biener wrote:
> On Wed, May 29, 2019 at 2:18 PM Aldy Hernandez <al...@redhat.com> wrote:
>>
>> As per the API, and the original documentation to value_range,
>> VR_UNDEFINED and VR_VARYING should never have equivalences. However,
>> equiv_add is tacking on equivalences blindly, and there are various
>> regressions that happen if I fix this oversight.
>>
>> void
>> value_range::equiv_add (const_tree var,
>> const value_range *var_vr,
>> bitmap_obstack *obstack)
>> {
>> if (!m_equiv)
>> m_equiv = BITMAP_ALLOC (obstack);
>> unsigned ver = SSA_NAME_VERSION (var);
>> bitmap_set_bit (m_equiv, ver);
>> if (var_vr && var_vr->m_equiv)
>> bitmap_ior_into (m_equiv, var_vr->m_equiv);
>> }
>>
>> Is this a bug in the documentation / API, or is equiv_add incorrect and
>> we should fix the fall-out elsewhere?
>
> I think this must have been crept in during the classification. If you
> go back to say GCC 7 you shouldn't see value-ranges with
> UNDEFINED/VARYING state in the lattice that have equivalences.
>
> It may not be easy to avoid with the new classy interface but we're
> certainly not tacking on them "blindly". At least we're not supposed
> to. As usual the intermediate state might be "broken" but
> intermediateness is not sth the new class "likes".
I don't remember changing anything (behavior-wise) in this space. If I
did it certainly wasn't intentional.
Given the code in gcc-7 looks like this:
> static void
> add_equivalence (bitmap *equiv, const_tree var)
> {
> unsigned ver = SSA_NAME_VERSION (var);
> value_range *vr = get_value_range (var);
>
> if (*equiv == NULL)
> *equiv = BITMAP_ALLOC (&vrp_equiv_obstack);
> bitmap_set_bit (*equiv, ver);
> if (vr && vr->equiv)
> bitmap_ior_into (*equiv, vr->equiv);
> }
I suspect we need to look up the call chain.
Jeff
