Patchwork [arm] Fix PR45701

login
register
mail settings
Submitter Yao Qi
Date Sept. 26, 2010, 3:03 p.m.
Message ID <4C9F60B2.2090108@codesourcery.com>
Download mbox | patch
Permalink /patch/65789/
State New
Headers show

Comments

Yao Qi - Sept. 26, 2010, 3:03 p.m.
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?
Richard Earnshaw - Sept. 27, 2010, 9:55 a.m.
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.

R.

Patch

gcc/
	PR target/45701
        * function.h (struct rtl_data): Replace field bool tail_call_emit
        by VEC (rtx, gc) tail_call_insns.
        * 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..182c1d1 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -3195,7 +3195,10 @@  expand_call (tree exp, rtx target, int ignore)
   if (tail_call_insns)
     {
       emit_insn (tail_call_insns);
-      crtl->tail_call_emit = true;
+      if (crtl->tail_call_insns == NULL)
+	crtl->tail_call_insns = VEC_alloc (rtx, gc, 64);
+
+      VEC_safe_push (rtx, gc, crtl->tail_call_insns, 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..7eaffc0 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,26 @@  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. */
+	  if (crtl->tail_call_insns)
+	    for (i = 0; 
+		 VEC_iterate(rtx, crtl->tail_call_insns, i, tail_call_insn);
+		 i++)
+	      {
+		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.h b/gcc/function.h
index 93a9b82..9254fa5 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 when the tail call 
+     hasn't been produced at all.  */
+  VEC (rtx, gc) *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);
+}