diff mbox

[arm] Fix PR45701

Message ID 4CA097B0.5060908@codesourcery.com
State New
Headers show

Commit Message

Yao Qi Sept. 27, 2010, 1:10 p.m. UTC 
Richard Earnshaw wrote:
> On Sun, 2010-09-26 at 23:03 +0800, Yao Qi wrote:
>> Richard Earnshaw wrote:
>>> On Wed, 2010-09-22 at 10:16 +0800, Yao Qi wrote:
>>> How does this track a function with more than one tailcall?  It seems
>>> that it only keeps track of the last tail-call emitted.  What happens if
>>> the first one uses R3, but the second one doesn't?
>>>
>> We can use VEC (rtx, gc) tail_call_insns to record all tail-call insns,
>> and check all of them in arm_get_frame_offsets to see if r3 has been
>> used for any tail-calls.  Beside this, I've added three new test cases.
>>
>> Tested on arm-none-linux-gnueabi and i686-pc-linux-gnu.  No regression.
>>    Is it OK?
>>
> 
> VEC seems a bit heavy-weight for this. Wouldn't an EXPR_LIST be better?
> After all, you only pass over it in a linear manner and there's no
> required order.
> 

Here is a new patch as you suggested.

Comments

Yao Qi Oct. 4, 2010, 10:04 a.m. UTC | #1 
On 09/27/2010 09:10 PM, Yao Qi wrote:
> Richard Earnshaw wrote:
>> On Sun, 2010-09-26 at 23:03 +0800, Yao Qi wrote:
>>> Richard Earnshaw wrote:
>>>> On Wed, 2010-09-22 at 10:16 +0800, Yao Qi wrote:
>>>> How does this track a function with more than one tailcall?  It seems
>>>> that it only keeps track of the last tail-call emitted.  What happens if
>>>> the first one uses R3, but the second one doesn't?
>>>>
>>> We can use VEC (rtx, gc) tail_call_insns to record all tail-call insns,
>>> and check all of them in arm_get_frame_offsets to see if r3 has been
>>> used for any tail-calls.  Beside this, I've added three new test cases.
>>>
>>> Tested on arm-none-linux-gnueabi and i686-pc-linux-gnu.  No regression.
>>>    Is it OK?
>>>
>>
>> VEC seems a bit heavy-weight for this. Wouldn't an EXPR_LIST be better?
>> After all, you only pass over it in a linear manner and there's no
>> required order.
>>
> 
> Here is a new patch as you suggested.
> 

Ping.
http://gcc.gnu.org/ml/gcc-patches/2010-09/msg02114.html
Yao Qi Nov. 15, 2010, 1:32 p.m. UTC | #2 
On 09/27/2010 09:10 PM, Yao Qi wrote:
> Richard Earnshaw wrote:
>> On Sun, 2010-09-26 at 23:03 +0800, Yao Qi wrote:
>>> Richard Earnshaw wrote:
>>>> On Wed, 2010-09-22 at 10:16 +0800, Yao Qi wrote:
>>>> How does this track a function with more than one tailcall?  It seems
>>>> that it only keeps track of the last tail-call emitted.  What happens if
>>>> the first one uses R3, but the second one doesn't?
>>>>
>>> We can use VEC (rtx, gc) tail_call_insns to record all tail-call insns,
>>> and check all of them in arm_get_frame_offsets to see if r3 has been
>>> used for any tail-calls.  Beside this, I've added three new test cases.
>>>
>>> Tested on arm-none-linux-gnueabi and i686-pc-linux-gnu.  No regression.
>>>    Is it OK?
>>>
>>
>> VEC seems a bit heavy-weight for this. Wouldn't an EXPR_LIST be better?
>> After all, you only pass over it in a linear manner and there's no
>> required order.
>>
> 
> Here is a new patch as you suggested.
> 

Ping.
http://gcc.gnu.org/ml/gcc-patches/2010-09/msg02114.html
Yao Qi Nov. 26, 2010, 1:51 a.m. UTC | #3 
On 09/27/2010 09:10 PM, Yao Qi wrote:
> Richard Earnshaw wrote:
>> On Sun, 2010-09-26 at 23:03 +0800, Yao Qi wrote:
>>> Richard Earnshaw wrote:
>>>> On Wed, 2010-09-22 at 10:16 +0800, Yao Qi wrote:
>>>> How does this track a function with more than one tailcall?  It seems
>>>> that it only keeps track of the last tail-call emitted.  What happens if
>>>> the first one uses R3, but the second one doesn't?
>>>>
>>> We can use VEC (rtx, gc) tail_call_insns to record all tail-call insns,
>>> and check all of them in arm_get_frame_offsets to see if r3 has been
>>> used for any tail-calls.  Beside this, I've added three new test cases.
>>>
>>> Tested on arm-none-linux-gnueabi and i686-pc-linux-gnu.  No regression.
>>>    Is it OK?
>>>
>>
>> VEC seems a bit heavy-weight for this. Wouldn't an EXPR_LIST be better?
>> After all, you only pass over it in a linear manner and there's no
>> required order.
>>
> 
> Here is a new patch as you suggested.
> 

