Re: ARM: Support for interrupt functions - mk II

[Nick Clifton <nickc at redhat dot com>]

Hi Phil,

: >+     case FT_ISR:
: >+     case FT_EXCEPTION:
: >+       /* Interrupt and exception handlers work with their own set of
: >+ 	 banked registers, so there is no need to save any of them
: >+ 	 onto the stack upon function entry.  */
: >+       break;
: >+ 
: >+     case FT_FIQ:
: >+       /* FIQ handlers only have registers r0 - r7 banked, so
: >+ 	 if they corrupt any others, they must save them.  */
: >+       for (reg = 8; reg <= 12; reg++)
: >+ 	if (regs_ever_live[reg])
: >+ 	  save_reg_mask |= (1 << reg);
: >+       break;
: 
: Hi Nick,
: 
: This is exactly backwards.  FIQ mode has R8..R14 banked, it's R0-R7 that must 
: be saved.  IRQ and exception modes only bank R13 and R14 so everything must be 
: saved.

Duh!

Yes you are right.  I was completely thrown by the diagram in the ARM
ARM on page 2-4, which I thought implied that there was a separate
register bank for user/system mode, except for the FIQ handlers which
shared some registers.  I really should have looked at the text more
closely.

OK, so I have revised that section of the patch to this:

+     case FT_ISR:
+     case FT_EXCEPTION:
+       /* Deciding which registers to save for an interrupt service
+ 	   routine is tricky.  We must save all registers that we corrupt,
+ 	   even if they are call used.  If we call another function then
+ 	   we have to save every register that that function might corrupt
+ 	   as well.  In practice this means all the call-used registers and
+ 	   any touched call-saved registers.  */
+       for (reg = 0; reg <= 12; reg++)
+ 	  if ((! current_function_is_leaf && call_used_regs [reg])
+ 	      || regs_ever_live [reg])
+ 	    save_reg_mask |= (1 << reg);
+       break;
+ 
+     case FT_FIQ:
+       /* FIQ handlers have registers r8 - r14 banked, so there is
+ 	   no need to save them.  The other registers must be saved
+          with the same criteria as for normal interrupt handlers.  */
+       for (reg = 0; reg <= 7; reg++)
+ 	  if ((! current_function_is_leaf && call_used_regs [reg])
+ 	      || regs_ever_live[reg])
+ 	    save_reg_mask |= (1 << reg);
+       break;

I have also taken the liberty of adding a new feature, now that I know
that FIQs should prefer to use registers 8 through 12.  This is the
reordering of the register alloc order array on a per-function basis,
so that FIQ handlers will preferentially used these banked registers,
but normal functions will use the default ordering.  This addition
requires a patch to the main body of gcc, so I will be submitting just
this new feature as a separate patch to the gcc-patches mailing list.

I have attached the ARM specific part of this new feature below.

Any other comments ?

Cheers
	Nick

Index: config/arm/arm.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/config/arm/arm.c,v
retrieving revision 1.107
diff -p -w -r1.107 arm.c
*** arm.c	2000/09/08 14:26:29	1.107
--- arm.c	2000/09/20 19:45:49
*************** thumb_unexpanded_epilogue ()
*** 8949,8961 ****
--- 9158,9205 ----
    return "";
  }
  
+ /* Special case handling of register class initialisation.  */
+ 
+ void
+ arm_reg_class_init ()
+ {
+   static int orig_order[FIRST_PSEUDO_REGISTER] = REG_ALLOC_ORDER;
+   static int fiq_order[FIRST_PSEUDO_REGISTER]  = FIQ_ALLOC_ORDER;
+   int i;
+ 
+   if (arm_current_func_type () == FT_FIQ)
+     {
+       /* If the current function is a FIQ handler, and the
+ 	 register ordering is not FIQ biased, change it.  */
+       
+       if (reg_alloc_order [0] == orig_order[0])
+ 	{
+ 	  for (i = ARRAY_SIZE (reg_alloc_order); i--;)
+ 	    reg_alloc_order [i] = fiq_order [i];
+ 	  
+ 	  for (i = ARRAY_SIZE (inv_reg_alloc_order); i--;)
+ 	    inv_reg_alloc_order[reg_alloc_order[i]] = i;
+ 	}
+     }
+   else if (reg_alloc_order [0] != orig_order[0])
+     {
+       /* Restore the default register alloc ordering.  */
+       
+       for (i = ARRAY_SIZE (reg_alloc_order); i--;)
+ 	reg_alloc_order [i] = orig_order [i];
+       
+       for (i = ARRAY_SIZE (inv_reg_alloc_order); i--;)
+ 	inv_reg_alloc_order[reg_alloc_order[i]] = i;
+     }
+ }
+ 
  /* Functions to save and restore machine-specific function data.  */
  
  static void