Re: [PATCH, ARM] Enable sibling calls for Thumb-2

[Julian Brown <julian at codesourcery dot com>]

On Thu, 18 Feb 2010 15:47:05 +0000
Richard Earnshaw <rearnsha@arm.com> wrote:

> On Mon, 2010-02-15 at 12:46 +0000, Julian Brown wrote:
> > This patch turns on sibling calls for Thumb-2. It works in cases
> > which failed with earlier patches, e.g. described in the thread
> > here:

[snip]

> > ChangeLog
> > 
> >     Julian Brown  <julian@codesourcery.com>
> >     Mark Mitchell  <mark@codesourcery.com>
> > 
> >     gcc/
> >     * config/arm/arm.c (arm_function_ok_for_sibcall): Allow sibling
> >     calls for Thumb-2.
> In the patch you wrote !TARGET_32BIT.  Why not use TARGET_THUMB1
> (particularly since the comment talks specifically about not doing
> this for Thumb1)?  

Fixed.

> >     (output_return_instruction): Use pop not ldmfd for Thumb-2.
> This bit doesn't appear in your patch.

Changes for unified-syntax mode have made that bit unnecessary now.
(I'm not sure if that was the case when I posted the patch
before -- I might have just forgotten to remove that bit from the
ChangeLog.)

> >     * config/arm/arm.h (USE_RETURN_INSN): Enable for Thumb-2.
> >     * config/arm/arm.md (*call_symbol, *call_value_symbol): Use for
> >     Thumb-2.
> >     (*call_insn, *call_value_insn): Don't use for Thumb-2.
> >     (sibcall, sibcall_value, *sibcall_insn, *sibcall_value_insn):
> > Use for Thumb-2.
> >     (return): New expander.
> >     (*arm_return): New name for ARM return insn.
> >     (*thumb2_return): New insn pattern.
> thumb2_return should be in thumb2.md

Fixed.

I've re-tested this on current trunk (with options to generate Thumb-2
code, cross to ARM Linux), and it still shows a couple of progressions.
OK to apply?

Thanks,

Julian

ChangeLog

    Julian Brown  <julian@codesourcery.com>
    Mark Mitchell  <mark@codesourcery.com>

    gcc/
    * config/arm/arm.c (arm_function_ok_for_sibcall): Only forbid
    sibling calls for Thumb-1.
    * config/arm/arm.h (USE_RETURN_INSN): Enable for Thumb-2.
    * config/arm/arm.md (*call_symbol, *call_value_symbol): Use for
    Thumb-2.
    (*call_insn, *call_value_insn): Don't use for Thumb-2.
    (sibcall, sibcall_value, *sibcall_insn, *sibcall_value_insn): Use
    for Thumb-2.
    (return): New expander.
    (*arm_return): New name for ARM return insn.
    * config/arm/thumb2.md (*thumb2_return): New insn pattern.

Index: gcc/config/arm/thumb2.md
===================================================================
--- gcc/config/arm/thumb2.md	(revision 158670)
+++ gcc/config/arm/thumb2.md	(working copy)
@@ -1054,6 +1054,19 @@
    (set_attr "length" "20")]
 )
 
