[PATCH, ARM, RFC] Fix gfortran.dg/vector_subscript_1.f90 for -Os -mthumb reload ICE

[Julian Brown <julian at codesourcery dot com>]

Hi,

The gfortran.dg/vector_subscript_1.f90 test case fails at -Os -mthumb
for ARM EABI, for a reason which I will attempt to explain, though I'm
not entirely certain that I'm seeing the whole picture.

The failure happens generating a reload for a nested function, on an
instruction which reads r12 (ip), the function's static chain pointer,
which is hardwired early in RTL generation:

vector_subscript_1.f90: In function 'test':
vector_subscript_1.f90:156: error: unable to find a register to spill
in class 'HI_REGS' vector_subscript_1.f90:156: error: this is the insn:
(insn:HI 4 3 5 2 vector_subscript_1.f90:19 (set (reg/f:SI 149
[ CHAIN.263 ]) (reg:SI 12 ip [ CHAIN.263 ])) 162 {*thumb1_movsi_insn}
(expr_list:REG_DEAD (reg:SI 12 ip [ CHAIN.263 ]) (nil)))

The corresponding fragment of 176r.greg is:

Spilling for insn 4.
Using reg 3 for reload 0
reload failure for reload 1

Reloads for insn # 4
Reload 0: LO_REGS, RELOAD_FOR_OUTPUT_ADDRESS (opnum = 0), can't
combine, secondary_reload_p
Reload 1: reload_out (SI) = (reg/f:SI 149 [ CHAIN.263 ])
        HI_REGS, RELOAD_FOR_OUTPUT (opnum = 0)
        reload_out_reg: (reg/f:SI 149 [ CHAIN.263 ])
        secondary_out_reload = 0

So, it seems like GCC wants to find a HI_REGS class register to spill.
But, saving HI_REGS (r8-r15) in Thumb-1 mode is very costly in terms of
the prologue/epilogue instructions necessary to save and restore
registers, so all are marked fixed in optimize_size mode, to
discourage their use. The last constraint in this pattern:

