From 8ebf6b99952ada09bf9ea0144dcd1d46363b0464 Mon Sep 17 00:00:00 2001 From: Richard Biener Date: Mon, 3 May 2021 15:11:28 +0200 Subject: [PATCH] middle-end/100394 - avoid DSE/DCE of pure call that throws There is -fdelete-dead-exceptions now and we're tracking nothrow and const/pure bits separately and I do remember that const or pure does _not_ imply nothrow. Now, in the light of the PR100382 fix which added a stmt_unremovable_because_of_non_call_eh_p guard to DSEs "DCE" I wondered how -fdelete-dead-exceptions applies to calls and whether stmt_unremovable_because_of_non_call_eh_p doing return (fun->can_throw_non_call_exceptions && !fun->can_delete_dead_exceptions && stmt_could_throw_p (fun, stmt)); really should conditionalize itself on fun->can_throw_non_call_exceptions. In fact DCE happily elides pure function calls that throw without a LHS (probably a consistency bug). The following testcase shows this: int x, y; int __attribute__((pure,noinline)) foo () { if (x) throw 1; return y; } int main() { int a[2]; x = 1; try { int res = foo (); a[0] = res; } catch (...) { return 0; } return 1; } note that if you wrap foo () into another noinline wrap_foo () { foo (); return 1; } function then we need to make sure to not DCE this call either even though it only throws externally. 2021-05-03 Richard Biener PR middle-end/100394 * calls.c (expand_call): Preserve possibly throwing calls. * cfgexpand.c (expand_call_stmt): When a call can throw signal RTL expansion there are side-effects. * tree-ssa-dce.c (mark_stmt_if_obviously_necessary): Simplify, mark all possibly throwing stmts necessary unless we can elide dead EH. * tree-ssa-dse.c (pass_dse::execute): Preserve exceptions unless -fdelete-dead-exceptions. * tree.h (DECL_PURE_P): Add note about exceptions. * g++.dg/torture/pr100382.C: New testcase. --- gcc/calls.c | 1 + gcc/cfgexpand.c | 5 ++++- gcc/testsuite/g++.dg/torture/pr100382.C | 24 ++++++++++++++++++++ gcc/tree-ssa-dce.c | 29 +++++++------------------ gcc/tree-ssa-dse.c | 3 ++- gcc/tree.h | 5 ++++- 6 files changed, 43 insertions(+), 24 deletions(-) create mode 100644 gcc/testsuite/g++.dg/torture/pr100382.C diff --git a/gcc/calls.c b/gcc/calls.c index 883d08ba5f280..f3da1839dc5a3 100644 --- a/gcc/calls.c +++ b/gcc/calls.c @@ -3808,6 +3808,7 @@ expand_call (tree exp, rtx target, int ignore) side-effects. */ if ((flags & (ECF_CONST | ECF_PURE)) && (!(flags & ECF_LOOPING_CONST_OR_PURE)) + && (flags & ECF_NOTHROW) && (ignore || target == const0_rtx || TYPE_MODE (rettype) == VOIDmode)) { diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c index a6b48d3e48f0f..556a0b22ed6aa 100644 --- a/gcc/cfgexpand.c +++ b/gcc/cfgexpand.c @@ -2795,7 +2795,10 @@ expand_call_stmt (gcall *stmt) CALL_EXPR_ARG (exp, i) = arg; } - if (gimple_has_side_effects (stmt)) + if (gimple_has_side_effects (stmt) + /* ??? Downstream in expand_expr_real_1 we assume that expressions + w/o side-effects do not throw so work around this here. */ + || stmt_could_throw_p (cfun, stmt)) TREE_SIDE_EFFECTS (exp) = 1; if (gimple_call_nothrow_p (stmt)) diff --git a/gcc/testsuite/g++.dg/torture/pr100382.C b/gcc/testsuite/g++.dg/torture/pr100382.C new file mode 100644 index 0000000000000..ffc4182cfceac --- /dev/null +++ b/gcc/testsuite/g++.dg/torture/pr100382.C @@ -0,0 +1,24 @@ +// { dg-do run } + +int x, y; +int __attribute__((pure,noinline)) foo () { if (x) throw 1; return y; } + +int __attribute__((noinline)) bar() +{ + int a[2]; + x = 1; + try { + int res = foo (); + a[0] = res; + } catch (...) { + return 0; + } + return 1; +} + +int main() +{ + if (bar ()) + __builtin_abort (); + return 0; +} diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c index 096cfc8721dbd..c091868a31352 100644 --- a/gcc/tree-ssa-dce.c +++ b/gcc/tree-ssa-dce.c @@ -199,14 +199,6 @@ mark_operand_necessary (tree op) static void mark_stmt_if_obviously_necessary (gimple *stmt, bool aggressive) { - /* With non-call exceptions, we have to assume that all statements could - throw. If a statement could throw, it can be deemed necessary. */ - if (stmt_unremovable_because_of_non_call_eh_p (cfun, stmt)) - { - mark_stmt_necessary (stmt, true); - return; - } - /* Statements that are implicitly live. Most function calls, asm and return statements are required. Labels and GIMPLE_BIND nodes are kept because they are control flow, and we have no way of @@ -250,14 +242,6 @@ mark_stmt_if_obviously_necessary (gimple *stmt, bool aggressive) && DECL_IS_REPLACEABLE_OPERATOR_NEW_P (callee)) return; - /* Most, but not all function calls are required. Function calls that - produce no result and have no side effects (i.e. const pure - functions) are unnecessary. */ - if (gimple_has_side_effects (stmt)) - { - mark_stmt_necessary (stmt, true); - return; - } /* IFN_GOACC_LOOP calls are necessary in that they are used to represent parameter (i.e. step, bound) of a lowered OpenACC partitioned loop. But this kind of partitioned loop might not @@ -269,8 +253,6 @@ mark_stmt_if_obviously_necessary (gimple *stmt, bool aggressive) mark_stmt_necessary (stmt, true); return; } - if (!gimple_call_lhs (stmt)) - return; break; } @@ -312,19 +294,24 @@ mark_stmt_if_obviously_necessary (gimple *stmt, bool aggressive) /* If the statement has volatile operands, it needs to be preserved. Same for statements that can alter control flow in unpredictable ways. */ - if (gimple_has_volatile_ops (stmt) || is_ctrl_altering_stmt (stmt)) + if (gimple_has_side_effects (stmt) || is_ctrl_altering_stmt (stmt)) { mark_stmt_necessary (stmt, true); return; } - if (stmt_may_clobber_global_p (stmt)) + /* If a statement could throw, it can be deemed necessary unless we + are allowed to remove dead EH. Test this after checking for + new/delete operators since we always elide their EH. */ + if (!cfun->can_delete_dead_exceptions + && stmt_could_throw_p (cfun, stmt)) { mark_stmt_necessary (stmt, true); return; } - if (gimple_vdef (stmt) && keep_all_vdefs_p ()) + if ((gimple_vdef (stmt) && keep_all_vdefs_p ()) + || stmt_may_clobber_global_p (stmt)) { mark_stmt_necessary (stmt, true); return; diff --git a/gcc/tree-ssa-dse.c b/gcc/tree-ssa-dse.c index d7cf747702850..63c876a1ff245 100644 --- a/gcc/tree-ssa-dse.c +++ b/gcc/tree-ssa-dse.c @@ -1220,7 +1220,8 @@ pass_dse::execute (function *fun) if (has_zero_uses (DEF_FROM_PTR (def_p)) && !gimple_has_side_effects (stmt) && !is_ctrl_altering_stmt (stmt) - && !stmt_unremovable_because_of_non_call_eh_p (cfun, stmt)) + && (!stmt_could_throw_p (fun, stmt) + || fun->can_delete_dead_exceptions)) { if (dump_file && (dump_flags & TDF_DETAILS)) { diff --git a/gcc/tree.h b/gcc/tree.h index 5a609b98b4c65..6d3cfc4c588e9 100644 --- a/gcc/tree.h +++ b/gcc/tree.h @@ -3133,7 +3133,10 @@ set_function_decl_type (tree decl, function_decl_type t, bool set) (FUNCTION_DECL_CHECK (NODE)->function_decl.returns_twice_flag) /* Nonzero in a FUNCTION_DECL means this function should be treated - as "pure" function (like const function, but may read global memory). */ + as "pure" function (like const function, but may read global memory). + Note that being pure or const for a function is orthogonal to being + nothrow, i.e. it is valid to have DECL_PURE_P set and TREE_NOTHROW + cleared. */ #define DECL_PURE_P(NODE) (FUNCTION_DECL_CHECK (NODE)->function_decl.pure_flag) /* Nonzero only if one of TREE_READONLY or DECL_PURE_P is nonzero AND