[PATCH] tree-optimization/104263 - avoid retaining abnormal edges for non-call/goto stmts

[PATCH] tree-optimization/104263 - avoid retaining abnormal edges for non-call/goto stmts

[Richard Biener]

This removes a premature optimization from
gimple_purge_dead_abnormal_call_edges which, after eliding the
last setjmp (or computed goto) statement from a function and
thus clearing cfun->calls_setjmp, leaves us with the abnormal
edges from other calls that are elided for example via inlining
or DCE.  That's a CFG / IL combination that should be impossible
(not addressing the fact that with cfun->calls_setjmp and
cfun->has_nonlocal_label cleared we should not have any abnormal
edge at all).

For the testcase in the PR this means that IPA inlining will
remove the abormal edges from the block after inlining the call
the edge was coming from.

Bootstrap / regtest running on x86_64-unknown-linux-gnu.

2022-01-28  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/104263
	* tree-cfg.cc (gimple_purge_dead_abnormal_call_edges):
	Purge edges also when !cfun->has_nonlocal_label
	and !cfun->calls_setjmp.

	* gcc.dg/tree-ssa/inline-13.c: New testcase.
---
 gcc/testsuite/gcc.dg/tree-ssa/inline-13.c | 27 +++++++++++++++++++++++
 gcc/tree-cfg.cc                           |  4 ----
 2 files changed, 27 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/inline-13.c

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/inline-13.c b/gcc/testsuite/gcc.dg/tree-ssa/inline-13.c
new file mode 100644
index 00000000000..94d8a9c709e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/inline-13.c
@@ -0,0 +1,27 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-early-inlining -fdump-tree-fixup_cfg3" } */
+
+int n;
+
+static int
+bar (void)
+{
+  int a;
+
+  n = 0;
+  a = 0;
+
+  return n;
+}
+
+__attribute__ ((pure, returns_twice)) int
+foo (void)
+{
+  n = bar () + 1;
+  foo ();
+
+  return 0;
+}
+
+/* Abnormal edges should be properly elided after IPA inlining of bar.  */
+/* { dg-final { scan-tree-dump-times "bb" 1 "fixup_cfg3" } } */
diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
index 2340cd7cef0..260a7fb97c6 100644
--- a/gcc/tree-cfg.cc
+++ b/gcc/tree-cfg.cc
@@ -8916,10 +8916,6 @@ gimple_purge_dead_abnormal_call_edges (basic_block bb)
   edge_iterator ei;
   gimple *stmt = last_stmt (bb);
 
-  if (!cfun->has_nonlocal_label
-      && !cfun->calls_setjmp)
-    return false;
-
   if (stmt && stmt_can_make_abnormal_goto (stmt))
     return false;
 
-- 
2.31.1

Re: [PATCH] tree-optimization/104263 - avoid retaining abnormal edges for non-call/goto stmts

[Jakub Jelinek]

On Fri, Jan 28, 2022 at 11:29:38AM +0100, Richard Biener wrote:
> This removes a premature optimization from
> gimple_purge_dead_abnormal_call_edges which, after eliding the
> last setjmp (or computed goto) statement from a function and
> thus clearing cfun->calls_setjmp, leaves us with the abnormal
> edges from other calls that are elided for example via inlining
> or DCE.  That's a CFG / IL combination that should be impossible
> (not addressing the fact that with cfun->calls_setjmp and
> cfun->has_nonlocal_label cleared we should not have any abnormal
> edge at all).
> 
> For the testcase in the PR this means that IPA inlining will
> remove the abormal edges from the block after inlining the call
> the edge was coming from.

Couldn't DCE when it clears calls_setjmp and doesn't set it again
(I think we never clear has_nonlocal_label) temporarily set
calls_setjmp and gimple_purge_all_dead_abnormal_call_edges
with it?
Or have next to calls_setjmp a maybe_calls_setjmp flag that
would be sticky like has_nonlocal_labels and would be never cleared?

	Jakub

Re: [PATCH] tree-optimization/104263 - avoid retaining abnormal edges for non-call/goto stmts

[Richard Biener]

On Fri, 28 Jan 2022, Jakub Jelinek wrote:

> On Fri, Jan 28, 2022 at 11:29:38AM +0100, Richard Biener wrote:
> > This removes a premature optimization from
> > gimple_purge_dead_abnormal_call_edges which, after eliding the
> > last setjmp (or computed goto) statement from a function and
> > thus clearing cfun->calls_setjmp, leaves us with the abnormal
> > edges from other calls that are elided for example via inlining
> > or DCE.  That's a CFG / IL combination that should be impossible
> > (not addressing the fact that with cfun->calls_setjmp and
> > cfun->has_nonlocal_label cleared we should not have any abnormal
> > edge at all).
> > 
> > For the testcase in the PR this means that IPA inlining will
> > remove the abormal edges from the block after inlining the call
> > the edge was coming from.
> 
> Couldn't DCE when it clears calls_setjmp and doesn't set it again
> (I think we never clear has_nonlocal_label) temporarily set
> calls_setjmp and gimple_purge_all_dead_abnormal_call_edges
> with it?
> Or have next to calls_setjmp a maybe_calls_setjmp flag that
> would be sticky like has_nonlocal_labels and would be never cleared?

I suppose we could do things like this.  Note in CFG cleanup
we call gimple_purge_dead_eh_edges on each block but not
gimple_purge_dead_abnormal_call_edges.  DCE, when resetting the
flag could also manually axe all abnormal edges in the function.

Still I think assuming there are no abnormal edges when neither
of the flag is set is premature (as can be seen here).  I also
don't think what we do in the function is very timing critical,
but sure, we walk all successor edges.

uninit has interesting code checking ->calls_setjmp conditionally
ignoring abnormal SSA names but only then ... (it should be
able to ignore them when _none_ of the flags is set instead).

That said, gimple_purge_dead_abnormal_call_edges wants to check
"are there possibly any abnormal edges in the function" and clearly
testing just the flags doesn't do it but resetting the flag was
important enough to cut out (sometimes bogus?) checks before
optimizations.

Richard.

Re: [PATCH] tree-optimization/104263 - avoid retaining abnormal edges for non-call/goto stmts

[Jakub Jelinek]

On Fri, Jan 28, 2022 at 12:04:00PM +0100, Richard Biener wrote:
> Still I think assuming there are no abnormal edges when neither
> of the flag is set is premature (as can be seen here).  I also
> don't think what we do in the function is very timing critical,
> but sure, we walk all successor edges.

Ok then.

	Jakub

Re: [PATCH] tree-optimization/104263 - avoid retaining abnormal edges for non-call/goto stmts

[Richard Biener]

On Fri, 28 Jan 2022, Jakub Jelinek wrote:

> On Fri, Jan 28, 2022 at 12:04:00PM +0100, Richard Biener wrote:
> > Still I think assuming there are no abnormal edges when neither
> > of the flag is set is premature (as can be seen here).  I also
> > don't think what we do in the function is very timing critical,
> > but sure, we walk all successor edges.
> 
> Ok then.

Just to add - gimple_purge_dead_abnormal_call_edges should only
be called if the caller determined a possible change.  I've checked
and only fixup_cfg calls it unconditionally (I guess on purpose).

Richard.