On Wed, Dec 09, 2015 at 11:41:44AM +0100, Richard Biener wrote:
> But I don't see why we have the asserts at all - they seem to be originated
> from
> development. IMHO factor_out_conditonal_conversion should simply return
> the new PHI it generates instead of relying on
> signle_non_signelton_phi_for_edges
> to return it.
Ok, that seems to work.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2015-12-09 Marek Polacek <pola...@redhat.com>
PR tree-optimization/66949
* tree-ssa-phiopt.c (tree_ssa_phiopt_worker): Don't call
single_non_singleton_phi_for_edges to get the PHI from
factor_out_conditional_conversion. Use NULL_TREE instead of NULL.
(factor_out_conditional_conversion): Adjust declaration. Make it
return the newly-created PHI.
* gcc.dg/torture/pr66949-1.c: New test.
* gcc.dg/torture/pr66949-2.c: New test.
diff --git gcc/testsuite/gcc.dg/torture/pr66949-1.c
gcc/testsuite/gcc.dg/torture/pr66949-1.c
index e69de29..1b765bc 100644
--- gcc/testsuite/gcc.dg/torture/pr66949-1.c
+++ gcc/testsuite/gcc.dg/torture/pr66949-1.c
@@ -0,0 +1,28 @@
+/* PR tree-optimization/66949 */
+/* { dg-do compile } */
+
+int a, *b = &a, c;
+
+unsigned short
+fn1 (unsigned short p1, unsigned int p2)
+{
+ return p2 > 1 || p1 >> p2 ? p1 : p1 << p2;
+}
+
+void
+fn2 ()
+{
+ int *d = &a;
+ for (a = 0; a < -1; a = 1)
+ ;
+ if (a < 0)
+ c = 0;
+ *b = fn1 (*d || c, *d);
+}
+
+int
+main ()
+{
+ fn2 ();
+ return 0;
+}
diff --git gcc/testsuite/gcc.dg/torture/pr66949-2.c
gcc/testsuite/gcc.dg/torture/pr66949-2.c
index e69de29..e6250a3 100644
--- gcc/testsuite/gcc.dg/torture/pr66949-2.c
+++ gcc/testsuite/gcc.dg/torture/pr66949-2.c
@@ -0,0 +1,23 @@
+/* PR tree-optimization/66949 */
+/* { dg-do compile } */
+
+char a;
+int b, c, d;
+extern int fn2 (void);
+
+short
+fn1 (short p1, short p2)
+{
+ return p2 == 0 ? p1 : p1 / p2;
+}
+
+int
+main (void)
+{
+ char e = 1;
+ int f = 7;
+ c = a >> f;
+ b = fn1 (c, 0 < d <= e && fn2 ());
+
+ return 0;
+}
diff --git gcc/tree-ssa-phiopt.c gcc/tree-ssa-phiopt.c
index 344cd2f..d7e8aa0 100644
--- gcc/tree-ssa-phiopt.c
+++ gcc/tree-ssa-phiopt.c
@@ -49,7 +49,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 gphi *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,
@@ -310,19 +310,19 @@ tree_ssa_phiopt_worker (bool do_store_elim, bool
do_hoist_loads)
/* Something is wrong if we cannot find the arguments in the PHI
node. */
- gcc_assert (arg0 != NULL && arg1 != NULL);
+ gcc_assert (arg0 != NULL_TREE && arg1 != NULL_TREE);
- if (factor_out_conditional_conversion (e1, e2, phi, arg0, arg1))
+ gphi *newphi = factor_out_conditional_conversion (e1, e2, phi,
+ arg0, arg1);
+ if (newphi != NULL)
{
+ phi = newphi;
/* factor_out_conditional_conversion may create a new PHI in
BB2 and eliminate an existing PHI in BB2. Recompute values
that may be affected by that change. */
- phis = phi_nodes (bb2);
- phi = single_non_singleton_phi_for_edges (phis, e1, e2);
- gcc_assert (phi);
arg0 = gimple_phi_arg_def (phi, e1->dest_idx);
arg1 = gimple_phi_arg_def (phi, e2->dest_idx);
- gcc_assert (arg0 != NULL && arg1 != NULL);
+ gcc_assert (arg0 != NULL_TREE && arg1 != NULL_TREE);
}
/* Do the replacement of conditional if it can be done. */
@@ -402,9 +402,9 @@ replace_phi_edge_with_variable (basic_block cond_block,
/* 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. */
+ to the result of PHI stmt. Return the newly-created PHI, if any. */
-static bool
+static gphi *
factor_out_conditional_conversion (edge e0, edge e1, gphi *phi,
tree arg0, tree arg1)
{
@@ -421,7 +421,7 @@ factor_out_conditional_conversion (edge e0, edge e1, gphi
*phi,
statement have the same unary operation, we can handle more
than two arguments too. */
if (gimple_phi_num_args (phi) != 2)
- return false;
+ return NULL;
/* First canonicalize to simplify tests. */
if (TREE_CODE (arg0) != SSA_NAME)
@@ -433,14 +433,14 @@ factor_out_conditional_conversion (edge e0, edge e1, gphi
*phi,
if (TREE_CODE (arg0) != SSA_NAME
|| (TREE_CODE (arg1) != SSA_NAME
&& TREE_CODE (arg1) != INTEGER_CST))
- return false;
+ return NULL;
/* Check if arg0 is an SSA_NAME and the stmt which defines arg0 is
a conversion. */
arg0_def_stmt = SSA_NAME_DEF_STMT (arg0);
if (!is_gimple_assign (arg0_def_stmt)
|| !gimple_assign_cast_p (arg0_def_stmt))
- return false;
+ return NULL;
/* Use the RHS as new_arg0. */
convert_code = gimple_assign_rhs_code (arg0_def_stmt);
@@ -455,7 +455,7 @@ factor_out_conditional_conversion (edge e0, edge e1, gphi
*phi,
arg1_def_stmt = SSA_NAME_DEF_STMT (arg1);
if (!is_gimple_assign (arg1_def_stmt)
|| gimple_assign_rhs_code (arg1_def_stmt) != convert_code)
- return false;
+ return NULL;
/* Use the RHS as new_arg1. */
new_arg1 = gimple_assign_rhs1 (arg1_def_stmt);
@@ -471,21 +471,21 @@ factor_out_conditional_conversion (edge e0, edge e1, gphi
*phi,
if (gimple_assign_cast_p (arg0_def_stmt))
new_arg1 = fold_convert (TREE_TYPE (new_arg0), arg1);
else
- return false;
+ return NULL;
}
else
- return false;
+ return NULL;
}
/* If arg0/arg1 have > 1 use, then this transformation actually increases
the number of expressions evaluated at runtime. */
if (!has_single_use (arg0)
|| (arg1_def_stmt && !has_single_use (arg1)))
- return false;
+ return NULL;
/* If types of new_arg0 and new_arg1 are different bailout. */
if (!types_compatible_p (TREE_TYPE (new_arg0), TREE_TYPE (new_arg1)))
- return false;
+ return NULL;
/* Create a new PHI stmt. */
result = PHI_RESULT (phi);
@@ -528,7 +528,7 @@ factor_out_conditional_conversion (edge e0, edge e1, gphi
*phi,
/* Remove the original PHI stmt. */
gsi = gsi_for_stmt (phi);
gsi_remove (&gsi, true);
- return true;
+ return newphi;
}
/* The function conditional_replacement does the main work of doing the
Marek
