Hi, On Tue, 23 Oct 2012, Richard Biener wrote:
> > > > ... for this. We should never "produce" UNDEFINED when the input > > > > wasn't > > > > UNDEFINED already. > > > > > > Why? > > > > Because doing so _always_ means an invalid lattice transition. UNDEFINED > > is TOP, anything not UNDEFINED is not TOP. So going from something to > > UNDEFINED is always going upward the lattice and hence in the wrong > > direction. > > Um, what do you mean with "input" then? Certainly intersecting > [2, 4] and [6, 8] yields UNDEFINED. Huh? It should yield VARYING, i.e. BOTTOM, not UNDEFINED, aka TOP. That's the whole point I'm trying to make. You're fixing up this very bug. > > > We shouldn't update the lattice this way, yes, but that is what > > > the patch ensures. > > > > An assert ensures. A work around works around a problem. I say that the > > problem is in those routines that produced the "new" UNDEFINED range in > > the first place, and it's not update_value_range's job to fix that after > > the fact. > > It is. See how CCPs set_lattice_vlaue adjusts the input as well. It actually does what I say update_value_range should do. It _asserts_ a valid transition, and the fixup before is correctly marked with a ??? comment about the improperty of the place of such fixing up. > It's just not convenient to repeat the adjustments everywhere. Sure. If the workers were correct there would be no need to do any adjustments. > > > The workers only compute a new value-range > > > for a stmt based on input value ranges. > > > > And if they produce UNDEFINED when the input wasn't so, then _that's_ > > where the bug is. > > See above. Hmm? See above. > > I'm not sure I understand. You claim that the workers have to produce > > UNDEFINED from non-UNDEFINED in some cases, otherwise we oscillate? > > That sounds strange. Or do you mean that we oscillate without your > > patch to update_value_range? That I believe, it's the natural result > > of going a lattice the wrong way, but I say that update_value_range is > > not the place to silently fix invalid transitions. > > No, I mean that going up the lattice results in oscillation. And that's why the workers are not supposed to do that. They can't willy-nilly create new UNDEFINED/TOP lattice values. Somehow we have a disconnect in this discussion over some very basic property, let's try to clean this up first. So, one question, are you claiming that a VRP worker like this: VR derive_new_range_from_operation (VR a, VR b) is _ever_ allowed to return UNDEFINED when a or b is something else than UNDEFINED? You seem to claim so AFAIU, but at the same time admit that this results in oscillation, and hence needs fixing up in the routine that uses the above result to install the lattice value in the map. I'm obviously saying that the above worker is not allowed to return UNDEFINED. Ciao, Michael.
