diff mbox

[PR66726] Fix regression caused by Factor conversion out of COND_EXPR

Message ID 574EBCEC.70805@linaro.org
State New
Headers show

Commit Message

Kugan Vivekanandarajah June 1, 2016, 10:46 a.m. UTC
Hi All,

Factoring out CONVERT_EXPR introduced a regression for (PR66726). I had 
to revert my previous patch due to some regressions. This is a much 
simplified version compared to the one I reverted.

There is a test-case (pr46309.c) in the test-suite which is valid for 
targets that has branch cost greater than 1.

This patch makes optimize_range_tests understand the factored out 
COND_EXPR. i.e., Updated the final_range_test_p to look for the new 
pattern. Changed the maybe_optimize_range_tests (which does the inter 
basic block range test optimization) accordingly.

With the patch
m68k-linux-gnu-gcc -O2 -S pr46309.c -fdump-tree-reassoc-details
grep -e "Optimizing range tests" -e into 
pr46309.c.*.reassoc1pr46309.c.114t.reassoc1:Optimizing range tests 
a_6(D) -[1, 1] and -[2, 2] and -[3, 3] and -[4, 4]
pr46309.c.114t.reassoc1: into (unsigned int) a_6(D) + 4294967295 > 3
pr46309.c.114t.reassoc1: into _10 = _13;
pr46309.c.114t.reassoc1:Optimizing range tests a_6(D) -[1, 1] and -[2, 
2] and -[3, 3] and -[4, 4]
pr46309.c.114t.reassoc1: into (unsigned int) a_6(D) + 4294967295 > 3
pr46309.c.114t.reassoc1: into _10 = _13;
pr46309.c.114t.reassoc1:Optimizing range tests a_4(D) -[1, 1] and -[3, 3]
pr46309.c.114t.reassoc1: into (a_4(D) & -3) != 1
pr46309.c.114t.reassoc1: into _6 = _8;
pr46309.c.114t.reassoc1:Optimizing range tests a_4(D) -[1, 1] and -[2, 2]
pr46309.c.114t.reassoc1: into (unsigned int) a_4(D) + 4294967295 > 1
pr46309.c.114t.reassoc1: into _6 = _9;
pr46309.c.114t.reassoc1:Optimizing range tests a_5(D) -[0, 31] and -[64, 95]
pr46309.c.114t.reassoc1: into (a_5(D) & 4294967231) > 31
pr46309.c.114t.reassoc1: into _7 = _9;
pr46309.c.114t.reassoc1:Optimizing range tests a_9(D) -[0, 31] and -[64, 95]
pr46309.c.114t.reassoc1: into (a_9(D) & 4294967231) > 31
pr46309.c.114t.reassoc1:Optimizing range tests a_9(D) -[128, 159] and 
-[192, 223]
pr46309.c.114t.reassoc1: into (a_9(D) & 4294967231) + 4294967168 > 31
pr46309.c.114t.reassoc1: into _13 = _18 | _15;
pr46309.c.116t.reassoc1:Optimizing range tests a_2(D) -[1, 1] and -[2, 
2] and -[3, 3] and -[4, 4]
pr46309.c.116t.reassoc1: into (unsigned int) a_2(D) + 4294967295 > 3
pr46309.c.116t.reassoc1:Optimizing range tests a_2(D) -[1, 1] and -[2, 
2] and -[3, 3] and -[4, 4]
pr46309.c.116t.reassoc1: into (unsigned int) a_2(D) + 4294967295 > 3
pr46309.c.116t.reassoc1:Optimizing range tests a_3(D) -[0, 31] and -[64, 95]
pr46309.c.116t.reassoc1: into (a_3(D) & 4294967231) > 31
pr46309.c.116t.reassoc1:Optimizing range tests a_5(D) -[0, 31] and -[64, 95]
pr46309.c.116t.reassoc1: into (a_5(D) & 4294967231) > 31
pr46309.c.116t.reassoc1:Optimizing range tests a_5(D) -[128, 159] and 
-[192, 223]
pr46309.c.116t.reassoc1: into (a_5(D) & 4294967231) + 4294967168 > 31


Bootstrapped and regression testing on x86-64-linux-gnu and 
ppc64le-linux-gnu doesn't have any new regressions. Also did regression 
testing arm variants which has branch cost greater than 1

Is this OK for trunk.

Thanks,
Kugan

gcc/ChangeLog:

