Message ID | 20140917182041.GT17454@tucnak.redhat.com |
---|---|
State | New |
Headers | show |
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
--- 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); +}