From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 84241 invoked by alias); 10 Jul 2015 17:21:41 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 84221 invoked by uid 89); 10 Jul 2015 17:21:40 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-ob0-f174.google.com Received: from mail-ob0-f174.google.com (HELO mail-ob0-f174.google.com) (209.85.214.174) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Fri, 10 Jul 2015 17:21:39 +0000 Received: by obbkm3 with SMTP id km3so195153577obb.1 for ; Fri, 10 Jul 2015 10:21:37 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.60.92.198 with SMTP id co6mr19866138oeb.3.1436548897364; Fri, 10 Jul 2015 10:21:37 -0700 (PDT) Received: by 10.60.231.195 with HTTP; Fri, 10 Jul 2015 10:21:37 -0700 (PDT) In-Reply-To: References: <20150709105439.GA19247@gmail.com> Date: Fri, 10 Jul 2015 17:21:00 -0000 Message-ID: Subject: Re: [PATCH] PR target/66819: Allow indirect sibcall with register arguments From: Uros Bizjak To: "H.J. Lu" Cc: "gcc-patches@gcc.gnu.org" Content-Type: text/plain; charset=UTF-8 X-SW-Source: 2015-07/txt/msg00913.txt.bz2 On Fri, Jul 10, 2015 at 7:10 PM, H.J. Lu wrote: > On Fri, Jul 10, 2015 at 9:30 AM, Uros Bizjak wrote: >> On Thu, Jul 9, 2015 at 12:54 PM, H.J. Lu wrote: >>> Indirect sibcall with register arguments is OK when there is register >>> available for argument passing. >>> >>> OK for trunk if there is no regression? >>> >>> >>> H.J. >>> --- >>> gcc/ >>> >>> PR target/66819 >>> * config/i386/i386.c (ix86_function_ok_for_sibcall): Allow >>> indirect sibcall with register arguments if register available >>> for argument passing. >>> (init_cumulative_args): Set cfun->machine->arg_reg_available_p >>> to cum->nregs != 0. Please update the above entry for nregs > 0. >>> (function_arg_advance_32): Set cfun->machine->arg_reg_available_p >>> to 0 when setting cum->nregs = 0. >> >> Do we also need similar functionality for 64bit ABIs? What happens if >> we are out of argument regs there? > > 64-bit is OK since we have rax, r10 and r11 as scratch registers which > aren't used to pass arguments. Maybe this fact should be added as a comment in some appropriate place. >>> * config/i386/i386.h (machine_function): Add arg_reg_available_p. >>> >>> gcc/testsuite/ >>> >>> PR target/66819 >>> * gcc.target/i386/pr66819-1.c: New test. >>> * gcc.target/i386/pr66819-2.c: Likewise. >>> * gcc.target/i386/pr66819-3.c: Likewise. >>> * gcc.target/i386/pr66819-4.c: Likewise. >>> * gcc.target/i386/pr66819-5.c: Likewise. >>> --- >>> gcc/config/i386/i386.c | 15 +++++++++------ >>> gcc/config/i386/i386.h | 3 +++ >>> gcc/testsuite/gcc.target/i386/pr66819-1.c | 8 ++++++++ >>> gcc/testsuite/gcc.target/i386/pr66819-2.c | 8 ++++++++ >>> gcc/testsuite/gcc.target/i386/pr66819-3.c | 10 ++++++++++ >>> gcc/testsuite/gcc.target/i386/pr66819-4.c | 12 ++++++++++++ >>> gcc/testsuite/gcc.target/i386/pr66819-5.c | 10 ++++++++++ >>> 7 files changed, 60 insertions(+), 6 deletions(-) >>> create mode 100644 gcc/testsuite/gcc.target/i386/pr66819-1.c >>> create mode 100644 gcc/testsuite/gcc.target/i386/pr66819-2.c >>> create mode 100644 gcc/testsuite/gcc.target/i386/pr66819-3.c >>> create mode 100644 gcc/testsuite/gcc.target/i386/pr66819-4.c >>> create mode 100644 gcc/testsuite/gcc.target/i386/pr66819-5.c >>> >>> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c >>> index 54ee6f3..85e59a8 100644 >>> --- a/gcc/config/i386/i386.c >>> +++ b/gcc/config/i386/i386.c >>> @@ -5628,12 +5628,12 @@ ix86_function_ok_for_sibcall (tree decl, tree exp) >>> if (!decl >>> || (TARGET_DLLIMPORT_DECL_ATTRIBUTES && DECL_DLLIMPORT_P (decl))) >>> { >>> - if (ix86_function_regparm (type, NULL) >= 3) >>> - { >>> - /* ??? Need to count the actual number of registers to be used, >>> - not the possible number of registers. Fix later. */ >>> - return false; >>> - } >>> + /* FIXME: The symbol indirect call doesn't need a >>> + call-clobbered register. But we don't know if >>> + this is a symbol indirect call or not here. */ >>> + if (ix86_function_regparm (type, NULL) >= 3 >>> + && !cfun->machine->arg_reg_available_p) >> >> Isn't enough to look at arg_reg_available here? > > We need to check ix86_function_regparm since nregs is 0 if > -mregparm=N isn't used and pr65753.c will fail. OK. Please add this comment, is not that obvious. > >>> + return false; >>> } >>> } >>> >>> @@ -6567,6 +6567,7 @@ init_cumulative_args (CUMULATIVE_ARGS *cum, /* Argument info to initialize */ >>> ? X86_64_REGPARM_MAX >>> : X86_64_MS_REGPARM_MAX); >>> } >>> + cfun->machine->arg_reg_available_p = cum->nregs != 0; >> >> false instead of 0. This is a boolean. > > Updated. > >>> if (TARGET_SSE) >>> { >>> cum->sse_nregs = SSE_REGPARM_MAX; >>> @@ -6636,6 +6637,7 @@ init_cumulative_args (CUMULATIVE_ARGS *cum, /* Argument info to initialize */ >>> else >>> cum->nregs = ix86_function_regparm (fntype, fndecl); >>> } >>> + cfun->machine->arg_reg_available_p = cum->nregs != 0; >> >> IMO, cum->nregs > 0 would be more descriptive. > > Updated. > >>> /* Set up the number of SSE registers used for passing SFmode >>> and DFmode arguments. Warn for mismatching ABI. */ >>> @@ -7584,6 +7586,7 @@ pass_in_reg: >>> { >>> cum->nregs = 0; >>> cum->regno = 0; >>> + cfun->machine->arg_reg_available_p = 0; >>> } >>> break; >>> >>> diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h >>> index 74334ff..0b6e304 100644 >>> --- a/gcc/config/i386/i386.h >>> +++ b/gcc/config/i386/i386.h >>> @@ -2479,6 +2479,9 @@ struct GTY(()) machine_function { >>> /* If true, it is safe to not save/restore DRAP register. */ >>> BOOL_BITFIELD no_drap_save_restore : 1; >>> >>> + /* If true, there is register available for argument passing. */ >>> + BOOL_BITFIELD arg_reg_available_p : 1; >> >> This is not a predicate, but a boolean flag. Please remove _p from the name. > > Updated. > > Here is the updated patch. OK for trunk? OK with a small comment additions. + /* If true, there is register available for argument passing. */ + BOOL_BITFIELD arg_reg_available : 1; + Please mention here that this is for 32bit targets only. Thanks, Uros.