diff mbox

[AVR] : Support tail calls

Message ID 4D873B1A.4050202@gjlay.de
State New
Headers show

Commit Message

Georg-Johann Lay March 21, 2011, 11:48 a.m. UTC
Denis Chertykov schrieb:
> 2011/3/18 Georg-Johann Lay <avr@gjlay.de>:
> 
>>> Is it tested for regressions ?
>>>
>>> Denis.
>> I ran tests against svn 170942 (latest 4.7.0 snapshot). Besides
>> timestamps, the diff looks like this:
>>
>> 1435a1436,1437
>>> XPASS: gcc.dg/sibcall-3.c execution test
>>> XPASS: gcc.dg/sibcall-4.c execution test
>> 1630,1631c1632,1633
>> < # of unexpected successes     4
>> < # of expected failures                132
>> ---
>>> # of unexpected successes     6
>>> # of expected failures                130
>> 1670c1672
>>
>> This is due to xfail for runtime-test of sibcall abilities for avr-*-*.
>>
> 
> 
> So, the patch can be committed.
> Please, prepare the final version of the patch.

Patch updated to trunk, some more comment.

Johann


2011-03-21  Georg-Johann Lay  <avr@gjlay.de>

	* config/avr/avr-protos.h (expand_epilogue): Change prototype
	* config/avr/avr.h (struct machine_function): Add field
	sibcall_fails.
	* config/avr/avr.c (init_cumulative_args,
	avr_function_arg_advance): Use it.
	* config/avr/avr.c (expand_epilogue): Add bool parameter. Handle
	sibcall	epilogues.
	(TARGET_FUNCTION_OK_FOR_SIBCALL): Define to...
	(avr_function_ok_for_sibcall): ...this new function.
	(avr_lookup_function_attribute1): New static Function.
	(avr_naked_function_p, interrupt_function_p,
	signal_function_p, avr_OS_task_function_p,
	avr_OS_main_function_p): Use it.
	* config/avr/avr.md ("sibcall", "sibcall_value",
	"sibcall_epilogue"): New expander.
	("*call_insn", "*call_value_insn"): New insn.
	("call_insn", "call_value_insn"): Remove
	("call", "call_value", "epilogue"): Change expander to handle
	sibling calls.

Comments

Denis Chertykov March 22, 2011, 6:01 p.m. UTC | #1
2011/3/21 Georg-Johann Lay <avr@gjlay.de>:
> Denis Chertykov schrieb:
>> 2011/3/18 Georg-Johann Lay <avr@gjlay.de>:
>>
>>>> Is it tested for regressions ?
>>>>
>>>> Denis.
>>> I ran tests against svn 170942 (latest 4.7.0 snapshot). Besides
>>> timestamps, the diff looks like this:
>>>
>>> 1435a1436,1437
>>>> XPASS: gcc.dg/sibcall-3.c execution test
>>>> XPASS: gcc.dg/sibcall-4.c execution test
>>> 1630,1631c1632,1633
>>> < # of unexpected successes     4
>>> < # of expected failures                132
>>> ---
>>>> # of unexpected successes     6
>>>> # of expected failures                130
>>> 1670c1672
>>>
>>> This is due to xfail for runtime-test of sibcall abilities for avr-*-*.
>>>
>>
>>
>> So, the patch can be committed.
>> Please, prepare the final version of the patch.
>
> Patch updated to trunk, some more comment.

Seems strange, I have a many messages from patch:
patching file avr-protos.h
patching file avr.md
Hunk #1 succeeded at 2646 (offset -1 lines).
Hunk #2 succeeded at 3234 (offset -9 lines).
patching file avr.c
Hunk #2 succeeded at 250 with fuzz 2 (offset 3 lines).
Hunk #3 succeeded at 352 (offset 3 lines).
Hunk #4 succeeded at 388 (offset 3 lines).
Hunk #5 succeeded at 397 (offset 3 lines).
Hunk #6 succeeded at 405 (offset 3 lines).
Hunk #7 succeeded at 413 (offset 3 lines).
Hunk #8 succeeded at 934 (offset 56 lines).
Hunk #9 succeeded at 945 (offset 56 lines).
Hunk #10 succeeded at 1089 with fuzz 1 (offset 57 lines).
Hunk #11 succeeded at 1703 (offset 57 lines).
Hunk #12 succeeded at 1754 (offset 57 lines).
Hunk #13 succeeded at 1778 (offset 57 lines).
patching file avr.h
Hunk #1 succeeded at 826 (offset 4 lines).

