To...

To: gcc-patches@gcc.gnu.org
Subject: PR 71020: Handle abnormal PHIs in tree-call-cdce.c
From: Richard Sandiford <richard.sandiford@arm.com>
Gcc: private.sent
--text follows this line--
The PR is about a case where tree-call-cdce.c causes two abnormal
PHIs for the same variable to be live at the same time, leading to
a coalescing failure.  It seemed like getting rid of these kinds of
input would be generally useful, so I added a utility to tree-dfa.c.

Tested on x86_64-linux-gnu.

gcc/
	PR middle-end/71020
	* tree-dfa.h (replace_abnormal_ssa_names): Declare.
	* tree-dfa.c (replace_abnormal_ssa_names): New function.
	* tree-call-cdce.c: Include tree-dfa.h.
	(can_guard_call_p): New function, extracted from...
	(can_use_internal_fn): ...here.
	(shrink_wrap_one_built_in_call_with_conds): Remove failure path
	and return void.
	(shrink_wrap_one_built_in_call): Likewise.
	(use_internal_fn): Likewise.
	(shrink_wrap_conditional_dead_built_in_calls): Update accordingly
	and return void.  Call replace_abnormal_ssa_names.
	(pass_call_cdce::execute): Check can_guard_call_p during the
	initial walk.  Assume shrink_wrap_conditional_dead_built_in_calls
	will always change something.

gcc/testsuite/
	* gcc.dg/torture/pr71020.c: New test.

From-SVN: r236393

gcc/ChangeLog

+2016-05-18  Richard Sandiford  <richard.sandiford@arm.com>
+
+	PR middle-end/71020
+	* tree-dfa.h (replace_abnormal_ssa_names): Declare.
+	* tree-dfa.c (replace_abnormal_ssa_names): New function.
+	* tree-call-cdce.c: Include tree-dfa.h.
+	(can_guard_call_p): New function, extracted from...
+	(can_use_internal_fn): ...here.
+	(shrink_wrap_one_built_in_call_with_conds): Remove failure path
+	and return void.
+	(shrink_wrap_one_built_in_call): Likewise.
+	(use_internal_fn): Likewise.
+	(shrink_wrap_conditional_dead_built_in_calls): Update accordingly
+	and return void.  Call replace_abnormal_ssa_names.
+	(pass_call_cdce::execute): Check can_guard_call_p during the
+	initial walk.  Assume shrink_wrap_conditional_dead_built_in_calls
+	will always change something.
+
 2016-05-18  Martin Jambor  <mjambor@suse.cz>
 
 	PR ipa/70646

gcc/testsuite/ChangeLog

+2016-05-18  Richard Sandiford  <richard.sandiford@arm.com>
+
+	* gcc.dg/torture/pr71020.c: New test.
+
 2016-05-18  Martin Jambor  <mjambor@suse.cz>
 
 	PR ipa/70646

gcc/testsuite/gcc.dg/torture/pr71020.c

+/* { dg-options "-funsafe-math-optimizations" } */
+
+double random_double (void);
+int setjmp (void *);
+void do_something (void);
+
+#define TEST_UNARY(FUNC)			\
+  double					\
+  FUNC##_dead (void *buffer)			\
+  {						\
+    double d = random_double ();		\
+    setjmp (buffer);				\
+    __builtin_##FUNC (d);			\
+    d += 1;					\
+    do_something ();				\
+    return d;					\
+  }						\
+						\
+  double					\
+  FUNC##_live (void *buffer)			\
+  {						\
+    double d = random_double ();		\
+    setjmp (buffer);				\
+    d = __builtin_##FUNC (d);			\
+    do_something ();				\
+    return d;					\
+  }
+
+
+#define TEST_BINARY(FUNC)			\
+  double					\
+  FUNC##_dead (void *buffer)			\
+  {						\
+    double d1 = random_double ();		\
+    double d2 = random_double ();		\
+    setjmp (buffer);				\
+    __builtin_##FUNC (d1, d2);			\
+    d1 += 1;					\
+    d2 += 1;					\
+    do_something ();				\
+    return d1 + d2;				\
+  }						\
+						\
+  double					\
+  FUNC##_live (void *buffer)			\
+  {						\
+    double d1 = random_double ();		\
+    double d2 = random_double ();		\
+    setjmp (buffer);				\
+    d1 = __builtin_##FUNC (d1, d2);		\
+    d2 += 1;					\
+    return d1 + d2;				\
+  }
+
+TEST_UNARY (acos)
+TEST_UNARY (asin)
+TEST_UNARY (asinh)
+TEST_UNARY (atan)
+TEST_UNARY (atanh)
+TEST_UNARY (cos)
+TEST_UNARY (cosh)
+TEST_UNARY (exp)
+TEST_UNARY (expm1)
+TEST_UNARY (exp2)
+TEST_UNARY (exp10)
+TEST_UNARY (log)
+TEST_UNARY (log2)
+TEST_UNARY (log10)
+TEST_UNARY (log1p)
+TEST_UNARY (significand)
+TEST_UNARY (sin)
+TEST_UNARY (sinh)
+TEST_UNARY (sqrt)
+
+TEST_BINARY (fmod)
+TEST_BINARY (remainder)

