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  <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
>
diff --git a/gcc/common.opt b/gcc/common.opt
index a1353e0..7988653 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2208,6 +2208,10 @@ fprefetch-loop-arrays
 Common Var(flag_prefetch_loop_arrays) Init(-1) Optimization
 Generate prefetch instructions, if available, for arrays in loops.
 
+fpreserve-traps
+Common Var(flag_preserve_traps)
+Treat trapping instructions as visible side-effects.
+
 fprofile
 Common Var(profile_flag)
 Enable basic program profiling code.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index af2ce18..277eef5 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -655,7 +655,7 @@ Objective-C and Objective-C++ Dialects}.
 @gccoptlist{-fcall-saved-@var{reg}  -fcall-used-@var{reg} @gol
 -ffixed-@var{reg}  -fexceptions @gol
 -fnon-call-exceptions  -fdelete-dead-exceptions  -funwind-tables @gol
--fasynchronous-unwind-tables @gol
+-fasynchronous-unwind-tables -fpreserve-traps @gol
 -fno-gnu-unique @gol
 -finhibit-size-directive  -fcommon  -fno-ident @gol
 -fpcc-struct-return  -fpic  -fPIC  -fpie  -fPIE  -fno-plt @gol
@@ -16262,6 +16262,17 @@ Generate unwind table in DWARF format, if supported by 
target machine.  The
 table is exact at each instruction boundary, so it can be used for stack
 unwinding from asynchronous events (such as debugger or garbage collector).
 
+@item -fpreserve-traps
+@opindex fpreserve-traps
+Prevent trapping instructions that don't otherwise contribute to the
+execution of a program, from being optimized away.  This option is
+orthogonal to exception handling, though the @option{-fnon-call-exceptions}
+option may be used to throw exceptions from a trapping instruction,
+if supported by the target.  The set of instructions considered trapping
+may be controlled with @option{-ftrapping-math} and @option{-ftrapv}.
+This option treats traps as observable side-effects, hence a trapping
+instruction prevents a function from being @code{pure} or @code{const}.
+
 @item -fno-gnu-unique
 @opindex fno-gnu-unique
 @opindex fgnu-unique
diff --git a/gcc/gimple.c b/gcc/gimple.c
index f1044e9..66edc1e 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -2090,7 +2090,8 @@ gimple_move_vops (gimple *new_stmt, gimple *old_stmt)
    statement to have side effects if:
 
    - It is a GIMPLE_CALL not marked with ECF_PURE or ECF_CONST.
-   - Any of its operands are marked TREE_THIS_VOLATILE or TREE_SIDE_EFFECTS.  
*/
+   - Any of its operands are marked TREE_THIS_VOLATILE or TREE_SIDE_EFFECTS.
+   - It may trap and -fpreserve-traps has been specified.  */
 
 bool
 gimple_has_side_effects (const gimple *s)
@@ -2108,6 +2109,12 @@ gimple_has_side_effects (const gimple *s)
       && gimple_asm_volatile_p (as_a <const gasm *> (s)))
     return true;
 
