Re: Problems in IPA passes

[Jeff Law <law at redhat dot com>]

On 10/31/2017 04:36 AM, Richard Biener wrote:
> On October 31, 2017 10:49:51 AM GMT+01:00, Kugan Vivekanandarajah
> <kugan.vivekanandarajah@linaro.org> wrote:
>> Hi Jeff,
>> 
>> On 31 October 2017 at 14:47, Jeff Law <law@redhat.com> wrote:
>>> On 10/29/2017 03:54 PM, Kugan Vivekanandarajah wrote:
>>>> Hi Jeff,
>>>> 
>>>> On 28 October 2017 at 18:28, Jeff Law <law@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