diff mbox

[PR66726] Factor conversion out of COND_EXPR

Message ID 5597D24B.8010900@linaro.org
State New
Headers show

Commit Message

Kugan Vivekanandarajah July 4, 2015, 12:32 p.m. UTC 
On 04/07/15 18:51, Bernhard Reutner-Fischer wrote:
> On Sat, Jul 04, 2015 at 12:58:58PM +1000, Kugan wrote:
>> Please find a patch that attempt to FIX PR66726 by factoring conversion
>> out of COND_EXPR as explained in the PR.
>>
>> Bootstrapped and regression tested on x86-64-none-linux-gnu with no new
>> regressions. Is this OK for trunk?
>>
>> Thanks,
>> Kugan
>>
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2015-07-03  Kugan Vivekanandarajah  <kuganv@linaro.org>
>> 	    Jeff Law  <law@redhat.com>
>>
>> 	PR middle-end/66726
>> 	* gcc.dg/tree-ssa/pr66726.c: New test.
> 
> I'd have scanned the details dump for "factor CONVERT_EXPR out" 1
> to make sure that it's this part that takes care of it.

Thanks for the comments. Please see the updated patch with the fixes.
Kugan

> 
>>
>> gcc/ChangeLog:
>>
>> 2015-07-03  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>
>> 	PR middle-end/66726
>> 	* tree-ssa-phiopt.c (factor_out_conditional_conversion): New function.
>> 	(tree_ssa_phiopt_worker): Call factor_out_conditional_conversion.
> 
>> diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
>> index d2a5cee..e8af086 100644
>> --- a/gcc/tree-ssa-phiopt.c
>> +++ b/gcc/tree-ssa-phiopt.c
> 
>> @@ -410,6 +413,108 @@ replace_phi_edge_with_variable (basic_block cond_block,
>>  	      bb->index);
>>  }
>>  
>> +/* PR66726: Factor conversion out of COND_EXPR.  If the argument of the PHI
> 
> s/the argument/the arguments/
> 
>> +   stmt are CONVERT_STMT, factor out the conversion and perform the conversion
>> +   to the result of PHI stmt.  */
>> +
>> +static bool
>> +factor_out_conditional_conversion (edge e0, edge e1, gphi *phi,
>> +				   tree arg0, tree arg1)
>> +{
>> +  gimple def0 = NULL, def1 = NULL, new_stmt;
>> +  tree new_arg0 = NULL_TREE, new_arg1 = NULL_TREE;
>> +  tree temp, result;
>> +  gimple_stmt_iterator gsi;
>> +
>> +  /* One of the argument has to be SSA_NAME and other argument can
> 
> s/the argument/the arguments/
> 
>> +     be an SSA_NAME of INTEGER_CST.  */
>> +  if ((TREE_CODE (arg0) != SSA_NAME
>> +       && TREE_CODE (arg0) != INTEGER_CST)
>> +      || (TREE_CODE (arg1) != SSA_NAME
>> +	  && TREE_CODE (arg1) != INTEGER_CST)
>> +      || (TREE_CODE (arg0) == INTEGER_CST
>> +	  && TREE_CODE (arg1) == INTEGER_CST))
> 
> inconsistent space for the && lines above; The first should have a
> leading tab.
> 
>> +    return false;
>> +
>> +  /* Handle only PHI statements with two arguments.  TODO: If all
>> +     other arguments to PHI are INTEGER_CST, we can handle more
>> +     than two arguments too.  */
>> +  if (gimple_phi_num_args (phi) != 2)
>> +    return false;
>> +
>> +  /* If arg0 is an SSA_NAME and the stmt which defines arg0 is
>> +     ai CONVERT_STMT, use the LHS as new_arg0.  */
> 
> s/ai/a/
> 
>> +  if (TREE_CODE (arg0) == SSA_NAME)
>> +    {
>> +      def0 = SSA_NAME_DEF_STMT (arg0);
>> +      if (!is_gimple_assign (def0)
>> +	  || !CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def0)))
>> +	return false;
>> +      new_arg0 = gimple_assign_rhs1 (def0);
>> +    }
>> +
>> +  /* If arg1 is an SSA_NAME and the stmt which defines arg0 is
>> +     ai CONVERT_STMT, use the LHS as new_arg1.  */
> 
> s/ai/a/
> 
>> +  if (TREE_CODE (arg1) == SSA_NAME)
>> +    {
>> +      def1 = SSA_NAME_DEF_STMT (arg1);
>> +      if (!is_gimple_assign (def1)
>> +	  || !CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def1)))
>> +	return false;
>> +      new_arg1 = gimple_assign_rhs1 (def1);
>> +    }
>> +
>> +  /* If arg0 is an INTEGER_CST, fold it to new type.  */
>> +  if (TREE_CODE (arg0) != SSA_NAME)
>> +    {
>> +      if (!POINTER_TYPE_P (TREE_TYPE (new_arg1))
>> +	  && int_fits_type_p (arg0, TREE_TYPE (new_arg1)))
>> +	new_arg0 = fold_convert (TREE_TYPE (new_arg1), arg0);
>> +      else
>> +	return false;
>> +    }
>> +
>> +  /* If arg1 is an INTEGER_CST, fold it to new type.  */
>> +  if (TREE_CODE (arg1) != SSA_NAME)
>> +    {
>> +      if (!POINTER_TYPE_P (TREE_TYPE (new_arg0))
>> +	  && int_fits_type_p (arg1, TREE_TYPE (new_arg0)))
>> +	new_arg1 = fold_convert (TREE_TYPE (new_arg0), arg1);
>> +      else
>> +	return false;
>> +    }
>> +
>> +  /* If types of new_arg0 and new_arg1 are different bailout.  */
>> +  if (TREE_TYPE (new_arg0) != TREE_TYPE (new_arg1))
>> +    return false;
>> +
>> +  /* Replace the PHI stmt with the new_arg0 and new_arg1.  Also insert
>> +     a new CONVER_STMT that converts the phi results.  */
> 
> s/CONVER_STMT/CONVERT_STMT/
> 
>> +  gsi = gsi_after_labels (gimple_bb (phi));
>> +  result = PHI_RESULT (phi);
>> +  temp = make_ssa_name (TREE_TYPE (new_arg0), phi);
>> +
>> +  if (dump_file && (dump_flags & TDF_DETAILS))
>> +    {
>> +      fprintf (dump_file, "PHI ");
>> +      print_generic_expr (dump_file, gimple_phi_result (phi), 0);
>> +      fprintf (dump_file,
>> +	       " changed to factor CONVERT_EXPR out from COND_EXPR.\n");
>> +      fprintf (dump_file, "New PHI_RESULT is ");
>> +      print_generic_expr (dump_file, temp, 0);
>> +      fprintf (dump_file, " and new stmt with CONVERT_EXPR defines ");
>> +      print_generic_expr (dump_file, result, 0);
>> +      fprintf (dump_file, ".\n");
>> +    }
>> +
>> +  gimple_phi_set_result (phi, temp);
>> +  SET_PHI_ARG_DEF (phi, e0->dest_idx, new_arg0);
>> +  SET_PHI_ARG_DEF (phi, e1->dest_idx, new_arg1);
>> +  new_stmt = gimple_build_assign (result, CONVERT_EXPR, temp);
>> +  gsi_insert_before (&gsi, new_stmt, GSI_SAME_STMT);
>> +  return true;
>> +}
>> +
>>  /*  The function conditional_replacement does the main work of doing the
>>      conditional replacement.  Return true if the replacement is done.
>>      Otherwise return false.
>> @@ -2144,6 +2249,26 @@ gate_hoist_loads (void)
>>     This pass also performs a fifth transformation of a slightly different
>>     flavor.
>>  
>> +   Factor conversion in COND_EXPR
>> +   ----------------------------------
> 
> excess trailing "----" ;)
> 
> thanks,
>> +
>> +   This transformation factors the conversion out of COND_EXPR with
>> +   factor_out_conditional_conversion.
>> +
>> +   For example:
>> +   if (a <= CST) goto <bb 3>; else goto <bb 4>;
>> +   <bb 3>:
>> +   tmp = (int) a;
>> +   <bb 4>:
>> +   tmp = PHI <tmp, CST>
>> +
>> +   Into:
>> +   if (a <= CST) goto <bb 3>; else goto <bb 4>;
>> +   <bb 3>:
>> +   <bb 4>:
>> +   a = PHI <a, CST>
>> +   tmp = (int) a;
>> +
>>     Adjacent Load Hoisting
>>     ----------------------
>>  
>
diff mbox

Patch

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr66726.c b/gcc/testsuite/gcc.dg/tree-ssa/pr66726.c
index e69de29..93f1ace 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/pr66726.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr66726.c
@@ -0,0 +1,13 @@ 
+
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-phiopt1-details" } */
+
+extern unsigned short mode_size[];
+int
+oof (int mode)
+{
+  return (64 < mode_size[mode] ? 64 : mode_size[mode]);
+}
+
+/* { dg-final { scan-tree-dump-times "factor CONVERT_EXPR out" 1 "phiopt1" } } */
+
diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
index d2a5cee..12ab9ee 100644
--- a/gcc/tree-ssa-phiopt.c
+++ b/gcc/tree-ssa-phiopt.c
@@ -73,6 +73,7 @@  along with GCC; see the file COPYING3.  If not see
 static unsigned int tree_ssa_phiopt_worker (bool, bool);
 static bool conditional_replacement (basic_block, basic_block,
 				     edge, edge, gphi *, tree, tree);
