diff mbox

[PR,debug/45136] revamp handling of BB boundary debug insns in sched

Message ID or62t5xu2d.fsf@livre.localdomain
State New
Headers show

Commit Message

Alexandre Oliva Jan. 31, 2011, 10:10 a.m. UTC 
On Jan 30, 2011, "Ulrich Weigand" <uweigand@de.ibm.com> wrote:

> Why is this df_insn_change_bb call necessary?

At some point, it was necessary to avoid miscompare-debug of some
testcases.  The problem is that, with debug insns, we can't distinguish
deleted insns that, without debug insns, would have ended up part of a
BB from those who'd end up between blocks.

It turned out that moving them all outside blocks wasn't enough to get
compare-debug to pass, because non-debug compilations had boundary
deleted insns in some cases, so I ended up discarding the BB of deleted
insns in debug dumps, and that fixed other failing testcases, but I
guess I didn't think of going back and trying to keep the insns inside
the block anyway.  This (not regtested) updated patch does just that.

I reverted the broken patch from the trunk, because I'm facing yet
another hardware crisis (my build machine died and so did its backup, so
I'm back to a much slower build machine, with a long queue of patches to
test).  In case you have spare cycles to verify that it fixes the
problem, and that this revised version passes, on SPU, a bootstrap with
BUILD_CONFIG="bootstrap-debug bootstrap-debug-lean bootstrap-debug-lib",
that would be very much appreciated.

I've verified that the patch fixes PRs 47498 and 47501, regressions
introduced by the original patch (thanks H.J. and Zdenek).

Comments

Jakub Jelinek Jan. 31, 2011, 2:48 p.m. UTC | #1 
On Mon, Jan 31, 2011 at 08:10:18AM -0200, Alexandre Oliva wrote:
> I reverted the broken patch from the trunk, because I'm facing yet
> another hardware crisis (my build machine died and so did its backup, so
> I'm back to a much slower build machine, with a long queue of patches to
> test).  In case you have spare cycles to verify that it fixes the
> problem, and that this revised version passes, on SPU, a bootstrap with
> BUILD_CONFIG="bootstrap-debug bootstrap-debug-lean bootstrap-debug-lib",
> that would be very much appreciated.

I've done a normal yes,rtl checking bootstrap with this on x86_64-linux and
i686-linux, no regressions.

> for gcc/ChangeLog
> from  Alexandre Oliva  <aoliva@redhat.com>
> 
> 	PR debug/47498
> 	PR debug/47501
> 	PR debug/45136
> 	PR debug/45130
> 	* haifa-sched.c (get_ebb_head_tail): Move notes across boundary
> 	debug insns.
> 	(no_real_insns_p, schedule_block, set_priorities): Drop special
> 	treatment of boundary debug insns.
> 	* sched-deps.c (sd_init_insn, sd_finish_insn): Don't mark debug
> 	insns.
> 	* sched-ebb.c (schedule_ebbs): Adjust skipping of debug insns.
> 	* sched-int.h (DEBUG_INSN_SCHED_P): Remove.
> 	(BOUNDARY_DEBUG_INSN_P): Likewise.
> 	(SCHEDULE_DEBUG_INSN_P): Likewise.
> 	* sched-rgn.c (init_ready_list): Drop special treatment of
> 	boundary debug insns.
> 	* final.c (rest_of_clean_state): Clear notes' BB.
> 
> for gcc/testsuite/ChangeLog
> from  Alexandre Oliva  <aoliva@redhat.com>
> 
> 	PR debug/47498
> 	PR debug/47501
> 	PR debug/45136
> 	PR debug/45130
> 	* debug/pr47498.c: New.
> 	* debug/pr47501.c: New.

	Jakub
