On Tue, 4 May 2021, Eric Botcazou wrote:
> > Do you have extra fixes in your tree or does -fnon-call-exceptions
> > somehow magically paper over the issue?
>
> This optimization is valid in Ada for pure functions. RM 10.2.1(18/3) says:
> "If a library unit is declared pure, then the implementation is permitted to
> omit a call on a library-level subprogram of the library unit if the results
> are not needed after the call.(...)[This permission applies even if the
> subprogram produces other side effects when called.]".
>
> > I suppose if the C or C++ frontends like to have pure/const attributed
> > functions not throw they could mark functions accordingly themselves.
> >
> > Which also means, the Ada functions are DECL_PURE_P but not 'pure'
> > according to the extend.texi documentation of the user-visible
> > attribute? That said, I think if my C++ testcase is valid then we
> > should amend this documentation (or if not then as well, to not
> > re-iterate this every N years). Do you agree?
>
> Yes, the documentation was written without considering other languages with
> exception handling, but it's originally an extension of the C language and
> documented in the manual of the C compiler, so that's not very surprising.
>
> Pure Ada functions are "const" in the GNU C sense if all their parameters are
> passed by copy and "pure" in the GNU C sense if their parameters not passed
> by value (i.e. by reference) are In parameters; in all the other cases, pure
> Ada functions are neither "const" nor "pure" in the GNU C sense.
>
> I think that we need to add an explicit sentence about exception handling to
> the declaration of DECL_PURE_P:
>
> /* Nonzero in a FUNCTION_DECL means this function should be treated
> as "pure" function (like const function, but may read global memory). */
> #define DECL_PURE_P(NODE) (FUNCTION_DECL_CHECK (NODE)>function_decl.pure_flag)
>
> Maybe: "Note that being pure or const for a function is orthogonal to being
> no-throw, i.e. it is valid to have DECL_PURE_P set and TREE_NOTHROW cleared".
Good idea.
So the reason it doesn't "break" with Ada is because with Ada we are
allowed to elide the EH.
I'm testing the revised patch below which also prepares for a
PR100409 fix in the C++ FE (the DCE hunk). I hope any Ada fallout
is catched by the testsuite.
Better ideas for the expand_call / expand_call_stmt kludges appreciated.
Richard.
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 <rguent...@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.
---
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 883d08ba5f2..f3da1839dc5 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 a6b48d3e48f..556a0b22ed6 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 00000000000..ffc4182cfce
--- /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 096cfc8721d..c091868a313 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 d7cf7477028..63c876a1ff2 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 5a609b98b4c..6d3cfc4c588 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
--
2.26.2
