On 10/2/19 6:56 AM, Aldy Hernandez wrote:
>>> diff --git a/gcc/range.cc b/gcc/range.cc
>>> new file mode 100644
>>> index 00000000000..5e4d90436f2
>>> --- /dev/null
>>> +++ b/gcc/range.cc
>> [ ... ]
>>> +
>>> +value_range_base
>>> +range_intersect (const value_range_base &r1, const value_range_base
>>> &r2)
>>> +{
>>> + value_range_base tmp (r1);
>>> + tmp.intersect (r2);
>>> + return tmp;
>>> +}
>> So low level question here. This code looks particularly well suited
>> for the NRV optimization. Can you check if NVR (named-value-return) is
>> triggering here, and if not why. ISTM these are likely used heavily and
>> NVR would be a win.
>
> AFAICT, even before the NRV pass, we have already lined up
> value_range_base::intersect to put its return value into RETVAL:
>
> const struct value_range_base & r1_2(D) = r1;
> const struct value_range_base & r2_4(D) = r2;
> <bb 2> [local count: 1073741824]:
> <retval> = *r1_2(D);
> value_range_base::intersect (&<retval>, r2_4(D));
> return <retval>;
>
> So...all good?
Yup. We construct into <retval> and return it.
>
>>
>>
>>> diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
>>> index 5ec4d17f23b..269a3cb090e 100644
>>> --- a/gcc/tree-vrp.c
>>> +++ b/gcc/tree-vrp.c
>> [ ... ]
>>
>>> @@ -67,7 +67,7 @@ along with GCC; see the file COPYING3. If not see
>>
>>> +
>>> +/* Return the inverse of a range. */
>>> +
>>> +void
>>> +value_range_base::invert ()
>>> +{
>>> + if (undefined_p ())
>>> + return;
>>> + if (varying_p ())
>>> + set_undefined ();
>>> + else if (m_kind == VR_RANGE)
>>> + m_kind = VR_ANTI_RANGE;
>>> + else if (m_kind == VR_ANTI_RANGE)
>>> + m_kind = VR_RANGE;
>>> + else
>>> + gcc_unreachable ();
>>> +}
>> I don't think this is right for varying_p. ISTM that if something is
>> VR_VARYING that inverting it is still VR_VARYING. VR_UNDEFINED seems
>> particularly bad given its optimistic treatment elsewhere.
>
> Done. Andrew agreed, as per his reply.
>
> Retested for all languages on x86-64 Linux.
>
> OK?
So the final version has asserts to ensure we don't get into the invert
method with varying/undefined. That works for me.
OK for the trunk.
Onward!
jeff
