re PR debug/90131 (wrong debug info at -O3)

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.

From-SVN: r270441

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 783ae7adc930aa6af4dbb42559565e4289236cf0..e589a9e751752c5c60c4df40f7533d6596f57ba3 100644 (file)
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,11 @@
+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.
+
 2019-04-18  Jakub Jelinek  <jakub@redhat.com>
 
        PR translation/79183

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 11580f2d006eaf6d2ae4dd177ca1631bbf72f8b9..ee03b5e9b55f39cf979a791e881e0890ef16f691 100644 (file)
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2019-04-18  Richard Biener  <rguenther@suse.de>
+
+       PR debug/90131
+       * gcc.dg/guality/pr90131.c: New testcase.
+
 2019-04-17  Jakub Jelinek  <jakub@redhat.com>
 
        PR c++/89325

diff --git a/gcc/testsuite/gcc.dg/guality/pr90131.c b/gcc/testsuite/gcc.dg/guality/pr90131.c
new file mode 100644 (file)
index 0000000..2b89aaa
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/guality/pr90131.c
@@ -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;
+}

diff --git a/gcc/tree-cfgcleanup.c b/gcc/tree-cfgcleanup.c
index 3c291f5473b6f2a0680caa9a0457f9a43410df0c..0a79a6a658fd33f3bf3999fd2c2668bf0c0ab96d 100644 (file)
--- a/gcc/tree-cfgcleanup.c
+++ b/gcc/tree-cfgcleanup.c
@@ -444,6 +444,45 @@ phi_alternatives_equal (basic_block dest, edge e1, edge e2)
   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_block bb)
       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);