[PATCH] 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; 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. 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?
2021-05-03 Richard Biener
* 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 000..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 (gimp
Re: [PATCH] Avoid DSE/DCE of pure call that throws
> 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...
Honza
>
> 2021-05-03 Richard Biener
>
> * 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 000..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
Re: [PATCH] Avoid DSE/DCE of pure call that throws
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
> >
> > * 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 000..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
> > @@ -2
Re: [PATCH] Avoid DSE/DCE of pure call that throws
> So - what is it? Are pure functions allowed to throw or not? We keep going back to this every N years and I'm not sure why... In Ada the answer is definitely yes, we have entire library units marked Pure and thus containing bunch of pure functions but they can throw like any other function. -- Eric Botcazou
Re: [PATCH] Avoid DSE/DCE of pure call that throws
On Mon, 3 May 2021, Eric Botcazou wrote: > > So - what is it? Are pure functions allowed to throw or not? > > We keep going back to this every N years and I'm not sure why... In Ada the > answer is definitely yes, we have entire library units marked Pure and thus > containing bunch of pure functions but they can throw like any other function. OK, so then we definitely have all the latent issues I paper over with the patch and that isn't even enough. Do you have extra fixes in your tree or does -fnon-call-exceptions somehow magically paper over the issue? 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? Thanks, Richard.
Re: [PATCH] Avoid DSE/DCE of pure call that throws
Hello,
On Mon, 3 May 2021, Jan Hubicka wrote:
> > (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
> >
> > ...
> >
> > 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?
I really want to say that const/pure and throw are mutually exclusive.
But in reality they are orthogonal, so a const function may throw. (It
certainly can in C++).
This is because while we implement exceptions with memory state, that
state in not accessible to the application. Exceptions could for instance
also be implemented via return type extension. And then, as long as other
guarantees of const or pure functions are adhered to (same input -> same
output), throwing an exception from a const function would be completely
natural. E.g. if a const function, given a specific set of arguments,
always throws the same value that would still allow to CSE two calls to it
in a row, and it would still allow to assume that no reachable memory was
changed by that call.
So, I think const and pure functions may validly throw (but then must
always do so given the same inputs).
Ciao,
Michael.
> 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...
>
> Honza
> >
> > 2021-05-03 Richard Biener
> >
> > * 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 000..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
Re: [PATCH] Avoid DSE/DCE of pure call that throws
On Mon, 3 May 2021, Michael Matz wrote:
> Hello,
>
> On Mon, 3 May 2021, Jan Hubicka wrote:
>
> > > (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
> > >
> > > ...
> > >
> > > 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?
>
> I really want to say that const/pure and throw are mutually exclusive.
> But in reality they are orthogonal, so a const function may throw. (It
> certainly can in C++).
>
> This is because while we implement exceptions with memory state, that
> state in not accessible to the application. Exceptions could for instance
> also be implemented via return type extension. And then, as long as other
> guarantees of const or pure functions are adhered to (same input -> same
> output), throwing an exception from a const function would be completely
> natural. E.g. if a const function, given a specific set of arguments,
> always throws the same value that would still allow to CSE two calls to it
> in a row, and it would still allow to assume that no reachable memory was
> changed by that call.
>
> So, I think const and pure functions may validly throw (but then must
> always do so given the same inputs).
I've filed PR100394 since it appearantly never worked (for C++).
Richard.
>
> Ciao,
> Michael.
>
>
> > 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...
> >
> > Honza
> > >
> > > 2021-05-03 Richard Biener
> > >
> > > * 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 000..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-
Re: [PATCH] Avoid DSE/DCE of pure call that throws
> 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". -- Eric Botcazou
Re: [PATCH] Avoid DSE/DCE of pure call that throws
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
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
