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]
Other format: [Raw text]

PR16634: Wrong-code for IRQ functions.


When generating interworked armv4t code gcc generates incorrect function 
epilogue code for IRQ/FIQ functions.

The current code works on the assumption that it is not possible to pop the 
return value directly into the PC in interworked v4t functions, and must go 
via in intermediate register. In fact it is safe to pop directly into PC when 
returning from an exception function (with ldmfd ...,pc}^) because the 
Thumbness comes from the SPSR, not the return address.

Popping the value into a register confuses the later epilogue generation code 
into thinking teh value wasn't pused, and needs adjustment.

The attached patch fixes this by popping the value directly into the PC when 
returning from interrupt functions, regardless of interworking.

AFAICS this has always been broken, so is not a regression. IMHO writing 
interrupt handlers directly in C is a fairly dubious feature anyway, so 
unless someone thinks it's important I'm defering this patch until mainline 
is open again.

Tested with cross to arm-none-eabi.
Applied to branches/csl/sourcerygxx-4_1.

Paul

2006-08-09  Paul Brook  <paul@codesourcery.com>

	PR target/16634
	* config/arm/arm.c (use_return_insn): Return 0 for Thumb interrupt
	functions.
	(print_multi_reg): Add rfe argument for IRQ returns.
	(arm_output_epilogue): Pop interrupt return address directly into PC.
	(arm_expand_prologue): Only adjust IRQ return address in Arm mode.

Index: gcc/config/arm/arm.c
===================================================================
--- gcc/config/arm/arm.c	(revision 116016)
+++ gcc/config/arm/arm.c	(working copy)
@@ -1537,8 +1537,9 @@ use_return_insn (int iscond, rtx sibling
   if (func_type & (ARM_FT_VOLATILE | ARM_FT_NAKED | ARM_FT_STACKALIGN))
     return 0;
 
-  /* So do interrupt functions that use the frame pointer.  */
-  if (IS_INTERRUPT (func_type) && frame_pointer_needed)
+  /* So do interrupt functions that use the frame pointer and Thumb
+     interrupt functions.  */
+  if (IS_INTERRUPT (func_type) && (frame_pointer_needed || TARGET_THUMB))
     return 0;
 
   offsets = arm_get_frame_offsets ();
@@ -1599,7 +1600,7 @@ use_return_insn (int iscond, rtx sibling
 
   /* Can't be done if interworking with Thumb, and any registers have been
      stacked.  */
-  if (TARGET_INTERWORK && saved_int_regs != 0)
+  if (TARGET_INTERWORK && saved_int_regs != 0 && !IS_INTERRUPT(func_type))
     return 0;
 
   /* On StrongARM, conditional returns are expensive if they aren't
@@ -8679,15 +8680,17 @@ fp_const_from_val (REAL_VALUE_TYPE *r)
 /* Output the operands of a LDM/STM instruction to STREAM.
    MASK is the ARM register set mask of which only bits 0-15 are important.
    REG is the base register, either the frame pointer or the stack pointer,
-   INSTR is the possibly suffixed load or store instruction.  */
+   INSTR is the possibly suffixed load or store instruction.
+   RFE is nonzero if the instruction should also copy spsr to cpsr.  */
 
 static void
 print_multi_reg (FILE *stream, const char *instr, unsigned reg,
-		 unsigned long mask)
+		 unsigned long mask, int rfe)
 {
   unsigned i;
   bool not_first = FALSE;
 
+  gcc_assert (!rfe || (mask & (1 << PC_REGNUM)));
   fputc ('\t', stream);
   asm_fprintf (stream, instr, reg);
   fputc ('{', stream);
@@ -8702,7 +8705,10 @@ print_multi_reg (FILE *stream, const cha
 	not_first = TRUE;
       }
 
-  fprintf (stream, "}\n");
+  if (rfe)
+    fprintf (stream, "}^\n");
+  else
+    fprintf (stream, "}\n");
 }
 
 
@@ -10366,16 +10372,17 @@ arm_output_epilogue (rtx sibling)
 	  || current_function_calls_alloca)
 	asm_fprintf (f, "\tsub\t%r, %r, #%d\n", SP_REGNUM, FP_REGNUM,
 		     4 * bit_count (saved_regs_mask));