(define_insn "*thumb1_movsi_insn"
  [(set (match_operand:SI 0 "nonimmediate_operand"
                           "=l,l,l,l,l,>,l,m,*lh")
	(match_operand:SI 1 "general_operand"
                           "l,I,J,K,>,l,mi,l,*lh"))]
  ...

seems to be causing the above failure, by causing just HI_REGS to be
preferred for this reload.

The first thing to wonder is why a larger register class is not chosen
for that alternative: I think the reason is that GENERAL_REGS (the
likely contender) isn't a true union of the LO_REGS and HI_REGS classes
indicated by the 'l' and 'h' constraint letters, because it has an
extra register too, the soft frame pointer:

{ 0x0200FFFF, 0x00000000, 0x00000000, 0x00000000 }, /* GENERAL_REGS */

So this patch adds a class which is the union of the HI_REGS and
LO_REGS classes, named HILO_REGS. This unfortunately isn't enough by
itself to fix the problem though: reload still chooses HI_REGS, and
still blows up in the same place.

We can fix that by changing each of the last constraint alternatives to
"*l*h" (or several variations [1]), which causes reload to be sensible
and use the new HILO_REGS class for the previously-breaking
instruction. I've only made this change at -Os, to avoid discouraging
usage of high registers when not optimising for size.

Are there any flaws in my reasoning above, or is this patch OK?
(Currently based on a Debian-patched 4.3, but I suspect mainline will
have the same issue. I still need to test that though).

[1] I ran CSiBE with the last constraint in "thumb1_movsi_insn" changed
to each of "*l*h", "l*h", "!*lh", "!lh" and "lh". All fix the reload
crash in question, though "!lh" and "lh" also cause an increase in code
size. I don't know enough about reload internals to know which version
is the most correct.

Cheers,

Julian

ChangeLog

    gcc/
    * config/arm/arm.h (reg_class): Add HILO_REGS class as union of
    HI_REGS and LO_REGS.
    (REG_CLASS_NAMES): Likewise.
    (REG_CLASS_CONTENTS): Likewise.
    (PREFERRED_RELOAD_CLASS): Prefer LO_REGS for HILO_REGS reloads.
    * config/arm/arm.md (*thumb1_movsi_insn): Only use
    for !optimize_size.
    (*thumb1_movsi_insn_osize): New. Use for optimize_size Thumb-1
    SImode moves.

Index: gcc/config/arm/arm.h
===================================================================
--- gcc/config/arm/arm.h	(revision 206057)
+++ gcc/config/arm/arm.h	(working copy)
@@ -1107,6 +1107,7 @@ enum reg_class
   STACK_REG,
   BASE_REGS,
   HI_REGS,
+  HILO_REGS,
   CC_REG,
   VFPCC_REG,
   GENERAL_REGS,
@@ -1132,6 +1133,7 @@ enum reg_class
   "STACK_REG",		\
   "BASE_REGS",		\
   "HI_REGS",		\
+  "HILO_REGS",		\
   "CC_REG",		\
   "VFPCC_REG",		\
   "GENERAL_REGS",	\
@@ -1156,6 +1158,7 @@ enum reg_class
   { 0x00002000, 0x00000000, 0x00000000, 0x00000000 }, /* STACK_REG */	\
   { 0x000020FF, 0x00000000, 0x00000000, 0x00000000 }, /* BASE_REGS */	\
   { 0x0000FF00, 0x00000000, 0x00000000, 0x00000000 }, /* HI_REGS */	\
+  { 0x0000FFFF, 0x00000000, 0x00000000, 0x00000000 }, /* HILO_REGS */	\
   { 0x01000000, 0x00000000, 0x00000000, 0x00000000 }, /* CC_REG */	\
   { 0x00000000, 0x00000000, 0x00000000, 0x80000000 }, /* VFPCC_REG */	\
   { 0x0200FFFF, 0x00000000, 0x00000000, 0x00000000 }, /* GENERAL_REGS */ \
@@ -1217,7 +1220,8 @@ enum reg_class
 #define PREFERRED_RELOAD_CLASS(X, CLASS)		\
   (TARGET_ARM ? (CLASS) :				\
    ((CLASS) == GENERAL_REGS || (CLASS) == HI_REGS	\
-    || (CLASS) == NO_REGS ? LO_REGS : (CLASS)))
+    || (CLASS) == HILO_REGS || (CLASS) == NO_REGS	\
+    ? LO_REGS : (CLASS)))
 
 /* Must leave BASE_REGS reloads alone */
 #define THUMB_SECONDARY_INPUT_RELOAD_CLASS(CLASS, MODE, X)		\
Index: gcc/config/arm/arm.md
===================================================================
--- gcc/config/arm/arm.md	(revision 206057)
+++ gcc/config/arm/arm.md	(working copy)
@@ -4823,7 +4823,30 @@
 	(match_operand:SI 1 "general_operand"      "l, I,J,K,>,l,mi,l,*lh"))]
   "TARGET_THUMB1
    && (   register_operand (operands[0], SImode) 
-       || register_operand (operands[1], SImode))"
+       || register_operand (operands[1], SImode))
+   && !optimize_size"
+  "@
+   mov	%0, %1
+   mov	%0, %1
+   #
+   #
+   ldmia\\t%1, {%0}
+   stmia\\t%0, {%1}
+   ldr\\t%0, %1
+   str\\t%1, %0
+   mov\\t%0, %1"
+  [(set_attr "length" "2,2,4,4,2,2,2,2,2")
+   (set_attr "type" "*,*,*,*,load1,store1,load1,store1,*")
+   (set_attr "pool_range" "*,*,*,*,*,*,1020,*,*")]
+)
+
+(define_insn "*thumb1_movsi_insn_osize"
+  [(set (match_operand:SI 0 "nonimmediate_operand" "=l,l,l,l,l,>,l, m,*l*h")
+	(match_operand:SI 1 "general_operand"      "l, I,J,K,>,l,mi,l,*l*h"))]
+  "TARGET_THUMB1
+   && (   register_operand (operands[0], SImode) 
+       || register_operand (operands[1], SImode))
+   && optimize_size"
   "@
    mov	%0, %1
    mov	%0, %1