On Wed, May 18, 2016 at 11:10 AM, Richard Sandiford <richard.sandif...@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)