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]

Re: c/2898: Illegal function return in ARM code when compiling with -mthumb-interwork -O2


>Doh! Wait a moment, we only want to avoid this sequence if really 
>returning -- popping to lr should be safe with a load even for the cases 
>concerned (though we should never be tail-calling out of an interrupt 
>function...).

I think this additional patch will improve matters.  It seems to DTRT with the 
testcase from 2898: we now get:

f:
        @ Function supports interworking.
        @ args = 0, pretend = 0, frame = 0
        @ frame_needed = 0, current_function_anonymous_args = 0
        str     lr, [sp, #-4]!
        mov     r3, r0
        ldrb    r2, [r3, #0]    @ zero_extendqisi2
        ldrb    ip, [r1, #0]
        cmp     r2, #0
        movne   r3, lr
        strb    ip, [r3, #0]
        ldr     lr, [sp], #4
        bx      lr
.Lfe1:

p.

2001-05-26  Philip Blundell  <philb@gnu.org>

	* config/arm/arm.c (output_return_instruction): Use LDR in more
	situations where this is allowed.

Index: arm.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/arm/arm.c,v
retrieving revision 1.146
diff -u -p -u -r1.146 arm.c
--- arm.c	2001/05/24 21:09:05	1.146
+++ arm.c	2001/05/26 14:19:13
@@ -7033,21 +7033,20 @@ output_return_instruction (operand, real
 
   live_regs_mask = arm_compute_save_reg_mask ();
 
-  /* On some ARM architectures it is faster to use LDR rather than LDM to
-     load a single register.  On other architectures, the cost is the same.
-     In 26 bit mode we have to use LDM in order to be able to restore the CPSR.  */
-  if ((live_regs_mask  == (1 << LR_REGNUM))
-      && ! TARGET_INTERWORK
-      && ! IS_INTERRUPT (func_type)
-      && (! really_return || TARGET_APCS_32))
+  if (live_regs_mask)
     {
-      if (! really_return)
-	sprintf (instr, "ldr%s\t%%|lr, [%%|sp], #4", conditional);
+      const char * return_reg;
+
+      /* If we do not have any special requirements for function exit 
+	 (eg interworking, or ISR) then we can load the return address 
+	 directly into the PC.  Otherwise we must load it into LR.  */
+      if (! TARGET_INTERWORK
+	  && ! IS_INTERRUPT (func_type)
+	  && really_return)
+	return_reg = reg_names [PC_REGNUM];
       else
-	sprintf (instr, "ldr%s\t%%|pc, [%%|sp], #4", conditional);
-    }
-  else if (live_regs_mask)
-    {
+	return_reg = reg_names [LR_REGNUM];
+
       if ((live_regs_mask & (1 << IP_REGNUM)) == (1 << IP_REGNUM))
 	/* There are two possible reasons for the IP register being saved.
 	   Either a stack frame was created, in which case IP contains the
@@ -7058,46 +7057,46 @@ output_return_instruction (operand, real
 	    live_regs_mask &= ~ (1 << IP_REGNUM);
 	    live_regs_mask |=   (1 << SP_REGNUM);
 	  }
-
-      /* Generate the load multiple instruction to restore the registers.  */
-      if (frame_pointer_needed)
-	sprintf (instr, "ldm%sea\t%%|fp, {", conditional);
-      else
-	sprintf (instr, "ldm%sfd\t%%|sp!, {", conditional);
-
-      for (reg = 0; reg <= SP_REGNUM; reg++)
-	if (live_regs_mask & (1 << reg))
-	  {
-	    strcat (instr, "%|");
-	    strcat (instr, reg_names[reg]);
-	    strcat (instr, ", ");
-	  }
 
-      if ((live_regs_mask & (1 << LR_REGNUM)) == 0)
+      /* On some ARM architectures it is faster to use LDR rather than LDM to
+	 load a single register.  On other architectures, the cost is the same.
+	 In 26 bit mode we have to use LDM in order to be able to restore the CPSR.  */
+      if ((live_regs_mask  == (1 << LR_REGNUM))
+	  && (! really_return || TARGET_APCS_32))
 	{
-	  /* If we are not restoring the LR register then we will
-	     have added one too many commas to the list above.
-	     Replace it with a closing brace.  */
-	  instr [strlen (instr) - 2] =  '}';
+	  sprintf (instr, "ldr%s\t%%|%s, [%%|sp], #4", conditional, return_reg);
 	}
       else
 	{
-	  strcat (instr, "%|");
+	  /* Generate the load multiple instruction to restore the registers.  */
+	  if (frame_pointer_needed)
+	    sprintf (instr, "ldm%sea\t%%|fp, {", conditional);
+	  else
+	    sprintf (instr, "ldm%sfd\t%%|sp!, {", conditional);
 
-	  /* At this point there should only be one or two registers left in
-	     live_regs_mask: always LR, and possibly PC if we created a stack
-	     frame.  LR contains the return address.  If we do not have any
-	     special requirements for function exit (eg interworking, or ISR)
-	     then we can load this value directly into the PC and save an
-	     instruction.  */
-	  if (! TARGET_INTERWORK
-	      && ! IS_INTERRUPT (func_type)
-	      && really_return)
-	    strcat (instr, reg_names [PC_REGNUM]);
+	  for (reg = 0; reg <= SP_REGNUM; reg++)
+	    if (live_regs_mask & (1 << reg))
+	      {
+		strcat (instr, "%|");
+		strcat (instr, reg_names[reg]);
+		strcat (instr, ", ");
+	      }
+	  
+	  if ((live_regs_mask & (1 << LR_REGNUM)) == 0)
+	    {
+	      /* If we are not restoring the LR register then we will
+		 have added one too many commas to the list above.
+		 Replace it with a closing brace.  */
+	      instr [strlen (instr) - 2] =  '}';
+	    }
 	  else
-	    strcat (instr, reg_names [LR_REGNUM]);
+	    {
+	      strcat (instr, "%|");
+
+	      strcat (instr, return_reg);
 
-	  strcat (instr, (TARGET_APCS_32 || !really_return) ? "}" : "}^");
+	      strcat (instr, (TARGET_APCS_32 || !really_return) ? "}" : "}^");
+	    }
 	}
 
       if (really_return)



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