On 7/24/19 12:33 PM, Richard Biener wrote: > On July 24, 2019 8:18:57 PM GMT+02:00, Jeff Law <l...@redhat.com> > wrote: >> On 7/24/19 11:00 AM, Richard Biener wrote: [ Big snip, ignore >> missing reply attributions... ] >> >>>> it. But I'd claim that if callers are required not to change >>>> these ranges, then the callers are fundamentally broken. I'm >>>> not sure what the "sanitization" is really buying you here. >>>> Can you point to something specific? >>>> >>>>> >>>>> But you lose the sanitizing that nobody can change it and the >>>>> changed info leaks to other SSA vars. >>>>> >>>>> As said, fix all callers to deal with NULL. >>>>> >>>>> But I argue the current code is exactly optimal and safe. >>>> ANd I'd argue that it's just plain broken and that the >>>> sanitization you're referring to points to something broken >>>> elsewhere, higher up in the callers. >>> >>> Another option is to make get_value_range return by value and >>> the only way to change the lattice to call an appropriate set >>> function. I think we already do the latter in all cases (but we >>> use get_value_range in the setter) and returning by reference is >>> just eliding the copy. >> OK, so what I think you're getting at (and please correct me if >> I'm wrong) is that once the lattice values are set, you don't want >> something changing the recorded ranges underneath? >> >> ISTM the way to enforce that is to embed the concept in the class >> and enforce it by not allowing direct manipulation of range by the >> clients. So a client that wants this behavior somehow tells the >> class that ranges are "set in stone" and from that point the >> setters don't allow changing the underlying ranges. > > Yes. You'll see that nearly all callers do > > Value_range vr = *get_value_range (name); > > Modify > > Update_value_range (name, &vr) ; > > And returning by reference was mostly an optimization. We _did_ have > callers Changing the range in place and the const varying catched > those. > > When returning by value we can return individual VARYINGs not in the > lattice if we decide that's what we want. > >> I just want to make sure we're on the same page WRT why you think >> the constant varying range object is useful. > > As said it's an optimization. We do not want to reallocate the > lattice. And we want lattice updating to happen in a controlled > manner, so returning a pointer into the lattice is bad design at this > point. But I would claim that the current state is dreadful. Consider that when gimple-fold asks for a new SSA_NAME, it could get a recycled one, in which case we get a real range. Or if it doens't get a recycled name, then we get the const varying node. The inconsistently is silly when we can just reallocate the underlying object.
Between recycling of SSA_NAMEs and allocating a bit of additional space (say rounding up to some reasonable boundary) I'd bet you'd never be able to measure the reallocation in practice. jeff
