diff mbox series

Don't DCE const/pure calls that can throw if cfg can't be altered (PR rtl-optimization/88870)

Message ID 20190116224311.GA30353@tucnak
State New
Headers show
Series Don't DCE const/pure calls that can throw if cfg can't be altered (PR rtl-optimization/88870) | expand

Commit Message

Jakub Jelinek Jan. 16, 2019, 10:43 p.m. UTC 
Hi!

For normal instructions, deletable_insn_p has:
  /* Don't delete insns that may throw if we cannot do so.  */
  if (!(cfun->can_delete_dead_exceptions && can_alter_cfg)
      && !insn_nothrow_p (insn))
    return false;

The following patch adds that for the const/pure non-looping calls
that are handled earlier in the function as well (I haven't moved this test
earlier so we don't check insn_nothrow_p on jump insns etc.).

The other change is just to handle those calls the same as non-const/pure,
if we never consider them to be deletable, there is no point in
find_call_stack_args for them, like we don't do that for normal calls.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2019-01-16  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/88870
	* dce.c (deletable_insn_p): Never delete const/pure calls that can
	throw if we can't alter the cfg or delete dead exceptions.
	(mark_insn): Don't call find_call_stack_args for such calls.

	* gcc.dg/pr88870.c: New test.



	Jakub

Comments

Jeff Law Jan. 16, 2019, 10:48 p.m. UTC | #1 
On 1/16/19 3:43 PM, Jakub Jelinek wrote:
> Hi!
> 
> For normal instructions, deletable_insn_p has:
>   /* Don't delete insns that may throw if we cannot do so.  */
>   if (!(cfun->can_delete_dead_exceptions && can_alter_cfg)
>       && !insn_nothrow_p (insn))
>     return false;
> 
> The following patch adds that for the const/pure non-looping calls
> that are handled earlier in the function as well (I haven't moved this test
> earlier so we don't check insn_nothrow_p on jump insns etc.).
> 
> The other change is just to handle those calls the same as non-const/pure,
> if we never consider them to be deletable, there is no point in
> find_call_stack_args for them, like we don't do that for normal calls.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2019-01-16  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR rtl-optimization/88870
> 	* dce.c (deletable_insn_p): Never delete const/pure calls that can
> 	throw if we can't alter the cfg or delete dead exceptions.
> 	(mark_insn): Don't call find_call_stack_args for such calls.
> 
> 	* gcc.dg/pr88870.c: New test.
OK.  Though I wonder if we want to continue to support
-fnon-call-exceptions.  With GCJ gone is there any value left in that
capability?  There's little doubt in my mind other parts of GCC are not
-fnon-call-exception safe.

jeff
Jakub Jelinek Jan. 16, 2019, 10:51 p.m. UTC | #2 
On Wed, Jan 16, 2019 at 03:48:07PM -0700, Jeff Law wrote:
> > 2019-01-16  Jakub Jelinek  <jakub@redhat.com>
> > 
> > 	PR rtl-optimization/88870
> > 	* dce.c (deletable_insn_p): Never delete const/pure calls that can
> > 	throw if we can't alter the cfg or delete dead exceptions.
> > 	(mark_insn): Don't call find_call_stack_args for such calls.
> > 
> > 	* gcc.dg/pr88870.c: New test.
> OK.  Though I wonder if we want to continue to support
> -fnon-call-exceptions.  With GCJ gone is there any value left in that
> capability?  There's little doubt in my mind other parts of GCC are not
> -fnon-call-exception safe.

AFAIK Ada and Go use -fnon-call-exceptions by default and heavily rely on
it.

	Jakub
Arnaud Charlet Jan. 17, 2019, 7:59 a.m. UTC | #3 
> > > 	PR rtl-optimization/88870
> > > 	* dce.c (deletable_insn_p): Never delete const/pure calls that can
> > > 	throw if we can't alter the cfg or delete dead exceptions.
> > > 	(mark_insn): Don't call find_call_stack_args for such calls.
> > > 
> > > 	* gcc.dg/pr88870.c: New test.
> > OK.  Though I wonder if we want to continue to support
> > -fnon-call-exceptions.  With GCJ gone is there any value left in that
> > capability?  There's little doubt in my mind other parts of GCC are not
> > -fnon-call-exception safe.
> 
> AFAIK Ada and Go use -fnon-call-exceptions by default and heavily rely on
> it.

Agreed.
Eric Botcazou Jan. 17, 2019, 11:27 a.m. UTC | #4 
> AFAIK Ada and Go use -fnon-call-exceptions by default and heavily rely on
> it.

Right, the Ada compiler uses -fnon-call-exceptions -fdelete-dead-exceptions 
and ACATS could not be passed with optimization enabled without the former.

Btw, there are a couple of improper direct uses of flag_non_call_exceptions in 
the sources, may I change them to cfun->can_throw_non_call_exceptions?

gimple-ssa-isolate-paths.c:      if (!flag_non_call_exceptions)
gimple-ssa-isolate-paths.c:  if (!flag_non_call_exceptions
tree-ssa-alias.c:  if (flag_non_call_exceptions && pi->pt.null)
Jakub Jelinek Jan. 17, 2019, 11:33 a.m. UTC | #5 
On Thu, Jan 17, 2019 at 12:27:31PM +0100, Eric Botcazou wrote:
> > AFAIK Ada and Go use -fnon-call-exceptions by default and heavily rely on
> > it.
> 
> Right, the Ada compiler uses -fnon-call-exceptions -fdelete-dead-exceptions 
> and ACATS could not be passed with optimization enabled without the former.
> 
> Btw, there are a couple of improper direct uses of flag_non_call_exceptions in 
> the sources, may I change them to cfun->can_throw_non_call_exceptions?

Yes.

> gimple-ssa-isolate-paths.c:      if (!flag_non_call_exceptions)
> gimple-ssa-isolate-paths.c:  if (!flag_non_call_exceptions
> tree-ssa-alias.c:  if (flag_non_call_exceptions && pi->pt.null)

	Jakub
diff mbox series

Patch

--- gcc/dce.c.jj	2019-01-01 12:37:21.308906844 +0100
+++ gcc/dce.c	2019-01-16 11:39:13.432604633 +0100
@@ -108,7 +108,10 @@  deletable_insn_p (rtx_insn *insn, bool f
       /* We can delete dead const or pure calls as long as they do not
          infinite loop.  */
       && (RTL_CONST_OR_PURE_CALL_P (insn)
-	  && !RTL_LOOPING_CONST_OR_PURE_CALL_P (insn)))
+	  && !RTL_LOOPING_CONST_OR_PURE_CALL_P (insn))
+      /* Don't delete calls that may throw if we cannot do so.  */
+      && ((cfun->can_delete_dead_exceptions && can_alter_cfg)
+	  || insn_nothrow_p (insn)))
     return find_call_stack_args (as_a <rtx_call_insn *> (insn), false,
 				 fast, arg_stores);
 
@@ -201,7 +204,9 @@  mark_insn (rtx_insn *insn, bool fast)
 	  && !df_in_progress
 	  && !SIBLING_CALL_P (insn)
 	  && (RTL_CONST_OR_PURE_CALL_P (insn)
-	      && !RTL_LOOPING_CONST_OR_PURE_CALL_P (insn)))
+	      && !RTL_LOOPING_CONST_OR_PURE_CALL_P (insn))
+	  && ((cfun->can_delete_dead_exceptions && can_alter_cfg)
+	      || insn_nothrow_p (insn)))
 	find_call_stack_args (as_a <rtx_call_insn *> (insn), true, fast, NULL);
     }
 }
--- gcc/testsuite/gcc.dg/pr88870.c.jj	2019-01-16 11:25:01.434552788 +0100
+++ gcc/testsuite/gcc.dg/pr88870.c	2019-01-16 11:24:48.172769383 +0100
@@ -0,0 +1,23 @@ 
+/* PR rtl-optimization/88870 */
+/* { dg-do compile } */
+/* { dg-options "-O1 -fexceptions -fnon-call-exceptions -ftrapv -fno-tree-dominator-opts" } */
+
+int a, b;
+
+void
+foo (int *x)
+{
+  int c = 0;
+  {
+    int d;
+    x = &c;
+    for (;;)
+      {
+        x = &d;
+        b = 0;
+        d = c + 1;
+        b = c = 1;
+        ++a;
+      }
+  }
+}