gcc/tree-call-cdce.c

@@ -35,6 +35,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree-into-ssa.h"
 #include "builtins.h"
 #include "internal-fn.h"
+#include "tree-dfa.h"
 
 
 /* This pass serves two closely-related purposes:
@@ -350,6 +351,15 @@ edom_only_function (gcall *call)
     }
 }
 
+/* Return true if it is structurally possible to guard CALL.  */
+
+static bool
+can_guard_call_p (gimple *call)
+{
+  return (!stmt_ends_bb_p (call)
+	  || find_fallthru_edge (gimple_bb (call)->succs));
+}
+
 /* A helper function to generate gimple statements for one bound
    comparison, so that the built-in function is called whenever
    TCODE <ARG, LBUB> is *false*.  TEMP_NAME1/TEMP_NAME2 are names
@@ -747,11 +757,9 @@ gen_shrink_wrap_conditions (gcall *bi_call, vec<gimple *> conds,
 #define ERR_PROB 0.01
 
 /* Shrink-wrap BI_CALL so that it is only called when one of the NCONDS
-   conditions in CONDS is false.
+   conditions in CONDS is false.  */
 
-   Return true on success, in which case the cfg will have been updated.  */
-
-static bool
+static void
 shrink_wrap_one_built_in_call_with_conds (gcall *bi_call, vec <gimple *> conds,
 					  unsigned int nconds)
 {
@@ -795,11 +803,10 @@ shrink_wrap_one_built_in_call_with_conds (gcall *bi_call, vec <gimple *> conds,
   /* Now find the join target bb -- split bi_call_bb if needed.  */
   if (stmt_ends_bb_p (bi_call))
     {
-      /* If the call must be the last in the bb, don't split the block,
-	 it could e.g. have EH edges.  */
+      /* We checked that there was a fallthrough edge in
+	 can_guard_call_p.  */
       join_tgt_in_edge_from_call = find_fallthru_edge (bi_call_bb->succs);
-      if (join_tgt_in_edge_from_call == NULL)
-        return false;
+      gcc_assert (join_tgt_in_edge_from_call);
       free_dominance_info (CDI_DOMINATORS);
     }
   else
@@ -898,28 +905,19 @@ shrink_wrap_one_built_in_call_with_conds (gcall *bi_call, vec <gimple *> conds,
                " into error conditions.\n",
                LOCATION_FILE (loc), LOCATION_LINE (loc));
     }
-
-  return true;
 }
 
 /* Shrink-wrap BI_CALL so that it is only called when it might set errno
-   (but is always called if it would set errno).
-
-   Return true on success, in which case the cfg will have been updated.  */
+   (but is always called if it would set errno).  */
 
-static bool
+static void
 shrink_wrap_one_built_in_call (gcall *bi_call)
 {
   unsigned nconds = 0;
   auto_vec<gimple *, 12> conds;
   gen_shrink_wrap_conditions (bi_call, conds, &nconds);
-  /* This can happen if the condition generator decides
-     it is not beneficial to do the transformation.  Just
-     return false and do not do any transformation for
-     the call.  */
-  if (nconds == 0)
-    return false;
-  return shrink_wrap_one_built_in_call_with_conds (bi_call, conds, nconds);
+  gcc_assert (nconds != 0);
+  shrink_wrap_one_built_in_call_with_conds (bi_call, conds, nconds);
 }
 
 /* Return true if built-in function call CALL could be implemented using
@@ -933,11 +931,6 @@ can_use_internal_fn (gcall *call)
   if (!gimple_vdef (call))
     return false;
 
-  /* Punt if we can't conditionalize the call.  */
-  basic_block bb = gimple_bb (call);
-  if (stmt_ends_bb_p (call) && !find_fallthru_edge (bb->succs))
-    return false;
-
   /* See whether there is an internal function for this built-in.  */
   if (replacement_internal_fn (call) == IFN_LAST)
     return false;
@@ -951,18 +944,25 @@ can_use_internal_fn (gcall *call)
   return true;
 }
 
-/* Implement built-in function call CALL using an internal function.
-   Return true on success, in which case the cfg will have changed.  */
+/* Implement built-in function call CALL using an internal function.  */
 
