On 10/31/2017 04:36 AM, Richard Biener wrote:
> On October 31, 2017 10:49:51 AM GMT+01:00, Kugan Vivekanandarajah
> <kugan.vivekanandara...@linaro.org> wrote:
>> Hi Jeff,
>>
>> On 31 October 2017 at 14:47, Jeff Law <l...@redhat.com> wrote:
>>> On 10/29/2017 03:54 PM, Kugan Vivekanandarajah wrote:
>>>> Hi Jeff,
>>>>
>>>> On 28 October 2017 at 18:28, Jeff Law <l...@redhat.com> wrote:
>>>>>
>>>>> Jan,
>>>>>
>>>>> What's the purpose behind calling vrp_meet and
>>>>> extract_range_from_unary_expr from within the IPA passes?
>>>>
>>>> This is used such that when we have an argument to a function
>>>> and
>> this
>>>> for which we know the VR and this intern is passed as a
>>>> parameter to another. For example:
>>>>
>>>> void foo (int i) { ... bar (unary_op (i)) ... }
>>>>
>>>> This is mainly to share what is done in tree-vrp.
>>> Presumably you never have equivalences or anything like that,
>>> which probably helps with not touching vrp_bitmap_obstack which
>>> isn't initialized when you run the IPA bits.
>>>
>>>>>
>>>>> AFAICT that is not safe to do. Various paths through those
>> routines
>>>>> will access static objects within tree-vrp.c which may not
>>>>> be initialized when IPA runs (vrp_equiv_obstack, vr_value).
>>>>
>>>> IPA-VRP does not track equivalence and vr_value is not used.
>>> But there's no enforcement and I'd be hard pressed to believe
>>> that
>> all
>>> the paths through the routines you use in tree-vrp aren't going
>>> to
>> touch
>>> vr_value, or vrp_bitmap_obstack. vrp_bitmap_obstack turns out to
>>> be incredibly tangled into the implementations within tree-vrp.c
>>> :(
>>>
>>
>> I looked into the usage and it does seem to be not using vr_value
>> unless I am missing something. There are two overloaded functions
>> here:
>>
>> extract_range_from_unary_expr (value_range *vr, enum tree_code
>> code, tree type, value_range *vr0_, tree op0_type) is safe as this
>> works with value_range and does not use get_value_range to access
>> vr_value.
>>
>> extract_range_from_unary_expr (value_range *vr, enum tree_code
>> code, tree type, tree op0) This is not safe as this takes tree as
>> an argument and gets value_range by calling get_value_range. May be
>> we should change the names to reflect this.
>
> At some point I wanted to isolate those functions not relying on
> global state but I never came to do this. Those are the building
> blocks alternative range propagation stuff like Andrew's could use.
So in my tree it's no longer global state :-)
> And yes, equivalences are ugly... Ideally we'd have a base
> value-range class without them used by IPA and VRP (non-early) would
> use a derived class.
I'll look into that -- it's a fairly large wart in terms of global
access in vrp.
In my local tree I'm just passing around the vrp_bitmap_obstack right
now. Nobody's accessing it via a global anymore. So at least we know
what routines directly or indirectly want to touch vrp_bitmap_obstack.
Jeff
