Problems in IPA passes

[Richard Biener]

On October 31, 2017 4:49:20 PM GMT+01:00, Jeff Law <law@redhat.com> wrote:
>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.

Maybe that's not necessary in most places given existing bitmaps know their obstack? IIRC most places do sth to equivalences only if a range already contains some. 

Richard. 

>
>Jeff