re PR debug/89892 (gcc generates wrong debug information at -O2)

2019-04-05  Richard Biener  <rguenther@suse.de>

	PR debug/89892
	PR debug/89905
	* tree-cfgcleanup.c (remove_forwarder_block): Always move
	debug bind stmts but reset them if they are not valid at the
	destination.

	* gcc.dg/guality/pr89892.c: New testcase.
	* gcc.dg/guality/pr89905.c: Likewise.
	* gcc.dg/guality/loop-1.c: Likewise.

From-SVN: r270165

gcc/ChangeLog

+2019-04-05  Richard Biener  <rguenther@suse.de>
+
+	PR debug/89892
+	PR debug/89905
+	* tree-cfgcleanup.c (remove_forwarder_block): Always move
+	debug bind stmts but reset them if they are not valid at the
+	destination.
+
 2019-04-05  Martin Liska  <mliska@suse.cz>
 
 	PR translation/89936

gcc/testsuite/ChangeLog

+2019-04-05  Richard Biener  <rguenther@suse.de>
+
+	PR debug/89892
+	PR debug/89905
+	* gcc.dg/guality/pr89892.c: New testcase.
+	* gcc.dg/guality/pr89905.c: Likewise.
+	* gcc.dg/guality/loop-1.c: Likewise.
+
 2019-04-05  Richard Sandiford  <richard.sandiford@arm.com>
 
 	PR tree-optimization/89956

gcc/testsuite/gcc.dg/guality/loop-1.c

+/* { dg-do run } */
+/* { dg-options "-fno-tree-scev-cprop -fno-tree-vectorize -g" } */
+
+#include "../nop.h"
+
+void __attribute__((noipa,noinline))
+foo (int n)
+{
+  if (n == 0)
+    return;
+  int i = 0;
+  do
+    {
+      ++i; /* { dg-final { gdb-test . "i" "0" } } */
+    }
+  while (i < n);
+  /* The following works only with final value replacement or with the NOP
+     but not without (which means -Og).  Vectorization breaks it, so disable
+     that.  At -O3 it currently fails, PR89983.  */
+  __asm__ volatile (NOP : : "g" (i) : "memory"); /* { dg-final { gdb-test . "i" "1" } } */
+}
+int main() { foo(1); }

gcc/testsuite/gcc.dg/guality/pr89892.c

+/* { dg-do run } */
+/* { dg-options "-g" } */
+
+void __attribute__((noinline))
+optimize_me_not ()
+{
+  __asm__ volatile ("" : : : "memory");
+}
+volatile int a;
+static short b[3][9][1] = {5};
+int c;
+int main() {
+    int i, d;
+    i = 0;
+    for (; i < 3; i++) {
+	c = 0;
+	for (; c < 9; c++) {
+	    d = 0;
+	    for (; d < 1; d++)
+	      a = b[i][c][d];
+	}
+    }
+    i = c = 0;
+    for (; c < 7; c++)
+      for (; d < 6; d++)
+	a;
+    /* 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 == 2 this is acceptable.  Optimally we'd produce a
+       debug stmt for the final value of the loop which would fix
+       the UNSUPPORTED cases.  */
+    optimize_me_not(); /* { dg-final { gdb-test . "i + 1" "1" } } */
+}

gcc/testsuite/gcc.dg/guality/pr89905.c

+/* { dg-do run } */
+/* { dg-options "-g" } */
+
+void __attribute__((noinline))
+optimize_me_not ()
+{
+  __asm__ volatile ("" : : : "memory");
+}
+char c, d = 22, f;
+short e, g;
+int h;
+char(a)() {}
+char(b)() { return 0; }
+void i() {
+    char j;
+    for (; h < 1;) {
+	short k[9] = {1, 1, 1, 1, 1, 1, 1, 1, 1};
+	int l, i = 5;
+	short m[3] = {0, 0, 0};
+	for (; h < 7; h++)
+	  for (; d >= 33;) {
+	      ++k[8];
+	      f = (c || a()) && g;
+	  }
+	i++;
+	j = b() || m[2];
+	l = 0;
+	for (; l <= 6; l = d)
+	  e = k[8];
+	/* i may very well be optimized out, so we cannot test for i == 6.
+	   Instead test i + 1 which will make the test UNSUPPORTED if i
+	   is optimized out.  Since the test previously had wrong debug
+	   with i == 5 this is acceptable.  Optimally we'd produce a
+	   debug stmt for the final value of the loop which would fix
+	   the UNSUPPORTED cases.  */
+	optimize_me_not(); /* { dg-final { gdb-test . "i + 1" "7" } } */
+    }
+}
+int main() { i(); }

gcc/tree-cfgcleanup.c

@@ -564,15 +564,39 @@ 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))
+  /* 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));
-	  gsi_move_before (&gsi, &gsi_to);
+	  /* 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));
     }