diff mbox

Fix -fcompare-debug failures caused by fixup_noreturn_call (PR debug/63284)

Message ID 20140917182041.GT17454@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Sept. 17, 2014, 6:20 p.m. UTC
Hi!

The following testcase fails on 4.9 branch (shows
undesirable difference in *.optimized dump on the trunk, but
doesn't report -fcompare-debug failure there, and in 4.8 is latent).
The problem is that if we have a noreturn stmt followed by only
debug stmt, fixup_noreturn_call will try to split_block after the
noreturn call, but with -g0 there are never debug stmts and thus
we would not split the block; that can lead to different order of bb
predecessors, different order of PHI args etc.

As nothing after noreturn call really matters, this patch if there are
only debug stmts after noreturn call removes the debug stmts instead of
the undesirable splitting of the block.

Bootstrapped/regtested on x86_64-linux and i686-linux on the trunk and on
the 4.9 branch, ok for trunk/4.9 and eventually 4.8 too?

2014-09-17  Jakub Jelinek  <jakub@redhat.com>

	PR debug/63284
	* tree-cfgcleanup.c (fixup_noreturn_call): Don't split block
	if there are only debug stmts after the noreturn call, instead
	remove the debug stmts.

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


	Jakub

Comments

Richard Biener Sept. 17, 2014, 6:24 p.m. UTC | #1
On September 17, 2014 8:20:41 PM CEST, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>The following testcase fails on 4.9 branch (shows
>undesirable difference in *.optimized dump on the trunk, but
>doesn't report -fcompare-debug failure there, and in 4.8 is latent).
>The problem is that if we have a noreturn stmt followed by only
>debug stmt, fixup_noreturn_call will try to split_block after the
>noreturn call, but with -g0 there are never debug stmts and thus
>we would not split the block; that can lead to different order of bb
>predecessors, different order of PHI args etc.
>
>As nothing after noreturn call really matters, this patch if there are
>only debug stmts after noreturn call removes the debug stmts instead of
>the undesirable splitting of the block.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux on the trunk and
>on
>the 4.9 branch, ok for trunk/4.9 and eventually 4.8 too?

OK everywhere.

Thanks,
Richard.

>
>2014-09-17  Jakub Jelinek  <jakub@redhat.com>
>
>	PR debug/63284
>	* tree-cfgcleanup.c (fixup_noreturn_call): Don't split block
>	if there are only debug stmts after the noreturn call, instead
>	remove the debug stmts.
>
>	* gcc.dg/pr63284.c: New test.
>
>--- gcc/tree-cfgcleanup.c.jj	2014-09-01 13:33:11.000000000 +0200
>+++ gcc/tree-cfgcleanup.c	2014-09-17 11:01:53.448165423 +0200
>@@ -565,7 +565,20 @@ fixup_noreturn_call (gimple stmt)
> 
>   /* First split basic block if stmt is not last.  */
>   if (stmt != gsi_stmt (gsi_last_bb (bb)))
>-    split_block (bb, stmt);
>+    {
>+      if (stmt == gsi_stmt (gsi_last_nondebug_bb (bb)))
>+	{
>+	  /* Don't split if there are only debug stmts
>+	     after stmt, that can result in -fcompare-debug
>+	     failures.  Remove the debug stmts instead,
>+	     they should be all unreachable anyway.  */
>+	  gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
>+	  for (gsi_next (&gsi); !gsi_end_p (gsi); )
>+	    gsi_remove (&gsi, true);
>+	}
>+      else
>+	split_block (bb, stmt);
>+    }
> 
>   changed |= remove_fallthru_edge (bb->succs);
> 
>--- gcc/testsuite/gcc.dg/pr63284.c.jj	2014-09-17 10:56:59.699677715
>+0200
>+++ gcc/testsuite/gcc.dg/pr63284.c	2014-09-17 10:56:39.000000000 +0200
>@@ -0,0 +1,42 @@
>+/* PR debug/63284 */
>+/* { dg-do compile } */
>+/* { dg-options "-O2 -fcompare-debug" } */
>+
>+int a[10], *b, *d, c, f;
>+int fn2 (void);
>+void fn3 (void);
>+void fn4 (int);
>+
>+static int
>+fn1 (int x)
>+{
>+  int e = a[0];
>+  if (e)
>+    return 1;
>+  if (b)
>+    switch (x)
>+      {
>+      case 1:
>+        if (d)
>+          e = fn2 ();
>+        else
>+          fn3 ();
>+        break;
>+      case 0:
>+        if (d)
>+          {
>+            fn3 ();
>+            if (c)
>+              fn4 (1);
>+          }
>+        else
>+          fn4 (0);
>+      }
>+  return e;
>+}
>+
>+void
>+fn6 (void)
>+{
>+  f = fn1 (0);
>+}
>
>	Jakub
diff mbox

Patch

--- gcc/tree-cfgcleanup.c.jj	2014-09-01 13:33:11.000000000 +0200
+++ gcc/tree-cfgcleanup.c	2014-09-17 11:01:53.448165423 +0200
@@ -565,7 +565,20 @@  fixup_noreturn_call (gimple stmt)
 
   /* First split basic block if stmt is not last.  */
   if (stmt != gsi_stmt (gsi_last_bb (bb)))
-    split_block (bb, stmt);
+    {
+      if (stmt == gsi_stmt (gsi_last_nondebug_bb (bb)))
+	{
+	  /* Don't split if there are only debug stmts
+	     after stmt, that can result in -fcompare-debug
+	     failures.  Remove the debug stmts instead,
+	     they should be all unreachable anyway.  */
+	  gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
+	  for (gsi_next (&gsi); !gsi_end_p (gsi); )
+	    gsi_remove (&gsi, true);
+	}
+      else
+	split_block (bb, stmt);
+    }
 
   changed |= remove_fallthru_edge (bb->succs);
 
--- gcc/testsuite/gcc.dg/pr63284.c.jj	2014-09-17 10:56:59.699677715 +0200
+++ gcc/testsuite/gcc.dg/pr63284.c	2014-09-17 10:56:39.000000000 +0200
@@ -0,0 +1,42 @@ 
+/* PR debug/63284 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcompare-debug" } */
+
+int a[10], *b, *d, c, f;
+int fn2 (void);
+void fn3 (void);
+void fn4 (int);
+
+static int
+fn1 (int x)
+{
+  int e = a[0];
+  if (e)
+    return 1;
+  if (b)
+    switch (x)
+      {
+      case 1:
+        if (d)
+          e = fn2 ();
+        else
+          fn3 ();
+        break;
+      case 0:
+        if (d)
+          {
+            fn3 ();
+            if (c)
+              fn4 (1);
+          }
+        else
+          fn4 (0);
+      }
+  return e;
+}
+
+void
+fn6 (void)
+{
+  f = fn1 (0);
+}