On 12/12/2017 01:39 AM, Richard Biener wrote: > > LGTM. I notice (existing code) that we use set_vr_value from push_value_range > vs. update_value_range from the regular code. Also for some unrelated missed > optimization I'd like to be able to set and unwind SSA range info via the > stack > as well. Both future things we might want to cleanup. I guess once the > SSA propagator VRP can go there's an opportunity to cleanup even more of the > code. Right. It's clearly an area ripe for a bit of cleanup. The avail_exprs and const/copies tables take the approach of recording everything in the unwind tables. The VRP bits try to be smarter and only use the unwinding stack when it's absolutely necessary. I'm torn as I can see pros and cons of both approaches. I'd certainly like to settle on just one for consistency's sake.
The API for update_value_range is painful -- it assumes the new range is a temporary range and that the equivs hung off it can be free'd. I'm not sure why it's done that way, but it introduces a certain amount of pain. So we have to be careful in how we use it. A part of me wants to classify the lowest level range structure (value_range) and introduce some invariants (ie, can they be shared or are they copied). But it feels like not enough gain right now. I absolutely agree that we'll want the push/pop scope primitives to be exposed. I've consistently seen value in that with the other tables, thus exposing those in the VRP bits seemed advisable. Jeff ps. Another report came in today that is almost certainly a DUP of 83298. I'll include its testcase in the commit.