Denis.
Richard Henderson March 22, 2011, 6:47 p.m. UTC | #2
On 03/22/2011 11:01 AM, Denis Chertykov wrote:
> Seems strange, I have a many messages from patch:
> patching file avr-protos.h
> patching file avr.md
> Hunk #1 succeeded at 2646 (offset -1 lines).
> Hunk #2 succeeded at 3234 (offset -9 lines).
> patching file avr.c
> Hunk #2 succeeded at 250 with fuzz 2 (offset 3 lines).
> Hunk #3 succeeded at 352 (offset 3 lines).
> Hunk #4 succeeded at 388 (offset 3 lines).
> Hunk #5 succeeded at 397 (offset 3 lines).
> Hunk #6 succeeded at 405 (offset 3 lines).
> Hunk #7 succeeded at 413 (offset 3 lines).
> Hunk #8 succeeded at 934 (offset 56 lines).
> Hunk #9 succeeded at 945 (offset 56 lines).
> Hunk #10 succeeded at 1089 with fuzz 1 (offset 57 lines).
> Hunk #11 succeeded at 1703 (offset 57 lines).
> Hunk #12 succeeded at 1754 (offset 57 lines).
> Hunk #13 succeeded at 1778 (offset 57 lines).
> patching file avr.h
> Hunk #1 succeeded at 826 (offset 4 lines).

Almost certainly fuzz from my dwarf unwind patch.


r~
Denis Chertykov March 22, 2011, 7:38 p.m. UTC | #3
2011/3/22 Richard Henderson <rth@redhat.com>:
> On 03/22/2011 11:01 AM, Denis Chertykov wrote:
>> Seems strange, I have a many messages from patch:
>> patching file avr-protos.h
>> patching file avr.md
>> Hunk #1 succeeded at 2646 (offset -1 lines).
>> Hunk #2 succeeded at 3234 (offset -9 lines).
>> patching file avr.c
>> Hunk #2 succeeded at 250 with fuzz 2 (offset 3 lines).
>> Hunk #3 succeeded at 352 (offset 3 lines).
>> Hunk #4 succeeded at 388 (offset 3 lines).
>> Hunk #5 succeeded at 397 (offset 3 lines).
>> Hunk #6 succeeded at 405 (offset 3 lines).
>> Hunk #7 succeeded at 413 (offset 3 lines).
>> Hunk #8 succeeded at 934 (offset 56 lines).
>> Hunk #9 succeeded at 945 (offset 56 lines).
>> Hunk #10 succeeded at 1089 with fuzz 1 (offset 57 lines).
>> Hunk #11 succeeded at 1703 (offset 57 lines).
>> Hunk #12 succeeded at 1754 (offset 57 lines).
>> Hunk #13 succeeded at 1778 (offset 57 lines).
>> patching file avr.h
>> Hunk #1 succeeded at 826 (offset 4 lines).
>
> Almost certainly fuzz from my dwarf unwind patch.

Oops, my bad. I forgot about you.
I have committed the patch.

Denis.
diff mbox

Patch

