diff mbox

[optimize,2/3] Simplify vrp abs conversion

Message ID 55C91C2D.7040407@acm.org
State New
Headers show

Commit Message

Nathan Sidwell Aug. 10, 2015, 9:48 p.m. UTC 
Richard,
in looking at how simplify_abs_using_ranges was doing its thing as a guide to a 
min/max vrp optimization, I noticed it was doing more work than necessary.

Firstly, it wasn't taking advantage of the range comparison functions only 
returning TRUE or FALSE nodes when there's a definite answer, and NULL 
otherwise.  Thus if we get a node, we don't have to (a) check if it's either 
true or false and (b) we only need to check for one of those values to determine 
which specific answer was given.

Also, it was checking for 'NOT (A >= B)' by inverting the result of a '>=' 
check, rather than simply doing a '<' check. (we're dealing with integer ranges, 
so that's all well defined)

Finally, there's a useless check for UNSIGNED_TYPE, which ends up doing nothing. 
  AFAICT 'ABS (unsigned)' gets folded out very early on.

booted and tested with the phi-min-max fix I just posted and the new VRP-min-max 
optimization I'm about to.

ok?

nathan

Comments

Richard Biener Aug. 11, 2015, 11:33 a.m. UTC | #1 
On Mon, 10 Aug 2015, Nathan Sidwell wrote:

> Richard,
> in looking at how simplify_abs_using_ranges was doing its thing as a guide to
> a min/max vrp optimization, I noticed it was doing more work than necessary.
> 
> Firstly, it wasn't taking advantage of the range comparison functions only
> returning TRUE or FALSE nodes when there's a definite answer, and NULL
> otherwise.  Thus if we get a node, we don't have to (a) check if it's either
> true or false and (b) we only need to check for one of those values to
> determine which specific answer was given.
> 
> Also, it was checking for 'NOT (A >= B)' by inverting the result of a '>='
> check, rather than simply doing a '<' check. (we're dealing with integer
> ranges, so that's all well defined)
> 
> Finally, there's a useless check for UNSIGNED_TYPE, which ends up doing
> nothing.  AFAICT 'ABS (unsigned)' gets folded out very early on.
> 
> booted and tested with the phi-min-max fix I just posted and the new
> VRP-min-max optimization I'm about to.
> 
> ok?

Ok.

Thanks,
Richard.
diff mbox

Patch

2015-08-10  Nathan Sidwell  <nathan@acm.org>

	* tree-vrp.c (simplify_abs_using_ranges): Simplify.

Index: tree-vrp.c
===================================================================
--- tree-vrp.c	(revision 226749)
+++ tree-vrp.c	(working copy)
@@ -9152,37 +9215,25 @@  simplify_div_or_mod_using_ranges (gimple
 static bool
 simplify_abs_using_ranges (gimple stmt)
 {
-  tree val = NULL;
   tree op = gimple_assign_rhs1 (stmt);
-  tree type = TREE_TYPE (op);
   value_range_t *vr = get_value_range (op);
 
-  if (TYPE_UNSIGNED (type))
-    {
-      val = integer_zero_node;
-    }
-  else if (vr)
+  if (vr)
     {
+      tree val = NULL;
       bool sop = false;
 
       val = compare_range_with_value (LE_EXPR, vr, integer_zero_node, &sop);
       if (!val)
 	{
+	  /* The range is neither <= 0 nor > 0.  Now see if it is
+	     either < 0 or >= 0.  */
 	  sop = false;
-	  val = compare_range_with_value (GE_EXPR, vr, integer_zero_node,
+	  val = compare_range_with_value (LT_EXPR, vr, integer_zero_node,
 					  &sop);
-
-	  if (val)
-	    {
-	      if (integer_zerop (val))
-		val = integer_one_node;
-	      else if (integer_onep (val))
-		val = integer_zero_node;
-	    }
 	}
 
-      if (val
-	  && (integer_onep (val) || integer_zerop (val)))
+      if (val)
 	{
 	  if (sop && issue_strict_overflow_warning (WARN_STRICT_OVERFLOW_MISC))
 	    {
@@ -9198,10 +9249,10 @@  simplify_abs_using_ranges (gimple stmt)
 	    }
 
 	  gimple_assign_set_rhs1 (stmt, op);
-	  if (integer_onep (val))
-	    gimple_assign_set_rhs_code (stmt, NEGATE_EXPR);
-	  else
+	  if (integer_zerop (val))
 	    gimple_assign_set_rhs_code (stmt, SSA_NAME);
+	  else
+	    gimple_assign_set_rhs_code (stmt, NEGATE_EXPR);
 	  update_stmt (stmt);
 	  return true;
 	}