Patchwork RFA: RL78: Improve prologues for interrupt handlers

login
register
mail settings
Submitter Nick Clifton
Date March 26, 2013, 11:30 a.m.
Message ID <87ppympfes.fsf@redhat.com>
Download mbox | patch
Permalink /patch/231180/
State New
Headers show

Comments

Nick Clifton - March 26, 2013, 11:30 a.m.
Hi DJ,

  I am submitting a patch on behalf of Renesas and KPIT.  They found a
  way to improve the prologues for interrupt handlers so that only the
  registers that actually need to be saved are pushed onto the stack.

  The patch has been tested with no regressions in the gcc testsuite for
  an rl78-elf toolchain, as well as various different interrupt handler
  configurations.

  OK to apply ?

Cheers
  Nick

gcc/ChangeLog
2013-03-26  Kaushik Phatak <kaushik.phatak@kpitcummins.com>

	* config/rl78/rl78.c (need_to_save): Change return type to bool.
	For interrupt functions: save call clobbered registers if the
        register is used or the handler is not a leaf function.
	(rl78_expand_prologue): Always recompute the frame information.
	For interrupt functions: only select bank 0 if one of the bank 0
	registers is going to be pushed.
DJ Delorie - March 27, 2013, 1:21 a.m.
The problem is, devirt hasn't happened yet when we compute the frame
size, so we *can't* know if bank 0 registers are used yet.

Also, I've had problems recomputing the frame size after reload.  If
the frame size changes for *any* reason between reload and prologue,
you get corrupt code, as the elimination offset from AP to SP changes.
That's why I avoid recomputing it.

Patch

Index: gcc/config/rl78/rl78.c
===================================================================
--- gcc/config/rl78/rl78.c	(revision 197093)
+++ gcc/config/rl78/rl78.c	(working copy)
@@ -373,34 +373,43 @@ 
   return true;
 }
 
-/* Returns nonzero if the given register needs to be saved by the
+/* Returns true if the given register needs to be saved by the
    current function.  */
-static int
-need_to_save (int regno)
+static bool
+need_to_save (unsigned int regno)
 {
   if (is_interrupt_func (cfun->decl))
     {
-      if (regno < 8)
-	return 1; /* don't know what devirt will need */
+       /* We don't need to save registers that have
+	  been reserved for interrupt handlers.  */
       if (regno > 23)
-	return 0; /* don't need to save interrupt registers */
-      if (crtl->is_leaf)
-	{
-	  return df_regs_ever_live_p (regno);
-	}
-      else
-	return 1;
+	return false;
+
+      /* If the handler is a non-leaf function then it may call
+	 non-interrupt aware routines which will happily clobber
+	 any call_used registers, so we have to preserve them.  */
+      if (!crtl->is_leaf && call_used_regs[regno])
+	return true;
+
+      /* Otherwise we only have to save a register, call_used
+	 or not, if it is used by this handler.  */
+      return df_regs_ever_live_p (regno);
     }
+
   if (regno == FRAME_POINTER_REGNUM && frame_pointer_needed)
-    return 1;
+    return true;
+
   if (fixed_regs[regno])
-    return 0;
+    return false;
+
   if (crtl->calls_eh_return)
-    return 1;
+    return true;
+
   if (df_regs_ever_live_p (regno)
       && !call_used_regs[regno])
-    return 1;
-  return 0;
+    return true;
+
+  return false;
 }
 
 /* We use this to wrap all emitted insns in the prologue.  */
@@ -833,14 +842,22 @@ 
   rtx sp = gen_rtx_REG (HImode, STACK_POINTER_REGNUM);
   int rb = 0;
 
-  if (!cfun->machine->computed)
-    rl78_compute_frame_info ();
 
+ /* Always re-compute the frame info - the
+    register usage may have changed.  */
+  rl78_compute_frame_info ();
+
   if (flag_stack_usage_info)
     current_function_static_stack_size = cfun->machine->framesize;
 
   if (is_interrupt_func (cfun->decl))
-    emit_insn (gen_sel_rb (GEN_INT (0)));
+    for (i = 0; i < 4; i++)
+      if (cfun->machine->need_to_push [i])
+	{
+	  /* Select Bank 0 if we are using any registers from Bank 0.   */
+	  emit_insn (gen_sel_rb (GEN_INT (0)));
+	  break;
+	}
 
   for (i = 0; i < 16; i++)
     if (cfun->machine->need_to_push [i])