Andrey, this is likely due to the selective scheduler merge.  Can you
investigate or delegate?

> Andrey, this is likely due to the selective scheduler merge.  Can you
> investigate or delegate?
We couldn't reproduce this with a cross from x86_64.  Also,  Adam Nemet fixed the problem with MIPS/sel-sched bootstrap (http://gcc.gnu.org/ml/gcc-patches/2008-09/msg00152.html) -- the mail mentions that he reached stage3 with his patch of 139918, this bug is of 139940.  Can this be reproduced natively on MIPS with 139918?  

This does not fail on a cross for us with 139918.  

You will note that I configured with --with-arch=sb1.

This in turn causes cc1 to be invoked with -march=sb1

I will attempt to test with a cross build.

My bootstrap gcc is: gcc (GCC) 4.4.0 20080223 (experimental) [trunk revision 132568]

There is the possibility that the stage1 compiler is being miscompiled, so I will try another bootstrap with: gcc (Debian 4.3.1-8) 4.3.1 to see if that changes anything.

I would suggest to try ira-merge branch to rule out any IRA related
problems.

I get the same ICE using gcc (Debian 4.3.1-8) 4.3.1 as the bootstrap compiler, so I am going with the theory that the bootstrap compiler is not the cause of this problem.

The problem is present in 139918

It is reproducible in a cross compiler as well.

This is my command line:

/home/daney/gccsvn/mipsel-trunk/gcc/cc1 -fpreprocessed j.i -quiet -march=sb1 -O2 -o j.s

Changing to -march={mips32,mips32r2,r5000} "fixes" the problem.  It seems to be sb1 specific.

(In reply to comment #2)
> > Andrey, this is likely due to the selective scheduler merge.  Can you
> > investigate or delegate?
> We couldn't reproduce this with a cross from x86_64.  Also,  Adam Nemet fixed
> the problem with MIPS/sel-sched bootstrap
> (http://gcc.gnu.org/ml/gcc-patches/2008-09/msg00152.html) -- the mail mentions
> that he reached stage3 with his patch of 139918, this bug is of 139940.  Can
> this be reproduced natively on MIPS with 139918?  
> 

The ICE first appears in 139854 (the sel-sched merge) it is still present in 139940 (and about 10 other version I tested between the two including 139918).  It is easily reproducible in a cross compile, but you must specify -march=sb1 to trigger it.

This may be related to PR 37378 and PR 37377.

I was unable to trigger the assert on the given testcase even with the cross-compiler configured exactly like the one in the bug report.  However, I was able to reproduce the ICE on a longer plain C test case.  

The assert that triggers actually means that we will never issue more insns on a cycle than issue_rate, i.e. cycle_issued_insns is always <= issue_rate.  However, both before and after selective scheduling merge, I see (the log is given for the trunk just before sel-sched merge):

;;      Ready list (t =   0):    117  101  100
;;              Choosed insn : 100; points: 3/3
;;        0-->   100 $2=sxn([$18])                     :sb1_ls0|sb1_ls1
;;              dependencies resolved: insn 104
;;              Ready-->Q: insn 104: queued for 1 cycles.
;;              tick updated: insn 104 into queue with cost=1
;;              dependencies resolved: insn 118
;;              tick updated: insn 118 into ready
;;      Ready list (t =   0):    118  117  101
;;              Choosed insn : 101; points: 3/2
;;        0-->   101 $16=sxn([$17])                    :sb1_ls0|sb1_ls1
;;              dependencies resolved: insn 119
;;              tick updated: insn 119 into ready
;;      Ready list (t =   0):    119  118  117
;;              Choosed insn : 117; points: 2/1
;;        0-->   117 $19=$19+0x1                       :sb1_ls1|sb1_ex1|sb1_ex0
;;      Ready list (t =   0):    119  118
;;              Choosed insn : 118; points: 1/1
;;        0-->   118 $18=$18+0x1                       :sb1_ls1|sb1_ex1|sb1_ex0
;;      Ready list (t =   0):    119
;;              Choosed insn : 119; points: -1/1

<assert is triggered here after the merge>

;;              Ready-->Q: insn 119: queued for 1 cycles.

This means that we've issued insns 100, 101, 117, and 118, while at the same time claiming that issue_rate is 3.  In mips_issue_rate, I see the following:

    case PROCESSOR_SB1:
    case PROCESSOR_SB1A:
      /* This is actually 4, but we get better performance if we claim 3.
         This is partly because of unwanted speculative code motion with the
         larger number, and partly because in most common cases we can't
         reach the theoretical max of 4.  */
      return 3;

So this is while the assert is triggered only for those arches.  

The assert was introduced with sel-sched merge just for purposes of stricter checking.  I was not aware of targets that claim issue rate which is lower than the maximal value that can be actually achieved.  I see that it is done on purpose, however hackish it may seem though.  So I'd propose to remove the assert in max_issue and add a comment saying that for some targets the assert may not be true.  If people are happy with this, I will prepare a patch.

The other solution will be to fix mips.c to make it not to lie to the scheduler about its issue rate, and fix the performance issues with e.g. lowering priority of speculative motions in the appropriate scheduler hooks.  But I believe that this is not appropriate for stage3. 

I think it's reasonable to disable the assert with a comment for now and
file a separate bugreport for the targets who lie about their issue rate.

(In reply to comment #12)
> I think it's reasonable to disable the assert with a comment for now and
> file a separate bugreport for the targets who lie about their issue rate.

Ok, with the below patch my failing test case compiles in an identical assembly as it was compiled before the sel-sched merge.  I've also bootstrapped it on x86_64.  If the patch will fix MIPS bootstrap, I think it can be committed as obvious.

2008-09-05  Andrey Belevantsev  <abel@ispras.ru>

        PR 37360
        * haifa-sched.c (max_issue): Do not assert that we never issue more insns than issue_rate.
        Add comment.

Index: gcc/haifa-sched.c
===================================================================
*** gcc/haifa-sched.c	(revision 140031)
--- gcc/haifa-sched.c	(working copy)
*************** max_issue (struct ready_list *ready, int
*** 2071,2077 ****
    /* Init max_points.  */
    max_points = 0;
    more_issue = issue_rate - cycle_issued_insns;
!   gcc_assert (more_issue >= 0);
  
    for (i = 0; i < n_ready; i++)
      if (!ready_try [i])
--- 2071,2084 ----
    /* 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);  */
  
    for (i = 0; i < n_ready; i++)
      if (!ready_try [i])

Created attachment 16231 [details]
Proposed patch

Thanks for the patch.

With the patch applied to the trunk my bootstrap has made it into stage 2, indicating that it has corrected the problem.

Could you explain why max_issue() should do anything
when more_issue <= 0?  I'd have expected it to early-out.

(In reply to comment #16)
> Could you explain why max_issue() should do anything
> when more_issue <= 0?  I'd have expected it to early-out.

But the whole point of the patch is that we _can_ actually issue more insns even when more_issue <= 0, because the backend is claiming lower issue_rate than can be achieved (see comment #11).  We can return 0 from max_issue when more_issue <= 0, but I don't see offhand how the theoretical value of 4 issued insns can then be achieved for sb1. 

Actually, I have skimmed through *_issue_rate functions on other targets, and it seems that only m32r has the same sort of comment for its issue_rate hook, but by default the hook passes correct value.  So I would be happy to do whatever seems beneficial to mips/m32r maintainers.

Subject: Bug 37360

Author: abel
Date: Tue Sep  9 14:19:31 2008
New Revision: 140151

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=140151
Log:
        PR rtl-optimization/37360
        * haifa-sched.c (max_issue): Do not assert that we never issue more
        insns than issue_rate.  Add comment.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/haifa-sched.c

Fixed in 140151.

Just for record, "./cc1 -march=sb1 -O3 j.i -fPIC" can reproduce this issue on latest trunk if r140151 is reversed.

Another way to fix this bug:

http://gcc.gnu.org/ml/gcc/2010-10/msg00281.html

David, are you still interested to try this patch on sb1?

I no longer have access to an SB1.  But you should be able to run the test case on a cross compiler to see how it is affected by any patch.

Author: jiez
Date: Sat Oct 23 00:38:13 2010
New Revision: 165880

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=165880
Log:
	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.

Added:
    trunk/gcc/testsuite/gcc.dg/pr37360.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/mips/mips.c
    trunk/gcc/haifa-sched.c
    trunk/gcc/testsuite/ChangeLog