Patchwork [mips] Return correct value from TARGET_SCHED_REORDER2 (PR rtl-optimization/37360)

login
register
mail settings
Submitter Jie Zhang
Date Oct. 23, 2010, 12:44 a.m.
Message ID <4CC22FF4.2020502@codesourcery.com>
Download mbox | patch
Permalink /patch/68985/
State New
Headers show

Comments

Jie Zhang - Oct. 23, 2010, 12:44 a.m.
On 10/21/2010 04:23 PM, Jie Zhang wrote:
> On 10/21/2010 04:18 PM, Andrey Belevantsev wrote:
>> On 21.10.2010 4:30, Jie Zhang wrote:
>>> On 10/21/2010 04:44 AM, Richard Sandiford wrote:
>>>> Jie Zhang<jie@codesourcery.com> writes:
>>>>> Currently TARGET_SCHED_REORDER2 returns mips_issue_rate (), which will
>>>>> reset can_issue_more to the issue_rate after scheduling each
>>>>> instruction. See haifa-sched.c:sched_block for the details. Because of
>>>>> this, issue_rate is not honored when scheduling. This caused PR
>>>>> rtl-optimization/37360 since the issue_rate is set to a value smaller
>>>>> than the number of instructions that scheduler DFA can issue in one
>>>>> cycle for mips sb1.
>>>>
>>>> The haifa-sched.c part looks good to me, but I can't approve it.
>>>> Thanks for tracking down the underlying problem.
>>>>
>>> Andrey and scheduler maintainers?
>>>
>> I'm all for the patch, life is easier for the scheduler when nobody
>> lies. I cannot approve it though. As I wrote earlier, we just need to be
>> on the lookout for possible fallout on other targets which may have the
>> same problem.
>>
> Thanks. Then let's wait for review from a scheduler maintainer.
>
I finally got an approval privately from Vladimir for the haifa-sched.c 
part. And I also found a compile warning in my original patch. The 
attached updated patch is what I have committed. Thanks!

Patch


	PR rtl-optimization/37360
	* config/mips/mips.c (cached_can_issue_more): New local variable.
	(mips_sched_reorder_1): New.
	(mips_sched_reorder): Use mips_sched_reorder_1.
	(mips_sched_reorder2): New.
	(mips_variable_issue): Set cached_can_issue_more.
	(TARGET_SCHED_REORDER2): Define to mips_sched_reorder2
	instead of mips_sched_reorder.

	Revert
	2008-09-09  Andrey Belevantsev  <abel@ispras.ru>
	PR rtl-optimization/37360
	* haifa-sched.c (max_issue): Do not assert that we never issue more
	insns than issue_rate.  Add comment.

	testsuite/
	PR rtl-optimization/37360
	* gcc.dg/pr37360.c: New test.

