PR 71020: Handle abnormal PHIs in tree-call-cdce.c

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.  OK to install?

Thanks,
Richard


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.

On Wed, May 18, 2016 at 11:10 AM, Richard Sandiford
<richard.sandiford@arm.com> wrote:
> 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.  OK to install?

Ok.

Thanks,
Richard.

> Thanks,
> Richard
>
>
> 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.
>
> Index: gcc/tree-dfa.h
> ===================================================================
> --- gcc/tree-dfa.h
> +++ 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);
>
>
> Index: gcc/tree-dfa.c
> ===================================================================
> --- gcc/tree-dfa.c
> +++ 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
>  {
> Index: gcc/tree-call-cdce.c
> ===================================================================
> --- gcc/tree-call-cdce.c
> +++ 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
> Index: gcc/testsuite/gcc.dg/torture/pr71020.c
> ===================================================================
> --- /dev/null
> +++ gcc/testsuite/gcc.dg/torture/pr71020.c
> @@ -0,0 +1,76 @@
> +/* { 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)