On 10/2/19 6:52 AM, Richard Biener wrote:
+
+/* Return the inverse of a range. */
+
+void
+value_range_base::invert ()
+{
+ if (undefined_p ())
+ return;
+ if (varying_p ())
+ set_undefined ();
+ else if (m_kind == VR_RANGE)
+ m_kind = VR_ANTI_RANGE;
+ else if (m_kind == VR_ANTI_RANGE)
+ m_kind = VR_RANGE;
+ else
+ gcc_unreachable ();
+}
I don't think this is right for varying_p. ISTM that if something is
VR_VARYING that inverting it is still VR_VARYING. VR_UNDEFINED seems
particularly bad given its optimistic treatment elsewhere.
VR_VARYING isn't a range, it's a lattice state (likewise for VR_UNDEFINED).
It doesn't make sense to invert a lattice state. How you treat
VR_VARYING/VR_UNDEFINED
depends on context and so depends what 'invert' would do. I suggest to assert
that varying/undefined is never inverted.
True for a lattice state, not true for a range in the new context of the
ranger where
a) varying == range for type and
b) undefined == unreachable
This is a carry over from really old code where we only got part of it
fixed right a while ago.
invert ( varying ) == varying because we still know nothing about it,
its still range for type.
invert (undefined) == undefined because undefined is unreachable
which is viral.
So indeed, varying should return varying... So If its undefined or
varying, we should just return from the invert call. ie, not do anything
to the range. in the case of a lattice state, doing nothing to it
should not be harmful. I also expect it will never get called for a
pure lattice state since its only invoked from range-ops, at which point
we only are dealing with the range it represents.
I took a look and this bug hasn't been triggered because its only used
in a couple of places.
1) op1_range for EQ_EXPR and NE_EXPR when we have a true OR false
constant condition in both the LHS and OP2 position, it sometimes
inverts it via this call.. so its only when there is a specific boolean
range of [TRUE,TRUE] or [FALSE,FALSE]. when any range is undefined
or varying in those routines, theres a different path for the result
2) fold() for logical NOT, which also has a preliminary check for
varying or undefined and does nothing in those cases ie, returns the
existing value. IN fact, you can probably remove the special casing in
logical_not with this fix, which is indicative that it is correct.
Andrew