Index: testsuite/gcc.dg/pr37360.c
===================================================================
--- testsuite/gcc.dg/pr37360.c	(revision 0)
+++ testsuite/gcc.dg/pr37360.c	(revision 0)
@@ -0,0 +1,21 @@ 
+/* PR rtl-optimization/37360 */
+/* { dg-do compile { target fpic } } */
+/* { dg-options "-O3 -fPIC" } */
+
+typedef unsigned int UQItype __attribute__ ((mode (QI)));
+typedef unsigned int USItype __attribute__ ((mode (SI)));
+
+extern const UQItype __popcount_tab[256];
+extern int __popcountsi2 (USItype);
+
+int
+__popcountsi2 (USItype x)
+{
+  int i, ret = 0;
+
+  for (i = 0; i < (4 * 8); i += 8)
+    ret += __popcount_tab[(x >> i) & 0xff];
+
+  return ret;
+}
+
Index: haifa-sched.c
===================================================================
--- haifa-sched.c	(revision 165879)
+++ haifa-sched.c	(working copy)
@@ -2479,14 +2479,7 @@  max_issue (struct ready_list *ready, int
   /* Init max_points.  */
   max_points = 0;
   more_issue = issue_rate - cycle_issued_insns;
-
-  /* ??? We used to assert here that we never issue more insns than issue_rate.
-     However, some targets (e.g. MIPS/SB1) claim lower issue rate than can be
-     achieved to get better performance.  Until these targets are fixed to use
-     scheduler hooks to manipulate insns priority instead, the assert should
-     be disabled.
-
-     gcc_assert (more_issue >= 0);  */
+  gcc_assert (more_issue >= 0);
 
   for (i = 0; i < n_ready; i++)
     if (!ready_try [i])
Index: config/mips/mips.c
===================================================================
--- config/mips/mips.c	(revision 165879)
+++ config/mips/mips.c	(working copy)
@@ -589,6 +589,10 @@  static const char *mips_hi_relocs[NUM_SY
 /* Target state for MIPS16.  */
 struct target_globals *mips16_globals;
 
+/* Cached value of can_issue_more. This is cached in mips_variable_issue hook
+   and returned from mips_sched_reorder2.  */
+static int cached_can_issue_more;
+
 /* Index R is the smallest register class that contains register R.  */
 const enum reg_class mips_regno_to_class[FIRST_PSEUDO_REGISTER] = {
   LEA_REGS,	LEA_REGS,	M16_REGS,	V1_REG,
@@ -12436,11 +12440,11 @@  mips_sched_init (FILE *file ATTRIBUTE_UN
   mips_ls2.falu1_turn_p = true;
 }
 
-/* Implement TARGET_SCHED_REORDER and TARGET_SCHED_REORDER2.  */
+/* Subroutine used by TARGET_SCHED_REORDER and TARGET_SCHED_REORDER2.  */
 
-static int
-mips_sched_reorder (FILE *file ATTRIBUTE_UNUSED, int verbose ATTRIBUTE_UNUSED,
-		    rtx *ready, int *nreadyp, int cycle ATTRIBUTE_UNUSED)
+static void
+mips_sched_reorder_1 (FILE *file ATTRIBUTE_UNUSED, int verbose ATTRIBUTE_UNUSED,
+		      rtx *ready, int *nreadyp, int cycle ATTRIBUTE_UNUSED)
 {
   if (!reload_completed
       && TUNE_MACC_CHAINS
@@ -12455,10 +12459,28 @@  mips_sched_reorder (FILE *file ATTRIBUTE
 
   if (TUNE_74K)
     mips_74k_agen_reorder (ready, *nreadyp);
+}
 
+/* Implement TARGET_SCHED_REORDER.  */
+
+static int
+mips_sched_reorder (FILE *file ATTRIBUTE_UNUSED, int verbose ATTRIBUTE_UNUSED,
+		    rtx *ready, int *nreadyp, int cycle ATTRIBUTE_UNUSED)
+{
+  mips_sched_reorder_1 (file, verbose, ready, nreadyp, cycle);
   return mips_issue_rate ();
 }
 
+/* Implement TARGET_SCHED_REORDER2.  */
+
+static int
+mips_sched_reorder2 (FILE *file ATTRIBUTE_UNUSED, int verbose ATTRIBUTE_UNUSED,
+		     rtx *ready, int *nreadyp, int cycle ATTRIBUTE_UNUSED)
+{
+  mips_sched_reorder_1 (file, verbose, ready, nreadyp, cycle);
+  return cached_can_issue_more;
+}
+
 /* Update round-robin counters for ALU1/2 and FALU1/2.  */
 
 static void
@@ -12516,6 +12538,7 @@  mips_variable_issue (FILE *file ATTRIBUT
 	      || recog_memoized (insn) < 0
 	      || get_attr_type (insn) != TYPE_MULTI);
 
+  cached_can_issue_more = more;
   return more;
 }
 
@@ -16417,7 +16440,7 @@  mips_shift_truncation_mask (enum machine
 #undef TARGET_SCHED_REORDER
 #define TARGET_SCHED_REORDER mips_sched_reorder
 #undef TARGET_SCHED_REORDER2
-#define TARGET_SCHED_REORDER2 mips_sched_reorder
+#define TARGET_SCHED_REORDER2 mips_sched_reorder2
 #undef TARGET_SCHED_VARIABLE_ISSUE
 #define TARGET_SCHED_VARIABLE_ISSUE mips_variable_issue
 #undef TARGET_SCHED_ADJUST_COST