-      print_multi_reg (f, "ldmfd\t%r, ", SP_REGNUM, saved_regs_mask);
+      print_multi_reg (f, "ldmfd\t%r, ", SP_REGNUM, saved_regs_mask, 0);
 
       if (IS_INTERRUPT (func_type))
 	/* Interrupt handlers will have pushed the
 	   IP onto the stack, so restore it now.  */
-	print_multi_reg (f, "ldmfd\t%r!, ", SP_REGNUM, 1 << IP_REGNUM);
+	print_multi_reg (f, "ldmfd\t%r!, ", SP_REGNUM, 1 << IP_REGNUM, 0);
     }
   else
     {
       HOST_WIDE_INT amount;
+      int rfe;
       /* Restore stack pointer if necessary.  */
       if (frame_pointer_needed)
 	{
@@ -10466,7 +10473,8 @@ arm_output_epilogue (rtx sibling)
 	    asm_fprintf (f, "\twldrd\t%r, [%r], #8\n", reg, SP_REGNUM);
 
       /* If we can, restore the LR into the PC.  */
-      if (ARM_FUNC_TYPE (func_type) == ARM_FT_NORMAL
+      if (ARM_FUNC_TYPE (func_type) != ARM_FT_INTERWORKED
+	  && (TARGET_ARM || ARM_FUNC_TYPE (func_type) == ARM_FT_NORMAL)
 	  && !IS_STACKALIGN (func_type)
 	  && really_return
 	  && current_function_pretend_args_size == 0
@@ -10475,12 +10483,16 @@ arm_output_epilogue (rtx sibling)
 	{
 	  saved_regs_mask &= ~ (1 << LR_REGNUM);
 	  saved_regs_mask |=   (1 << PC_REGNUM);
+	  rfe = IS_INTERRUPT (func_type);
 	}
+      else
+	rfe = 0;
 
       /* Load the registers off the stack.  If we only have one register
 	 to load use the LDR instruction - it is faster.  For Thumb-2
 	 always use pop and the assembler will pick the best instruction.*/
-      if (TARGET_ARM && saved_regs_mask == (1 << LR_REGNUM))
+      if (TARGET_ARM && saved_regs_mask == (1 << LR_REGNUM)
+	  && !IS_INTERRUPT(func_type))
 	{
 	  asm_fprintf (f, "\tldr\t%r, [%r], #4\n", LR_REGNUM, SP_REGNUM);
 	}
@@ -10491,11 +10503,13 @@ arm_output_epilogue (rtx sibling)
 	       (i.e. "ldmfd sp!...").  We know that the stack pointer is
 	       in the list of registers and if we add writeback the
 	       instruction becomes UNPREDICTABLE.  */
-	    print_multi_reg (f, "ldmfd\t%r, ", SP_REGNUM, saved_regs_mask);
+	    print_multi_reg (f, "ldmfd\t%r, ", SP_REGNUM, saved_regs_mask,
+			     rfe);
 	  else if (TARGET_ARM)
-	    print_multi_reg (f, "ldmfd\t%r!, ", SP_REGNUM, saved_regs_mask);
+	    print_multi_reg (f, "ldmfd\t%r!, ", SP_REGNUM, saved_regs_mask,
+			     rfe);
 	  else
-	    print_multi_reg (f, "pop\t", SP_REGNUM, saved_regs_mask);
+	    print_multi_reg (f, "pop\t", SP_REGNUM, saved_regs_mask, 0);
 	}
 
       if (current_function_pretend_args_size)
@@ -11349,7 +11363,8 @@ arm_expand_prologue (void)
      can be done with a single instruction.  */
   if ((func_type == ARM_FT_ISR || func_type == ARM_FT_FIQ)
       && (live_regs_mask & (1 << LR_REGNUM)) != 0
-      && ! frame_pointer_needed)
+      && ! frame_pointer_needed
+      && TARGET_ARM)
     emit_insn (gen_rtx_SET (SImode,
 			    gen_rtx_REG (SImode, LR_REGNUM),
 			    gen_rtx_PLUS (SImode,

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]