Richard Henderson Feb. 1, 2011, 4:30 p.m. UTC | #2 
On 01/31/2011 02:10 AM, Alexandre Oliva wrote:
> 	PR debug/47498
> 	PR debug/47501
> 	PR debug/45136
> 	PR debug/45130
> 	* haifa-sched.c (get_ebb_head_tail): Move notes across boundary
> 	debug insns.
> 	(no_real_insns_p, schedule_block, set_priorities): Drop special
> 	treatment of boundary debug insns.
> 	* sched-deps.c (sd_init_insn, sd_finish_insn): Don't mark debug
> 	insns.
> 	* sched-ebb.c (schedule_ebbs): Adjust skipping of debug insns.
> 	* sched-int.h (DEBUG_INSN_SCHED_P): Remove.
> 	(BOUNDARY_DEBUG_INSN_P): Likewise.
> 	(SCHEDULE_DEBUG_INSN_P): Likewise.
> 	* sched-rgn.c (init_ready_list): Drop special treatment of
> 	boundary debug insns.
> 	* final.c (rest_of_clean_state): Clear notes' BB.

Revised patch is ok.


r~
diff mbox

Patch

for gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR debug/47498
	PR debug/47501
	PR debug/45136
	PR debug/45130
	* haifa-sched.c (get_ebb_head_tail): Move notes across boundary
	debug insns.
	(no_real_insns_p, schedule_block, set_priorities): Drop special
	treatment of boundary debug insns.
	* sched-deps.c (sd_init_insn, sd_finish_insn): Don't mark debug
	insns.
	* sched-ebb.c (schedule_ebbs): Adjust skipping of debug insns.
	* sched-int.h (DEBUG_INSN_SCHED_P): Remove.
	(BOUNDARY_DEBUG_INSN_P): Likewise.
	(SCHEDULE_DEBUG_INSN_P): Likewise.
	* sched-rgn.c (init_ready_list): Drop special treatment of
	boundary debug insns.
	* final.c (rest_of_clean_state): Clear notes' BB.

for gcc/testsuite/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR debug/47498
	PR debug/47501
	PR debug/45136
	PR debug/45130
	* debug/pr47498.c: New.
	* debug/pr47501.c: New.

