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]

Re: AM30/AM33 SP handling

On Apr 22, 2000, Jeffrey A Law <law@cygnus.com> wrote:

>   In message <oraein9nmk.fsf@zecarneiro.lsd.ic.unicamp.br>you write:
>> On Apr 21, 2000, Jeffrey A Law <law@cygnus.com> wrote:
>> 
>> >   In message <orpurj9run.fsf@zecarneiro.lsd.ic.unicamp.br>you write:
>> >> * config/mn10300/mn10300.md (movsi): Move SP handling to
>> >> separate patterns.
>> 
>> > I fail to see the benefit of the movsi change.
>> 
>> Unfortunately, it's necessary to avoid a reload failure.

>> It might work to simply remove the `x' from this entry

It doesn't.

>> but breaking the SP handling allows us to use data registers to
>> copy from/to SP on AM33.

> I would like to see a more detailed analysis of this problem -- testcases,
> rtl dumps and analysis.

Ok, here's a minimal testcase, derived from newlib/libm/math/kf_tan.c:

extern float T[];
float __kernel_tanf() {
	float w;
	return (T[0]+w*(T[2]+w*(T[3]+w*(T[4]+w*(T[1]+w*T[0])))));
}

% cc1 kf_tan.i -O2 -quiet
kf_tan.i: In function `__kernel_tanf':
kf_tan.i:6: Unable to find a register to spill in class `SP_REGS'.
kf_tan.i:6: This is the insn:
(insn 12 148 145 (set (reg:SI 26)
        (plus:SI (reg:SI 26)
            (const_int 8 [0x8]))) 23 {subsi3-1} (insn_list 10 (nil))
    (expr_list:REG_EQUAL (const:SI (plus:SI (symbol_ref:SI ("T"))
                (const_int 8 [0x8])))
        (nil)))
kf_tan.i:6: Internal compiler error in `spill_failure', at reload1.c:1825

