On Sat, Jul 10, 2021 at 9:26 AM Roger Sayle <ro...@nextmovesoftware.com> wrote:
>
>
> Hi Richard and Eric,
> Of course, you're both completely right. Rather than argue that
> -fnon-call-exceptions without -fexceptions (and without
> -fdelete-dead-exceptions) has some implicit undocumented semantics,
> trapping instructions should be completely orthogonal to exception
> handling.
>
> This patch adds a new code generation option -fpreserve-traps, the
> (obvious) semantics of which is demonstrated by the expanded test
> case below. The current behaviour of gcc is to eliminate calls
> to may_trap_1, may_trap_2, may_trap_3 etc. from foo, but these are
> all retained with -fpreserve-traps.
>
> Historically, the semantics of -fnon-call-exceptions vs. traps has
> been widely misunderstood, with different levels of optimization
> producing different outcomes, as shown by the impressive list of PRs
> affected by this solution. Hopefully, this new documentation will
> clarify things.
I've reviewed the cited PRs and most of them would have
individual fixes and are not fixed by your patch, though
-fpreserve-traps would offer a workaround in some cases.
Now, -fpreserve-traps follows the unfortunate precedence of
tieing IL semantics to a (global) flag rather than to individual
stmts. I'm not sure -fpreserve-traps is something good to offer
since on its own it looks not too useful and for making use of
it one still needs -fnon-call-exceptions [-fexceptions].
There's still the open question what -fnon-call-exceptions on its
own should do - IMHO it doesn't make sense to allow unwiding
from a trapping memory reference but not from the call it resides
in which means -fnon-call-exceptions should better enable
-fexceptions?
There's also valid points made in some of the PRs (some of which
look like dups of each other) that an asm with memory operands
should be trapping and thus throwing with -fnon-call-exceptions
even when it is not volatile and that some builtin functions like
memcpy should not be nothrow with -fnon-call-exceptions.
There's const-correctness pieces in your patch - those are OK
under the obvious rule and you might want to push them separately.
Thanks,
Richard.
>
> This patch has been tested on x86_64-pc-linux-gnu with a "make
> bootstrap" and "make -k check" with no new failures.
>
> Ok for mainline?
>
>
> 2021-07-09 Roger Sayle <ro...@nextmovesoftware.com>
> Eric Botcazou <ebotca...@adacore.com>
> Richard Biener <rguent...@suse.de>
>
> gcc/ChangeLog
> PR tree-optimization/38943
> PR middle-end/39801
> PR middle-end/64711
> PR target/70387
> PR tree-optimization/94357
> * common.opt (fpreserve-traps): New code generation option.
> * doc/invoke.texi (-fpreserve-traps): Document new option.
> * gimple.c (gimple_has_side_effects): Consider trapping to
> be a side-effect when -fpreserve-traps is specified.
> (gimple_could_trap_p_1): Make S argument a "const gimple*".
> Preserve constness in call to gimple_asm_volatile_p.
> (gimple_could_trap_p): Make S argument a "const gimple*".
> * gimple.h (gimple_could_trap_p_1, gimple_could_trap_p):
> Update function prototypes.
> * ipa-pure-const.c (check_stmt): When preserving traps,
> a trapping statement should be considered a side-effect,
> so the function is neither const nor pure.
>
> gcc/testsuite/ChangeLog
> PR tree-optimization/38943
> PR middle-end/39801
> PR middle-end/64711
> PR target/70387
> PR tree-optimization/94357
> * gcc.dg/pr38943.c: New test case.
>
> --
> Roger Sayle, PhD.
> CEO and founder
> NextMove Software Limited
> Registered in England No. 07588305
> Registered Office: Innovation Centre, 320 Cambridge Science Park, Cambridge,
> CB4 0WG
>
> -----Original Message-----
> From: Richard Biener <richard.guent...@gmail.com>
> Sent: 08 July 2021 11:19
> To: Roger Sayle <ro...@nextmovesoftware.com>; Eric Botcazou
> <botca...@adacore.com>
> Cc: GCC Patches <gcc-patches@gcc.gnu.org>
> Subject: Re: [PATCH] PR tree-optimization/38943: Preserve trapping
> instructions with -fnon-call-exceptions
>
> On Thu, Jul 8, 2021 at 11:54 AM Roger Sayle <ro...@nextmovesoftware.com>
> wrote:
> >
> >
> > This patch addresses PR tree-optimization/38943 where gcc may optimize
> > away trapping instructions even when -fnon-call-exceptions is specified.
> > Interestingly this only affects the C compiler (when -fexceptions is
> > not
> > specified) as g++ (or -fexceptions) supports C++-style exception
> > handling, where -fnon-call-exceptions triggers the stmt_could_throw_p
> > machinery.
> > Without -fexceptions, trapping instructions aren't always considered
> > visible side-effects.
>
> But -fnon-call-exceptions without -fexceptions doesn't make much sense, does
> it? I see the testcase behaves correctly when -fexceptions is also specified.
>
> The call vanishes in DCE because stmt_could_throw_p starts with
>
> bool
> stmt_could_throw_p (function *fun, gimple *stmt) {
> if (!flag_exceptions)
> return false;
>
> the documentation of -fnon-call-exceptions says
>
> Generate code that allows trapping instructions to throw exceptions.
>
> so either the above check is wrong or -fnon-call-exceptions should imply
> -fexceptions (or we should diagnose missing -fexceptions)
>
> >
> > This patch fixes this in two place. Firstly, gimple_has_side_effects
> > is tweaked such that gimple_could_trap_p is considered a side-effect
> > if the current function can throw non-call exceptions.
>
> But exceptions are not considered side-effects - they are explicit in the IL
> and thus passes are supposed to check for those and preserve dead
> (externally) throwing stmts if not told otherwise
> (flag_delete_dead_exceptions).
>
> > And secondly,
> > check_stmt in ipa-pure-const.c is tweaked such that a function
> > containing a trapping statement is considered to have a side-effect
> > with -fnon-call-exceptions, and therefore cannot be pure or const.
>
> EH is orthogonal to pure/const, so I think that's wrong.
>
> > Calling gimple_could_trap_p (which previously took a non-const gimple)
> > from gimple_has_side_effects (which takes a const gimple) required
> > improving the const-safety of gimple_could_trap_p (a relatively minor
> > tweak) and its prototypes. Hopefully this is considered a clean-up/
> > improvement.
>
> Yeah, even an obvious one I think - you can push that independently.
>
> > This patch has been tested on x86_64-pc-linux-gnu with a "make
> > bootstrap" and "make -k check" with no new failures. This should be
> > relatively safe, as there are no changes in behaviour unless the user
> > explicitly specifies -fnon-call-exceptions, when the C compiler then
> > behaves more like the C++/Ada compiler.
> >
> > Ok for mainline?
> >
> >
> > 2021-07-08 Roger Sayle <ro...@nextmovesoftware.com>
> >
> > gcc/ChangeLog
> > PR tree-optimization/38943
> > * gimple.c (gimple_has_side_effects): Consider trapping to
> > be a side-effect when -fnon-call-exceptions is specified.
> > (gimple_coult_trap_p_1): Make S argument a "const gimple*".
> > Preserve constness in call to gimple_asm_volatile_p.
> > (gimple_could_trap_p): Make S argument a "const gimple*".
> > * gimple.h (gimple_could_trap_p_1, gimple_could_trap_p):
> > Update function prototypes.
> > * ipa-pure-const.c (check_stmt): When the current function
> > can throw non-call exceptions, a trapping statement should
> > be considered a side-effect, so the function is neither
> > const nor pure.
> >
> > gcc/testsuite/ChangeLog
> > PR tree-optimization/38943
> > * gcc.dg/pr38943.c: New test case.
> >
> > Roger
> > --
> > Roger Sayle
> > NextMove Software
> > Cambridge, UK
> >