2016-06-01  Kugan Vivekanandarajah  <kuganv@linaro.org>

	PR middle-end/66726
	* tree-ssa-reassoc.c (optimize_vec_cond_expr): Handle tcc_compare stmt
	whose result is used in PHI
	(final_range_test_p): Likewise.
	(maybe_optimize_range_tests): Likewise.
diff mbox

Patch

diff --git a/gcc/tree-ssa-reassoc.c b/gcc/tree-ssa-reassoc.c
index 430bcc8..851ae6d 100644
--- a/gcc/tree-ssa-reassoc.c
+++ b/gcc/tree-ssa-reassoc.c
@@ -3007,18 +3007,33 @@  optimize_vec_cond_expr (tree_code opcode, vec<operand_entry *> *ops)
    # _345 = PHI <_123(N), 1(...), 1(...)>
    where _234 has bool type, _123 has single use and
    bb N has a single successor M.  This is commonly used in
+   the last block of a range test.
+
+   Also Return true if STMT is tcc_compare like:
+   <bb N>:
+   ...
+   _234 = a_2(D) == 2;
+
+   <bb M>:
+   # _345 = PHI <_234(N), 1(...), 1(...)>
+   _346 = (int) _345;
+   where _234 has booltype, single use and
+   bb N has a single successor M.  This is commonly used in
    the last block of a range test.  */
 
 static bool
 final_range_test_p (gimple *stmt)
 {
-  basic_block bb, rhs_bb;
+  basic_block bb, rhs_bb, lhs_bb;
   edge e;
   tree lhs, rhs;
   use_operand_p use_p;
   gimple *use_stmt;
 
-  if (!gimple_assign_cast_p (stmt))
+  if (!gimple_assign_cast_p (stmt)
+      && (!is_gimple_assign (stmt)
+	  || (TREE_CODE_CLASS (gimple_assign_rhs_code (stmt))
+	      != tcc_comparison)))
     return false;
   bb = gimple_bb (stmt);
   if (!single_succ_p (bb))
@@ -3029,11 +3044,16 @@  final_range_test_p (gimple *stmt)
 
   lhs = gimple_assign_lhs (stmt);
   rhs = gimple_assign_rhs1 (stmt);
-  if (!INTEGRAL_TYPE_P (TREE_TYPE (lhs))
-      || TREE_CODE (rhs) != SSA_NAME
-      || TREE_CODE (TREE_TYPE (rhs)) != BOOLEAN_TYPE)
+  if (gimple_assign_cast_p (stmt)
+      && (!INTEGRAL_TYPE_P (TREE_TYPE (lhs))
+	  || TREE_CODE (rhs) != SSA_NAME
+	  || TREE_CODE (TREE_TYPE (rhs)) != BOOLEAN_TYPE))
     return false;
 
+  if (!gimple_assign_cast_p (stmt)
+      && (TREE_CODE (TREE_TYPE (lhs)) != BOOLEAN_TYPE))
+      return false;
+
   /* Test whether lhs is consumed only by a PHI in the only successor bb.  */
   if (!single_imm_use (lhs, &use_p, &use_stmt))
     return false;
@@ -3043,10 +3063,20 @@  final_range_test_p (gimple *stmt)
     return false;
 
   /* And that the rhs is defined in the same loop.  */
-  rhs_bb = gimple_bb (SSA_NAME_DEF_STMT (rhs));
-  if (rhs_bb == NULL
-      || !flow_bb_inside_loop_p (loop_containing_stmt (stmt), rhs_bb))
-    return false;
+  if (gimple_assign_cast_p (stmt))
+    {
+      if (TREE_CODE (rhs) != SSA_NAME
+	  || !(rhs_bb = gimple_bb (SSA_NAME_DEF_STMT (rhs)))
+	  || !flow_bb_inside_loop_p (loop_containing_stmt (stmt), rhs_bb))
+	return false;
+    }
+  else
+    {
+      if (TREE_CODE (lhs) != SSA_NAME
+	  || !(lhs_bb = gimple_bb (SSA_NAME_DEF_STMT (lhs)))
+	  || !flow_bb_inside_loop_p (loop_containing_stmt (stmt), lhs_bb))
+	return false;
+    }
 
   return true;
 }
