diff mbox

[PR66726] Factor conversion out of COND_EXPR

Message ID 55A6073D.1020800@linaro.org
State New
Headers show

Commit Message

Kugan Vivekanandarajah July 15, 2015, 7:09 a.m. UTC
Here is a patch to fix to teach tree-ssa-reassoc the sinking the cast.
Bootstrapped and regression tested on x86-64-none-linux-gnu with no new
regressions. Also regression tested on qemu arm.

I also verified the issue Andreas Schwab raised is fixed on arm
cortex-a5 where the same issue was present. Does this make sense?

Thanks,
Kugan

gcc/ChangeLog:

2015-07-15  Kugan Vivekanandarajah  <kuganv@linaro.org>

	PR middle-end/66726
	* tree-ssa-reassoc.c (optimize_range_tests): Handle sinking the cast
after PHI.
	(final_range_test_p): Detect sinking the cast after PHI.
	(maybe_optimize_range_tests): Handle sinking the cast after PHI.

Comments

Jeff Law July 15, 2015, 6:45 p.m. UTC | #1
On 07/15/2015 01:09 AM, Kugan wrote:
>
> 2015-07-15  Kugan Vivekanandarajah<kuganv@linaro.org>
>
> 	PR middle-end/66726
> 	* tree-ssa-reassoc.c (optimize_range_tests): Handle sinking the cast
> after PHI.
> 	(final_range_test_p): Detect sinking the cast after PHI.
> 	(maybe_optimize_range_tests): Handle sinking the cast after PHI.
Can we tweak
>
>
> p.txt
>
>
> diff --git a/gcc/tree-ssa-reassoc.c b/gcc/tree-ssa-reassoc.c
> index 932c83a..3058eb5 100644
> --- a/gcc/tree-ssa-reassoc.c
> +++ b/gcc/tree-ssa-reassoc.c

>       return false;
>     bb = gimple_bb (stmt);
>     if (!single_succ_p (bb))
> @@ -2729,9 +2743,8 @@ 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 (TREE_CODE (TREE_TYPE (rhs)) != BOOLEAN_TYPE
> +      && TREE_CODE (TREE_TYPE (lhs)) != BOOLEAN_TYPE)
>       return false;
So you're ensuring that one of the two is a boolean...  Note that 
previously we ensured that the rhs was a boolean and the lhs was an 
integral type (which I believe is true for booleans).

Thus if we had
bool x;
int y;

x = (bool) y;

The old code would have rejected that case.  But I think it gets through 
now, right?

I think once that issue is addressed, this will be good for the trunk.

jeff
diff mbox

Patch

diff --git a/gcc/tree-ssa-reassoc.c b/gcc/tree-ssa-reassoc.c
index 932c83a..3058eb5 100644
--- a/gcc/tree-ssa-reassoc.c
+++ b/gcc/tree-ssa-reassoc.c
@@ -2707,18 +2707,32 @@  optimize_range_tests (enum tree_code opcode,
    # _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.  */
+   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))
@@ -2729,9 +2743,8 @@  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 (TREE_CODE (TREE_TYPE (rhs)) != BOOLEAN_TYPE
+      && TREE_CODE (TREE_TYPE (lhs)) != BOOLEAN_TYPE)
     return false;
 
   /* Test whether lhs is consumed only by a PHI in the only successor bb.  */
@@ -2743,10 +2756,21 @@  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 (!INTEGRAL_TYPE_P (TREE_TYPE (lhs))
+	  || 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;
 }
@@ -3132,6 +3156,8 @@  maybe_optimize_range_tests (gimple stmt)
 
 	  /* stmt is
 	     _123 = (int) _234;
+	     OR
+	     _234 = a_2(D) == 2;
 
 	     followed by:
 	     <bb M>:
@@ -3161,6 +3187,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.  */
@@ -3173,6 +3201,22 @@  maybe_optimize_range_tests (gimple stmt)
 	      ops.safe_push (oe);
 	      bb_ent.last_idx++;
 	    }
+	  else if (!get_ops (lhs, code, &ops,
+			     loop_containing_stmt (stmt))
+		   && is_gimple_assign (stmt)
+		   && (TREE_CODE_CLASS (gimple_assign_rhs_code (stmt))
+		       == tcc_comparison)
+		   && has_single_use (lhs))
+	    {
+	      /* Push the _234 range test itself.  */
+	      operand_entry_t 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++;
+	    }
 	  else
 	    bb_ent.last_idx = ops.length ();
 	  bb_ent.op = rhs;
@@ -3267,7 +3311,8 @@  maybe_optimize_range_tests (gimple stmt)
 		    else if (gimple_assign_cast_p (use_stmt))
 		      cast_stmt = use_stmt;
 		    else
-		      gcc_unreachable ();
+		      cast_stmt = NULL;
+
 		  if (cast_stmt)
 		    {
 		      gcc_assert (bb == last_bb);