Index: gcc/haifa-sched.c
===================================================================
--- gcc/haifa-sched.c.orig	2011-01-31 03:08:08.641988972 -0200
+++ gcc/haifa-sched.c	2011-01-31 07:51:24.769236414 -0200
@@ -1900,8 +1900,33 @@  get_ebb_head_tail (basic_block beg, basi
     beg_head = NEXT_INSN (beg_head);
 
   while (beg_head != beg_tail)
-    if (NOTE_P (beg_head) || BOUNDARY_DEBUG_INSN_P (beg_head))
+    if (NOTE_P (beg_head))
       beg_head = NEXT_INSN (beg_head);
+    else if (DEBUG_INSN_P (beg_head))
+      {
+	rtx note, next;
+
+	for (note = NEXT_INSN (beg_head);
+	     note != beg_tail;
+	     note = next)
+	  {
+	    next = NEXT_INSN (note);
+	    if (NOTE_P (note))
+	      {
+		if (sched_verbose >= 9)
+		  fprintf (sched_dump, "reorder %i\n", INSN_UID (note));
+
+		reorder_insns_nobb (note, note, PREV_INSN (beg_head));
+
+		if (BLOCK_FOR_INSN (note) != beg)
+		  df_insn_change_bb (note, beg);
+	      }
+	    else if (!DEBUG_INSN_P (note))
+	      break;
+	  }
+
+	break;
+      }
     else
       break;
 
@@ -1913,8 +1938,36 @@  get_ebb_head_tail (basic_block beg, basi
     end_head = NEXT_INSN (end_head);
 
   while (end_head != end_tail)
-    if (NOTE_P (end_tail) || BOUNDARY_DEBUG_INSN_P (end_tail))
+    if (NOTE_P (end_tail))
       end_tail = PREV_INSN (end_tail);
+    else if (DEBUG_INSN_P (end_tail))
+      {
+	rtx note, prev;
+
+	for (note = PREV_INSN (end_tail);
+	     note != end_head;
+	     note = prev)
+	  {
+	    prev = PREV_INSN (note);
+	    if (NOTE_P (note))
+	      {
+		if (sched_verbose >= 9)
+		  fprintf (sched_dump, "reorder %i\n", INSN_UID (note));
+
+		reorder_insns_nobb (note, note, end_tail);
+
+		if (end_tail == BB_END (end))
+		  BB_END (end) = note;
+
+		if (BLOCK_FOR_INSN (note) != end)
+		  df_insn_change_bb (note, end);
+	      }
+	    else if (!DEBUG_INSN_P (note))
+	      break;
+	  }
+
+	break;
+      }
     else
       break;
 
@@ -1928,8 +1981,7 @@  no_real_insns_p (const_rtx head, const_r
 {
   while (head != NEXT_INSN (tail))
     {
-      if (!NOTE_P (head) && !LABEL_P (head)
-	  && !BOUNDARY_DEBUG_INSN_P (head))
+      if (!NOTE_P (head) && !LABEL_P (head))
 	return 0;
       head = NEXT_INSN (head);
     }
@@ -2812,7 +2864,7 @@  schedule_block (basic_block *target_bb)
   last_scheduled_insn = prev_head;
 
   gcc_assert ((NOTE_P (last_scheduled_insn)
-	       || BOUNDARY_DEBUG_INSN_P (last_scheduled_insn))
+	       || DEBUG_INSN_P (last_scheduled_insn))
 	      && BLOCK_FOR_INSN (last_scheduled_insn) == *target_bb);
 
   /* Initialize INSN_QUEUE.  Q_SIZE is the total number of insns in the
@@ -3319,7 +3371,7 @@  set_priorities (rtx head, rtx tail)
 	current_sched_info->sched_max_insns_priority;
   rtx prev_head;
 
-  if (head == tail && (! INSN_P (head) || BOUNDARY_DEBUG_INSN_P (head)))
+  if (head == tail && ! INSN_P (head))
     gcc_unreachable ();
 
   n_insn = 0;
Index: gcc/sched-deps.c
===================================================================
--- gcc/sched-deps.c.orig	2011-01-31 03:38:46.725237282 -0200
+++ gcc/sched-deps.c	2011-01-31 06:48:03.552251313 -0200
@@ -715,9 +715,6 @@  sd_init_insn (rtx insn)
   INSN_FORW_DEPS (insn) = create_deps_list ();
   INSN_RESOLVED_FORW_DEPS (insn) = create_deps_list ();
 
-  if (DEBUG_INSN_P (insn))
-    DEBUG_INSN_SCHED_P (insn) = TRUE;
-
   /* ??? It would be nice to allocate dependency caches here.  */
 }
 
@@ -727,12 +724,6 @@  sd_finish_insn (rtx insn)
 {
   /* ??? It would be nice to deallocate dependency caches here.  */
 
-  if (DEBUG_INSN_P (insn))
-    {
-      gcc_assert (DEBUG_INSN_SCHED_P (insn));
-      DEBUG_INSN_SCHED_P (insn) = FALSE;
-    }
-
   free_deps_list (INSN_HARD_BACK_DEPS (insn));
   INSN_HARD_BACK_DEPS (insn) = NULL;
 
Index: gcc/sched-ebb.c
===================================================================
--- gcc/sched-ebb.c.orig	2011-01-31 03:08:08.833989355 -0200
+++ gcc/sched-ebb.c	2011-01-31 07:22:50.071236904 -0200
@@ -598,9 +598,9 @@  schedule_ebbs (void)
 	 a note or two.  */
       while (head != tail)
 	{
-	  if (NOTE_P (head) || BOUNDARY_DEBUG_INSN_P (head))
+	  if (NOTE_P (head) || DEBUG_INSN_P (head))
 	    head = NEXT_INSN (head);
-	  else if (NOTE_P (tail) || BOUNDARY_DEBUG_INSN_P (tail))
+	  else if (NOTE_P (tail) || DEBUG_INSN_P (tail))
 	    tail = PREV_INSN (tail);
 	  else if (LABEL_P (head))
 	    head = NEXT_INSN (head);
Index: gcc/sched-int.h
===================================================================
--- gcc/sched-int.h.orig	2011-01-31 03:08:08.863991859 -0200
+++ gcc/sched-int.h	2011-01-31 06:48:03.589247797 -0200
@@ -887,23 +887,6 @@  extern VEC(haifa_deps_insn_data_def, hea
 #define IS_SPECULATION_BRANCHY_CHECK_P(INSN) \
   (RECOVERY_BLOCK (INSN) != NULL && RECOVERY_BLOCK (INSN) != EXIT_BLOCK_PTR)
 
-/* The unchanging bit tracks whether a debug insn is to be handled
-   like an insn (i.e., schedule it) or like a note (e.g., it is next
-   to a basic block boundary.  */
-#define DEBUG_INSN_SCHED_P(insn) \
-  (RTL_FLAG_CHECK1("DEBUG_INSN_SCHED_P", (insn), DEBUG_INSN)->unchanging)
-
-/* True if INSN is a debug insn that is next to a basic block
-   boundary, i.e., it is to be handled by the scheduler like a
-   note.  */
-#define BOUNDARY_DEBUG_INSN_P(insn) \
-  (DEBUG_INSN_P (insn) && !DEBUG_INSN_SCHED_P (insn))
-/* True if INSN is a debug insn that is not next to a basic block
-   boundary, i.e., it is to be handled by the scheduler like an
-   insn.  */
-#define SCHEDULE_DEBUG_INSN_P(insn) \
-  (DEBUG_INSN_P (insn) && DEBUG_INSN_SCHED_P (insn))
-
 /* Dep status (aka ds_t) of the link encapsulates information, that is needed
    for speculative scheduling.  Namely, it is 4 integers in the range
    [0, MAX_DEP_WEAK] and 3 bits.
Index: gcc/sched-rgn.c
===================================================================
--- gcc/sched-rgn.c.orig	2011-01-31 03:08:08.963991733 -0200
+++ gcc/sched-rgn.c	2011-01-31 06:48:03.615235850 -0200
@@ -2138,7 +2138,7 @@  init_ready_list (void)
 	src_head = head;
 
 	for (insn = src_head; insn != src_next_tail; insn = NEXT_INSN (insn))
-	  if (INSN_P (insn) && !BOUNDARY_DEBUG_INSN_P (insn))
+	  if (INSN_P (insn))
 	    try_ready (insn);
       }
 }
Index: gcc/final.c
===================================================================
--- gcc/final.c.orig	2011-01-31 03:08:09.077988958 -0200
+++ gcc/final.c	2011-01-31 06:48:03.643238135 -0200
@@ -4402,7 +4402,11 @@  rest_of_clean_state (void)
 	    if (LABEL_P (insn))
 	      INSN_UID (insn) = CODE_LABEL_NUMBER (insn);
 	    else
-	      INSN_UID (insn) = 0;
+	      {
+		if (NOTE_P (insn))
+		  set_block_for_insn (insn, NULL);
+		INSN_UID (insn) = 0;
+	      }
 	}
     }
 
@@ -4423,7 +4427,6 @@  rest_of_clean_state (void)
 	       && NOTE_KIND (insn) != NOTE_INSN_BLOCK_END
 	       && NOTE_KIND (insn) != NOTE_INSN_CFA_RESTORE_STATE)))
 	print_rtl_single (final_output, insn);
-
     }
 
   if (final_output)
Index: gcc/testsuite/gcc.dg/debug/pr47498.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gcc/testsuite/gcc.dg/debug/pr47498.c	2011-01-31 06:58:07.180239331 -0200
@@ -0,0 +1,11 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -fsched2-use-superblocks -fcompare-debug" } */
+
+int bar(void *);
+
+void foo (void *p)
+{
+  int i = 1;
+  while (i)
+    i = bar (p);
+}
Index: gcc/testsuite/gcc.dg/debug/pr47501.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gcc/testsuite/gcc.dg/debug/pr47501.c	2011-01-31 07:30:29.627237105 -0200
@@ -0,0 +1,11 @@ 
+/* { dg-do compile } */
+/* { dg-options "-Os -fmodulo-sched -fcompare-debug" } */
+
+void
+foo (void)
+{
+  unsigned numlen;
+  unsigned foldlen;
+  for (; foldlen; foldlen -= numlen)
+    foo ();
+}