Skip to content

Commit

Permalink
middle-end/100394 - avoid DSE/DCE of pure call that throws
Browse files Browse the repository at this point in the history
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  <rguenther@suse.de>

	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.
  • Loading branch information
rguenth committed May 5, 2021
1 parent 61d48b1 commit 8ebf6b9
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 24 deletions.
1 change: 1 addition & 0 deletions gcc/calls.c
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
Expand Down
5 changes: 4 additions & 1 deletion gcc/cfgexpand.c
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
24 changes: 24 additions & 0 deletions gcc/testsuite/g++.dg/torture/pr100382.C
Original file line number Diff line number Diff line change
@@ -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;
}
29 changes: 8 additions & 21 deletions gcc/tree-ssa-dce.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion gcc/tree-ssa-dse.c
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
Expand Down
5 changes: 4 additions & 1 deletion gcc/tree.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 8ebf6b9

Please sign in to comment.