[PATCH, sched] Cleanup and improve multipass_dfa_lookahead_guard

[PATCH, sched] Cleanup and improve multipass_dfa_lookahead_guard

[Maxim Kuvyrkov]

[-- Attachment #1: Type: text/plain, Size: 1161 bytes --]

Hi,

This patch cleans up haifa-sched.c:choose_ready() function while allow more powerful customization by backends at the same time.

The primary change is that targetm.sched.first_cycle_multipass_dfa_lookahead_guard hook is converted from returning a boolean OK/Nada value to returning an action to be applied to a given instruction in the ready list.

If return value is N == 0, then insn is OK (previous corresponding return value was 1).
If return value is N  < 0, then insn should not be considered for scheduling for -N cycles (I'm going to use this for ARM scheduling in an upcoming patch).
If return value is N  > 0, then insn can still be issued on the current cycle, just not immediately now (previous corresponding return value was 0).

Changes to ia64 and rs6000 are mostly mechanical to update hook return values, but port maintainers may wish to review those.

Tested on x86_64-linux-gnu, arm-linux-gnueabihf, and by building ia64-linux-gnu cc1.  Changes to PowerPC were seriously eyeballed.  If maintainers so to wish, I will carry out any additional testing.

OK to apply?

Thank you,

--
Maxim Kuvyrkov
www.linaro.org



[-- Attachment #2: 0004-Cleanup-and-improve-multipass_dfa_lookahead_guard.patch --]
[-- Type: application/octet-stream, Size: 20713 bytes --]

From f9ca8dc6141b732a11c8a7aadbd6b67ce019d228 Mon Sep 17 00:00:00 2001
From: Maxim Kuvyrkov <maxim.kuvyrkov@linaro.org>
Date: Fri, 28 Mar 2014 17:53:24 +1300
Subject: [PATCH 4/4] Cleanup and improve multipass_dfa_lookahead_guard

	* config/i386/i386.c (core2i7_first_cycle_multipass_filter_ready_try,)
	(core2i7_first_cycle_multipass_begin,)
	(core2i7_first_cycle_multipass_issue,)
	(core2i7_first_cycle_multipass_backtrack): Update signature.
	* config/ia64/ia64.c
	(ia64_first_cycle_multipass_dfa_lookahead_guard_spec): Remove.
	(ia64_first_cycle_multipass_dfa_lookahead_guard): Update signature.
	(TARGET_SCHED_FIRST_CYCLE_MULTIPASS_DFA_LOOKAHEAD_GUARD_SPEC): Remove
	hook definition.
	(ia64_first_cycle_multipass_dfa_lookahead_guard): Merge logic from
	ia64_first_cycle_multipass_dfa_lookahead_guard_spec.  Update return
	values.
	* config/rs6000/rs6000.c (rs6000_use_sched_lookahead_guard): Update
	return values.
	* doc/tm.texi: Regenerate.
	* doc/tm.texi.in
	(TARGET_SCHED_FIRST_CYCLE_MULTIPASS_DFA_LOOKAHEAD_GUARD_SPEC): Remove.
	* haifa-sched.c (ready_try): Make signed to allow negative values.
	(rebug_ready_list_1): Update.
	(choose_ready): Simplify.
	(sched_extend_ready_list): Update.
---
 gcc/config/i386/i386.c     |   10 +++--
 gcc/config/ia64/ia64.c     |   39 ++++++++-----------
 gcc/config/rs6000/rs6000.c |   14 ++++---
 gcc/doc/tm.texi            |   31 ++++++---------
 gcc/doc/tm.texi.in         |    2 -
 gcc/haifa-sched.c          |   92 +++++++++++++++++++++++++-------------------
 gcc/sched-int.h            |    2 +-
 gcc/sel-sched.c            |    2 +-
 gcc/target.def             |   38 ++++++------------
 9 files changed, 108 insertions(+), 122 deletions(-)

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 6ffb788..a6a8156 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -26383,7 +26383,7 @@ static int min_insn_size (rtx);
 static void
 core2i7_first_cycle_multipass_filter_ready_try
 (const_ix86_first_cycle_multipass_data_t data,
- char *ready_try, int n_ready, bool first_cycle_insn_p)
+ signed char *ready_try, int n_ready, bool first_cycle_insn_p)
 {
   while (n_ready--)
     {
@@ -26415,7 +26415,8 @@ core2i7_first_cycle_multipass_filter_ready_try
 
 /* Prepare for a new round of multipass lookahead scheduling.  */
 static void
-core2i7_first_cycle_multipass_begin (void *_data, char *ready_try, int n_ready,
+core2i7_first_cycle_multipass_begin (void *_data,
+				     signed char *ready_try, int n_ready,
 				     bool first_cycle_insn_p)
 {
   ix86_first_cycle_multipass_data_t data
@@ -26436,7 +26437,8 @@ core2i7_first_cycle_multipass_begin (void *_data, char *ready_try, int n_ready,
 /* INSN is being issued in current solution.  Account for its impact on
    the decoder model.  */
 static void
-core2i7_first_cycle_multipass_issue (void *_data, char *ready_try, int n_ready,
+core2i7_first_cycle_multipass_issue (void *_data,
+				     signed char *ready_try, int n_ready,
 				     rtx insn, const void *_prev_data)
 {
   ix86_first_cycle_multipass_data_t data
@@ -26474,7 +26476,7 @@ core2i7_first_cycle_multipass_issue (void *_data, char *ready_try, int n_ready,
 /* Revert the effect on ready_try.  */
 static void
 core2i7_first_cycle_multipass_backtrack (const void *_data,
-					 char *ready_try,
+					 signed char *ready_try,
 					 int n_ready ATTRIBUTE_UNUSED)
 {
   const_ix86_first_cycle_multipass_data_t data
diff --git a/gcc/config/ia64/ia64.c b/gcc/config/ia64/ia64.c
index 7118146..6275ab1 100644
--- a/gcc/config/ia64/ia64.c
+++ b/gcc/config/ia64/ia64.c
@@ -169,8 +169,7 @@ static int ia64_first_cycle_multipass_dfa_lookahead (void);
 static void ia64_dependencies_evaluation_hook (rtx, rtx);
 static void ia64_init_dfa_pre_cycle_insn (void);
 static rtx ia64_dfa_pre_cycle_insn (void);
-static int ia64_first_cycle_multipass_dfa_lookahead_guard (rtx);
-static bool ia64_first_cycle_multipass_dfa_lookahead_guard_spec (const_rtx);
+static int ia64_first_cycle_multipass_dfa_lookahead_guard (rtx, int);
 static int ia64_dfa_new_cycle (FILE *, int, rtx, int, int, int *);
 static void ia64_h_i_d_extended (void);
 static void * ia64_alloc_sched_context (void);
@@ -496,10 +495,6 @@ static const struct attribute_spec ia64_attribute_table[] =
 #undef TARGET_SCHED_GEN_SPEC_CHECK
 #define TARGET_SCHED_GEN_SPEC_CHECK ia64_gen_spec_check
 
-#undef TARGET_SCHED_FIRST_CYCLE_MULTIPASS_DFA_LOOKAHEAD_GUARD_SPEC
-#define TARGET_SCHED_FIRST_CYCLE_MULTIPASS_DFA_LOOKAHEAD_GUARD_SPEC\
-  ia64_first_cycle_multipass_dfa_lookahead_guard_spec
-
 #undef TARGET_SCHED_SKIP_RTX_P
 #define TARGET_SCHED_SKIP_RTX_P ia64_skip_rtx_p
 
@@ -7535,28 +7530,24 @@ ia64_variable_issue (FILE *dump ATTRIBUTE_UNUSED,
    can be chosen.  */
 
 static int
-ia64_first_cycle_multipass_dfa_lookahead_guard (rtx insn)
+ia64_first_cycle_multipass_dfa_lookahead_guard (rtx insn, int ready_index)
 {
   gcc_assert (insn && INSN_P (insn));
-  return ((!reload_completed
-	   || !safe_group_barrier_needed (insn))
-	  && ia64_first_cycle_multipass_dfa_lookahead_guard_spec (insn)
-	  && (!mflag_sched_mem_insns_hard_limit
-	      || !is_load_p (insn)
-	      || mem_ops_in_group[current_cycle % 4] < ia64_max_memory_insns));
-}
 
-/* We are choosing insn from the ready queue.  Return nonzero if INSN
-   can be chosen.  */
+  /* Size of ALAT is 32.  As far as we perform conservative
+     data speculation, we keep ALAT half-empty.  */
+  if ((TODO_SPEC (insn) & BEGIN_DATA) && pending_data_specs >= 16)
+    return ready_index == 0 ? -1 : 1;
 
-static bool
-ia64_first_cycle_multipass_dfa_lookahead_guard_spec (const_rtx insn)
-{
-  gcc_assert (insn  && INSN_P (insn));
-  /* Size of ALAT is 32.  As far as we perform conservative data speculation,
-     we keep ALAT half-empty.  */
-  return (pending_data_specs < 16
-	  || !(TODO_SPEC (insn) & BEGIN_DATA));
+  if (ready_index == 0)
+    return 0;
+
+  if ((!reload_completed
+       || !safe_group_barrier_needed (insn))
+      && (!mflag_sched_mem_insns_hard_limit
+	  || !is_load_p (insn)
+	  || mem_ops_in_group[current_cycle % 4] < ia64_max_memory_insns))
+    return 0;
 }
 
 /* The following variable value is pseudo-insn used by the DFA insn
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index cd7b8c1..75bd52a 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -26798,20 +26798,22 @@ rs6000_use_sched_lookahead (void)
 
 /* We are choosing insn from the ready queue.  Return nonzero if INSN can be chosen.  */
 static int
-rs6000_use_sched_lookahead_guard (rtx insn)
+rs6000_use_sched_lookahead_guard (rtx insn, int ready_index)
 {
+  if (ready_index == 0)
+    return 0;
+
   if (rs6000_cpu_attr != CPU_CELL)
-    return 1;
+    return 0;
 
-   if (insn == NULL_RTX || !INSN_P (insn))
-     abort ();
+  gcc_assert (insn != NULL_RTX && INSN_P (insn));
 
   if (!reload_completed
       || is_nonpipeline_insn (insn)
       || is_microcoded_insn (insn))
-    return 0;
+    return 1;
 
-  return 1;
+  return 0;
 }
 
 /* Determine if PAT refers to memory. If so, set MEM_REF to the MEM rtx
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 1a26bcd..05342b3d8a 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -6716,26 +6716,32 @@ schedules to choose the best one.
 The default is no multipass scheduling.
 @end deftypefn
 
-@deftypefn {Target Hook} int TARGET_SCHED_FIRST_CYCLE_MULTIPASS_DFA_LOOKAHEAD_GUARD (rtx @var{insn})
+@deftypefn {Target Hook} int TARGET_SCHED_FIRST_CYCLE_MULTIPASS_DFA_LOOKAHEAD_GUARD (rtx @var{insn}, int @var{ready_index})
 
 This hook controls what insns from the ready insn queue will be
 considered for the multipass insn scheduling.  If the hook returns
-zero for @var{insn}, the insn will be not chosen to
-be issued.
+zero for @var{insn}, the insn will be considered in multipass scheduling.
+Positive return values will remove @var{insn} from consideration on
+the current round of multipass scheduling.
+Negative return values will remove @var{insn} from consideration for given
+number of cycles.
+Backends should be careful about returning non-zero for highest priority
+instruction at position 0 in the ready list.  @var{ready_index} is passed
+to allow backends make correct judgements.
 
 The default is that any ready insns can be chosen to be issued.
 @end deftypefn
 
-@deftypefn {Target Hook} void TARGET_SCHED_FIRST_CYCLE_MULTIPASS_BEGIN (void *@var{data}, char *@var{ready_try}, int @var{n_ready}, bool @var{first_cycle_insn_p})
+@deftypefn {Target Hook} void TARGET_SCHED_FIRST_CYCLE_MULTIPASS_BEGIN (void *@var{data}, signed char *@var{ready_try}, int @var{n_ready}, bool @var{first_cycle_insn_p})
 This hook prepares the target backend for a new round of multipass
 scheduling.
 @end deftypefn
 
-@deftypefn {Target Hook} void TARGET_SCHED_FIRST_CYCLE_MULTIPASS_ISSUE (void *@var{data}, char *@var{ready_try}, int @var{n_ready}, rtx @var{insn}, const void *@var{prev_data})
+@deftypefn {Target Hook} void TARGET_SCHED_FIRST_CYCLE_MULTIPASS_ISSUE (void *@var{data}, signed char *@var{ready_try}, int @var{n_ready}, rtx @var{insn}, const void *@var{prev_data})
 This hook is called when multipass scheduling evaluates instruction INSN.
 @end deftypefn
 
-@deftypefn {Target Hook} void TARGET_SCHED_FIRST_CYCLE_MULTIPASS_BACKTRACK (const void *@var{data}, char *@var{ready_try}, int @var{n_ready})
+@deftypefn {Target Hook} void TARGET_SCHED_FIRST_CYCLE_MULTIPASS_BACKTRACK (const void *@var{data}, signed char *@var{ready_try}, int @var{n_ready})
 This is called when multipass scheduling backtracks from evaluation of
 an instruction.
 @end deftypefn
@@ -6843,19 +6849,6 @@ a pattern for a branchy check corresponding to a simple check denoted by
 @var{insn} should be generated.  In this case @var{label} can't be null.
 @end deftypefn
 
-@deftypefn {Target Hook} bool TARGET_SCHED_FIRST_CYCLE_MULTIPASS_DFA_LOOKAHEAD_GUARD_SPEC (const_rtx @var{insn})
-This hook is used as a workaround for
-@samp{TARGET_SCHED_FIRST_CYCLE_MULTIPASS_DFA_LOOKAHEAD_GUARD} not being
-called on the first instruction of the ready list.  The hook is used to
-discard speculative instructions that stand first in the ready list from
-being scheduled on the current cycle.  If the hook returns @code{false},
-@var{insn} will not be chosen to be issued.
-For non-speculative instructions,
-the hook should always return @code{true}.  For example, in the ia64 backend
-the hook is used to cancel data speculative insns when the ALAT table
-is nearly full.
-@end deftypefn
-
 @deftypefn {Target Hook} void TARGET_SCHED_SET_SCHED_FLAGS (struct spec_info_def *@var{spec_info})
 This hook is used by the insn scheduler to find out what features should be
 enabled/used.
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 9c81f20..8a66842 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -5019,8 +5019,6 @@ them: try the first ones in this list first.
 
 @hook TARGET_SCHED_GEN_SPEC_CHECK
 
-@hook TARGET_SCHED_FIRST_CYCLE_MULTIPASS_DFA_LOOKAHEAD_GUARD_SPEC
-
 @hook TARGET_SCHED_SET_SCHED_FLAGS
 
 @hook TARGET_SCHED_SMS_RES_MII
diff --git a/gcc/haifa-sched.c b/gcc/haifa-sched.c
index 6db5983..b8c7862 100644
--- a/gcc/haifa-sched.c
+++ b/gcc/haifa-sched.c
@@ -345,7 +345,7 @@ size_t dfa_state_size;
 
 /* The following array is used to find the best insn from ready when
    the automaton pipeline interface is used.  */
-char *ready_try = NULL;
+signed char *ready_try = NULL;
 
 /* The ready list.  */
 struct ready_list ready = {NULL, 0, 0, 0, 0};
@@ -5126,7 +5126,7 @@ early_queue_to_ready (state_t state, struct ready_list *ready)
    If READY_TRY is non-zero then only print insns that max_issue
    will consider.  */
 static void
-debug_ready_list_1 (struct ready_list *ready, char *ready_try)
+debug_ready_list_1 (struct ready_list *ready, signed char *ready_try)
 {
   rtx *p;
   int i;
@@ -5566,10 +5566,9 @@ choose_ready (struct ready_list *ready, bool first_cycle_insn_p,
     }
   else
     {
-      /* Try to choose the better insn.  */
+      /* Try to choose the best insn.  */
       int index = 0, i;
       rtx insn;
-      ds_t ts;
 
       insn = ready_element (ready, 0);
       if (INSN_CODE (insn) < 0)
@@ -5578,42 +5577,56 @@ choose_ready (struct ready_list *ready, bool first_cycle_insn_p,
 	  return 0;
 	}
 
-      ts = TODO_SPEC (insn);
-      if ((ts & SPECULATIVE)
-	  && (targetm.sched.first_cycle_multipass_dfa_lookahead_guard_spec
-	      && (!targetm.sched
-		  .first_cycle_multipass_dfa_lookahead_guard_spec (insn))))
-	/* Discard speculative instruction that stands first in the ready
-	   list.  */
+      /* Filter the search space.  */
+      for (i = 0; i < ready->n_ready; i++)
 	{
-	  change_queue_index (insn, 1);
-	  return 1;
-	}
+	  ready_try[i] = 0;
 
-      for (i = 0; i < ready->n_ready; i++)
-	ready_try [i] = 0;
+	  insn = ready_element (ready, i);
 
-      /* Let the target filter the search space.  */
-      for (i = 1; i < ready->n_ready; i++)
-	if (!ready_try[i])
-	  {
-	    insn = ready_element (ready, i);
-
-	    /* If this insn is recognizable we should have already
-	       recognized it earlier.
-	       ??? Not very clear where this is supposed to be done.
-	       See dep_cost_1.  */
-	    gcc_checking_assert (INSN_CODE (insn) >= 0
-				 || recog_memoized (insn) < 0);
-
-	    ready_try [i]
-	      = (/* INSN_CODE check can be omitted here as it is also done later
-		    in max_issue ().  */
-		 INSN_CODE (insn) < 0
-		 || (targetm.sched.first_cycle_multipass_dfa_lookahead_guard
-		     && !targetm.sched.first_cycle_multipass_dfa_lookahead_guard
-		     (insn)));
-	  }
+	  /* If this insn is recognizable we should have already
+	     recognized it earlier.
+	     ??? Not very clear where this is supposed to be done.
+	     See dep_cost_1.  */
+	  gcc_checking_assert (INSN_CODE (insn) >= 0
+			       || recog_memoized (insn) < 0);
+	  if (INSN_CODE (insn) < 0)
+	    {
+	      /* Non-recognized insns at position 0 are handled above.  */
+	      gcc_assert (i > 0);
+	      ready_try[i] = 1;
+	      continue;
+	    }
+
+	  if (targetm.sched.first_cycle_multipass_dfa_lookahead_guard)
+	    {
+	      ready_try[i]
+		= (targetm.sched.first_cycle_multipass_dfa_lookahead_guard
+		    (insn, i));
+
+	      if (ready_try[i] < 0)
+		/* Queue instruction for several cycles.
+		   We need to restart choose_ready as we have changed
+		   the ready list.  */
+		{
+		  change_queue_index (insn, -ready_try[i]);
+		  return 1;
+		}
+
+	      /* Make sure that we didn't end up with 0'th insn filtered out.
+		 Don't be tempted to make life easier for backends and just
+		 requeue 0'th insn if (ready_try[0] == 0) and restart
+		 choose_ready.  Backends should be very considerate about
+		 requeueing instructions -- especially the highest priority
+		 one at position 0.  */
+	      gcc_assert (ready_try[i] == 0 || i > 0);
+	      if (ready_try[i])
+		continue;
+	    }
+
+	  gcc_assert (ready_try[i] == 0);
+	  /* INSN made it through the scrutiny of filters!  */
+	}
 
       if (max_issue (ready, 1, curr_state, first_cycle_insn_p, &index) == 0)
 	{
@@ -7193,8 +7206,9 @@ sched_extend_ready_list (int new_sched_ready_n_insns)
 
   gcc_assert (new_sched_ready_n_insns >= sched_ready_n_insns);
 
-  ready_try = (char *) xrecalloc (ready_try, new_sched_ready_n_insns,
-                                  sched_ready_n_insns, sizeof (*ready_try));
+  ready_try = (signed char *) xrecalloc (ready_try, new_sched_ready_n_insns,
+					 sched_ready_n_insns,
+					 sizeof (*ready_try));
 
   /* We allocate +1 element to save initial state in the choice_stack[0]
      entry.  */
diff --git a/gcc/sched-int.h b/gcc/sched-int.h
index 6e024c2..ffe618c 100644
--- a/gcc/sched-int.h
+++ b/gcc/sched-int.h
@@ -170,7 +170,7 @@ struct ready_list
   int n_debug;
 };
 
-extern char *ready_try;
+extern signed char *ready_try;
 extern struct ready_list ready;
 
 extern int max_issue (struct ready_list *, int, state_t, bool, int *);
diff --git a/gcc/sel-sched.c b/gcc/sel-sched.c
index e255d62..0c864ac 100644
--- a/gcc/sel-sched.c
+++ b/gcc/sel-sched.c
@@ -4225,7 +4225,7 @@ invoke_dfa_lookahead_guard (void)
       if (! have_hook || i == 0)
         r = 0;
       else
-        r = !targetm.sched.first_cycle_multipass_dfa_lookahead_guard (insn);
+        r = targetm.sched.first_cycle_multipass_dfa_lookahead_guard (insn, i);
 
       gcc_assert (INSN_CODE (insn) >= 0);
 
diff --git a/gcc/target.def b/gcc/target.def
index dd167bb..5fe2e82 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -1174,11 +1174,17 @@ DEFHOOK
  "\n\
 This hook controls what insns from the ready insn queue will be\n\
 considered for the multipass insn scheduling.  If the hook returns\n\
-zero for @var{insn}, the insn will be not chosen to\n\
-be issued.\n\
+zero for @var{insn}, the insn will be considered in multipass scheduling.\n\
+Positive return values will remove @var{insn} from consideration on\n\
+the current round of multipass scheduling.\n\
+Negative return values will remove @var{insn} from consideration for given\n\
+number of cycles.\n\
+Backends should be careful about returning non-zero for highest priority\n\
+instruction at position 0 in the ready list.  @var{ready_index} is passed\n\
+to allow backends make correct judgements.\n\
 \n\
 The default is that any ready insns can be chosen to be issued.",
- int, (rtx insn), NULL)
+ int, (rtx insn, int ready_index), NULL)
 
 /* This hook prepares the target for a new round of multipass
    scheduling.
@@ -1193,7 +1199,7 @@ DEFHOOK
 (first_cycle_multipass_begin,
  "This hook prepares the target backend for a new round of multipass\n\
 scheduling.",
- void, (void *data, char *ready_try, int n_ready, bool first_cycle_insn_p),
+ void, (void *data, signed char *ready_try, int n_ready, bool first_cycle_insn_p),
  NULL)
 
 /* This hook is called when multipass scheduling evaluates instruction INSN.
@@ -1209,7 +1215,7 @@ scheduling.",
 DEFHOOK
 (first_cycle_multipass_issue,
  "This hook is called when multipass scheduling evaluates instruction INSN.",
- void, (void *data, char *ready_try, int n_ready, rtx insn,
+ void, (void *data, signed char *ready_try, int n_ready, rtx insn,
 	const void *prev_data), NULL)
 
 /* This hook is called when multipass scheduling backtracks from evaluation of
@@ -1225,7 +1231,7 @@ DEFHOOK
 (first_cycle_multipass_backtrack,
  "This is called when multipass scheduling backtracks from evaluation of\n\
 an instruction.",
- void, (const void *data, char *ready_try, int n_ready), NULL)
+ void, (const void *data, signed char *ready_try, int n_ready), NULL)
 
 /* This hook notifies the target about the result of the concluded current
    round of multipass scheduling.
@@ -1421,26 +1427,6 @@ a pattern for a branchy check corresponding to a simple check denoted by\n\
 @var{insn} should be generated.  In this case @var{label} can't be null.",
  rtx, (rtx insn, rtx label, unsigned int ds), NULL)
 
-/* The following member value is a pointer to a function controlling
-   what insns from the ready insn queue will be considered for the
-   multipass insn scheduling.  If the hook returns zero for the insn
-   passed as the parameter, the insn will not be chosen to be
-   issued.  This hook is used to discard speculative instructions,
-   that stand at the first position of the ready list.  */
-DEFHOOK
-(first_cycle_multipass_dfa_lookahead_guard_spec,
- "This hook is used as a workaround for\n\
-@samp{TARGET_SCHED_FIRST_CYCLE_MULTIPASS_DFA_LOOKAHEAD_GUARD} not being\n\
-called on the first instruction of the ready list.  The hook is used to\n\
-discard speculative instructions that stand first in the ready list from\n\
-being scheduled on the current cycle.  If the hook returns @code{false},\n\
-@var{insn} will not be chosen to be issued.\n\
-For non-speculative instructions,\n\
-the hook should always return @code{true}.  For example, in the ia64 backend\n\
-the hook is used to cancel data speculative insns when the ALAT table\n\
-is nearly full.",
- bool, (const_rtx insn), NULL)
-
 /* The following member value is a pointer to a function that provides
    information about the speculation capabilities of the target.
    The parameter is a pointer to spec_info variable.  */
-- 
1.7.9.5

Re: [PATCH, sched] Cleanup and improve multipass_dfa_lookahead_guard

[Jeff Law]

On 05/19/14 22:27, Maxim Kuvyrkov wrote:
> Hi,
>
> This patch cleans up haifa-sched.c:choose_ready() function while
> allow more powerful customization by backends at the same time.
>
> The primary change is that
> targetm.sched.first_cycle_multipass_dfa_lookahead_guard hook is
> converted from returning a boolean OK/Nada value to returning an
> action to be applied to a given instruction in the ready list.
>
> If return value is N == 0, then insn is OK (previous corresponding
> return value was 1). If return value is N  < 0, then insn should not
> be considered for scheduling for -N cycles (I'm going to use this for
> ARM scheduling in an upcoming patch). If return value is N  > 0, then
> insn can still be issued on the current cycle, just not immediately
> now (previous corresponding return value was 0).
>
> Changes to ia64 and rs6000 are mostly mechanical to update hook
> return values, but port maintainers may wish to review those.
>
> Tested on x86_64-linux-gnu, arm-linux-gnueabihf, and by building
> ia64-linux-gnu cc1.  Changes to PowerPC were seriously eyeballed.  If
> maintainers so to wish, I will carry out any additional testing.
This looks OK to me.

I think it's safe to work from the assumption that the hook will never 
return INT_MIN :-)

jeff

Re: [PATCH, sched] Cleanup and improve multipass_dfa_lookahead_guard

[Pat Haugen]

On 05/19/2014 11:27 PM, Maxim Kuvyrkov wrote:
> Changes to ia64 and rs6000 are mostly mechanical to update hook return values, but port maintainers may wish to review those.
I'm not a maintainer, but the rs6000 changes look good to me.

-Pat

Re: [PATCH, sched] Cleanup and improve multipass_dfa_lookahead_guard

[David Edelsohn]

On Tue, May 20, 2014 at 5:46 PM, Pat Haugen <pthaugen@linux.vnet.ibm.com> wrote:
> On 05/19/2014 11:27 PM, Maxim Kuvyrkov wrote:
>>
>> Changes to ia64 and rs6000 are mostly mechanical to update hook return
>> values, but port maintainers may wish to review those.
>
> I'm not a maintainer, but the rs6000 changes look good to me.

The rs6000 changes are okay with me too.

Thanks, David

Re: [PATCH, sched] Cleanup and improve multipass_dfa_lookahead_guard

[Andreas Schwab]

../../gcc/config/ia64/ia64.c: In function 'int ia64_first_cycle_multipass_dfa_lookahead_guard(rtx, int)':
../../gcc/config/ia64/ia64.c:7551:1: error: control reaches end of non-void function [-Werror=return-type]

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH, sched] Cleanup and improve multipass_dfa_lookahead_guard

[Maxim Kuvyrkov]

On May 23, 2014, at 7:23 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:

> ../../gcc/config/ia64/ia64.c: In function 'int ia64_first_cycle_multipass_dfa_lookahead_guard(rtx, int)':
> ../../gcc/config/ia64/ia64.c:7551:1: error: control reaches end of non-void function [-Werror=return-type]

Fixed, sorry about the breakage.  The patch is trivial.

Thank you,

--
Maxim Kuvyrkov
www.linaro.org


2014-05-23  Maxim Kuvyrkov  <maxim.kuvyrkov@linaro.org>

	Fix bootstrap error on ia64
	* config/ia64/ia64.c (ia64_first_cycle_multipass_dfa_lookahead_guard):
	Return default value.

Index: gcc/config/ia64/ia64.c
===================================================================
--- gcc/config/ia64/ia64.c	(revision 210844)
+++ gcc/config/ia64/ia64.c	(working copy)
@@ -7548,6 +7548,8 @@ ia64_first_cycle_multipass_dfa_lookahead_guard
 	  || !is_load_p (insn)
 	  || mem_ops_in_group[current_cycle % 4] < ia64_max_memory_insns))
     return 0;
+
+  return 1;
 }
 
 /* The following variable value is pseudo-insn used by the DFA insn

Re: [PATCH, sched] Cleanup and improve multipass_dfa_lookahead_guard

[Jeff Law]

On 05/23/14 01:35, Maxim Kuvyrkov wrote:
> On May 23, 2014, at 7:23 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
>
>> ../../gcc/config/ia64/ia64.c: In function 'int ia64_first_cycle_multipass_dfa_lookahead_guard(rtx, int)':
>> ../../gcc/config/ia64/ia64.c:7551:1: error: control reaches end of non-void function [-Werror=return-type]
>
> Fixed, sorry about the breakage.  The patch is trivial.
>
> Thank you,
>
> --
> Maxim Kuvyrkov
> www.linaro.org
>
>
> 2014-05-23  Maxim Kuvyrkov  <maxim.kuvyrkov@linaro.org>
>
> 	Fix bootstrap error on ia64
> 	* config/ia64/ia64.c (ia64_first_cycle_multipass_dfa_lookahead_guard):
> 	Return default value.
If you haven't done so already, go ahead and check this in.

jeff

Re: [PATCH, sched] Cleanup and improve multipass_dfa_lookahead_guard

[Andreas Schwab]

Maxim Kuvyrkov <maxim.kuvyrkov@linaro.org> writes:

> @@ -7535,28 +7530,24 @@ ia64_variable_issue (FILE *dump ATTRIBUTE_UNUSED,
>     can be chosen.  */
>  
>  static int
> -ia64_first_cycle_multipass_dfa_lookahead_guard (rtx insn)
> +ia64_first_cycle_multipass_dfa_lookahead_guard (rtx insn, int ready_index)
>  {
>    gcc_assert (insn && INSN_P (insn));
> -  return ((!reload_completed
> -	   || !safe_group_barrier_needed (insn))
> -	  && ia64_first_cycle_multipass_dfa_lookahead_guard_spec (insn)
> -	  && (!mflag_sched_mem_insns_hard_limit
> -	      || !is_load_p (insn)
> -	      || mem_ops_in_group[current_cycle % 4] < ia64_max_memory_insns));
> -}
>  
> -/* We are choosing insn from the ready queue.  Return nonzero if INSN
> -   can be chosen.  */
> +  /* Size of ALAT is 32.  As far as we perform conservative
> +     data speculation, we keep ALAT half-empty.  */
> +  if ((TODO_SPEC (insn) & BEGIN_DATA) && pending_data_specs >= 16)
> +    return ready_index == 0 ? -1 : 1;

$ gcc/xgcc -B gcc/ ../gcc/testsuite/gcc.c-torture/execute/pr51581-1.c -fno-diagnostics-show-caret -fdiagnostics-color=never -w -O3 -fomit-frame-pointer -funroll-loops -c
../gcc/testsuite/gcc.c-torture/execute/pr51581-1.c: In function ‘f1’:
../gcc/testsuite/gcc.c-torture/execute/pr51581-1.c:15:1: internal compiler error: in operator[], at vec.h:736
0x40000000014455af vec<_haifa_insn_data, va_heap, vl_embed>::operator[](unsigned int)
        ../../gcc/vec.h:736
0x40000000014455af vec<_haifa_insn_data, va_heap, vl_ptr>::operator[](unsigned int)
        ../../gcc/vec.h:1202
0x40000000014455af ia64_first_cycle_multipass_dfa_lookahead_guard
        ../../gcc/config/ia64/ia64.c:7539

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH, sched] Cleanup and improve multipass_dfa_lookahead_guard

[Andreas Schwab]

Regtested on ia64-suse-linux and installed as obvious.

Andreas.

	* config/ia64/ia64.c
	(ia64_first_cycle_multipass_dfa_lookahead_guard): Check
	pending_data_specs first.

diff --git a/gcc/config/ia64/ia64.c b/gcc/config/ia64/ia64.c
index 118e5bf..4c5390b 100644
--- a/gcc/config/ia64/ia64.c
+++ b/gcc/config/ia64/ia64.c
@@ -7536,7 +7536,7 @@ ia64_first_cycle_multipass_dfa_lookahead_guard (rtx insn, int ready_index)
 
   /* Size of ALAT is 32.  As far as we perform conservative
      data speculation, we keep ALAT half-empty.  */
-  if ((TODO_SPEC (insn) & BEGIN_DATA) && pending_data_specs >= 16)
+  if (pending_data_specs >= 16 && (TODO_SPEC (insn) & BEGIN_DATA))
     return ready_index == 0 ? -1 : 1;
 
   if (ready_index == 0)
-- 
2.0.0

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH, sched] Cleanup and improve multipass_dfa_lookahead_guard

[Maxim Kuvyrkov]

On Jun 2, 2014, at 7:54 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:

> Regtested on ia64-suse-linux and installed as obvious.
> 
> Andreas.
> 
> 	* config/ia64/ia64.c
> 	(ia64_first_cycle_multipass_dfa_lookahead_guard): Check
> 	pending_data_specs first.
> 
> diff --git a/gcc/config/ia64/ia64.c b/gcc/config/ia64/ia64.c
> index 118e5bf..4c5390b 100644
> --- a/gcc/config/ia64/ia64.c
> +++ b/gcc/config/ia64/ia64.c
> @@ -7536,7 +7536,7 @@ ia64_first_cycle_multipass_dfa_lookahead_guard (rtx insn, int ready_index)
> 
>   /* Size of ALAT is 32.  As far as we perform conservative
>      data speculation, we keep ALAT half-empty.  */
> -  if ((TODO_SPEC (insn) & BEGIN_DATA) && pending_data_specs >= 16)
> +  if (pending_data_specs >= 16 && (TODO_SPEC (insn) & BEGIN_DATA))
>     return ready_index == 0 ? -1 : 1;
> 
>   if (ready_index == 0)

Thanks for fixing this.  To make it a bit more robust I would suggest using same condition as in ia64_variable_issue():
===
  if (sched_deps_info->generate_spec_deps && !sel_sched_p ())
    /* Modulo scheduling does not extend h_i_d when emitting
       new instructions.  Don't use h_i_d, if we don't have to.  */
    {
      if (DONE_SPEC (insn) & BEGIN_DATA)
	pending_data_specs++;
      if (CHECK_SPEC (insn) & BEGIN_DATA)
	pending_data_specs--;
    }
===

The underlaying problem is that TODO_SPEC(insn) should not be accessed for sel-sched.  Because pending_data_specs turns out to always be null during sel-sched, "pending_data_specs >= 16" can be used as a shorthand for
 "sched_deps_info->generate_spec_deps && !sel_sched_p () && pending_data_specs >= 16"

--
Maxim Kuvyrkov
www.linaro.org