On 11/07/2017 02:52 AM, Richard Biener wrote:
> On Mon, Nov 6, 2017 at 6:01 PM, Jeff Law <l...@redhat.com> wrote:
>>
>>
>> So I spent a fair amount of time over the weekend trying to figure out
>> how to stage in the vrp cleanups. I don't want to drop a massive
>> unreviewable kit on everyone. It's hard on the reviewers and its hard
>> on me too -- with stuff moving around it's hard to easily see that the
>> implementation isn't changing unexpectedly.
>>
>>
>>
>> What I've come up with to hopefully make this dramatically easier is to:
>>
>> 1. Go ahead with creating new .h files for the new classes, but keep the
>> implementations inside tree-vrp.c (for now).
>>
>> 2. Use some delegating member functions to minimize deltas during the
>> transition. So for example, the evrp class has a getter/setter
>> vr_values, that just delegates to the vr_values class. This means that
>> an evrp member function can still use get_value_range (for example).
>>
>>
>> #1 and #2 mean that we minimize the textual changes to bring in the new
>> class structure. Typically we're going to see free functions move into
>> a class hierarchy. So for example we currently have
>>
>> static value_range *
>> get_value_range (const_tree var)
>> { ... ]
>>
>> That would change to
>>
>> value_range
>> vr_values::get_value_range (const_tree var)
>>
>>
>> With virtually no changes to its body or callers. That's pretty easy to
>> review. When there are changes to an implementation they'll be a lot
>> easier to see. So we get the class structure installed and then proceed to:
>>
>>
>>
>> #3 Pull the member functions out of tree-vrp into their respective new
>> files. Right now I've got tree-evrp range-analyzer and vr-values for
>> the evrp optimization, generic range analysis and access to vr-values.
>> The names, of course, can certainly change.
>>
>> There'll be some free functions that will need to be shared. Those
>> routines are context free -- ie, they can be used anytime as they don't
>> access any class or global data. Everything else will be accessed
>> through the class instances.
>>
>> #4 Consider removal of the delegators. So for a call within the evrp
>> bits to get_value_range would change from
>>
>> value_range *vr = get_value_range (op);
>>
>> to
>>
>> value_range *vr = vr_values.get_value_range (op);
>>
>>
>> We're still calling the same function in both instances. The first just
>> has to go through the delegator. The second pulls the vr_values
>> instance out of the evrp class and calls the function directly.
>>
>>
>> Thoughts?
>
> Sounds like a good plan overall.
And I forgot #5. Enable in DOM and remove threading code in tree-vrp.c
and associated cleanups :-) and #6 enable in sprintf warning pass.
jeff
