[PATCH take 2] PR tree-optimization/38943: Preserve trapping instructions with -fpreserve-traps

[Roger Sayle]

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.

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  <roger@nextmovesoftware.com>
	    Eric Botcazou  <ebotcazou@adacore.com>
	    Richard Biener  <rguenther@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.guenther@gmail.com> 
Sent: 08 July 2021 11:19
To: Roger Sayle <roger@nextmovesoftware.com>; Eric Botcazou <botcazou@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 <roger@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  <roger@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
>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: patche6.txt
URL: <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20210710/85f513b7/attachment-0001.txt>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: pr38943.c
URL: <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20210710/85f513b7/attachment-0001.c>