diff mbox series

PR tree-optimization/38943: Preserve trapping instructions with -fnon-call-exceptions

Message ID 002a01d773df$202b5150$6081f3f0$@nextmovesoftware.com
State New
Headers show
Series PR tree-optimization/38943: Preserve trapping instructions with -fnon-call-exceptions | expand

Commit Message

Roger Sayle July 8, 2021, 9:53 a.m. UTC
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.

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.  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.

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.

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

/* { dg-do compile } */
/* { dg-options "-O2 -fnon-call-exceptions -fdump-tree-optimized" } */

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

void bar()
{
  foo (3.0f);
}

/* { dg-final { scan-tree-dump "foo \\(3" "optimized" } } */

Comments

Richard Biener July 8, 2021, 10:18 a.m. UTC | #1
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
>
Eric Botcazou July 8, 2021, 12:54 p.m. UTC | #2
> 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.

I think this will pessimize Ada, which defaults to -fnon-call-exceptions but 
where we do *not* want to preserve trapping instructions just because they may 
trap (i.e. -fdelete-dead-exceptions is enabled by default).

And, as noticed by Richard, EH is orthogonal to side effects and pure/const.
diff mbox series

Patch

diff --git a/gcc/gimple.c b/gcc/gimple.c
index f1044e9..4b150b0 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 -fnon-call-exceptions has been specified.  */
 
 bool
 gimple_has_side_effects (const gimple *s)
@@ -2108,6 +2109,10 @@  gimple_has_side_effects (const gimple *s)
       && gimple_asm_volatile_p (as_a <const gasm *> (s)))
     return true;
 
+  if (cfun->can_throw_non_call_exceptions
+      && gimple_could_trap_p (s))
+    return true;
+
   if (is_gimple_call (s))
     {
       int flags = gimple_call_flags (s);
@@ -2129,7 +2134,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 +2151,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 +2197,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..436cbcd 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 (cfun->can_throw_non_call_exceptions
+	   && 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,