On Mon, 3 May 2021, Jan Hubicka wrote:
> > 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. Now the question is whether this testcase is valid
> > (it should not abort). The documentation of 'pure' must be read
> > as that 'pure' is not valid for 'foo' since throwing an exception
> > is obviously an observable effect on the state of the program
> > (in particular for the testcase at hand). But for example
> > IPA pure const does not cause us to infer nothrow on pure
> > declared functions and the C++ program
> >
> > extern int __attribute__((pure)) foo();
> > int bar()
> > {
> > try
> > {
> > return foo ();
> > }
> > catch (...)
> > {
> > return -1;
> > }
> > }
> >
> > is compiled with full EH in place. (clang elides all of it, btw)
> >
> > The set of changes below do not succeed in it passing but at least
> > the throwing call prevails but somehow EH info is hosed and you'll
> > get
> >
> > > ./pr100382.exe
> > terminate called without an active exception
> > Aborted (core dumped)
> >
> > So - what is it? Are pure functions allowed to throw or not?
>
> I was one adding pure functions and it was really intended to be same
> thing as const+reading memory. So does const function throw or not?
> Internally we definitly want to make this separate - with symbol
> summaries it is now reasonably easy to do. I was thinking of doing
> similar summary as modref handles to pass down separate info tracked by
> pure-const patch rather than trying to re-synthetize it to rather random
> attributes we have now...
Yes, we handle 'const' as throwing (well, we don't optimize it as
not throwing...). But since 'const' is a subset of 'pure' functions
if pure functions cannot throw then const functions can not either.
extern int __attribute__((const)) foo();
int bar()
{
try
{
return foo ();
}
catch (...)
{
return -1;
}
}
has EH emitted as well.
Richard.
> Honza
> >
> > 2021-05-03 Richard Biener <[email protected]>
> >
> > * 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.
> > * tree-ssa-dse.c (pass_dse::execute): Preserve exceptions unless
> > -fdelete-dead-exceptions.
> >
> > * g++.dg/tree-ssa/pr100382.C: New testcase.
> > ---
> > gcc/calls.c | 1 +
> > gcc/cfgexpand.c | 5 ++++-
> > gcc/testsuite/g++.dg/tree-ssa/pr100382.C | 25 ++++++++++++++++++++++++
> > gcc/tree-ssa-dce.c | 21 +++-----------------
> > gcc/tree-ssa-dse.c | 3 ++-
> > 5 files changed, 35 insertions(+), 20 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/tree-ssa/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/tree-ssa/pr100382.C
> > b/gcc/testsuite/g++.dg/tree-ssa/pr100382.C
> > new file mode 100644
> > index 00000000000..b9948d3034a
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/tree-ssa/pr100382.C
> > @@ -0,0 +1,25 @@
> > +// { dg-do run }
> > +// { dg-options "-O2" }
> > +
> > +int x, y;
> > +int __attribute__((pure,noinline)) foo () { if (x) throw; 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..ff0389af36f 100644
> > --- a/gcc/tree-ssa-dce.c
> > +++ b/gcc/tree-ssa-dce.c
> > @@ -250,14 +250,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 +261,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 +302,14 @@ 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))
> > - {
> > - mark_stmt_necessary (stmt, true);
> > - return;
> > - }
> > -
> > - if (stmt_may_clobber_global_p (stmt))
> > + if (gimple_has_side_effects (stmt) || is_ctrl_altering_stmt (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 dfa6d314727..386667b9f7f 100644
> > --- a/gcc/tree-ssa-dse.c
> > +++ b/gcc/tree-ssa-dse.c
> > @@ -1219,7 +1219,8 @@ pass_dse::execute (function *fun)
> > dead SSA defs. */
> > if (has_zero_uses (DEF_FROM_PTR (def_p))
> > && !gimple_has_side_effects (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))
> > {
> > --
> > 2.26.2
>
--
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
