On 11/10/20 5:41 AM, Aldy Hernandez wrote:
[Andrew, this doesn't fix a PR per se.  It's just something I stumbled
upon in my other -fstrict-enums fixes.  It passes tests.  What do you
think?  Should we push it?]

Imagine an enum in an -fstrict-enums world:

        enum foo { zero, one, two, three };

Building the inverse of [0,3] currently yields VARYING (instead of
[4,+INF]).  This is because the setter sees 0 and 3 as the extremes of
the type, and per the current code, calculates the inverse to VARYING.
BTW, it really should be UNDEFINED, but this is legacy code I'm afraid
to touch:

Didnt we conclude at one point that neither VARYING nor UNDEFINED can be inverted?  There was too much ambiguity about what it meant, so callers need to check for undefined and varying before they can use invert.  This makes sure they get their expected results.

in fact, I see this in invert():

gcc_assert (!undefined_p () && !varying_p ());


but it comes after the legacy check..  so maybe there is some attempt to preserve legacy behaviour..  dunno.



       if (is_min && is_max)
        {
          /* We cannot deal with empty ranges, drop to varying.
             ???  This could be VR_UNDEFINED instead.  */
          set_varying (type);
          return;
        }

However, the extremes of a VARYING are the extremes of the underlying
type ([0, 0xff..ff]), even in the presence of -fstrict-enums.

This fixes the setter to use the extremes of the underlying type.

gcc/ChangeLog:

        * value-range.cc (irange::set): Use wi::{min,max}_value
        instead of TYPE_{MIN,MAX}_VALUE when building anti-ranges.
        (range_tests_strict_enum): New tests.
---
  gcc/value-range.cc | 27 +++++++++++++++++++++------
  1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index f83a824a982..bbd7d69a766 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -274,11 +274,13 @@ irange::set (tree min, tree max, value_range_kind kind)
    // Anti-ranges that can be represented as ranges should be so.
    if (kind == VR_ANTI_RANGE)
      {
-      /* For -fstrict-enums we may receive out-of-range ranges so consider
-         values < -INF and values > INF as -INF/INF as well.  */
-      bool is_min = vrp_val_is_min (min);
-      bool is_max = vrp_val_is_max (max);
        tree type = TREE_TYPE (min);
+      unsigned prec = TYPE_PRECISION (type);
+      signop sign = TYPE_SIGN (type);
+      wide_int type_max = wi::max_value (prec, sign);
+      wide_int type_min = wi::min_value (prec, sign);
+      bool is_min = wi::to_wide (min) == type_min;
+      bool is_max = wi::to_wide (max) == type_max;
if (is_min && is_max)
        {
@@ -302,14 +304,14 @@ irange::set (tree min, tree max, value_range_kind kind)
          {
          tree one = build_int_cst (TREE_TYPE (max), 1);
          min = int_const_binop (PLUS_EXPR, max, one);
-         max = vrp_val_max (TREE_TYPE (max));
+         max = wide_int_to_tree (type, type_max);
          kind = VR_RANGE;
          }
        else if (is_max)
          {
          tree one = build_int_cst (TREE_TYPE (min), 1);
          max = int_const_binop (MINUS_EXPR, min, one);
-         min = vrp_val_min (TREE_TYPE (min));
+         min = wide_int_to_tree (type, type_min);
          kind = VR_RANGE;
          }
      }
@@ -2276,6 +2278,19 @@ range_tests_strict_enum ()
    ir1 = vr1;
    ASSERT_TRUE (ir1 == vr1);
    ASSERT_FALSE (ir1.varying_p ());
+
+  // Legacy inversion of [0, 3] should be [4, MAX].
+  vr1 = int_range<1> (build_int_cstu (rtype, 0), build_int_cstu (rtype, 3));
+  vr1.invert ();
+  tree type_max = wide_int_to_tree (rtype,
+                                   wi::max_value (TYPE_PRECISION (rtype),
+                                                  TYPE_SIGN (rtype)));
+  ASSERT_TRUE (vr1 == int_range<1> (build_int_cstu (rtype, 4), type_max));
+
+  // Multi-range inversion of [0, 3] should be [4, MAX].
+  ir1 = int_range<2> (build_int_cstu (rtype, 0), build_int_cstu (rtype, 3));
+  ir1.invert ();
+  ASSERT_TRUE (vr1 == int_range<2> (build_int_cstu (rtype, 4), type_max));
  }
static void

Reply via email to