Ping.
http://gcc.gnu.org/ml/gcc-patches/2010-09/msg02114.html

gcc/
        PR target/45701
        * function.h (struct rtl_data): Replace field bool
	tail_call_emit by rtx tail_call_insns.
	* function.c (free_after_compilation): Free EXPR_LIST.
        * call.c (expand_call): Modified code to match new data
	structures.
        * cfgcleanup.c (rest_of_handle_jump): Likewise.
        * config/i386/i386.c (find_drap_reg): Likewise.
        * config/arm/arm.c (arm_output_epilogue):Likewise.
        (arm_get_frame_offsets): Use r3 for padding stack to 8-byte
	alignment if r3 is not used to pass values to tail call.

gcc/testsuite/
        PR target/45701
        * gcc.target/arm/pr45701-1.c: New test.
        * gcc.target/arm/pr45701-2.c: New test.
	* gcc.target/arm/pr45701-3.c: New test.
diff mbox

Patch

gcc/
        PR target/45701
        * function.h (struct rtl_data): Replace field bool tail_call_emit
        by rtx tail_call_insns.
	* function.c (free_after_compilation): Free EXPR_LIST. 
        * call.c (expand_call): Modified code to match new data structures.
        * cfgcleanup.c (rest_of_handle_jump): Likewise.
        * config/i386/i386.c (find_drap_reg): Likewise.
        * config/arm/arm.c (arm_output_epilogue):Likewise.
        (arm_get_frame_offsets): Use r3 for padding stack to 8-byte alignment
        if r3 is not used to pass values to tail call.

gcc/testsuite/
        PR target/45701
        * gcc.target/arm/pr45701-1.c: New test.
        * gcc.target/arm/pr45701-2.c: New test.
	* gcc.target/arm/pr45701-3.c: New test.

diff --git a/gcc/calls.c b/gcc/calls.c
index 3888831..9c3252b 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -3195,7 +3195,9 @@  expand_call (tree exp, rtx target, int ignore)
   if (tail_call_insns)
     {
       emit_insn (tail_call_insns);
-      crtl->tail_call_emit = true;
+
+      crtl->tail_call_insns = 
+	alloc_EXPR_LIST(0, tail_call_insns, crtl->tail_call_insns);
     }
   else
     emit_insn (normal_call_insns);
diff --git a/gcc/cfgcleanup.c b/gcc/cfgcleanup.c
index 9ded1e6..c6b8f28 100644
--- a/gcc/cfgcleanup.c
+++ b/gcc/cfgcleanup.c
@@ -2394,7 +2394,7 @@  cleanup_cfg (int mode)
 static unsigned int
 rest_of_handle_jump (void)
 {
-  if (crtl->tail_call_emit)
+  if (crtl->tail_call_insns)
     fixup_tail_calls ();
   return 0;
 }
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 6f260ec..42f9664 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -14490,7 +14490,7 @@  arm_output_epilogue (rtx sibling)
 		  && !crtl->calls_eh_return
 		  && bit_count(saved_regs_mask) * 4 == count
 		  && !IS_INTERRUPT (func_type)
