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.
Thanks,
Richard.
>
> jeff
