[PATCH] Fix PR90131, wrong-debug
Richard Biener
rguenther@suse.de
Thu Apr 18 10:21:00 GMT 2019
This fixes another case similar to the fixed PR89892, mergephi
via remove_forwarder_block_with_phi caused out-of-date debug
binds to become live and thus needs similar treatment as
remove_forwarder_block. Previously it didn't even bother
to move debug stmts because the destination always has
multiple predecessors. Now we have to move and reset them.
Fixed by factoring out a worker from remove_forwarder_block and
using that from remove_forwarder_block_with_phi as well.
Bootstrap & regtest running on x86_64-unknown-linux-gnu.
Richard.
2019-04-18 Richard Biener <rguenther@suse.de>
PR debug/90131
* tree-cfgcleanup.c (move_debug_stmts_from_forwarder): Split
out from ...
(remove_forwarder_block): ... here.
(remove_forwarder_block_with_phi): Also move debug stmts here.
* gcc.dg/guality/pr90131.c: New testcase.
Index: gcc/tree-cfgcleanup.c
===================================================================
--- gcc/tree-cfgcleanup.c (revision 270437)
+++ gcc/tree-cfgcleanup.c (working copy)
@@ -444,6 +444,45 @@ phi_alternatives_equal (basic_block dest
return true;
}
+/* Move debug stmts from the forwarder block SRC to DEST. */
+
+static void
+move_debug_stmts_from_forwarder (basic_block src, basic_block dest)
+{
+ if (!MAY_HAVE_DEBUG_STMTS)
+ return;
+
+ bool can_move_debug_stmts = single_pred_p (dest);
+ gimple_stmt_iterator gsi_to = gsi_after_labels (dest);
+ for (gimple_stmt_iterator gsi = gsi_after_labels (src); !gsi_end_p (gsi);)
+ {
+ gimple *debug = gsi_stmt (gsi);
+ gcc_assert (is_gimple_debug (debug));
+ /* Move debug binds anyway, but not anything else like begin-stmt
+ markers unless they are always valid at the destination. */
+ if (can_move_debug_stmts
+ || gimple_debug_bind_p (debug))
+ {
+ gsi_move_before (&gsi, &gsi_to);
+ /* Reset debug-binds that are not always valid at the destination.
+ Simply dropping them can cause earlier values to become live,
+ generating wrong debug information.
+ ??? There are several things we could improve here. For
+ one we might be able to move stmts to the predecessor.
+ For anther, if the debug stmt is immediately followed by a
+ (debug) definition in the destination (on a post-dominated path?)
+ we can elide it without any bad effects. */
+ if (!can_move_debug_stmts)
+ {
+ gimple_debug_bind_reset_value (debug);
+ update_stmt (debug);
+ }
+ }
+ else
+ gsi_next (&gsi);
+ }
+}
+
/* Removes forwarder block BB. Returns false if this failed. */
static bool
@@ -454,7 +493,6 @@ remove_forwarder_block (basic_block bb)
gimple *stmt;
edge_iterator ei;
gimple_stmt_iterator gsi, gsi_to;
- bool can_move_debug_stmts;
/* We check for infinite loops already in tree_forwarder_block_p.
However it may happen that the infinite loop is created
@@ -503,8 +541,6 @@ remove_forwarder_block (basic_block bb)
}
}
- can_move_debug_stmts = MAY_HAVE_DEBUG_STMTS && single_pred_p (dest);
-
basic_block pred = NULL;
if (single_pred_p (bb))
pred = single_pred (bb);
@@ -566,40 +602,7 @@ remove_forwarder_block (basic_block bb)
/* Move debug statements. Reset them if the destination does not
have a single predecessor. */
- if (!gsi_end_p (gsi))
- {
- gsi_to = gsi_after_labels (dest);
- do
- {
- gimple *debug = gsi_stmt (gsi);
- gcc_assert (is_gimple_debug (debug));
- /* Move debug binds anyway, but not anything else
- like begin-stmt markers unless they are always
- valid at the destination. */
- if (can_move_debug_stmts
- || gimple_debug_bind_p (debug))
- {
- gsi_move_before (&gsi, &gsi_to);
- /* Reset debug-binds that are not always valid at the
- destination. Simply dropping them can cause earlier
- values to become live, generating wrong debug information.
- ??? There are several things we could improve here. For
- one we might be able to move stmts to the predecessor.
- For anther, if the debug stmt is immediately followed
- by a (debug) definition in the destination (on a
- post-dominated path?) we can elide it without any bad
- effects. */
- if (!can_move_debug_stmts)
- {
- gimple_debug_bind_reset_value (debug);
- update_stmt (debug);
- }
- }
- else
- gsi_next (&gsi);
- }
- while (!gsi_end_p (gsi));
- }
+ move_debug_stmts_from_forwarder (bb, dest);
bitmap_set_bit (cfgcleanup_altered_bbs, dest->index);
@@ -1282,6 +1285,10 @@ remove_forwarder_block_with_phi (basic_b
redirect_edge_var_map_clear (e);
}
+ /* Move debug statements. Reset them if the destination does not
+ have a single predecessor. */
+ move_debug_stmts_from_forwarder (bb, dest);
+
/* Update the dominators. */
dombb = get_immediate_dominator (CDI_DOMINATORS, bb);
domdest = get_immediate_dominator (CDI_DOMINATORS, dest);
Index: gcc/testsuite/gcc.dg/guality/pr90131.c
===================================================================
--- gcc/testsuite/gcc.dg/guality/pr90131.c (nonexistent)
+++ gcc/testsuite/gcc.dg/guality/pr90131.c (working copy)
@@ -0,0 +1,35 @@
+/* { dg-do run } */
+/* { dg-options "-g" } */
+
+void __attribute__((noinline))
+optimize_me_not ()
+{
+ __asm__ volatile ("" : : : "memory");
+}
+volatile long a;
+int b[9][1];
+static short c[2][1] = {3};
+int main()
+{
+ int i, d, e;
+ i = 0;
+ for (; i < 9; i++)
+ a = b[i][0];
+ i = 0;
+ for (; i < 2; i++)
+ {
+ d = 0;
+ for (; d < 1; d++)
+ {
+ e = 0;
+ for (; e < 1; e++)
+ a = c[i][e];
+ /* i may very well be optimized out, so we cannot test for i == 0.
+ Instead test i + 1 which will make the test UNSUPPORTED if i
+ is optimized out. Since the test previously had wrong debug
+ with i == 9 this is acceptable. */
+ optimize_me_not(); /* { dg-final { gdb-test . "i + 1" "1" } } */
+ }
+ }
+ return 0;
+}
More information about the Gcc-patches
mailing list