-static bool
+static void
 use_internal_fn (gcall *call)
 {
+  /* We'll be inserting another call with the same arguments after the
+     lhs has been set, so prevent any possible coalescing failure from
+     having both values live at once.  See PR 71020.  */
+  replace_abnormal_ssa_names (call);
+
   unsigned nconds = 0;
   auto_vec<gimple *, 12> conds;
   if (can_test_argument_range (call))
+    {
       gen_shrink_wrap_conditions (call, conds, &nconds);
-  if (nconds == 0 && !edom_only_function (call))
-    return false;
+      gcc_assert (nconds != 0);
+    }
+  else
+    gcc_assert (edom_only_function (call));
 
   internal_fn ifn = replacement_internal_fn (call);
   gcc_assert (ifn != IFN_LAST);
@@ -1008,35 +1008,26 @@ use_internal_fn (gcall *call)
 	}
     }
 
-  if (!shrink_wrap_one_built_in_call_with_conds (call, conds, nconds))
-    /* It's too late to back out now.  */
-    gcc_unreachable ();
-  return true;
+  shrink_wrap_one_built_in_call_with_conds (call, conds, nconds);
 }
 
 /* The top level function for conditional dead code shrink
    wrapping transformation.  */
 
-static bool
+static void
 shrink_wrap_conditional_dead_built_in_calls (vec<gcall *> calls)
 {
-  bool changed = false;
   unsigned i = 0;
 
   unsigned n = calls.length ();
-  if (n == 0)
-    return false;
-
   for (; i < n ; i++)
     {
       gcall *bi_call = calls[i];
       if (gimple_call_lhs (bi_call))
-	changed |= use_internal_fn (bi_call);
+	use_internal_fn (bi_call);
       else
-	changed |= shrink_wrap_one_built_in_call (bi_call);
+	shrink_wrap_one_built_in_call (bi_call);
     }
-
-  return changed;
 }
 
 namespace {
@@ -1079,7 +1070,6 @@ pass_call_cdce::execute (function *fun)
 {
   basic_block bb;
   gimple_stmt_iterator i;
-  bool something_changed = false;
   auto_vec<gcall *> cond_dead_built_in_calls;
   FOR_EACH_BB_FN (bb, fun)
     {
@@ -1096,7 +1086,8 @@ pass_call_cdce::execute (function *fun)
 	      && gimple_call_builtin_p (stmt, BUILT_IN_NORMAL)
 	      && (gimple_call_lhs (stmt)
 		  ? can_use_internal_fn (stmt)
-		  : can_test_argument_range (stmt)))
+		  : can_test_argument_range (stmt))
+	      && can_guard_call_p (stmt))
             {
               if (dump_file && (dump_flags & TDF_DETAILS))
                 {
@@ -1114,19 +1105,12 @@ pass_call_cdce::execute (function *fun)
   if (!cond_dead_built_in_calls.exists ())
     return 0;
 
-  something_changed
-    = shrink_wrap_conditional_dead_built_in_calls (cond_dead_built_in_calls);
-
-  if (something_changed)
-    {
+  shrink_wrap_conditional_dead_built_in_calls (cond_dead_built_in_calls);
   free_dominance_info (CDI_POST_DOMINATORS);
   /* As we introduced new control-flow we need to insert PHI-nodes
      for the call-clobbers of the remaining call.  */
   mark_virtual_operands_for_renaming (fun);
   return TODO_update_ssa;
-    }
-
-  return 0;
 }
 
 } // anon namespace

gcc/tree-dfa.c

@@ -823,6 +823,29 @@ stmt_references_abnormal_ssa_name (gimple *stmt)
   return false;
 }
 
+/* If STMT takes any abnormal PHI values as input, replace them with
+   local copies.  */
+
+void
+replace_abnormal_ssa_names (gimple *stmt)
+{
+  ssa_op_iter oi;
+  use_operand_p use_p;
+
+  FOR_EACH_SSA_USE_OPERAND (use_p, stmt, oi, SSA_OP_USE)
+    {
+      tree op = USE_FROM_PTR (use_p);
+      if (TREE_CODE (op) == SSA_NAME && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (op))
+	{
+	  gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
+	  tree new_name = make_ssa_name (TREE_TYPE (op));
+	  gassign *assign = gimple_build_assign (new_name, op);
+	  gsi_insert_before (&gsi, assign, GSI_SAME_STMT);
+	  SET_USE (use_p, new_name);
+	}
+    }
+}
+
 /* Pair of tree and a sorting index, for dump_enumerated_decls.  */
 struct GTY(()) numbered_tree
 {

gcc/tree-dfa.h

@@ -35,6 +35,7 @@ extern tree get_addr_base_and_unit_offset_1 (tree, HOST_WIDE_INT *,
 					     tree (*) (tree));
 extern tree get_addr_base_and_unit_offset (tree, HOST_WIDE_INT *);
 extern bool stmt_references_abnormal_ssa_name (gimple *);
+extern void replace_abnormal_ssa_names (gimple *);
 extern void dump_enumerated_decls (FILE *, int);