This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH v3] Fix PR81503 (SLSR invalid fold)


> On Aug 28, 2017, at 7:37 AM, Richard Biener <richard.guenther@gmail.com> wrote:
> 
> Not sure, but would it be fixed in a similar way when writing
<snip>
> ?

Thanks, Richard, that works very well.  I decided this was a good opportunity to also
simplify the control flow a little with early returns.  Here's the result.  Bootstrapped and
tested on powerpc64le-linux-gnu with no regressions.  Is this ok for trunk, and
eventually for backport to gcc 5, 6, and 7?  (I can omit the control flow cleanups for
the older releases if desired.)

Thanks,
Bill

[gcc]

2017-08-03  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
	    Jakub Jelinek  <jakub@redhat.com>
	    Richard Biener  <rguenther@suse.de>

	PR tree-optimization/81503
	* gimple-ssa-strength-reduction.c (replace_mult_candidate): Ensure
	folded constant fits in the target type; reorder tests for clarity.

[gcc/testsuite]

2017-08-03  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
	    Jakub Jelinek  <jakub@redhat.com>
	    Richard Biener  <rguenther@suse.de>

	PR tree-optimization/81503
	* gcc.c-torture/execute/pr81503.c: New file.


Index: gcc/gimple-ssa-strength-reduction.c
===================================================================
--- gcc/gimple-ssa-strength-reduction.c	(revision 251369)
+++ gcc/gimple-ssa-strength-reduction.c	(working copy)
@@ -2089,104 +2089,104 @@ replace_mult_candidate (slsr_cand_t c, tree basis_
   tree target_type = TREE_TYPE (gimple_assign_lhs (c->cand_stmt));
   enum tree_code cand_code = gimple_assign_rhs_code (c->cand_stmt);
 
-  /* It is highly unlikely, but possible, that the resulting
-     bump doesn't fit in a HWI.  Abandon the replacement
-     in this case.  This does not affect siblings or dependents
-     of C.  Restriction to signed HWI is conservative for unsigned
-     types but allows for safe negation without twisted logic.  */
-  if (wi::fits_shwi_p (bump)
-      && bump.to_shwi () != HOST_WIDE_INT_MIN
-      /* It is not useful to replace casts, copies, negates, or adds of
-	 an SSA name and a constant.  */
-      && cand_code != SSA_NAME
-      && !CONVERT_EXPR_CODE_P (cand_code)
-      && cand_code != PLUS_EXPR
-      && cand_code != POINTER_PLUS_EXPR
-      && cand_code != MINUS_EXPR
-      && cand_code != NEGATE_EXPR)
+  /* It is not useful to replace casts, copies, negates, or adds of
+     an SSA name and a constant.  */
+  if (cand_code == SSA_NAME
+      || CONVERT_EXPR_CODE_P (cand_code)
+      || cand_code == PLUS_EXPR
+      || cand_code == POINTER_PLUS_EXPR
+      || cand_code == MINUS_EXPR
+      || cand_code == NEGATE_EXPR)
+    return;
+
+  enum tree_code code = PLUS_EXPR;
+  tree bump_tree;
+  gimple *stmt_to_print = NULL;
+
+  if (wi::neg_p (bump))
     {
-      enum tree_code code = PLUS_EXPR;
-      tree bump_tree;
-      gimple *stmt_to_print = NULL;
+      code = MINUS_EXPR;
+      bump = -bump;
+    }
 
-      /* If the basis name and the candidate's LHS have incompatible
-	 types, introduce a cast.  */
-      if (!useless_type_conversion_p (target_type, TREE_TYPE (basis_name)))
-	basis_name = introduce_cast_before_cand (c, target_type, basis_name);
-      if (wi::neg_p (bump))
+  /* It is possible that the resulting bump doesn't fit in target_type.
+     Abandon the replacement in this case.  This does not affect
+     siblings or dependents of C.  */
+  if (bump != wi::ext (bump, TYPE_PRECISION (target_type),
+		       TYPE_SIGN (target_type)))
+    return;
+
+  bump_tree = wide_int_to_tree (target_type, bump);
+
+  /* If the basis name and the candidate's LHS have incompatible types,
+     introduce a cast.  */
+  if (!useless_type_conversion_p (target_type, TREE_TYPE (basis_name)))
+    basis_name = introduce_cast_before_cand (c, target_type, basis_name);
+
+  if (dump_file && (dump_flags & TDF_DETAILS))
+    {
+      fputs ("Replacing: ", dump_file);
+      print_gimple_stmt (dump_file, c->cand_stmt, 0);
+    }
+
+  if (bump == 0)
+    {
+      tree lhs = gimple_assign_lhs (c->cand_stmt);
+      gassign *copy_stmt = gimple_build_assign (lhs, basis_name);
+      gimple_stmt_iterator gsi = gsi_for_stmt (c->cand_stmt);
+      slsr_cand_t cc = c;
+      gimple_set_location (copy_stmt, gimple_location (c->cand_stmt));
+      gsi_replace (&gsi, copy_stmt, false);
+      c->cand_stmt = copy_stmt;
+      while (cc->next_interp)
 	{
-	  code = MINUS_EXPR;
-	  bump = -bump;
+	  cc = lookup_cand (cc->next_interp);
+	  cc->cand_stmt = copy_stmt;
 	}
-
-      bump_tree = wide_int_to_tree (target_type, bump);
-
       if (dump_file && (dump_flags & TDF_DETAILS))
+	stmt_to_print = copy_stmt;
+    }
+  else
+    {
+      tree rhs1, rhs2;
+      if (cand_code != NEGATE_EXPR) {
+	rhs1 = gimple_assign_rhs1 (c->cand_stmt);
+	rhs2 = gimple_assign_rhs2 (c->cand_stmt);
+      }
+      if (cand_code != NEGATE_EXPR
+	  && ((operand_equal_p (rhs1, basis_name, 0)
+	       && operand_equal_p (rhs2, bump_tree, 0))
+	      || (operand_equal_p (rhs1, bump_tree, 0)
+		  && operand_equal_p (rhs2, basis_name, 0))))
 	{
-	  fputs ("Replacing: ", dump_file);
-	  print_gimple_stmt (dump_file, c->cand_stmt, 0);
+	  if (dump_file && (dump_flags & TDF_DETAILS))
+	    {
+	      fputs ("(duplicate, not actually replacing)", dump_file);
+	      stmt_to_print = c->cand_stmt;
+	    }
 	}
-
-      if (bump == 0)
+      else
 	{
-	  tree lhs = gimple_assign_lhs (c->cand_stmt);
-	  gassign *copy_stmt = gimple_build_assign (lhs, basis_name);
 	  gimple_stmt_iterator gsi = gsi_for_stmt (c->cand_stmt);
 	  slsr_cand_t cc = c;
-	  gimple_set_location (copy_stmt, gimple_location (c->cand_stmt));
-	  gsi_replace (&gsi, copy_stmt, false);
-	  c->cand_stmt = copy_stmt;
+	  gimple_assign_set_rhs_with_ops (&gsi, code, basis_name, bump_tree);
+	  update_stmt (gsi_stmt (gsi));
+	  c->cand_stmt = gsi_stmt (gsi);
 	  while (cc->next_interp)
 	    {
 	      cc = lookup_cand (cc->next_interp);
-	      cc->cand_stmt = copy_stmt;
+	      cc->cand_stmt = gsi_stmt (gsi);
 	    }
 	  if (dump_file && (dump_flags & TDF_DETAILS))
-	    stmt_to_print = copy_stmt;
+	    stmt_to_print = gsi_stmt (gsi);
 	}
-      else
-	{
-	  tree rhs1, rhs2;
-	  if (cand_code != NEGATE_EXPR) {
-	    rhs1 = gimple_assign_rhs1 (c->cand_stmt);
-	    rhs2 = gimple_assign_rhs2 (c->cand_stmt);
-	  }
-	  if (cand_code != NEGATE_EXPR
-	      && ((operand_equal_p (rhs1, basis_name, 0)
-		   && operand_equal_p (rhs2, bump_tree, 0))
-		  || (operand_equal_p (rhs1, bump_tree, 0)
-		      && operand_equal_p (rhs2, basis_name, 0))))
-	    {
-	      if (dump_file && (dump_flags & TDF_DETAILS))
-		{
-		  fputs ("(duplicate, not actually replacing)", dump_file);
-		  stmt_to_print = c->cand_stmt;
-		}
-	    }
-	  else
-	    {
-	      gimple_stmt_iterator gsi = gsi_for_stmt (c->cand_stmt);
-	      slsr_cand_t cc = c;
-	      gimple_assign_set_rhs_with_ops (&gsi, code,
-					      basis_name, bump_tree);
-	      update_stmt (gsi_stmt (gsi));
-              c->cand_stmt = gsi_stmt (gsi);
-	      while (cc->next_interp)
-		{
-		  cc = lookup_cand (cc->next_interp);
-		  cc->cand_stmt = gsi_stmt (gsi);
-		}
-	      if (dump_file && (dump_flags & TDF_DETAILS))
-		stmt_to_print = gsi_stmt (gsi);
-	    }
-	}
+    }
   
-      if (dump_file && (dump_flags & TDF_DETAILS))
-	{
-	  fputs ("With: ", dump_file);
-	  print_gimple_stmt (dump_file, stmt_to_print, 0);
-	  fputs ("\n", dump_file);
-  	}
+  if (dump_file && (dump_flags & TDF_DETAILS))
+    {
+      fputs ("With: ", dump_file);
+      print_gimple_stmt (dump_file, stmt_to_print, 0);
+      fputs ("\n", dump_file);
     }
 }
 
Index: gcc/testsuite/gcc.c-torture/execute/pr81503.c
===================================================================
--- gcc/testsuite/gcc.c-torture/execute/pr81503.c	(nonexistent)
+++ gcc/testsuite/gcc.c-torture/execute/pr81503.c	(working copy)
@@ -0,0 +1,15 @@
+unsigned short a = 41461;
+unsigned short b = 3419;
+int c = 0;
+
+void foo() {
+  if (a + b * ~(0 != 5))
+    c = -~(b * ~(0 != 5)) + 2147483647;
+}
+
+int main() {
+  foo();
+  if (c != 2147476810)
+    return -1;
+  return 0;
+}


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]