+  /* Treat trapping instructions as having side-effects when
+     -fpreserve_traps has been specified.  */
+  if (flag_preserve_traps
+      && gimple_could_trap_p (s))
+    return true;
+
   if (is_gimple_call (s))
     {
       int flags = gimple_call_flags (s);
@@ -2129,7 +2136,7 @@ gimple_has_side_effects (const gimple *s)
    S is a GIMPLE_ASSIGN, the LHS of the assignment is also checked.  */
 
 bool
-gimple_could_trap_p_1 (gimple *s, bool include_mem, bool include_stores)
+gimple_could_trap_p_1 (const gimple *s, bool include_mem, bool include_stores)
 {
   tree t, div = NULL_TREE;
   enum tree_code op;
@@ -2146,7 +2153,7 @@ gimple_could_trap_p_1 (gimple *s, bool include_mem, bool 
include_stores)
   switch (gimple_code (s))
     {
     case GIMPLE_ASM:
-      return gimple_asm_volatile_p (as_a <gasm *> (s));
+      return gimple_asm_volatile_p (as_a <const gasm *> (s));
 
     case GIMPLE_CALL:
       t = gimple_call_fndecl (s);
@@ -2192,7 +2199,7 @@ gimple_could_trap_p_1 (gimple *s, bool include_mem, bool 
include_stores)
 /* Return true if statement S can trap.  */
 
 bool
-gimple_could_trap_p (gimple *s)
+gimple_could_trap_p (const gimple *s)
 {
   return gimple_could_trap_p_1 (s, true, true);
 }
diff --git a/gcc/gimple.h b/gcc/gimple.h
index e7dc2a4..1a2e120 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -1601,8 +1601,8 @@ void gimple_set_lhs (gimple *, tree);
 gimple *gimple_copy (gimple *);
 void gimple_move_vops (gimple *, gimple *);
 bool gimple_has_side_effects (const gimple *);
-bool gimple_could_trap_p_1 (gimple *, bool, bool);
-bool gimple_could_trap_p (gimple *);
+bool gimple_could_trap_p_1 (const gimple *, bool, bool);
+bool gimple_could_trap_p (const gimple *);
 bool gimple_assign_rhs_could_trap_p (gimple *);
 extern void dump_gimple_statistics (void);
 unsigned get_gimple_rhs_num_ops (enum tree_code);
diff --git a/gcc/ipa-pure-const.c b/gcc/ipa-pure-const.c
index f045108..e2a4bcf 100644
--- a/gcc/ipa-pure-const.c
+++ b/gcc/ipa-pure-const.c
@@ -765,6 +765,14 @@ check_stmt (gimple_stmt_iterator *gsip, funct_state local, 
bool ipa)
       if (dump_file)
        fprintf (dump_file, "    Volatile stmt is not const/pure\n");
     }
+  else if (flag_preserve_traps
+          && gimple_code (stmt) != GIMPLE_CALL
+          && gimple_could_trap_p (stmt))
+    {
+      local->pure_const_state = IPA_NEITHER;
+      if (dump_file)
+       fprintf (dump_file, "    Trapping stmt is not const/pure\n");
+    }
 
   /* Look for loads and stores.  */
   walk_stmt_load_store_ops (stmt, local,
/* { dg-do compile } */
/* { dg-options "-O2 -fpreserve-traps -ftrapv -fdump-tree-optimized" } */

int __attribute__((noinline)) may_trap_1 (int x) {
  return 10/x;
}

int __attribute__((noinline)) may_trap_2 (int *x) {
  return *x;
}

int __attribute__((noinline)) may_trap_3 (int *x) {
  return *x;
}

int __attribute__((noinline)) may_trap_4 (int x) {
  int too_small[5];
  return too_small[x];
}

float __attribute__((noinline)) may_trap_5 (float x) {
  return 2.0f / x;
};

int __attribute__((noinline)) may_trap_6 (int x) {
  return x + x;
};

void foo()
{
  may_trap_1 (0);  // division by zero
  may_trap_2 (0);  // invalid memory access
  may_trap_3 ((int *)1);  // mis-aligned memory access
  may_trap_4 (10);  // out-of-bounds memory access
  may_trap_5 (3.0f);  // floating-point trap
  may_trap_6 ((int)((~0u)>>1));  // integer overflow with -ftrapv
}

/* { dg-final { scan-tree-dump "may_trap_1 \\(0" "optimized" } } */
/* { dg-final { scan-tree-dump "may_trap_2 \\(0" "optimized" } } */
/* { dg-final { scan-tree-dump "may_trap_3 \\(1" "optimized" } } */
/* { dg-final { scan-tree-dump "may_trap_4 \\(10" "optimized" } } */
/* { dg-final { scan-tree-dump "may_trap_5 \\(3" "optimized" } } */
/* { dg-final { scan-tree-dump "may_trap_6 \\(\[1-9\]" "optimized" } } */

Reply via email to