@@ -3440,6 +3470,8 @@  maybe_optimize_range_tests (gimple *stmt)
 
 	  /* stmt is
 	     _123 = (int) _234;
+	     OR
+	     _234 = a_2(D) == 2;
 
 	     followed by:
 	     <bb M>:
@@ -3469,6 +3501,8 @@  maybe_optimize_range_tests (gimple *stmt)
 	     of the bitwise or resp. and, recursively.  */
 	  if (!get_ops (rhs, code, &ops,
 			loop_containing_stmt (stmt))
+	      && (TREE_CODE_CLASS (gimple_assign_rhs_code (stmt))
+		  != tcc_comparison)
 	      && has_single_use (rhs))
 	    {
 	      /* Otherwise, push the _234 range test itself.  */
@@ -3481,10 +3515,36 @@  maybe_optimize_range_tests (gimple *stmt)
 	      oe->stmt_to_insert = NULL;
 	      ops.safe_push (oe);
 	      bb_ent.last_idx++;
+	      bb_ent.op = rhs;
+	    }
+	  else if (is_gimple_assign (stmt)
+		   && (TREE_CODE_CLASS (gimple_assign_rhs_code (stmt))
+		       == tcc_comparison))
+	    {
+	      if (!get_ops (lhs, code, &ops,
+			    loop_containing_stmt (stmt))
+		  && has_single_use (rhs))
+		{
+		  operand_entry *oe = operand_entry_pool.allocate ();
+		  oe->op = lhs;
+		  oe->rank = code;
+		  oe->id = 0;
+		  oe->count = 1;
+		  ops.safe_push (oe);
+		  bb_ent.last_idx++;
+		  bb_ent.op = lhs;
+		}
+	      else
+		{
+		  bb_ent.last_idx = ops.length ();
+		  bb_ent.op = rhs;
+		}
 	    }
 	  else
-	    bb_ent.last_idx = ops.length ();
-	  bb_ent.op = rhs;
+	    {
+	      bb_ent.last_idx = ops.length ();
+	      bb_ent.op = rhs;
+	    }
 	  bbinfo.safe_push (bb_ent);
 	  continue;
 	}
@@ -3566,7 +3626,7 @@  maybe_optimize_range_tests (gimple *stmt)
 		{
 		  imm_use_iterator iter;
 		  use_operand_p use_p;
-		  gimple *use_stmt, *cast_stmt = NULL;
+		  gimple *use_stmt, *cast_or_tcc_cmp_stmt = NULL;
 
 		  FOR_EACH_IMM_USE_STMT (use_stmt, iter, bbinfo[idx].op)
 		    if (is_gimple_debug (use_stmt))
@@ -3575,17 +3635,25 @@  maybe_optimize_range_tests (gimple *stmt)
 			     || gimple_code (use_stmt) == GIMPLE_PHI)
 		      FOR_EACH_IMM_USE_ON_STMT (use_p, iter)
 			SET_USE (use_p, new_op);
+		    else if ((is_gimple_assign (use_stmt)
+			      && (TREE_CODE_CLASS
+				  (gimple_assign_rhs_code (use_stmt))
+				  == tcc_comparison)))
+		      cast_or_tcc_cmp_stmt = use_stmt;
 		    else if (gimple_assign_cast_p (use_stmt))
-		      cast_stmt = use_stmt;
+		      cast_or_tcc_cmp_stmt = use_stmt;
 		    else
 		      gcc_unreachable ();
-		  if (cast_stmt)
+
+		  if (cast_or_tcc_cmp_stmt)
 		    {
 		      gcc_assert (bb == last_bb);
-		      tree lhs = gimple_assign_lhs (cast_stmt);
+		      tree lhs = gimple_assign_lhs (cast_or_tcc_cmp_stmt);
 		      tree new_lhs = make_ssa_name (TREE_TYPE (lhs));
 		      enum tree_code rhs_code
-			= gimple_assign_rhs_code (cast_stmt);
+			= gimple_assign_cast_p (cast_or_tcc_cmp_stmt)
+			? gimple_assign_rhs_code (cast_or_tcc_cmp_stmt)
+			: CONVERT_EXPR;
 		      gassign *g;
 		      if (is_gimple_min_invariant (new_op))
 			{
@@ -3594,8 +3662,9 @@  maybe_optimize_range_tests (gimple *stmt)
 			}
 		      else
 			g = gimple_build_assign (new_lhs, rhs_code, new_op);
-		      gimple_stmt_iterator gsi = gsi_for_stmt (cast_stmt);
-		      gimple_set_uid (g, gimple_uid (cast_stmt));
+		      gimple_stmt_iterator gsi
+			= gsi_for_stmt (cast_or_tcc_cmp_stmt);
+		      gimple_set_uid (g, gimple_uid (cast_or_tcc_cmp_stmt));
 		      gimple_set_visited (g, true);
 		      gsi_insert_before (&gsi, g, GSI_SAME_STMT);
 		      FOR_EACH_IMM_USE_STMT (use_stmt, iter, lhs)