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.


git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@236393 138bc75d-0d04-0410-961f-82ee72b054a4

1 files changed, 44 insertions, 60 deletions

diff --git a/gcc/tree-call-cdce.c b/gcc/tree-call-cdce.c
index 647be3937a3..8df9b08010f 100644
--- a/gcc/tree-call-cdce.c
+++ b/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:
@@ -349,6 +350,15 @@ edom_only_function (gcall *call)
       return false;
     }
 }
+
+/* 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
@@ -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;
+    {
+      gen_shrink_wrap_conditions (call, conds, &nconds);
+      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)
-    {
-      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;
+  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;
 }
 
 } // anon namespace