-		  && !crtl->tail_call_emit)
+		  && !crtl->tail_call_insns)
 		{
 		  unsigned long mask;
                   /* Preserve return values, of any size.  */
@@ -15113,11 +15113,25 @@  arm_get_frame_offsets (void)
       if (frame_size + crtl->outgoing_args_size == 0)
 	{
 	  int reg = -1;
-
+	  bool r3_used_in_tail_call = false;
+	  int i;
+	  rtx tail_call_insn;
+	  
+	  /* r3 is safe to use if tail calls don't use it. */
+	  for (tail_call_insn = crtl->tail_call_insns; 
+	       tail_call_insn;
+	       tail_call_insn = XEXP (tail_call_insn, 1))
+	    {
+	      if (find_regno_fusage (tail_call_insn, USE, 3))
+		{
+		  r3_used_in_tail_call = true;
+		  break;
+		}
+	    }
 	  /* If it is safe to use r3, then do so.  This sometimes 
 	     generates better code on Thumb-2 by avoiding the need to
 	     use 32-bit push/pop instructions.  */
-	  if (!crtl->tail_call_emit
+	  if (!r3_used_in_tail_call
 	      && arm_size_return_regs () <= 12
 	      && (offsets->saved_regs_mask & (1 << 3)) == 0)
 	    {
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 19d6387..973b8a7 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -8853,7 +8853,7 @@  find_drap_reg (void)
 	 Since function with tail call may use any caller-saved
 	 registers in epilogue, DRAP must not use caller-saved
 	 register in such case.  */
-      if (DECL_STATIC_CHAIN (decl) || crtl->tail_call_emit)
+      if (DECL_STATIC_CHAIN (decl) || crtl->tail_call_insns)
 	return R13_REG;
 
       return R10_REG;
@@ -8864,7 +8864,7 @@  find_drap_reg (void)
 	 Since function with tail call may use any caller-saved
 	 registers in epilogue, DRAP must not use caller-saved
 	 register in such case.  */
-      if (DECL_STATIC_CHAIN (decl) || crtl->tail_call_emit)
+      if (DECL_STATIC_CHAIN (decl) || crtl->tail_call_insns)
 	return DI_REG;
 
       /* Reuse static chain register if it isn't used for parameter
diff --git a/gcc/function.c b/gcc/function.c
index fac8b75..f67369b 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -214,6 +214,9 @@  free_after_compilation (struct function *f)
   if (crtl->emit.regno_pointer_align)
     free (crtl->emit.regno_pointer_align);
 
+  if (crtl->tail_call_insns)
+    free_EXPR_LIST_list (&crtl->tail_call_insns);
+
   memset (crtl, 0, sizeof (struct rtl_data));
   f->eh = NULL;
   f->machine = NULL;
diff --git a/gcc/function.h b/gcc/function.h
index 93a9b82..b8444f7 100644
--- a/gcc/function.h
+++ b/gcc/function.h
@@ -393,8 +393,9 @@  struct GTY(()) rtl_data {
   /* Nonzero if the current function needs an lsda for exception handling.  */
   bool uses_eh_lsda;
 
-  /* Set when the tail call has been produced.  */
-  bool tail_call_emit;
+  /* Insns that produced for tail calls.  It is NULL_RTX when the tail call 
+     hasn't been produced at all.  */
+  rtx tail_call_insns;
 
   /* Nonzero if code to initialize arg_pointer_save_area has been emitted.  */
   bool arg_pointer_save_area_init;
diff --git a/gcc/testsuite/gcc.target/arm/pr45701-1.c b/gcc/testsuite/gcc.target/arm/pr45701-1.c
new file mode 100644
index 0000000..f94f578
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr45701-1.c
@@ -0,0 +1,24 @@ 
+/* { dg-do compile } */
+/* { dg-options "-march=armv7-a -mthumb -Os" }  */
+/* { dg-final { scan-assembler "push\t\{r3" } } */
+/* { dg-final { scan-assembler-not "r8" } } */
+
+extern int hist_verify;
+extern char *pre_process_line (char*);
+extern char* str_cpy (char*, char*);
+extern int str_len (char*);
+extern char* x_malloc (int);
+#define savestring(x) (char *)str_cpy (x_malloc (1 + str_len (x)), (x))
+
+char *
+history_expand_line_internal (char* line)
+{
+  char *new_line;
+  int old_verify;
+
+  old_verify = hist_verify;
+  hist_verify = 0;
+  new_line = pre_process_line (line);
+  hist_verify = old_verify;
+  return (new_line == line) ? savestring (line) : new_line;
+}
diff --git a/gcc/testsuite/gcc.target/arm/pr45701-2.c b/gcc/testsuite/gcc.target/arm/pr45701-2.c
new file mode 100644
index 0000000..68d5178
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr45701-2.c
@@ -0,0 +1,26 @@ 
+/* { dg-do compile } */
+/* { dg-options "-march=armv7-a -mthumb -Os" }  */
+/* { dg-final { scan-assembler "push\t\{r3" } } */
+/* { dg-final { scan-assembler-not "r8" } } */
+
+extern int hist_verify;
+extern char *pre_process_line (char*);
+extern char* savestring1 (char*, char*);
+extern char* str_cpy (char*, char*);
+extern int str_len (char*);
+extern char* x_malloc (int);
+#define savestring(x) (char *)str_cpy (x_malloc (1 + str_len (x)), (x))
+
+char *
+history_expand_line_internal (char* line)
+{
+  char *new_line;
+  int old_verify;
+
+  old_verify = hist_verify;
+  hist_verify = 0;
+  new_line = pre_process_line (line);
+  hist_verify = old_verify;
+  /* Two tail calls here, but r3 is not used to pass values.  */
+  return (new_line == line) ? savestring (line) : savestring1 (new_line, line);
+}
diff --git a/gcc/testsuite/gcc.target/arm/pr45701-3.c b/gcc/testsuite/gcc.target/arm/pr45701-3.c
new file mode 100644
index 0000000..8255bae
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr45701-3.c
@@ -0,0 +1,27 @@ 
+/* { dg-do compile } */
+/* { dg-options "-march=armv7-a -mthumb -Os" }  */
+/* { dg-final { scan-assembler "push\t.*r8" } } */
+/* { dg-final { scan-assembler-not "push\t*r3" } } */
+
+extern int hist_verify;
+extern char *pre_process_line (char*);
+extern char* savestring1 (char*, char*, int, int);
+extern char* str_cpy (char*, char*);
+extern int str_len (char*);
+extern char* x_malloc (int);
+#define savestring(x) (char *)str_cpy (x_malloc (1 + str_len (x)), (x))
+
+char *
+history_expand_line_internal (char* line)
+{
+  char *new_line;
+  int old_verify;
+
+  old_verify = hist_verify;
+  hist_verify = 0;
+  new_line = pre_process_line (line);
+  hist_verify = old_verify;
+  /* Two tail calls here, but r3 is used to pass values.  */
+  return (new_line == line) ? savestring (line) : 
+    savestring1 (new_line, line, 0, old_verify+1);
+}