From cf8be00de9a9e31a2f05894d429fe38948a0c255 Mon Sep 17 00:00:00 2001 From: Richard Sandiford Date: Wed, 18 May 2016 14:01:31 +0000 Subject: [PATCH] To... To: gcc-patches@gcc.gnu.org Subject: PR 71020: Handle abnormal PHIs in tree-call-cdce.c From: Richard Sandiford 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 | 18 +++++ gcc/testsuite/ChangeLog | 4 + gcc/testsuite/gcc.dg/torture/pr71020.c | 76 ++++++++++++++++++ gcc/tree-call-cdce.c | 104 +++++++++++-------------- gcc/tree-dfa.c | 23 ++++++ gcc/tree-dfa.h | 1 + 6 files changed, 166 insertions(+), 60 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/torture/pr71020.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 1f6d29b29d1..4ceebb4789f 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,21 @@ +2016-05-18 Richard Sandiford + + 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 PR ipa/70646 diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index ec993ce7831..d7189e7257d 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,7 @@ +2016-05-18 Richard Sandiford + + * gcc.dg/torture/pr71020.c: New test. + 2016-05-18 Martin Jambor PR ipa/70646 diff --git a/gcc/testsuite/gcc.dg/torture/pr71020.c b/gcc/testsuite/gcc.dg/torture/pr71020.c new file mode 100644 index 00000000000..9b22280b8f4 --- /dev/null +++ b/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) 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 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 conds, unsigned int nconds) { @@ -795,11 +803,10 @@ shrink_wrap_one_built_in_call_with_conds (gcall *bi_call, vec 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 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 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 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 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 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 diff --git a/gcc/tree-dfa.c b/gcc/tree-dfa.c index f6986c1f0ca..9a3b072ae2f 100644 --- a/gcc/tree-dfa.c +++ b/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 { diff --git a/gcc/tree-dfa.h b/gcc/tree-dfa.h index c94dc557796..08864cf470b 100644 --- a/gcc/tree-dfa.h +++ b/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); -- 2.30.2