diff mbox

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

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

Commit Message

Alexandre Oliva Sept. 4, 2010, 3:44 p.m. UTC 
We used to mark and skip debug insns at beginning and end of basic
blocks during sched initialization, so as to leave them alone along with
any notes that might appear between them and non-debug insns, because in
a compilation without debug insns, the notes would remain in place for
being outside the blocks.

This worked pretty well, except when multi-block sched happened to pull
an insn from a later block, causing a debug insns (and notes between
them and other insns) to become boundary insns.  We wouldn't
re-recognize them as boundary insns or notes, so we'd end up moving the
notes about, whereas the non-debug compilation would leave them in
place, causing -fcompare-debug differences.

This patch does away with the notion of boundary debug insns and notes.
Instead, whenever we're skip boundary notes and we find debug insns, we
search further for notes and move them past the debug insns, so that
they're left alone by the scheduler, as they would in a non-debug
compilation.

There are two catches:

- if notes ever become meaningful among debug insns, this will kind of
change their meaning.  I suppose we can cross that bridge when we get to
it.

- if there's a long chain of debug insns at the beginning or end of a
block, we'll iterate over all of them looking for notes every time we
call get_ebb_head_tail for such blocks.  I'm not sure this could become
a performance issue, and I don't quite see how to avoid checking it
every time, considering that the status of a debug insn may change from
one call to another, without changes to a previously checked boundary
debug insn, e.g.:

(debug_insn 51 ...
(note 52 ...
(insn 53 ...
(debug_insn 55 ...
(note 54 ...
end of block

say the patch moved note 54 past debug_insn 55, so debug_insn 55 is
regarded as the scheduling boundary.  Then, sched moves insn 53 to an
earlier block.  Now, we have to move note 52 past debug_insn 55, but if
we optimized the search at the end of the block to stop at debug_insn
55, we'd miss the change of status of note 52 and debug_insn 51.

Thoughts?

This patch was regstrapped on x86_64-linux-gnu and i686-pc-linux-gnu.
Ok to install?
diff mbox

Patch

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

	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): Don't skip 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.

Index: gcc/haifa-sched.c
===================================================================
--- gcc/haifa-sched.c.orig	2010-09-04 05:54:19.592549051 -0300
+++ gcc/haifa-sched.c	2010-09-04 05:56:06.000000000 -0300
@@ -1919,8 +1919,30 @@  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));
+	      }
+	    else if (!DEBUG_INSN_P (note))
+	      break;
+	  }
+
+	break;
+      }
     else
       break;
 
@@ -1932,8 +1954,32 @@  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))
+		  df_insn_change_bb (note, NULL);
+	      }
+	    else if (!DEBUG_INSN_P (note))
+	      break;
+	  }
+
+	break;
+      }
     else
       break;
 
@@ -1947,8 +1993,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);
     }
@@ -2811,7 +2856,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
@@ -3314,7 +3359,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	2010-09-04 05:54:19.592549051 -0300
+++ gcc/sched-deps.c	2010-09-04 12:23:01.000000000 -0300
@@ -711,9 +711,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.  */
 }
 
@@ -723,12 +720,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	2010-09-04 05:54:19.587550050 -0300
+++ gcc/sched-ebb.c	2010-09-04 05:56:06.000000000 -0300
@@ -608,9 +608,9 @@  schedule_ebbs (void)
 	 a note or two.  */
       while (head != tail)
 	{
-	  if (NOTE_P (head) || BOUNDARY_DEBUG_INSN_P (head))
+	  if (NOTE_P (head))
 	    head = NEXT_INSN (head);
-	  else if (NOTE_P (tail) || BOUNDARY_DEBUG_INSN_P (tail))
+	  else if (NOTE_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	2010-09-04 05:54:19.592549051 -0300
+++ gcc/sched-int.h	2010-09-04 05:56:06.000000000 -0300
@@ -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	2010-09-04 05:55:51.592549051 -0300
+++ gcc/sched-rgn.c	2010-09-04 12:23:38.000000000 -0300
@@ -2143,7 +2143,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);
       }
 }