+static bool factor_out_conditional_conversion (edge, edge, gphi *, tree, tree);
 static int value_replacement (basic_block, basic_block,
 			      edge, edge, gimple, tree, tree);
 static bool minmax_replacement (basic_block, basic_block,
@@ -342,6 +343,8 @@  tree_ssa_phiopt_worker (bool do_store_elim, bool do_hoist_loads)
 	    cfgchanged = true;
 	  else if (minmax_replacement (bb, bb1, e1, e2, phi, arg0, arg1))
 	    cfgchanged = true;
+	  else if (factor_out_conditional_conversion (e1, e2, phi, arg0, arg1))
+	    cfgchanged = true;
 	}
     }
 
@@ -410,6 +413,108 @@  replace_phi_edge_with_variable (basic_block cond_block,
 	      bb->index);
 }
 
+/* PR66726: Factor conversion out of COND_EXPR.  If the arguments of the PHI
+   stmt are CONVERT_STMT, factor out the conversion and perform the conversion
+   to the result of PHI stmt.  */
+
+static bool
+factor_out_conditional_conversion (edge e0, edge e1, gphi *phi,
+				   tree arg0, tree arg1)
+{
+  gimple def0 = NULL, def1 = NULL, new_stmt;
+  tree new_arg0 = NULL_TREE, new_arg1 = NULL_TREE;
+  tree temp, result;
+  gimple_stmt_iterator gsi;
+
+  /* One of the arguments has to be SSA_NAME and other argument can
+     be an SSA_NAME of INTEGER_CST.  */
+  if ((TREE_CODE (arg0) != SSA_NAME
+       && TREE_CODE (arg0) != INTEGER_CST)
+      || (TREE_CODE (arg1) != SSA_NAME
+	  && TREE_CODE (arg1) != INTEGER_CST)
+      || (TREE_CODE (arg0) == INTEGER_CST
+	  && TREE_CODE (arg1) == INTEGER_CST))
+    return false;
+
+  /* Handle only PHI statements with two arguments.  TODO: If all
+     other arguments to PHI are INTEGER_CST, we can handle more
+     than two arguments too.  */
+  if (gimple_phi_num_args (phi) != 2)
+    return false;
+
+  /* If arg0 is an SSA_NAME and the stmt which defines arg0 is
+     a CONVERT_STMT, use the LHS as new_arg0.  */
+  if (TREE_CODE (arg0) == SSA_NAME)
+    {
+      def0 = SSA_NAME_DEF_STMT (arg0);
+      if (!is_gimple_assign (def0)
+	  || !CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def0)))
+	return false;
+      new_arg0 = gimple_assign_rhs1 (def0);
+    }
+
+  /* If arg1 is an SSA_NAME and the stmt which defines arg0 is
+     a CONVERT_STMT, use the LHS as new_arg1.  */
+  if (TREE_CODE (arg1) == SSA_NAME)
+    {
+      def1 = SSA_NAME_DEF_STMT (arg1);
+      if (!is_gimple_assign (def1)
+	  || !CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def1)))
+	return false;
+      new_arg1 = gimple_assign_rhs1 (def1);
+    }
+
+  /* If arg0 is an INTEGER_CST, fold it to new type.  */
+  if (TREE_CODE (arg0) != SSA_NAME)
+    {
+      if (!POINTER_TYPE_P (TREE_TYPE (new_arg1))
+	  && int_fits_type_p (arg0, TREE_TYPE (new_arg1)))
+	new_arg0 = fold_convert (TREE_TYPE (new_arg1), arg0);
+      else
+	return false;
+    }
+
+  /* If arg1 is an INTEGER_CST, fold it to new type.  */
+  if (TREE_CODE (arg1) != SSA_NAME)
+    {
+      if (!POINTER_TYPE_P (TREE_TYPE (new_arg0))
+	  && int_fits_type_p (arg1, TREE_TYPE (new_arg0)))
+	new_arg1 = fold_convert (TREE_TYPE (new_arg0), arg1);
+      else
+	return false;
+    }
+
+  /* If types of new_arg0 and new_arg1 are different bailout.  */
+  if (TREE_TYPE (new_arg0) != TREE_TYPE (new_arg1))
+    return false;
+
+  /* Replace the PHI stmt with the new_arg0 and new_arg1.  Also insert
+     a new CONVERT_STMT that converts the phi results.  */
+  gsi = gsi_after_labels (gimple_bb (phi));
+  result = PHI_RESULT (phi);
+  temp = make_ssa_name (TREE_TYPE (new_arg0), phi);
+
+  if (dump_file && (dump_flags & TDF_DETAILS))
+    {
+      fprintf (dump_file, "PHI ");
+      print_generic_expr (dump_file, gimple_phi_result (phi), 0);
+      fprintf (dump_file,
+	       " changed to factor CONVERT_EXPR out from COND_EXPR.\n");
+      fprintf (dump_file, "New PHI_RESULT is ");
+      print_generic_expr (dump_file, temp, 0);
+      fprintf (dump_file, " and new stmt with CONVERT_EXPR defines ");
+      print_generic_expr (dump_file, result, 0);
+      fprintf (dump_file, ".\n");
+    }
+
+  gimple_phi_set_result (phi, temp);
+  SET_PHI_ARG_DEF (phi, e0->dest_idx, new_arg0);
+  SET_PHI_ARG_DEF (phi, e1->dest_idx, new_arg1);
+  new_stmt = gimple_build_assign (result, CONVERT_EXPR, temp);
+  gsi_insert_before (&gsi, new_stmt, GSI_SAME_STMT);
+  return true;
+}
+
 /*  The function conditional_replacement does the main work of doing the
     conditional replacement.  Return true if the replacement is done.
     Otherwise return false.
@@ -2144,6 +2249,26 @@  gate_hoist_loads (void)
    This pass also performs a fifth transformation of a slightly different
    flavor.
 
+   Factor conversion in COND_EXPR
+   ------------------------------
+
+   This transformation factors the conversion out of COND_EXPR with
+   factor_out_conditional_conversion.
+
+   For example:
+   if (a <= CST) goto <bb 3>; else goto <bb 4>;
+   <bb 3>:
+   tmp = (int) a;
+   <bb 4>:
+   tmp = PHI <tmp, CST>
+
+   Into:
+   if (a <= CST) goto <bb 3>; else goto <bb 4>;
+   <bb 3>:
+   <bb 4>:
+   a = PHI <a, CST>
+   tmp = (int) a;
+
    Adjacent Load Hoisting
    ----------------------