From patchwork Mon Sep 27 13:10:08 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yao Qi X-Patchwork-Id: 65844 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id E85C4B70A3 for ; Mon, 27 Sep 2010 23:10:46 +1000 (EST) Received: (qmail 20034 invoked by alias); 27 Sep 2010 13:10:34 -0000 Received: (qmail 19769 invoked by uid 22791); 27 Sep 2010 13:10:31 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL, BAYES_00, TW_CF, TW_CP, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 27 Sep 2010 13:10:22 +0000 Received: (qmail 25598 invoked from network); 27 Sep 2010 13:10:20 -0000 Received: from unknown (HELO ?192.168.0.100?) (yao@127.0.0.2) by mail.codesourcery.com with ESMTPA; 27 Sep 2010 13:10:20 -0000 Message-ID: <4CA097B0.5060908@codesourcery.com> Date: Mon, 27 Sep 2010 21:10:08 +0800 From: Yao Qi User-Agent: Thunderbird 2.0.0.24 (X11/20100411) MIME-Version: 1.0 To: Richard Earnshaw CC: gcc-patches@gcc.gnu.org Subject: Re: [patch, arm] Fix PR45701 References: <4C9966FE.2050707@codesourcery.com> <1285258289.13878.217.camel@e102346-lin.cambridge.arm.com> <4C9F60B2.2090108@codesourcery.com> <1285581337.22518.21.camel@e102346-lin.cambridge.arm.com> In-Reply-To: <1285581337.22518.21.camel@e102346-lin.cambridge.arm.com> X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org 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. 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); +}