From patchwork Fri Jun 7 18:15:47 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "H.J. Lu" X-Patchwork-Id: 1112160 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-502598-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="EzpUwJ+n"; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="F9vAPdEv"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 45L9hj036zz9sBb for ; Sat, 8 Jun 2019 04:16:35 +1000 (AEST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:references:in-reply-to:from:date:message-id :subject:to:cc:content-type; q=dns; s=default; b=imTyHbPBeiTWuXz nDdqJ008LZOXEmvNu0YcGs6FPK3XgxUIPBpG6dqRSP3x2LZ8Az+ftvS4cv9F8ILX 63hSo02EGO5JklY4nowyTUGrllA/eJwIOuhB7SqE5sU/+24+VfG18v+ErxHW2Sac /iTjznQp2akDEfnAjKOXGbZWzkqQ= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:references:in-reply-to:from:date:message-id :subject:to:cc:content-type; s=default; bh=SBaQwG/ZOnAxaJySMYCaN gSS/5s=; b=EzpUwJ+nLcp9Z4sRTp9ei1JEEDEsAzq75+92gMjhPr1XjI7bKikbw Vy652CVd8V+RJwFyHFnQvIcQuqH/f4YLB4CK6mFDjlQhs/vqubI/mBcGL+DZSpiG 6mE2DS/CxEY89LajAV/WxqojZvIrN2ENPFoayrN4IYmfPPd4Pv7COc= Received: (qmail 4082 invoked by alias); 7 Jun 2019 18:16:27 -0000 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 Received: (qmail 4071 invoked by uid 89); 7 Jun 2019 18:16:27 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-14.1 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=error_at, Agreed X-HELO: mail-ot1-f68.google.com Received: from mail-ot1-f68.google.com (HELO mail-ot1-f68.google.com) (209.85.210.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 07 Jun 2019 18:16:25 +0000 Received: by mail-ot1-f68.google.com with SMTP id z24so2723313oto.1 for ; Fri, 07 Jun 2019 11:16:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=NUgXhv5DsPia16A07Iimj3vNdrlDLNCaxK8C/Ju/t9Y=; b=F9vAPdEvE7TZnSZ+G/LA420Q3SgrNW+ESxuKlsxRODfBuKjFivyziF4BRvg/Z1w1wX pT7Poa6penwHYGYGOIIOXiwDo8/XuP9wFkwF0L31B1p+ujYhXpLHsztfSuQw1gTyVDsc O2S3cDvB8YIV3QUquhxSDaXPviAmXNlEIlURAQmNCbDv6ZsqaxvLDc3KlYo8A6NPTBNY a8tMsTUaFMQE/ymhlFuEke31sLbHy0S/VoJUNa5DlAMhuuS8yKeYCyTfOlBUIEimpDZq xFScwz1JOMYAHEtB1Mn0t7CD+zPtflFkIODvjM9VmeBJAPX4EuQWdtVmKbqJO7eQXaXy LMXA== MIME-Version: 1.0 References: In-Reply-To: From: "H.J. Lu" Date: Fri, 7 Jun 2019 11:15:47 -0700 Message-ID: Subject: V2: [PATCH] Update preferred_stack_boundary only when expanding function call To: Richard Biener Cc: Richard Sandiford , GCC Patches , Jakub Jelinek , Jeffrey Law X-IsSubscribed: yes On Fri, Jun 7, 2019 at 1:22 AM Richard Biener wrote: > > On Fri, 7 Jun 2019, Richard Sandiford wrote: > > > "H.J. Lu" writes: > > > locate_and_pad_parm is called when expanding function call from > > > initialize_argument_information and when generating function body > > > from assign_parm_find_entry_rtl: > > > > > > /* Remember if the outgoing parameter requires extra alignment on the > > > calling function side. */ > > > if (crtl->stack_alignment_needed < boundary) > > > crtl->stack_alignment_needed = boundary; > > > if (crtl->preferred_stack_boundary < boundary) > > > crtl->preferred_stack_boundary = boundary; > > > > > > stack_alignment_needed and preferred_stack_boundary should be updated > > > only when expanding function call, not when generating function body. > > > Add an argument, outgoing_p, to locate_and_pad_parm to indicate for > > > expanding function call. Update stack_alignment_needed and > > > preferred_stack_boundary if the parameter is passed on stack and only > > > when expanding function call. > > > > > > Tested on Linux/x86-64. > > > > > > OK for trunk? > > > > > > Thanks. > > > > > > -- > > > H.J. > > > > > > From e91e20ad8e10373db2c6d8f99a3da0bbf46c5c34 Mon Sep 17 00:00:00 2001 > > > From: "H.J. Lu" > > > Date: Wed, 5 Jun 2019 12:55:19 -0700 > > > Subject: [PATCH] Update preferred_stack_boundary only when expanding function > > > call > > > > > > locate_and_pad_parm is called when expanding function call from > > > initialize_argument_information and when generating function body > > > from assign_parm_find_entry_rtl: > > > > > > /* Remember if the outgoing parameter requires extra alignment on the > > > calling function side. */ > > > if (crtl->stack_alignment_needed < boundary) > > > crtl->stack_alignment_needed = boundary; > > > if (crtl->preferred_stack_boundary < boundary) > > > crtl->preferred_stack_boundary = boundary; > > > > > > stack_alignment_needed and preferred_stack_boundary should be updated > > > only when expanding function call, not when generating function body. > > > Add an argument, outgoing_p, to locate_and_pad_parm to indicate for > > > expanding function call. Update stack_alignment_needed and > > > preferred_stack_boundary if the parameter is passed on stack and only > > > when expanding function call. > > > > > > Tested on Linux/x86-64. > > > > > > gcc/ > > > > > > PR rtl-optimization/90765 > > > * function.c (assign_parm_find_entry_rtl): Pass false to > > > locate_and_pad_parm. > > > (locate_and_pad_parm): Add an argument, outgoing_p, to indicate > > > for expanding function call. Update stack_alignment_needed and > > > preferred_stack_boundary only if outgoing_p is true and the > > > the parameter is passed on stack. > > > * function.h (locate_and_pad_parm): Add an argument, outgoing_p, > > > defaulting to true. > > > > > > gcc/testsuite/ > > > > > > PR rtl-optimization/90765 > > > * gcc.target/i386/pr90765-1.c: New test. > > > * gcc.target/i386/pr90765-2.c: Likewise. > > > --- > > > gcc/function.c | 21 +++++++++++++-------- > > > gcc/function.h | 3 ++- > > > gcc/testsuite/gcc.target/i386/pr90765-1.c | 11 +++++++++++ > > > gcc/testsuite/gcc.target/i386/pr90765-2.c | 18 ++++++++++++++++++ > > > 4 files changed, 44 insertions(+), 9 deletions(-) > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr90765-1.c > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr90765-2.c > > > > > > diff --git a/gcc/function.c b/gcc/function.c > > > index e30ee259bec..9b6673f6f0d 100644 > > > --- a/gcc/function.c > > > +++ b/gcc/function.c > > > @@ -2601,7 +2601,7 @@ assign_parm_find_entry_rtl (struct assign_parm_data_all *all, > > > locate_and_pad_parm (data->promoted_mode, data->passed_type, in_regs, > > > all->reg_parm_stack_space, > > > entry_parm ? data->partial : 0, current_function_decl, > > > - &all->stack_args_size, &data->locate); > > > + &all->stack_args_size, &data->locate, false); > > > > > > /* Update parm_stack_boundary if this parameter is passed in the > > > stack. */ > > > @@ -3954,7 +3954,8 @@ locate_and_pad_parm (machine_mode passed_mode, tree type, int in_regs, > > > int reg_parm_stack_space, int partial, > > > tree fndecl ATTRIBUTE_UNUSED, > > > struct args_size *initial_offset_ptr, > > > - struct locate_and_pad_arg_data *locate) > > > + struct locate_and_pad_arg_data *locate, > > > + bool outgoing_p) > > > { > > > tree sizetree; > > > pad_direction where_pad; > > > @@ -4021,12 +4022,16 @@ locate_and_pad_parm (machine_mode passed_mode, tree type, int in_regs, > > > } > > > } > > > > > > - /* Remember if the outgoing parameter requires extra alignment on the > > > - calling function side. */ > > > - if (crtl->stack_alignment_needed < boundary) > > > - crtl->stack_alignment_needed = boundary; > > > - if (crtl->preferred_stack_boundary < boundary) > > > - crtl->preferred_stack_boundary = boundary; > > > + if (outgoing_p && !in_regs) > > > + { > > > + /* Remember if the outgoing parameter requires extra alignment on > > > + the calling function side if this parameter is passed in the > > > + stack. */ > > > + if (crtl->stack_alignment_needed < boundary) > > > + crtl->stack_alignment_needed = boundary; > > > + if (crtl->preferred_stack_boundary < boundary) > > > + crtl->preferred_stack_boundary = boundary; > > > + } > > > > Just my opinion, but I think this shows that the code isn't really > > in the right place. IMO locate_and_pad_parm should just describe > > the ABI and is too low-level to be modifying global state like this. > > > > I think we could instead do this by walking over the argument vectors > > in a subroutine of expand_call and emit_library_call_value_1. > > expand_call is also where we do: > > > > /* Ensure current function's preferred stack boundary is at least > > what we need. Stack alignment may also increase preferred stack > > boundary. */ > > if (crtl->preferred_stack_boundary < preferred_stack_boundary) > > crtl->preferred_stack_boundary = preferred_stack_boundary; > > else > > preferred_stack_boundary = crtl->preferred_stack_boundary; > > Agreed - that would be much cleaner. > Here is the updated patch to add update_stack_alignment_for_call. Tested on Linux/x86-64. OK for trunk? Thanks. From 00d81edc1e62c43c2084483df055ea68f4047679 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" Date: Wed, 5 Jun 2019 12:55:19 -0700 Subject: [PATCH] Update preferred_stack_boundary only when expanding function call locate_and_pad_parm is called when expanding function call from initialize_argument_information and when generating function body from assign_parm_find_entry_rtl: /* Remember if the outgoing parameter requires extra alignment on the calling function side. */ if (crtl->stack_alignment_needed < boundary) crtl->stack_alignment_needed = boundary; if (crtl->preferred_stack_boundary < boundary) crtl->preferred_stack_boundary = boundary; stack_alignment_needed and preferred_stack_boundary should be updated only when expanding function call, not when generating function body. Add update_stack_alignment_for_call to update stack alignment when outgoing parameter is passed in the stack. gcc/ PR rtl-optimization/90765 * calls.c (update_stack_alignment_for_call): New function. (initialize_argument_information): Call update_stack_alignment_for_call when outgoing parameter is passed in the stack. (emit_library_call_value_1): Likewise. * function.c (locate_and_pad_parm): Don't update stack_alignment_needed and preferred_stack_boundary. gcc/testsuite/ PR rtl-optimization/90765 * gcc.target/i386/pr90765-1.c: New test. * gcc.target/i386/pr90765-2.c: Likewise. --- gcc/calls.c | 33 ++++++++++++++++++----- gcc/function.c | 7 ----- gcc/testsuite/gcc.target/i386/pr90765-1.c | 11 ++++++++ gcc/testsuite/gcc.target/i386/pr90765-2.c | 18 +++++++++++++ 4 files changed, 55 insertions(+), 14 deletions(-) create mode 100644 gcc/testsuite/gcc.target/i386/pr90765-1.c create mode 100644 gcc/testsuite/gcc.target/i386/pr90765-2.c diff --git a/gcc/calls.c b/gcc/calls.c index c8a42680041..8ba82fbb6c0 100644 --- a/gcc/calls.c +++ b/gcc/calls.c @@ -1841,6 +1841,19 @@ maybe_complain_about_tail_call (tree call_expr, const char *reason) error_at (EXPR_LOCATION (call_expr), "cannot tail-call: %s", reason); } +/* Update stack alignment when the parameter is passed in the stack + since the outgoing parameter requires extra alignment on the calling + function side. */ + +static void +update_stack_alignment_for_call (struct locate_and_pad_arg_data *locate) +{ + if (crtl->stack_alignment_needed < locate->boundary) + crtl->stack_alignment_needed = locate->boundary; + if (crtl->preferred_stack_boundary < locate->boundary) + crtl->preferred_stack_boundary = locate->boundary; +} + /* Fill in ARGS_SIZE and ARGS array based on the parameters found in CALL_EXPR EXP. @@ -2162,15 +2175,18 @@ initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED, if (args[i].reg == 0 || args[i].partial != 0 || reg_parm_stack_space > 0 || args[i].pass_on_stack) - locate_and_pad_parm (mode, type, + { + locate_and_pad_parm (mode, type, #ifdef STACK_PARMS_IN_REG_PARM_AREA - 1, + 1, #else - args[i].reg != 0, + args[i].reg != 0, #endif - reg_parm_stack_space, - args[i].pass_on_stack ? 0 : args[i].partial, - fndecl, args_size, &args[i].locate); + reg_parm_stack_space, + args[i].pass_on_stack ? 0 : args[i].partial, + fndecl, args_size, &args[i].locate); + update_stack_alignment_for_call (&args[i].locate); + } #ifdef BLOCK_REG_PADDING else /* The argument is passed entirely in registers. See at which @@ -4861,7 +4877,10 @@ emit_library_call_value_1 (int retval, rtx orgfun, rtx value, if (argvec[count].reg == 0 || argvec[count].partial != 0 || reg_parm_stack_space > 0) - args_size.constant += argvec[count].locate.size.constant; + { + args_size.constant += argvec[count].locate.size.constant; + update_stack_alignment_for_call (&argvec[count].locate); + } targetm.calls.function_arg_advance (args_so_far, Pmode, (tree) 0, true); diff --git a/gcc/function.c b/gcc/function.c index e30ee259bec..45b65dc0fd2 100644 --- a/gcc/function.c +++ b/gcc/function.c @@ -4021,13 +4021,6 @@ locate_and_pad_parm (machine_mode passed_mode, tree type, int in_regs, } } - /* Remember if the outgoing parameter requires extra alignment on the - calling function side. */ - if (crtl->stack_alignment_needed < boundary) - crtl->stack_alignment_needed = boundary; - if (crtl->preferred_stack_boundary < boundary) - crtl->preferred_stack_boundary = boundary; - if (ARGS_GROW_DOWNWARD) { locate->slot_offset.constant = -initial_offset_ptr->constant; diff --git a/gcc/testsuite/gcc.target/i386/pr90765-1.c b/gcc/testsuite/gcc.target/i386/pr90765-1.c new file mode 100644 index 00000000000..178c3ff8054 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr90765-1.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -mavx512f" } */ +/* { dg-final { scan-assembler-not "and\[lq\]?\[\\t \]*\\$-64,\[\\t \]*%\[re\]?sp" } } */ + +typedef int __v16si __attribute__ ((__vector_size__ (64))); + +void +foo (__v16si x, int i0, int i1, int i2, int i3, int i4, int i5, __v16si *p) +{ + *p = x; +} diff --git a/gcc/testsuite/gcc.target/i386/pr90765-2.c b/gcc/testsuite/gcc.target/i386/pr90765-2.c new file mode 100644 index 00000000000..45cf1f03747 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr90765-2.c @@ -0,0 +1,18 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -mavx512f" } */ +/* { dg-final { scan-assembler "and\[lq\]?\[\\t \]*\\$-64,\[\\t \]*%\[re\]?sp" } } */ +/* { dg-skip-if "" { x86_64-*-mingw* } } */ + +typedef int __v16si __attribute__ ((__vector_size__ (64))); + +extern void foo (__v16si, __v16si, __v16si, __v16si, __v16si, __v16si, + __v16si, __v16si, __v16si, int, int, int, int, int, + int, __v16si *); + +extern __v16si x, y; + +void +bar (void) +{ + foo (x, x, x, x, x, x, x, x, x, 0, 1, 2, 3, 4, 5, &y); +} -- 2.20.1