This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Patch Fix PR1936,24894,31644,31786 AVR target
- From: Andrew Hutchinson <andrewhutchinson at cox dot net>
- To: Anatoly Sokolov <aesok at post dot ru>, gcc-patches at gcc dot gnu dot org, "Weddington, Eric" <eweddington at cso dot atmel dot com>
- Date: Fri, 01 Feb 2008 23:44:34 -0500
- Subject: Patch Fix PR1936,24894,31644,31786 AVR target
Anatoly, could please review and comit this solution if it meets your
approval.
The attached patch addresses BASE_POINTER spill failure ICE bugs:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19636
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31786
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24894
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31644
The patch has been tested against gcc head of 20080121 , but with latest
avr target revision (as noted on diff listing)
All example test cases included in bug reports now pass compilation
without failure. (whereas they all failed before at -O2)
Testsuite (execute) has been run and there were no regressions.
=== gcc Summary ===
# of expected passes 11799
# of unexpected failures 52
# of unresolved testcases 23
# of unsupported tests 682
/cygdrive/e/awhconf/gcc/xgcc version 4.3.0 20080121 (experimental) (GCC)
The cause of problem was trying to use base pointer register to reload
an expression that got spilled out of a register - when the were no base
pointers left available!
This typically occurs when r30,r31 - they only freely available base
pointer - was used for function pointer or EEPROM addressing.
The fix is based around introducing r26,r27 as an additional 'emergency'
base pointer register.
1) The LEGITIMIZE_RELOAD_ADDRESS substitution is disabled. This allow
gcc to explore alternatives (which often avoid need for base pointer!)
2) 'legitimate_address_p()' now accepts R26 as base pointer. No new
assembler patterns were needed as these already existed.
These two changes remove basic problem, but as R26 cannot be used as
base pointer without extra instructions code quality suffers. To
prevent this the following changes were made to limit unnecessary usage
of r26 as base pointer, while still retaining situations where r26,r27
is a good solution.
3) Register allocation order (priority) is changed. R26,R27 is now
demoted to the last "call used" pair. R30/31 is now higher priority and
if available will be used as base pointer before R26/27. R28/R29 remains
at lower priority (since it is call saved). I also cleaned up declarations.
4) Small change made to 'avr_hard_regno_mode_ok' to exclude possibility
that a large operand can span off the range of available registers (R31+)
5) Existing Q Constraint was used to for memory addressing using base
pointer registers. This preferred category has been expanded to include
pre_inc and post_dec of pointer registers - thus promoting r26/r27 for
this situation.
6) The general memory 'm' constraint is where r26/27 can be still
assigned as a base pointer. This has been demoted using '?m' to give it
a lower score than "Q" constraint. (do not use ! - this prevent it being
used after reload)
In addition:
6) HI and SImode patterns have "Q" constraint added - this was missing
before. If anything pointers are more useful on larger operands!
7) 'legitimate_address_p()' was rejecting address registers that were
contained inside subreg RTL patterns. e.g. (subreg:HI ((reg:SI 46) 0)).
Such patterns are created when a structure is stored in register. If
this structure contained pointer, it would not be recognized immediately
as a valid address. e.g same as (reg:HI 46). This led to poor pointer
code often missing easy opportunities for post increment and base
+offset instructions. The simple solution (taken from MIPS port) is to
take the inner register.
With above changes, it as rare to see r26 used as base pointer - and
beneficial when it does get used. Overall perhaps a small win on code
size and speed. But importantly 4 less bugs!
Andy
Index: avr.c
===================================================================
--- avr.c (revision 132049)
+++ avr.c (working copy)
@@ -946,6 +946,8 @@
reload_completed ? "(reload_completed)": "",
reload_in_progress ? "(reload_in_progress)": "",
reg_renumber ? "(reg_renumber)" : "");
+ if (!strict && GET_CODE (x) == SUBREG)
+ x = SUBREG_REG (x);
if (GET_CODE (x) == PLUS
&& REG_P (XEXP (x, 0))
&& GET_CODE (XEXP (x, 1)) == CONST_INT
@@ -971,6 +973,7 @@
if (fit)
{
if (! strict
+ || REGNO (XEXP (x,0)) == REG_X
|| REGNO (XEXP (x,0)) == REG_Y
|| REGNO (XEXP (x,0)) == REG_Z)
r = BASE_POINTER_REGS;
@@ -1938,7 +1941,7 @@
/* This is a paranoid case. LEGITIMIZE_RELOAD_ADDRESS must exclude
it but I have this situation with extremal
optimization options. */
-
+
*l = 4;
if (reg_base == reg_dest)
return (AS2 (adiw,r26,%o1) CR_TAB
@@ -4821,43 +4824,9 @@
order_regs_for_local_alloc (void)
{
unsigned int i;
- static const int order_0[] = {
- 24,25,
- 18,19,
- 20,21,
- 22,23,
- 30,31,
- 26,27,
- 28,29,
- 17,16,15,14,13,12,11,10,9,8,7,6,5,4,3,2,
- 0,1,
- 32,33,34,35
- };
- static const int order_1[] = {
- 18,19,
- 20,21,
- 22,23,
- 24,25,
- 30,31,
- 26,27,
- 28,29,
- 17,16,15,14,13,12,11,10,9,8,7,6,5,4,3,2,
- 0,1,
- 32,33,34,35
- };
- static const int order_2[] = {
- 25,24,
- 23,22,
- 21,20,
- 19,18,
- 30,31,
- 26,27,
- 28,29,
- 17,16,
- 15,14,13,12,11,10,9,8,7,6,5,4,3,2,
- 1,0,
- 32,33,34,35
- };
+ static const int order_0[] = REG_ALLOC_ORDER_0;
+ static const int order_1[] = REG_ALLOC_ORDER_1;
+ static const int order_2[] = REG_ALLOC_ORDER_2;
const int *order = (TARGET_ORDER_1 ? order_1 :
TARGET_ORDER_2 ? order_2 :
@@ -5471,6 +5440,14 @@
|| xx == arg_pointer_rtx)
return 1; /* XXX frame & arg pointer checks */
}
+ else if (GET_CODE (x) == PRE_DEC || GET_CODE (x) == POST_INC)
+ {
+ int regno = REGNO (XEXP (x, 0));
+ if (regno == REG_Z || regno == REG_Y || regno == REG_X)
+ return 1;
+ }
+
+
return 0;
}
@@ -5673,7 +5650,7 @@
return 1;
/* Modes larger than QImode occupy consecutive registers. */
- if (regno + GET_MODE_SIZE (mode) > FIRST_PSEUDO_REGISTER)
+ if (regno <= (REG_Z + 1) && (regno + GET_MODE_SIZE (mode)) > (REG_Z + 2))
return 0;
/* All modes larger than QImode should start in an even register. */
Index: avr.h
===================================================================
--- avr.h (revision 132049)
+++ avr.h (working copy)
@@ -198,19 +198,29 @@
1,1,/* STACK */ \
1,1 /* arg pointer */ }
-#define REG_ALLOC_ORDER { \
- 24,25, \
- 18,19, \
- 20,21, \
- 22,23, \
- 30,31, \
- 26,27, \
- 28,29, \
- 17,16,15,14,13,12,11,10,9,8,7,6,5,4,3,2, \
- 0,1, \
- 32,33,34,35 \
- }
+#define REG_ALLOC_ORDER_0 {\
+ 18,22,20,24,19,23,21,25,30,31,26,27,28,29, \
+ 17,16,15,14,13,12,11,10,9,8,7,6,5,4,3,2,\
+ 0,1,\
+ 32,33,34,35 }
+
+#define REG_ALLOC_ORDER_1 {\
+ 18,19,20,21,22,23,24,25,30,31,26,27,28,29,\
+ 17,16,15,14,13,12,11,10,9,8,7,6,5,4,3,2,\
+ 0,1,\
+ 32,33,34,35 }
+
+#define REG_ALLOC_ORDER_2 { \
+ 25,24,23,22,21,20,19,18,30,31,26,27,28,29,\
+ 17,16,15,14,13,12,11,10,9,8,7,6,5,4,3,2,\
+ 1,0,\
+ 32,33,34,35 }
+
+
+#define REG_ALLOC_ORDER REG_ALLOC_ORDER_0
+
+
#define ORDER_REGS_FOR_LOCAL_ALLOC order_regs_for_local_alloc ()
@@ -452,11 +462,14 @@
OPNUM, TYPE); \
goto WIN; \
} \
+ if(0) \
+ { \
push_reload (XEXP (X, 0), NULL_RTX, &XEXP (X, 0), NULL, \
BASE_POINTER_REGS, GET_MODE (X), VOIDmode, 0, 0, \
OPNUM, TYPE); \
goto WIN; \
} \
+ } \
else if (! (frame_pointer_needed && XEXP (X,0) ==
frame_pointer_rtx)) \
{ \
push_reload (X, NULL_RTX, &X, NULL, \
Index: avr.md
===================================================================
--- avr.md (revision 132049)
+++ avr.md (working copy)
@@ -169,8 +169,8 @@
")
(define_insn "*movqi"
- [(set (match_operand:QI 0 "nonimmediate_operand" "=r,d,Qm,r,q,r,*r")
- (match_operand:QI 1 "general_operand" "r,i,rL,Qm,r,q,i"))]
+ [(set (match_operand:QI 0 "nonimmediate_operand" "=r,d,Q?m,r,q,r,*r")
+ (match_operand:QI 1 "general_operand" "r,i,rL,Q?m,r,q,i"))]
"(register_operand (operands[0],QImode)
|| register_operand (operands[1], QImode) || const0_rtx ==
operands[1])"
"* return output_movqi (insn, operands, NULL);"
@@ -250,8 +250,8 @@
(set_attr "cc" "none")])
(define_insn "*movhi"
- [(set (match_operand:HI 0 "nonimmediate_operand" "=r,r,m,d,*r,q,r")
- (match_operand:HI 1 "general_operand" "r,m,rL,i,i,r,q"))]
+ [(set (match_operand:HI 0 "nonimmediate_operand" "=r,r,Q?m,d,*r,q,r")
+ (match_operand:HI 1 "general_operand" "r,Q?m,rL,i,i,r,q"))]
"(register_operand (operands[0],HImode)
|| register_operand (operands[1],HImode) || const0_rtx == operands[1])"
"* return output_movhi (insn, operands, NULL);"
@@ -328,8 +328,8 @@
(define_insn "*movsi"
- [(set (match_operand:SI 0 "nonimmediate_operand" "=r,r,r,Qm,!d,r")
- (match_operand:SI 1 "general_operand" "r,L,Qm,rL,i,i"))]
+ [(set (match_operand:SI 0 "nonimmediate_operand" "=r,r,r,Q?m,!d,r")
+ (match_operand:SI 1 "general_operand" "r,L,Q?m,rL,i,i"))]
"(register_operand (operands[0],SImode)
|| register_operand (operands[1],SImode) || const0_rtx == operands[1])"
"* return output_movsisf (insn, operands, NULL);"