Registers 23 (w), 26 (T+2) and 31 (T[0]), the ones with the most
conflicts, are not allocated in lreg.  23 and 31 aren't because all
DATA registers are already taken, and 26 isn't because the only
ADDRESS register still available, `a1', isn't preserved across
function calls.

Registers 23 and 31 shouldn't get in trouble because they can only be
allocated to DATA_OR_ADDRESS_REGS; register 26 is the one that
requires SP_OR_ADDRESS_REGS, else DATA_OR_ADDRESS_REGS.

In the lreg dump, I find:

(insn 148 10 12 (set (reg:SI 26)
        (reg:SI 25)) 8 {movsi+1} (nil)
    (nil))

(insn 12 148 145 (set (reg:SI 26)
        (plus:SI (reg:SI 26)
            (const_int 8 [0x8]))) 23 {subsi3-1} (insn_list 10 (nil))
    (expr_list:REG_EQUAL (const:SI (plus:SI (symbol_ref:SI ("T"))
                (const_int 8 [0x8])))
        (nil)))

reg 25 is (symbol_ref:SI ("T")), allocated to a0.  The greg dump ends
with:

Spilling for insn 148.
Spilling for insn 12.

but find_reloads() had decided SP_REGS was the best register class for
reg 26, and, since SP cannot be spilled, it loses.


Fortunately, I have a less disruptive solution, whose only drawback is
to not enable moving data regs from/to SP on AM33, which could be
easily done by creating a new AM33-only movsi pattern.  I'll
investigate if there are any gains from that.  Meanwhile, here's the
patch.  Ok to install?


Index: gcc/ChangeLog
from  Alexandre Oliva  <aoliva@cygnus.com>

	* config/mn10300/mn10300.h (REG_CLASS_FROM_LETTER): Return
	EXTENDED_REGS only if TARGET_AM33.
	* config/mn10300/mn10300.md (movsi, addsi): Do not allow
	reload for SP.

Index: gcc/config/mn10300/mn10300.h
===================================================================
RCS file: /cvs/gcc/egcs/gcc/config/mn10300/mn10300.h,v
retrieving revision 1.30
diff -u -r1.30 mn10300.h
--- gcc/config/mn10300/mn10300.h	2000/04/21 21:34:19	1.30
+++ gcc/config/mn10300/mn10300.h	2000/04/23 11:38:57
@@ -287,8 +287,10 @@
 #define REG_CLASS_FROM_LETTER(C) \
   ((C) == 'd' ? DATA_REGS : \
    (C) == 'a' ? ADDRESS_REGS : \
+   (C) == 'y' ? SP_REGS : \
+   ! TARGET_AM33 ? NO_REGS : \
    (C) == 'x' ? EXTENDED_REGS : \
-   (C) == 'y' ? SP_REGS : NO_REGS)
+   NO_REGS)
 
 /* Macros to check register numbers against specific register classes.  */
 
Index: gcc/config/mn10300/mn10300.md
===================================================================
RCS file: /cvs/gcc/egcs/gcc/config/mn10300/mn10300.md,v
retrieving revision 1.29
diff -u -r1.29 mn10300.md
--- gcc/config/mn10300/mn10300.md	2000/04/22 00:43:22	1.29
+++ gcc/config/mn10300/mn10300.md	2000/04/23 11:38:57
@@ -296,9 +296,9 @@
 
 (define_insn ""
   [(set (match_operand:SI 0 "general_operand"
-				"=dx,ax,dx,a,dxm,dxm,axm,axm,dx,dx,ax,ax,axR,y")
+				"=dx,ax,dx,a,dxm,dxm,axm,axm,dx,dx,ax,ax,axR,!y")
 	(match_operand:SI 1 "general_operand"
-				"0,0,I,I,dx,ax,dx,ax,dixm,aixm,dixm,aixm,xy,axR"))]
+				"0,0,I,I,dx,ax,dx,ax,dixm,aixm,dixm,aixm,!y,axR"))]
   "register_operand (operands[0], SImode)
    || register_operand (operands[1], SImode)"
   "*
@@ -744,23 +744,10 @@
 	(plus:SI (match_operand:SI 1 "register_operand" "")
 		 (match_operand:SI 2 "nonmemory_operand" "")))]
   ""
-  "
-{
-  /* We can't add a variable amount directly to the stack pointer;
-     so do so via a temporary register.  */
-  if (operands[0] == stack_pointer_rtx
-      && GET_CODE (operands[1]) != CONST_INT
-      && GET_CODE (operands[2]) != CONST_INT)
-   {
-     rtx temp = gen_reg_rtx (SImode);
-     emit_move_insn (temp, gen_rtx_PLUS (SImode, operands[1], operands[2]));
-     emit_move_insn (operands[0], temp);
-     DONE;
-   }
-}")
+  "")
 
 (define_insn ""
-  [(set (match_operand:SI 0 "register_operand" "=dx,ax,ax,dax,xy,!dax")
+  [(set (match_operand:SI 0 "register_operand" "=dx,ax,ax,dax,!y,!dax")
 	(plus:SI (match_operand:SI 1 "register_operand" "%0,0,0,0,0,dax")
 		 (match_operand:SI 2 "nonmemory_operand" "J,J,L,daxi,i,dax")))]
   "TARGET_AM33"
@@ -838,7 +825,7 @@
   [(set_attr "cc" "set_zn,none_0hit,none_0hit,set_zn,none_0hit,set_zn")])
 
 (define_insn ""
-  [(set (match_operand:SI 0 "register_operand" "=dx,ax,ax,dax,xy,!dax")
+  [(set (match_operand:SI 0 "register_operand" "=dx,ax,ax,dax,!y,!dax")
 	(plus:SI (match_operand:SI 1 "register_operand" "%0,0,0,0,0,dax")
 		 (match_operand:SI 2 "nonmemory_operand" "J,J,L,daxi,i,dax")))]
   ""


>> > It may also be dangerous -- at one time the movXX patterns were
>> > special in that they needed to support all the possible movXX
>> > alternatives in a single pattern.  I do not know if the movXX
>> > patterns are still special in that respect.

>> They don't seem to be.  I've had no regressions in the testsuite with
>> this patch.

> That's not generally enough to make that kind of assertion.  While
> the testsuite is reasonably good at finding bugs, you can't assume
> that because the testsuite shows no regressions that everything is
> OK.

Ok.  Any pointers about how to find out whether the movXX patterns are
still special?

-- 
Alexandre Oliva    Enjoy Guaranį, see http://www.ic.unicamp.br/~oliva/
Cygnus Solutions, a Red Hat company        aoliva@{redhat, cygnus}.com
Free Software Developer and Evangelist    CS PhD student at IC-Unicamp
oliva@{lsd.ic.unicamp.br, gnu.org}   Write to mailing lists, not to me
Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]