Re: [PR86153] simplify more overflow tests in VRP

[Alexandre Oliva <aoliva@redhat.com>]

On Dec 19, 2018, Alexandre Oliva <aoliva@redhat.com> wrote:

> +      op1 = x;
> +      code = (code == LT_EXPR || code == LE_EXPR) ? LE_EXPR : GT_EXPR;

So...  I've done some more testing, and this alone seems to overlap
nicely with what's in the trunk ATM, with a few exceptions:

- for some reason, LE/GT do not get us some simplifications that EQ/NE
  do when x is zero, despite the unsigned type for op0, that ought to
  make them equivalent.  That could probably be improved elsewhere, but
  it's easy enough to remedy with:

  if (integer_zerop (x))
    code = (code == LT_EXPR || code == LE_EXPR) ? EQ_EXPR : NE_EXPR;
  else
    code = (code == LT_EXPR || code == LE_EXPR) ? LE_EXPR : GT_EXPR;
   
- other differences I observed involved other case we currently
  transform to equality, namely, if x is (u)max-1, we test op1 against
  zero.  The transformation quoted above does better than this in at
  least 3 situations: a selftest in vec.c, cp_genericize_r in
  cp/cp-gimplify.c, and cp_maybe_mangle_decomp in cp/decl.c.  In these
  cases, op0 has a known range from 0 to INT_MAX or INT_MAX-1, so a
  compare of op0 vs 2u*INT_MAX can be folded to a constant, but the
  compare of op1 vs 0 we currently use cannot.

- the fact that in some cases a test against one of the overflow-related
  variables can be optimized when a test against the other can't
  surprised me; I would have expected the ranges and equivalences and
  whatnot to be such that this would never happen, but it does.  It
  suggests we could get some additional folding out of trying op1 vs
  max-x when op0 vs x fails to resolve to a constant.


FTR, here's the patchlet (-b) I've used to look for differences.


commit 2fe0b2784815882c3e2821b171979b54c3ffdc55
Author: Alexandre Oliva <aoliva@redhat.com>
Date:   Sat Dec 29 00:21:42 2018 -0200

    [PR86153/83239] identify vrp overflow simplification differences to investigate

diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index d71a703ab550..a40c41e4d139 100644
--- a/gcc/vr-values.c
+++ b/gcc/vr-values.c
@@ -2296,7 +2296,6 @@ vr_values::vrp_evaluate_conditional_warnv_with_ops (enum tree_code code,
 						    bool *strict_overflow_p,
 						    bool *only_ranges)
 {
-  tree ret;
   if (only_ranges)
     *only_ranges = true;
 
@@ -2316,6 +2315,40 @@ vr_values::vrp_evaluate_conditional_warnv_with_ops (enum tree_code code,
   tree x;
   if (overflow_comparison_p (code, op0, op1, use_equiv_p, &x))
     {
+      tree ret1 = ovrflow_vrp_evaluate_conditional_warnv_with_ops (code, op0, op1, x,
+								   use_equiv_p,
+								   strict_overflow_p,
+								   only_ranges);
+
+      op1 = x;
+      if (integer_zerop (x))
+	code = (code == LT_EXPR || code == LE_EXPR) ? EQ_EXPR : NE_EXPR;
+      else
+	code = (code == LT_EXPR || code == LE_EXPR) ? LE_EXPR : GT_EXPR;
+
+      tree ret2 = rest_of_vrp_evaluate_conditional_warnv_with_ops (code, op0, op1,
+								   use_equiv_p,
+								   strict_overflow_p,
+								   only_ranges);
+      gcc_assert (ret1 == ret2
+		  || (ret1 && ret2 && operand_equal_p (ret1, ret2, 0)));
+
+      return ret2;
+    }
+  else
+    return rest_of_vrp_evaluate_conditional_warnv_with_ops (code, op0, op1,
+							    use_equiv_p,
+							    strict_overflow_p,
+							    only_ranges);
+}
+
+tree vr_values::
+ovrflow_vrp_evaluate_conditional_warnv_with_ops (enum tree_code code,
+						 tree op0, tree op1, tree x,
+						 bool use_equiv_p,
+						 bool *strict_overflow_p,
+						 bool *only_ranges)
+{
   wide_int max = wi::max_value (TYPE_PRECISION (TREE_TYPE (op0)), UNSIGNED);
   /* B = A - 1; if (A < B) -> B = A - 1; if (A == 0)
      B = A - 1; if (A > B) -> B = A - 1; if (A != 0)
@@ -2369,7 +2402,21 @@ vr_values::vrp_evaluate_conditional_warnv_with_ops (enum tree_code code,
       if (vri.undefined_p ())
 	return boolean_true_node;
     }
-    }
+
+  return rest_of_vrp_evaluate_conditional_warnv_with_ops (code, op0, op1,
+							  use_equiv_p,
+							  strict_overflow_p,
+							  only_ranges);
+}
+
+tree vr_values::
+rest_of_vrp_evaluate_conditional_warnv_with_ops (enum tree_code code,
+						 tree op0, tree op1,
+						 bool use_equiv_p,
+						 bool *strict_overflow_p,
+						 bool *only_ranges)
+{
+  tree ret;
 
   if ((ret = vrp_evaluate_conditional_warnv_with_ops_using_ranges
 	       (code, op0, op1, strict_overflow_p)))
diff --git a/gcc/vr-values.h b/gcc/vr-values.h
index 6785cb68fa76..e30719e82599 100644
--- a/gcc/vr-values.h
+++ b/gcc/vr-values.h
@@ -85,6 +85,17 @@ class vr_values
   tree vrp_evaluate_conditional_warnv_with_ops (enum tree_code,
 						tree, tree, bool,
 						bool *, bool *);
+  tree rest_of_vrp_evaluate_conditional_warnv_with_ops (enum tree_code code,
+							tree op0, tree op1,
+							bool use_equiv_p,
+							bool *strict_overflow_p,
+							bool *only_ranges);
+  tree ovrflow_vrp_evaluate_conditional_warnv_with_ops (enum tree_code code,
+							tree op0, tree op1, tree x,
+							bool use_equiv_p,
+							bool *strict_overflow_p,
+							bool *only_ranges);
+
   void extract_range_from_assignment (value_range *, gassign *);
   void extract_range_from_assert (value_range *, tree);
   void extract_range_from_ssa_name (value_range *, tree);


-- 
Alexandre Oliva, freedom fighter   https://FSFLA.org/blogs/lxo
Be the change, be Free!         FSF Latin America board member
GNU Toolchain Engineer                Free Software Evangelist
Hay que enGNUrecerse, pero sin perder la terGNUra jamás-GNUChe