+;; Note: this is not predicable, to avoid issues with linker-generated
+;; interworking stubs.
+(define_insn "*thumb2_return"
+  [(return)]
+  "TARGET_THUMB2 && USE_RETURN_INSN (FALSE)"
+  "*
+  {
+    return output_return_instruction (const_true_rtx, TRUE, FALSE);
+  }"
+  [(set_attr "type" "load1")
+   (set_attr "length" "12")]
+)
+
 (define_insn_and_split "thumb2_eh_return"
   [(unspec_volatile [(match_operand:SI 0 "s_register_operand" "r")]
 		    VUNSPEC_EH_RETURN)
Index: gcc/config/arm/arm.c
===================================================================
--- gcc/config/arm/arm.c	(revision 158670)
+++ gcc/config/arm/arm.c	(working copy)
@@ -4796,8 +4796,8 @@ arm_function_ok_for_sibcall (tree decl, 
     return false;
 
   /* Never tailcall something for which we have no decl, or if we
-     are in Thumb mode.  */
-  if (decl == NULL || TARGET_THUMB)
+     are generating code for Thumb-1.  */
+  if (decl == NULL || TARGET_THUMB1)
     return false;
 
   /* The PIC register is live on entry to VxWorks PLT entries, so we
Index: gcc/config/arm/arm.h
===================================================================
--- gcc/config/arm/arm.h	(revision 158670)
+++ gcc/config/arm/arm.h	(working copy)
@@ -1809,10 +1809,8 @@ typedef struct
 
 /* Determine if the epilogue should be output as RTL.
    You should override this if you define FUNCTION_EXTRA_EPILOGUE.  */
-/* This is disabled for Thumb-2 because it will confuse the
-   conditional insn counter.  */
 #define USE_RETURN_INSN(ISCOND)				\
-  (TARGET_ARM ? use_return_insn (ISCOND, NULL) : 0)
+  (TARGET_32BIT ? use_return_insn (ISCOND, NULL) : 0)
 
 /* Definitions for register eliminations.
 
Index: gcc/config/arm/arm.md
===================================================================
--- gcc/config/arm/arm.md	(revision 158670)
+++ gcc/config/arm/arm.md	(working copy)
@@ -8650,7 +8650,7 @@
 	 (match_operand 1 "" ""))
    (use (match_operand 2 "" ""))
    (clobber (reg:SI LR_REGNUM))]
-  "TARGET_ARM
+  "TARGET_32BIT
    && (GET_CODE (operands[0]) == SYMBOL_REF)
    && !arm_is_long_call_p (SYMBOL_REF_DECL (operands[0]))"
   "*
@@ -8666,7 +8666,7 @@
 	(match_operand:SI 2 "" "")))
    (use (match_operand 3 "" ""))
    (clobber (reg:SI LR_REGNUM))]
-  "TARGET_ARM
+  "TARGET_32BIT
    && (GET_CODE (operands[1]) == SYMBOL_REF)
    && !arm_is_long_call_p (SYMBOL_REF_DECL (operands[1]))"
   "*
@@ -8681,7 +8681,7 @@
 	 (match_operand:SI 1 "" ""))
    (use (match_operand 2 "" ""))
    (clobber (reg:SI LR_REGNUM))]
-  "TARGET_THUMB
+  "TARGET_THUMB1
    && GET_CODE (operands[0]) == SYMBOL_REF
    && !arm_is_long_call_p (SYMBOL_REF_DECL (operands[0]))"
   "bl\\t%a0"
@@ -8695,7 +8695,7 @@
 	      (match_operand 2 "" "")))
    (use (match_operand 3 "" ""))
    (clobber (reg:SI LR_REGNUM))]
-  "TARGET_THUMB
+  "TARGET_THUMB1
    && GET_CODE (operands[1]) == SYMBOL_REF
    && !arm_is_long_call_p (SYMBOL_REF_DECL (operands[1]))"
   "bl\\t%a1"
@@ -8709,7 +8709,7 @@
 		    (match_operand 1 "general_operand" ""))
 	      (return)
 	      (use (match_operand 2 "" ""))])]
-  "TARGET_ARM"
+  "TARGET_32BIT"
   "
   {
     if (operands[2] == NULL_RTX)
@@ -8723,7 +8723,7 @@
 			 (match_operand 2 "general_operand" "")))
 	      (return)
 	      (use (match_operand 3 "" ""))])]
-  "TARGET_ARM"
+  "TARGET_32BIT"
   "
   {
     if (operands[3] == NULL_RTX)
@@ -8736,7 +8736,7 @@
 	(match_operand 1 "" ""))
   (return)
   (use (match_operand 2 "" ""))]
-  "TARGET_ARM && GET_CODE (operands[0]) == SYMBOL_REF"
+  "TARGET_32BIT && GET_CODE (operands[0]) == SYMBOL_REF"
   "*
   return NEED_PLT_RELOC ? \"b%?\\t%a0(PLT)\" : \"b%?\\t%a0\";
   "
@@ -8749,15 +8749,20 @@
 	     (match_operand 2 "" "")))
   (return)
   (use (match_operand 3 "" ""))]
-  "TARGET_ARM && GET_CODE (operands[1]) == SYMBOL_REF"
+  "TARGET_32BIT && GET_CODE (operands[1]) == SYMBOL_REF"
   "*
   return NEED_PLT_RELOC ? \"b%?\\t%a1(PLT)\" : \"b%?\\t%a1\";
   "
   [(set_attr "type" "call")]
 )
 
+(define_expand "return"
+  [(return)]
+  "TARGET_32BIT && USE_RETURN_INSN (FALSE)"
+  "")
+
 ;; Often the return insn will be the same as loading from memory, so set attr
-(define_insn "return"
+(define_insn "*arm_return"
   [(return)]
   "TARGET_ARM && USE_RETURN_INSN (FALSE)"
   "*