Index: config/avr/avr-protos.h
===================================================================
--- config/avr/avr-protos.h	(Revision 171215)
+++ config/avr/avr-protos.h	(Arbeitskopie)
@@ -76,7 +76,7 @@  extern const char *lshrsi3_out (rtx insn
 extern bool avr_rotate_bytes (rtx operands[]);
 
 extern void expand_prologue (void);
-extern void expand_epilogue (void);
+extern void expand_epilogue (bool);
 extern int avr_epilogue_uses (int regno);
 
 extern void avr_output_bld (rtx operands[], int bit_nr);
Index: config/avr/avr.md
===================================================================
--- config/avr/avr.md	(Revision 171215)
+++ config/avr/avr.md	(Arbeitskopie)
@@ -2647,94 +2647,91 @@ 
 ;; call
 
 (define_expand "call"
-  [(call (match_operand:HI 0 "call_insn_operand" "")
-         (match_operand:HI 1 "general_operand" ""))]
+  [(parallel[(call (match_operand:HI 0 "call_insn_operand" "")
+                   (match_operand:HI 1 "general_operand" ""))
+             (use (const_int 0))])]
   ;; Operand 1 not used on the AVR.
+  ;; Operand 2 is 1 for tail-call, 0 otherwise.
+  ""
+  "")
+
+(define_expand "sibcall"
+  [(parallel[(call (match_operand:HI 0 "call_insn_operand" "")
+                   (match_operand:HI 1 "general_operand" ""))
+             (use (const_int 1))])]
+  ;; Operand 1 not used on the AVR.
+  ;; Operand 2 is 1 for tail-call, 0 otherwise.
   ""
   "")
 
 ;; call value
 
 (define_expand "call_value"
-  [(set (match_operand 0 "register_operand" "")
-        (call (match_operand:HI 1 "call_insn_operand" "")
-              (match_operand:HI 2 "general_operand" "")))]
+  [(parallel[(set (match_operand 0 "register_operand" "")
+                  (call (match_operand:HI 1 "call_insn_operand" "")
+                        (match_operand:HI 2 "general_operand" "")))
+             (use (const_int 0))])]
   ;; Operand 2 not used on the AVR.
+  ;; Operand 3 is 1 for tail-call, 0 otherwise.
   ""
   "")
 
-(define_insn "call_insn"
-  [(call (mem:HI (match_operand:HI 0 "nonmemory_operand" "!z,*r,s,n"))
-         (match_operand:HI 1 "general_operand" "X,X,X,X"))]
-;; We don't need in saving Z register because r30,r31 is a call used registers
+(define_expand "sibcall_value"
+  [(parallel[(set (match_operand 0 "register_operand" "")
+                  (call (match_operand:HI 1 "call_insn_operand" "")
+                        (match_operand:HI 2 "general_operand" "")))
+             (use (const_int 1))])]
+  ;; Operand 2 not used on the AVR.
+  ;; Operand 3 is 1 for tail-call, 0 otherwise.
+  ""
+  "")
+
+(define_insn "*call_insn"
+  [(parallel[(call (mem:HI (match_operand:HI 0 "nonmemory_operand" "z,s,z,s"))
+                   (match_operand:HI 1 "general_operand"           "X,X,X,X"))
+             (use (match_operand:HI 2 "const_int_operand"          "L,L,P,P"))])]
   ;; Operand 1 not used on the AVR.
-  "(register_operand (operands[0], HImode) || CONSTANT_P (operands[0]))"
-  "*{
-  if (which_alternative==0)
-     return \"%!icall\";
-  else if (which_alternative==1)
-    {
-      if (AVR_HAVE_MOVW)
-	return (AS2 (movw, r30, %0) CR_TAB
-               \"%!icall\");
-      else
-	return (AS2 (mov, r30, %A0) CR_TAB
-		AS2 (mov, r31, %B0) CR_TAB
-		\"%!icall\");
-    }
-  else if (which_alternative==2)
-    return AS1(%~call,%x0);
-  return (AS2 (ldi,r30,lo8(%0)) CR_TAB
-          AS2 (ldi,r31,hi8(%0)) CR_TAB
-          \"%!icall\");
-}"
-  [(set_attr "cc" "clobber,clobber,clobber,clobber")
+  ;; Operand 2 is 1 for tail-call, 0 otherwise.
+  ""
+  "@
+    %!icall
+    %~call %x0
+    %!ijmp
+    %~jmp %x0"
+  [(set_attr "cc" "clobber")
    (set_attr_alternative "length"
-			 [(const_int 1)
-			  (if_then_else (eq_attr "mcu_have_movw" "yes")
-					(const_int 2)
-					(const_int 3))
-			  (if_then_else (eq_attr "mcu_mega" "yes")
-					(const_int 2)
-					(const_int 1))
-			  (const_int 3)])])
-
-(define_insn "call_value_insn"
-  [(set (match_operand 0 "register_operand" "=r,r,r,r")
-        (call (mem:HI (match_operand:HI 1 "nonmemory_operand" "!z,*r,s,n"))
-;; We don't need in saving Z register because r30,r31 is a call used registers
-              (match_operand:HI 2 "general_operand" "X,X,X,X")))]
+                         [(const_int 1)
+                          (if_then_else (eq_attr "mcu_mega" "yes")
+                                        (const_int 2)
+                                        (const_int 1))
+                          (const_int 1)
+                          (if_then_else (eq_attr "mcu_mega" "yes")
+                                        (const_int 2)
+                                        (const_int 1))])])
+
+(define_insn "*call_value_insn"
+  [(parallel[(set (match_operand 0 "register_operand"                   "=r,r,r,r")
+                  (call (mem:HI (match_operand:HI 1 "nonmemory_operand"  "z,s,z,s"))
+                        (match_operand:HI 2 "general_operand"            "X,X,X,X")))
+             (use (match_operand:HI 3 "const_int_operand"                "L,L,P,P"))])]
   ;; Operand 2 not used on the AVR.
-  "(register_operand (operands[0], VOIDmode) || CONSTANT_P (operands[0]))"
-  "*{
-  if (which_alternative==0)
-     return \"%!icall\";
-  else if (which_alternative==1)
-    {
-      if (AVR_HAVE_MOVW)
-	return (AS2 (movw, r30, %1) CR_TAB
-		\"%!icall\");
-      else
-	return (AS2 (mov, r30, %A1) CR_TAB
-		AS2 (mov, r31, %B1) CR_TAB
-		\"%!icall\");
-    }
-  else if (which_alternative==2)
-    return AS1(%~call,%x1);
-  return (AS2 (ldi, r30, lo8(%1)) CR_TAB
-          AS2 (ldi, r31, hi8(%1)) CR_TAB
-          \"%!icall\");
-}"
-  [(set_attr "cc" "clobber,clobber,clobber,clobber")
+  ;; Operand 3 is 1 for tail-call, 0 otherwise.
+  ""
+  "@
+    %!icall
+    %~call %x1
+    %!ijmp
+    %~jmp %x1"
+  [(set_attr "cc" "clobber")
    (set_attr_alternative "length"
-			 [(const_int 1)
-			  (if_then_else (eq_attr "mcu_have_movw" "yes")
-					(const_int 2)
-					(const_int 3))
-			  (if_then_else (eq_attr "mcu_mega" "yes")
-					(const_int 2)
-					(const_int 1))
-			  (const_int 3)])])
+                         [(const_int 1)
+                          (if_then_else (eq_attr "mcu_mega" "yes")
+                                        (const_int 2)
+                                        (const_int 1))
+                          (const_int 1)
+                          (if_then_else (eq_attr "mcu_mega" "yes")
+                                        (const_int 2)
+                                        (const_int 1))])])
 
 (define_insn "nop"
   [(const_int 0)]
@@ -3246,8 +3243,15 @@ 
 (define_expand "epilogue"
   [(const_int 0)]
   ""
-  "
   {
-    expand_epilogue (); 
+    expand_epilogue (false /* sibcall_p */);
     DONE;
-  }")
+  })
+
+(define_expand "sibcall_epilogue"
+  [(const_int 0)]
+  ""
+  {
+    expand_epilogue (true /* sibcall_p */);
+    DONE;
+  })
Index: config/avr/avr.c
===================================================================
--- config/avr/avr.c	(Revision 171215)
+++ config/avr/avr.c	(Arbeitskopie)
@@ -102,6 +102,7 @@  static rtx avr_function_arg (CUMULATIVE_
 static void avr_function_arg_advance (CUMULATIVE_ARGS *, enum machine_mode,
 				      const_tree, bool);
 static void avr_help (void);
+static bool avr_function_ok_for_sibcall (tree, tree);
 
 /* Allocate registers from r25 to r8 for parameters for function calls.  */
 #define FIRST_CUM_REG 26
@@ -246,6 +247,9 @@  static const struct default_options avr_
 #undef TARGET_HELP
 #define TARGET_HELP avr_help
 
+#undef TARGET_FUNCTION_OK_FOR_SIBCALL
+#define TARGET_FUNCTION_OK_FOR_SIBCALL avr_function_ok_for_sibcall
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 
 static void
@@ -345,17 +349,34 @@  avr_regno_reg_class (int r)
   return ALL_REGS;
 }
 
+/* A helper for the subsequent function attribute used to dig for
+   attribute 'name' in a FUNCTION_DECL or FUNCTION_TYPE */
+
+static inline int
+avr_lookup_function_attribute1 (const_tree func, const char *name)
+{
+  if (FUNCTION_DECL == TREE_CODE (func))
+    {
+      if (NULL_TREE != lookup_attribute (name, DECL_ATTRIBUTES (func)))
+        {
+          return true;
+        }
+      
+      func = TREE_TYPE (func);
+    }
+
+  gcc_assert (TREE_CODE (func) == FUNCTION_TYPE
+              || TREE_CODE (func) == METHOD_TYPE);
+  
+  return NULL_TREE != lookup_attribute (name, TYPE_ATTRIBUTES (func));
+}
+
 /* Return nonzero if FUNC is a naked function.  */
 
 static int
 avr_naked_function_p (tree func)
 {
-  tree a;
-
-  gcc_assert (TREE_CODE (func) == FUNCTION_DECL);
-  
-  a = lookup_attribute ("naked", TYPE_ATTRIBUTES (TREE_TYPE (func)));
-  return a != NULL_TREE;
+  return avr_lookup_function_attribute1 (func, "naked");
 }
 
 /* Return nonzero if FUNC is an interrupt function as specified
@@ -364,13 +385,7 @@  avr_naked_function_p (tree func)
 static int
 interrupt_function_p (tree func)
 {
-  tree a;
-
-  if (TREE_CODE (func) != FUNCTION_DECL)
-    return 0;
-
-  a = lookup_attribute ("interrupt", DECL_ATTRIBUTES (func));
-  return a != NULL_TREE;
+  return avr_lookup_function_attribute1 (func, "interrupt");
 }
 
 /* Return nonzero if FUNC is a signal function as specified
@@ -379,13 +394,7 @@  interrupt_function_p (tree func)
 static int
 signal_function_p (tree func)
 {
-  tree a;
-
-  if (TREE_CODE (func) != FUNCTION_DECL)
-    return 0;
-
-  a = lookup_attribute ("signal", DECL_ATTRIBUTES (func));
-  return a != NULL_TREE;
+  return avr_lookup_function_attribute1 (func, "signal");
 }
 
 /* Return nonzero if FUNC is a OS_task function.  */
@@ -393,12 +402,7 @@  signal_function_p (tree func)
 static int
 avr_OS_task_function_p (tree func)
 {
-  tree a;
-
-  gcc_assert (TREE_CODE (func) == FUNCTION_DECL);
-  
-  a = lookup_attribute ("OS_task", TYPE_ATTRIBUTES (TREE_TYPE (func)));
-  return a != NULL_TREE;
+  return avr_lookup_function_attribute1 (func, "OS_task");
 }
 
 /* Return nonzero if FUNC is a OS_main function.  */
@@ -406,12 +410,7 @@  avr_OS_task_function_p (tree func)
 static int
 avr_OS_main_function_p (tree func)
 {
-  tree a;
-
-  gcc_assert (TREE_CODE (func) == FUNCTION_DECL);
-  
-  a = lookup_attribute ("OS_main", TYPE_ATTRIBUTES (TREE_TYPE (func)));
-  return a != NULL_TREE;
+  return avr_lookup_function_attribute1 (func, "OS_main");
 }
 
 /* Return the number of hard registers to push/pop in the prologue/epilogue
@@ -879,7 +878,7 @@  avr_epilogue_uses (int regno ATTRIBUTE_U
 /*  Output RTL epilogue.  */
 
 void
-expand_epilogue (void)
+expand_epilogue (bool sibcall_p)
 {
   int reg;
   int live_seq;
@@ -890,6 +889,8 @@  expand_epilogue (void)
   /* epilogue: naked  */
   if (cfun->machine->is_naked)
     {
+      gcc_assert (!sibcall_p);
+      
       emit_jump_insn (gen_return ());
       return;
     }
@@ -1031,7 +1032,8 @@  expand_epilogue (void)
           emit_insn (gen_popqi (zero_reg_rtx));
         }
 
-      emit_jump_insn (gen_return ());
+      if (!sibcall_p)
+        emit_jump_insn (gen_return ());
     }
 }
 
@@ -1644,6 +1646,10 @@  init_cumulative_args (CUMULATIVE_ARGS *c
   cum->regno = FIRST_CUM_REG;
   if (!libname && stdarg_p (fntype))
     cum->nregs = 0;
+
+  /* Assume the calle may be tail called */
+  
+  cfun->machine->sibcall_fails = 0;
 }
 
 /* Returns the number of registers to allocate for a function argument.  */
@@ -1691,6 +1697,23 @@  avr_function_arg_advance (CUMULATIVE_ARG
   cum->nregs -= bytes;
   cum->regno -= bytes;
 
+  /* A parameter is being passed in a call-saved register. As the original
+     contents of these regs has to be restored before leaving the function,
+     a function must not pass arguments in call-saved regs in order to get
+     tail-called. */
+  
+  if (cum->regno >= 0
+      && !call_used_regs[cum->regno])
+    {
+      /* FIXME: We ship info on failing tail-call in struct machine_function.
+         This uses internals of calls.c:expand_call() and the way args_so_far
+         is used. targetm.function_ok_for_sibcall() needs to be extended to
+         pass &args_so_far, too. At present, CUMULATIVE_ARGS is target
+         dependent so that such an extension is not wanted. */
+      
+      cfun->machine->sibcall_fails = 1;
+    }
+
   if (cum->nregs <= 0)
     {
       cum->nregs = 0;
@@ -1698,6 +1721,65 @@  avr_function_arg_advance (CUMULATIVE_ARG
     }
 }
 
+/* Implement `TARGET_FUNCTION_OK_FOR_SIBCALL' */
+/* Decide whether we can make a sibling call to a function.  DECL is the
+   declaration of the function being targeted by the call and EXP is the
+   CALL_EXPR representing the call. */
+
+static bool
+avr_function_ok_for_sibcall (tree decl_callee, tree exp_callee)
+{
+  tree fntype_callee;
+
+  /* Tail-calling must fail if callee-saved regs are used to pass
+     function args.  We must not tail-call when `epilogue_restores'
+     is used.  Unfortunately, we cannot tell at this point if that
+     actually will happen or not, and we cannot step back from
+     tail-calling. Thus, we inhibit tail-calling with -mcall-prologues. */
+  
+  if (cfun->machine->sibcall_fails
+      || TARGET_CALL_PROLOGUES)
+    {
+      return false;
+    }
+  
+  fntype_callee = TREE_TYPE (CALL_EXPR_FN (exp_callee));
+
+  if (decl_callee)
+    {
+      decl_callee = TREE_TYPE (decl_callee);
+    }
+  else
+    {
+      decl_callee = fntype_callee;
+      
+      while (FUNCTION_TYPE != TREE_CODE (decl_callee)
+             && METHOD_TYPE != TREE_CODE (decl_callee))
+        {
+          decl_callee = TREE_TYPE (decl_callee);
+        }
+    }
+
+  /* Ensure that caller and callee have compatible epilogues */
+  
+  if (interrupt_function_p (current_function_decl)
+      || signal_function_p (current_function_decl)
+      || avr_naked_function_p (decl_callee)
+      || avr_naked_function_p (current_function_decl)
+      /* FIXME: For OS_task and OS_main, we are over-conservative.
+         This is due to missing documentation of these attributes
+         and what they actually should do and should not do. */
+      || (avr_OS_task_function_p (decl_callee)
+          != avr_OS_task_function_p (current_function_decl))
+      || (avr_OS_main_function_p (decl_callee)
+          != avr_OS_main_function_p (current_function_decl)))
+    {
+      return false;
+    }
+ 
+  return true;
+}
+
 /***********************************************************************
   Functions for outputting various mov's for a various modes
 ************************************************************************/
Index: config/avr/avr.h
===================================================================
--- config/avr/avr.h	(Revision 171215)
+++ config/avr/avr.h	(Arbeitskopie)
@@ -822,4 +822,7 @@  struct GTY(()) machine_function
   
   /* Current function stack size.  */
   int stack_usage;
+
+  /* 'true' if a callee might be tail called */
+  int sibcall_fails;
 };