On Dec 12, 2017, David Edelsohn <dje....@gmail.com> wrote: > Something in this series broke bootstrap on AIX, probably Power in general.
And probably a number of other platforms as well. Here's a patch that seems to have fixed lots of problems. Without it, we might move debug (bind) stmts from a forwarder block about to be removed, inserting them before the label of a successor block. debug bind stmts before labels are not welcome (they might become insns outside any blocks, and not be properly adjusted; in this specific testcase, a reg lowered to a concatn remained shared because the out-of-block debug insn was not adjusted), and I'm reconsidering even debug markers, but for now, I'm leaving markers before and among labels. To fix the problem, I've rearranged the forwarder block remover to insert stmts that were before or among labels before labels, but those that are after labels (or in a block without labels) are inserted after any labels in the destination block. This should keep debug insns in order, except when the forwarder block has no labels, whereas the successor block had markers before the labels. I'm undecided between moving all markers in the successor after any labels in it, before making other changes, or to rule out markers before or among labels altogether. For now, to restore bootstrap and builds on most platforms, this will do. There are still unrelated -fcompare-debug issues in the supplied AIX testcase, but Jakub's reduced testcase passes even -fcompare-debug. Regstrapping on x86_64-linux-gnu and i686-linux-gnu. Hopefully I'll have feedback about its bootstrap on powerpc-aix and sparc-solaris as well before it goes in. Ok to install? [SFN] don't move after-label debug stmts before labels When removing a forwarder block (that contains debug stmts), be careful to not insert before labels debug stmts that appear after labels. for gcc/ChangeLog PR bootstrap/83396 PR debug/83391 * tree-cfgcleanup.c (remove_forwarder_block): Keep after labels debug stmts appearing after labels. for gcc/testsuite/ChangeLog PR bootstrap/83396 PR debug/83391 * gcc.dg/torture/pr83396.c: New. * g++.dg/torture/pr83391.C: New. --- gcc/testsuite/g++.dg/torture/pr83391.C | 34 +++++++++++++++++++++++++++++ gcc/testsuite/gcc.dg/torture/pr83396.c | 37 ++++++++++++++++++++++++++++++++ gcc/tree-cfgcleanup.c | 29 ++++++++++++++++++++++--- 3 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/g++.dg/torture/pr83391.C create mode 100644 gcc/testsuite/gcc.dg/torture/pr83396.c diff --git a/gcc/testsuite/g++.dg/torture/pr83391.C b/gcc/testsuite/g++.dg/torture/pr83391.C new file mode 100644 index 000000000000..098a1101a3f4 --- /dev/null +++ b/gcc/testsuite/g++.dg/torture/pr83391.C @@ -0,0 +1,34 @@ +/* PR debug/83391 */ +/* { dg-do compile } */ + +unsigned char a; +enum E { F, G, H } b; +int c, d; + +void +foo () +{ + int e; + bool f; + E g = b; + while (1) + { + unsigned char h = a ? d : 0; + switch (g) + { + case 0: + f = h <= 'Z' || h >= 'a' && h <= 'z'; + break; + case 1: + { + unsigned char i = h; + e = 0; + } + if (e || h) + g = H; + /* FALLTHRU */ + default: + c = 0; + } + } +} diff --git a/gcc/testsuite/gcc.dg/torture/pr83396.c b/gcc/testsuite/gcc.dg/torture/pr83396.c new file mode 100644 index 000000000000..4c0c30ceaab3 --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/pr83396.c @@ -0,0 +1,37 @@ +/* PR bootstrap/83396 */ +/* { dg-do compile } */ + +int fn1 (void); +void fn2 (void *, const char *); +void fn3 (void); + +void +fn4 (long long x) +{ + fn3 (); +} + +void +fn5 (long long x) +{ + if (x) + fn3(); +} + +void +fn6 (long long x) +{ + switch (fn1 ()) + { + case 0: + fn5 (x); + case 2: + fn2 (0, ""); + break; + case 1: + case 3: + fn4(x); + case 5: + fn2 (0, ""); + } +} diff --git a/gcc/tree-cfgcleanup.c b/gcc/tree-cfgcleanup.c index 0bee21756f2b..e30a93d504b6 100644 --- a/gcc/tree-cfgcleanup.c +++ b/gcc/tree-cfgcleanup.c @@ -536,14 +536,23 @@ remove_forwarder_block (basic_block bb) defined labels and labels with an EH landing pad number to the new block, so that the redirection of the abnormal edges works, jump targets end up in a sane place and debug information for - labels is retained. */ + labels is retained. + + While at that, move any debug stmts that appear before or among + labels, but not those that can only appear after labels, unless + the destination block didn't have labels of its own. */ gsi_to = gsi_start_bb (dest); - for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); ) + gsi = gsi_start_bb (bb); + gimple_stmt_iterator gsie = gsi_after_labels (bb); + gimple_stmt_iterator gsie_to = gsi_after_labels (dest); + bool can_move_early_debug_stmts = can_move_debug_stmts + && (gsi_stmt (gsi) != gsi_stmt (gsie) || gsi_stmt (gsi_to) != gsi_stmt (gsie_to)); + while (gsi_stmt (gsi) != gsi_stmt (gsie)) { tree decl; label = gsi_stmt (gsi); if (is_gimple_debug (label) - ? can_move_debug_stmts + ? can_move_early_debug_stmts : ((decl = gimple_label_label (as_a <glabel *> (label))), EH_LANDING_PAD_NR (decl) != 0 || DECL_NONLOCAL (decl) @@ -557,6 +566,20 @@ remove_forwarder_block (basic_block bb) gsi_next (&gsi); } + /* Move debug statements if the destination has a single predecessor. */ + if (can_move_debug_stmts && !gsi_end_p (gsi)) + { + gcc_assert (gsi_stmt (gsi) == gsi_stmt (gsie)); + do + { + gimple *debug = gsi_stmt (gsi); + gcc_assert (is_gimple_debug (debug)); + gsi_remove (&gsi, false); + gsi_insert_before (&gsie_to, debug, GSI_SAME_STMT); + } + while (!gsi_end_p (gsi)); + } + bitmap_set_bit (cfgcleanup_altered_bbs, dest->index); /* Update the dominators. */ -- Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/ You must be the change you wish to see in the world. -- Gandhi Be Free! -- http://FSFLA.org/ FSF Latin America board member Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer