This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Patch to fix a MIPS movstrsi scheduling bug.


This define_split in mips.md loses rtx-unchanging & alias information:

    ;; Split a block move into 2 parts, the first part is everything
    ;; except for the last move, and the second part is just the last
    ;; store, which is exactly 1 instruction (ie, not a usw), so it can
    ;; fill a delay slot.  This also prevents a bug in delayed branches
    ;; from showing up, which reuses one of the registers in our clobbers.

    (define_split
      [(set (mem:BLK (match_operand:SI 0 "register_operand" ""))
            (mem:BLK (match_operand:SI 1 "register_operand" "")))
       (clobber (match_operand:SI 4 "register_operand" ""))
       (clobber (match_operand:SI 5 "register_operand" ""))
       (clobber (match_operand:SI 6 "register_operand" ""))
       (clobber (match_operand:SI 7 "register_operand" ""))
       (use (match_operand:SI 2 "small_int" ""))
       (use (match_operand:SI 3 "small_int" ""))
       (use (const_int 0))]

      "reload_completed && !TARGET_DEBUG_D_MODE && INTVAL (operands[2]) > 0"

      ;; All but the last move
      [(parallel [(set (mem:BLK (match_dup 0))
                       (mem:BLK (match_dup 1)))
                  (clobber (match_dup 4))
                  (clobber (match_dup 5))
                  (clobber (match_dup 6))
                  (clobber (match_dup 7))
                  (use (match_dup 2))
                  (use (match_dup 3))
                  (use (const_int 1))])

       ;; The last store, so it can fill a delay slot
       (parallel [(set (mem:BLK (match_dup 0))
                       (mem:BLK (match_dup 1)))
                  (clobber (match_dup 4))
                  (clobber (match_dup 5))
                  (clobber (match_dup 6))
                  (clobber (match_dup 7))
                  (use (match_dup 2))
                  (use (match_dup 3))
                  (use (const_int 2))])]

      "")

Now that could be fixed by making operands 0 and 1 include
the MEM.  But I'm a bit uneasy about this code anyway.
The (const_int 2) pattern uses values left in scratch
registers by the (const_int 1) pattern, but nothing in the
rtl seems to say that.

It seems safer to just disable the split, at least for 3.3.
I guess the "proper" fix would be to expand the movstrsi
stuff as rtl, giving the scheduler a chance to do more with it.

FWIW, this code appears in revision 1.1 of mips.md in the Cygnus
CVS repository (dated Feb 1993).  I hope the dbr bug has been fixed
by now.  It didn't show up on a mips64-elf build & regression test.

The test case fails for -O3 -funroll-loops.

OK for mainline?

Richard


	* config/mips/mips.md: Disable the movstrsi define_split.

Index: config/mips/mips.md
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/mips/mips.md,v
retrieving revision 1.150
diff -c -d -p -F^[(a-zA-Z0-9_^#] -r1.150 mips.md
*** config/mips/mips.md	6 Nov 2002 18:10:58 -0000	1.150
--- config/mips/mips.md	26 Nov 2002 18:14:04 -0000
*************** (define_insn ""
*** 6547,6552 ****
--- 6547,6555 ----
  ;; fill a delay slot.  This also prevents a bug in delayed branches
  ;; from showing up, which reuses one of the registers in our clobbers.
  
+ ;; ??? Disabled because it doesn't preserve alias information for
+ ;; operands 0 and 1.  Also, the rtl for the second insn doesn't mention
+ ;; that it uses the registers clobbered by the first.
  (define_split
    [(set (mem:BLK (match_operand:SI 0 "register_operand" ""))
  	(mem:BLK (match_operand:SI 1 "register_operand" "")))
*************** (define_split
*** 6558,6564 ****
     (use (match_operand:SI 3 "small_int" ""))
     (use (const_int 0))]
  
!   "reload_completed && !TARGET_DEBUG_D_MODE && INTVAL (operands[2]) > 0"
  
    ;; All but the last move
    [(parallel [(set (mem:BLK (match_dup 0))
--- 6561,6567 ----
     (use (match_operand:SI 3 "small_int" ""))
     (use (const_int 0))]
  
!   "reload_completed && 0 && INTVAL (operands[2]) > 0"
  
    ;; All but the last move
    [(parallel [(set (mem:BLK (match_dup 0))
*** /dev/null	Thu Apr 11 15:25:15 2002
--- testsuite/gcc.c-torture/execute/20021127-1.c	Tue Nov 26 18:17:40 2002
***************
*** 0 ****
--- 1,20 ----
+ int __attribute__ ((noinline))
+ foo ()
+ {
+   const int a[8] = { 0, 1, 2, 3, 4, 5, 6, 7 };
+   int i, sum;
+ 
+   sum = 0;
+   for (i = 0; i < sizeof (a) / sizeof (*a); i++)
+     sum += a[i];
+ 
+   return sum;
+ }
+ 
+ int
+ main ()
+ {
+   if (foo () != 28)
+     abort ();
+   exit (0);
+ }


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]