On Tue, 23 Oct 2012, Michael Matz wrote: > 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.
I suppose we can argue about this one. When using VARYING here this VARYING can leak out from unreachable paths in the CFG. > > > > 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. Well, consider 0 * VARYING. That's still [0, 0], not VARYING. Workers should compute the best approximation for a range of an operation, otherwise we cannot use them to build up operations by pieces (like we do for -b). > 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. Yes. Do you say it never may return a RANGE if either a or b is VARYING? The whole point is that we have both derive_new_range_from_operation and update_lattice_value. Only the latter knows how we iterate and that this iteration restricts the values we may update the lattice with. derive_new_range_from_operation is supposed to be generally useful. Richard. -- Richard Biener <rguent...@suse.de> SUSE / SUSE Labs SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 GF: Jeff Hawn, Jennifer Guild, Felix Imend
