Message ID | 20220414011811.62291-1-hongyu.wang@intel.com |
---|---|
State | Committed |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id D1434385781A for <patchwork@sourceware.org>; Thu, 14 Apr 2022 01:18:44 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org D1434385781A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1649899124; bh=FQjtC6vTzInmYB5b5MUPfNV5j6ZExsmbaMOgBnRqpsw=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=DHsVYkObAwPMjLdvsb1OlACGIxJwsdEowWFCjOhGz1PmqLzfBp8v7foiAOYEVzOsm IP2S2bihnKzqfc5jHGZcX8mdTZvwvqlm6HOR1TLCPmLXJQYg2mUNryUogsPUw9T1id 4/koYZcWrukTnmhGPD4e8UofYNpH/w2ESzXRdShs= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by sourceware.org (Postfix) with ESMTPS id 3E64F3858D3C for <gcc-patches@gcc.gnu.org>; Thu, 14 Apr 2022 01:18:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 3E64F3858D3C X-IronPort-AV: E=McAfee;i="6400,9594,10316"; a="244703221" X-IronPort-AV: E=Sophos;i="5.90,258,1643702400"; d="scan'208";a="244703221" Received: from orsmga007.jf.intel.com ([10.7.209.58]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Apr 2022 18:18:13 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.90,258,1643702400"; d="scan'208";a="552438727" Received: from scymds01.sc.intel.com ([10.148.94.138]) by orsmga007.jf.intel.com with ESMTP; 13 Apr 2022 18:18:13 -0700 Received: from shliclel320.sh.intel.com (shliclel320.sh.intel.com [10.239.236.50]) by scymds01.sc.intel.com with ESMTP id 23E1IChR026178; Wed, 13 Apr 2022 18:18:12 -0700 To: ubizjak@gmail.com Subject: [PATCH] i386: Disable stv under optimize_size [PR 105034] Date: Thu, 14 Apr 2022 09:18:11 +0800 Message-Id: <20220414011811.62291-1-hongyu.wang@intel.com> X-Mailer: git-send-email 2.18.1 X-Spam-Status: No, score=-10.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_SHORT, SPF_HELO_NONE, SPF_SOFTFAIL, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: Hongyu Wang via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Hongyu Wang <hongyu.wang@intel.com> Cc: gcc-patches@gcc.gnu.org Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
i386: Disable stv under optimize_size [PR 105034]
|
|
Commit Message
Hongyu Wang
April 14, 2022, 1:18 a.m. UTC
Hi, From -Os point of view, stv converts scalar register to vector mode which introduces extra reg conversion and increase instruction size. Disabling stv under optimize_size would avoid such code size increment and no need to touch ix86_size_cost that has not been tuned for long time. Bootstrapped/regtested on x86_64-pc-linux-gnu{-m32,}, Ok for master? gcc/ChangeLog: PR target/105034 * config/i386/i386-features.cc (pass_stv::gate()): Block out optimize_size. gcc/testsuite/ChangeLog: PR target/105034 * gcc.target/i386/pr105034.c: New test. --- gcc/config/i386/i386-features.cc | 3 ++- gcc/testsuite/gcc.target/i386/pr105034.c | 23 +++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gcc.target/i386/pr105034.c
Comments
On Thu, Apr 14, 2022 at 3:18 AM Hongyu Wang via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > Hi, > > From -Os point of view, stv converts scalar register to vector mode > which introduces extra reg conversion and increase instruction size. > Disabling stv under optimize_size would avoid such code size increment > and no need to touch ix86_size_cost that has not been tuned for long > time. > > Bootstrapped/regtested on x86_64-pc-linux-gnu{-m32,}, > > Ok for master? > > gcc/ChangeLog: > > PR target/105034 > * config/i386/i386-features.cc (pass_stv::gate()): Block out > optimize_size. > > gcc/testsuite/ChangeLog: > > PR target/105034 > * gcc.target/i386/pr105034.c: New test. > --- > gcc/config/i386/i386-features.cc | 3 ++- > gcc/testsuite/gcc.target/i386/pr105034.c | 23 +++++++++++++++++++++++ > 2 files changed, 25 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/gcc.target/i386/pr105034.c > > diff --git a/gcc/config/i386/i386-features.cc b/gcc/config/i386/i386-features.cc > index 6fe41c3c24f..f57281e672f 100644 > --- a/gcc/config/i386/i386-features.cc > +++ b/gcc/config/i386/i386-features.cc > @@ -1911,7 +1911,8 @@ public: > virtual bool gate (function *) > { > return ((!timode_p || TARGET_64BIT) > - && TARGET_STV && TARGET_SSE2 && optimize > 1); > + && TARGET_STV && TARGET_SSE2 && optimize > 1 > + && !optimize_size); optimize_function_for_speed ()? > } > > virtual unsigned int execute (function *) > diff --git a/gcc/testsuite/gcc.target/i386/pr105034.c b/gcc/testsuite/gcc.target/i386/pr105034.c > new file mode 100644 > index 00000000000..d997e26e9ed > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr105034.c > @@ -0,0 +1,23 @@ > +/* PR target/105034 */ > +/* { dg-do compile } */ > +/* { dg-options "-Os -msse4.1" } */ > + > +#define max(a,b) (((a) > (b))? (a) : (b)) > +#define min(a,b) (((a) < (b))? (a) : (b)) > + > +int foo(int x) > +{ > + return max(x,0); > +} > + > +int bar(int x) > +{ > + return min(x,0); > +} > + > +unsigned int baz(unsigned int x) > +{ > + return min(x,1); > +} > + > +/* { dg-final { scan-assembler-not "xmm" } } */ > -- > 2.18.1 >
> > optimize_function_for_speed ()? > Yes, updated patch with optimize_function_for_speed_p() gcc/ChangeLog: PR target/105034 * config/i386/i386-features.cc (pass_stv::gate()): Add optimize_function_for_speed_p (). gcc/testsuite/ChangeLog: PR target/105034 * gcc.target/i386/pr105034.c: New test. --- gcc/config/i386/i386-features.cc | 3 ++- gcc/testsuite/gcc.target/i386/pr105034.c | 23 +++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gcc.target/i386/pr105034.c diff --git a/gcc/config/i386/i386-features.cc b/gcc/config/i386/i386-features.cc index 6fe41c3c24f..a49c3aa1525 100644 --- a/gcc/config/i386/i386-features.cc +++ b/gcc/config/i386/i386-features.cc @@ -1911,7 +1911,8 @@ public: virtual bool gate (function *) { return ((!timode_p || TARGET_64BIT) - && TARGET_STV && TARGET_SSE2 && optimize > 1); + && TARGET_STV && TARGET_SSE2 && optimize > 1 + && optimize_function_for_speed_p (cfun)); } virtual unsigned int execute (function *) diff --git a/gcc/testsuite/gcc.target/i386/pr105034.c b/gcc/testsuite/gcc.target/i386/pr105034.c new file mode 100644 index 00000000000..d997e26e9ed --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr105034.c @@ -0,0 +1,23 @@ +/* PR target/105034 */ +/* { dg-do compile } */ +/* { dg-options "-Os -msse4.1" } */ + +#define max(a,b) (((a) > (b))? (a) : (b)) +#define min(a,b) (((a) < (b))? (a) : (b)) + +int foo(int x) +{ + return max(x,0); +} + +int bar(int x) +{ + return min(x,0); +} + +unsigned int baz(unsigned int x) +{ + return min(x,1); +} + +/* { dg-final { scan-assembler-not "xmm" } } */
On Thu, Apr 14, 2022 at 9:55 AM Hongyu Wang <wwwhhhyyy333@gmail.com> wrote: > > > > > optimize_function_for_speed ()? > > > > Yes, updated patch with optimize_function_for_speed_p() > > gcc/ChangeLog: > > PR target/105034 > * config/i386/i386-features.cc (pass_stv::gate()): Add > optimize_function_for_speed_p (). > > gcc/testsuite/ChangeLog: > > PR target/105034 > * gcc.target/i386/pr105034.c: New test. > --- > gcc/config/i386/i386-features.cc | 3 ++- > gcc/testsuite/gcc.target/i386/pr105034.c | 23 +++++++++++++++++++++++ > 2 files changed, 25 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/gcc.target/i386/pr105034.c > > diff --git a/gcc/config/i386/i386-features.cc b/gcc/config/i386/i386-features.cc > index 6fe41c3c24f..a49c3aa1525 100644 > --- a/gcc/config/i386/i386-features.cc > +++ b/gcc/config/i386/i386-features.cc > @@ -1911,7 +1911,8 @@ public: > virtual bool gate (function *) please name the parameter ... > { > return ((!timode_p || TARGET_64BIT) > - && TARGET_STV && TARGET_SSE2 && optimize > 1); > + && TARGET_STV && TARGET_SSE2 && optimize > 1 > + && optimize_function_for_speed_p (cfun)); ... and use it here instead of referencing 'cfun' Richard. > } > > virtual unsigned int execute (function *) > diff --git a/gcc/testsuite/gcc.target/i386/pr105034.c > b/gcc/testsuite/gcc.target/i386/pr105034.c > new file mode 100644 > index 00000000000..d997e26e9ed > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr105034.c > @@ -0,0 +1,23 @@ > +/* PR target/105034 */ > +/* { dg-do compile } */ > +/* { dg-options "-Os -msse4.1" } */ > + > +#define max(a,b) (((a) > (b))? (a) : (b)) > +#define min(a,b) (((a) < (b))? (a) : (b)) > + > +int foo(int x) > +{ > + return max(x,0); > +} > + > +int bar(int x) > +{ > + return min(x,0); > +} > + > +unsigned int baz(unsigned int x) > +{ > + return min(x,1); > +} > + > +/* { dg-final { scan-assembler-not "xmm" } } */ > -- > 2.18.1 > > Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> 于2022年4月14日周四 14:56写道: > > > > On Thu, Apr 14, 2022 at 3:18 AM Hongyu Wang via Gcc-patches > > <gcc-patches@gcc.gnu.org> wrote: > > > > > > Hi, > > > > > > From -Os point of view, stv converts scalar register to vector mode > > > which introduces extra reg conversion and increase instruction size. > > > Disabling stv under optimize_size would avoid such code size increment > > > and no need to touch ix86_size_cost that has not been tuned for long > > > time. > > > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu{-m32,}, > > > > > > Ok for master? > > > > > > gcc/ChangeLog: > > > > > > PR target/105034 > > > * config/i386/i386-features.cc (pass_stv::gate()): Block out > > > optimize_size. > > > > > > gcc/testsuite/ChangeLog: > > > > > > PR target/105034 > > > * gcc.target/i386/pr105034.c: New test. > > > --- > > > gcc/config/i386/i386-features.cc | 3 ++- > > > gcc/testsuite/gcc.target/i386/pr105034.c | 23 +++++++++++++++++++++++ > > > 2 files changed, 25 insertions(+), 1 deletion(-) > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr105034.c > > > > > > diff --git a/gcc/config/i386/i386-features.cc b/gcc/config/i386/i386-features.cc > > > index 6fe41c3c24f..f57281e672f 100644 > > > --- a/gcc/config/i386/i386-features.cc > > > +++ b/gcc/config/i386/i386-features.cc > > > @@ -1911,7 +1911,8 @@ public: > > > virtual bool gate (function *) > > > { > > > return ((!timode_p || TARGET_64BIT) > > > - && TARGET_STV && TARGET_SSE2 && optimize > 1); > > > + && TARGET_STV && TARGET_SSE2 && optimize > 1 > > > + && !optimize_size); > > > > optimize_function_for_speed ()? > > > > > } > > > > > > virtual unsigned int execute (function *) > > > diff --git a/gcc/testsuite/gcc.target/i386/pr105034.c b/gcc/testsuite/gcc.target/i386/pr105034.c > > > new file mode 100644 > > > index 00000000000..d997e26e9ed > > > --- /dev/null > > > +++ b/gcc/testsuite/gcc.target/i386/pr105034.c > > > @@ -0,0 +1,23 @@ > > > +/* PR target/105034 */ > > > +/* { dg-do compile } */ > > > +/* { dg-options "-Os -msse4.1" } */ > > > + > > > +#define max(a,b) (((a) > (b))? (a) : (b)) > > > +#define min(a,b) (((a) < (b))? (a) : (b)) > > > + > > > +int foo(int x) > > > +{ > > > + return max(x,0); > > > +} > > > + > > > +int bar(int x) > > > +{ > > > + return min(x,0); > > > +} > > > + > > > +unsigned int baz(unsigned int x) > > > +{ > > > + return min(x,1); > > > +} > > > + > > > +/* { dg-final { scan-assembler-not "xmm" } } */ > > > -- > > > 2.18.1 > > >
> > virtual bool gate (function *) > > please name the parameter ... > > > { > > return ((!timode_p || TARGET_64BIT) > > - && TARGET_STV && TARGET_SSE2 && optimize > 1); > > + && TARGET_STV && TARGET_SSE2 && optimize > 1 > > + && optimize_function_for_speed_p (cfun)); > > ... and use it here instead of referencing 'cfun' Updated. Thanks! gcc/ChangeLog: PR target/105034 * config/i386/i386-features.cc (pass_stv::gate()): Name param to fun and add optimize_function_for_speed_p (fun). gcc/testsuite/ChangeLog: PR target/105034 * gcc.target/i386/pr105034.c: New test. --- gcc/config/i386/i386-features.cc | 5 +++-- gcc/testsuite/gcc.target/i386/pr105034.c | 23 +++++++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gcc.target/i386/pr105034.c diff --git a/gcc/config/i386/i386-features.cc b/gcc/config/i386/i386-features.cc index 6fe41c3c24f..26be2986486 100644 --- a/gcc/config/i386/i386-features.cc +++ b/gcc/config/i386/i386-features.cc @@ -1908,10 +1908,11 @@ public: {} /* opt_pass methods: */ - virtual bool gate (function *) + virtual bool gate (function *fun) { return ((!timode_p || TARGET_64BIT) - && TARGET_STV && TARGET_SSE2 && optimize > 1); + && TARGET_STV && TARGET_SSE2 && optimize > 1 + && optimize_function_for_speed_p (fun)); } virtual unsigned int execute (function *) diff --git a/gcc/testsuite/gcc.target/i386/pr105034.c b/gcc/testsuite/gcc.target/i386/pr105034.c new file mode 100644 index 00000000000..d997e26e9ed --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr105034.c @@ -0,0 +1,23 @@ +/* PR target/105034 */ +/* { dg-do compile } */ +/* { dg-options "-Os -msse4.1" } */ + +#define max(a,b) (((a) > (b))? (a) : (b)) +#define min(a,b) (((a) < (b))? (a) : (b)) + +int foo(int x) +{ + return max(x,0); +} + +int bar(int x) +{ + return min(x,0); +} + +unsigned int baz(unsigned int x) +{ + return min(x,1); +} + +/* { dg-final { scan-assembler-not "xmm" } } */
On Thu, Apr 14, 2022 at 10:31 AM Hongyu Wang <wwwhhhyyy333@gmail.com> wrote: > > > > virtual bool gate (function *) > > > > please name the parameter ... > > > > > { > > > return ((!timode_p || TARGET_64BIT) > > > - && TARGET_STV && TARGET_SSE2 && optimize > 1); > > > + && TARGET_STV && TARGET_SSE2 && optimize > 1 > > > + && optimize_function_for_speed_p (cfun)); > > > > ... and use it here instead of referencing 'cfun' > > Updated. Thanks! As a general comment it would be nicer if the cost metric itself would focus on size costs when optimizing for size and speed costs when optimizing for speed so that individual STV opportunities can be enabled/disabled based on it. At least I see the chance that there will be a case where STV improves code size that will regress if we simply disable it for -Os. Like when I do typedef int v4si __attribute__((vector_size(16))); #define min(a,b) ((a)<(b)?(a):(b)) v4si foo (v4si a, v4si b) { a[0] = min (a[0], b[0]); return a; } there's a xmm to grp move penalty for scalar code that could go away (but oddly enough we're not arranging for the use of pminsd here - seems we're confused about vec_select/vec_merge). Richard. > gcc/ChangeLog: > > PR target/105034 > * config/i386/i386-features.cc (pass_stv::gate()): Name param > to fun and add optimize_function_for_speed_p (fun). > > gcc/testsuite/ChangeLog: > > PR target/105034 > * gcc.target/i386/pr105034.c: New test. > --- > gcc/config/i386/i386-features.cc | 5 +++-- > gcc/testsuite/gcc.target/i386/pr105034.c | 23 +++++++++++++++++++++++ > 2 files changed, 26 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/i386/pr105034.c > > diff --git a/gcc/config/i386/i386-features.cc b/gcc/config/i386/i386-features.cc > index 6fe41c3c24f..26be2986486 100644 > --- a/gcc/config/i386/i386-features.cc > +++ b/gcc/config/i386/i386-features.cc > @@ -1908,10 +1908,11 @@ public: > {} > > /* opt_pass methods: */ > - virtual bool gate (function *) > + virtual bool gate (function *fun) > { > return ((!timode_p || TARGET_64BIT) > - && TARGET_STV && TARGET_SSE2 && optimize > 1); > + && TARGET_STV && TARGET_SSE2 && optimize > 1 > + && optimize_function_for_speed_p (fun)); > } > > virtual unsigned int execute (function *) > diff --git a/gcc/testsuite/gcc.target/i386/pr105034.c > b/gcc/testsuite/gcc.target/i386/pr105034.c > new file mode 100644 > index 00000000000..d997e26e9ed > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr105034.c > @@ -0,0 +1,23 @@ > +/* PR target/105034 */ > +/* { dg-do compile } */ > +/* { dg-options "-Os -msse4.1" } */ > + > +#define max(a,b) (((a) > (b))? (a) : (b)) > +#define min(a,b) (((a) < (b))? (a) : (b)) > + > +int foo(int x) > +{ > + return max(x,0); > +} > + > +int bar(int x) > +{ > + return min(x,0); > +} > + > +unsigned int baz(unsigned int x) > +{ > + return min(x,1); > +} > + > +/* { dg-final { scan-assembler-not "xmm" } } */ > -- > 2.18.1 > > > Richard Biener <richard.guenther@gmail.com> 于2022年4月14日周四 16:06写道: > > > > On Thu, Apr 14, 2022 at 9:55 AM Hongyu Wang <wwwhhhyyy333@gmail.com> wrote: > > > > > > > > > > > optimize_function_for_speed ()? > > > > > > > > > > Yes, updated patch with optimize_function_for_speed_p() > > > > > > gcc/ChangeLog: > > > > > > PR target/105034 > > > * config/i386/i386-features.cc (pass_stv::gate()): Add > > > optimize_function_for_speed_p (). > > > > > > gcc/testsuite/ChangeLog: > > > > > > PR target/105034 > > > * gcc.target/i386/pr105034.c: New test. > > > --- > > > gcc/config/i386/i386-features.cc | 3 ++- > > > gcc/testsuite/gcc.target/i386/pr105034.c | 23 +++++++++++++++++++++++ > > > 2 files changed, 25 insertions(+), 1 deletion(-) > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr105034.c > > > > > > diff --git a/gcc/config/i386/i386-features.cc b/gcc/config/i386/i386-features.cc > > > index 6fe41c3c24f..a49c3aa1525 100644 > > > --- a/gcc/config/i386/i386-features.cc > > > +++ b/gcc/config/i386/i386-features.cc > > > @@ -1911,7 +1911,8 @@ public: > > > virtual bool gate (function *) > > > > please name the parameter ... > > > > > { > > > return ((!timode_p || TARGET_64BIT) > > > - && TARGET_STV && TARGET_SSE2 && optimize > 1); > > > + && TARGET_STV && TARGET_SSE2 && optimize > 1 > > > + && optimize_function_for_speed_p (cfun)); > > > > ... and use it here instead of referencing 'cfun' > > > > Richard. > > > > > } > > > > > > virtual unsigned int execute (function *) > > > diff --git a/gcc/testsuite/gcc.target/i386/pr105034.c > > > b/gcc/testsuite/gcc.target/i386/pr105034.c > > > new file mode 100644 > > > index 00000000000..d997e26e9ed > > > --- /dev/null > > > +++ b/gcc/testsuite/gcc.target/i386/pr105034.c > > > @@ -0,0 +1,23 @@ > > > +/* PR target/105034 */ > > > +/* { dg-do compile } */ > > > +/* { dg-options "-Os -msse4.1" } */ > > > + > > > +#define max(a,b) (((a) > (b))? (a) : (b)) > > > +#define min(a,b) (((a) < (b))? (a) : (b)) > > > + > > > +int foo(int x) > > > +{ > > > + return max(x,0); > > > +} > > > + > > > +int bar(int x) > > > +{ > > > + return min(x,0); > > > +} > > > + > > > +unsigned int baz(unsigned int x) > > > +{ > > > + return min(x,1); > > > +} > > > + > > > +/* { dg-final { scan-assembler-not "xmm" } } */ > > > -- > > > 2.18.1 > > > > > > Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> 于2022年4月14日周四 14:56写道: > > > > > > > > On Thu, Apr 14, 2022 at 3:18 AM Hongyu Wang via Gcc-patches > > > > <gcc-patches@gcc.gnu.org> wrote: > > > > > > > > > > Hi, > > > > > > > > > > From -Os point of view, stv converts scalar register to vector mode > > > > > which introduces extra reg conversion and increase instruction size. > > > > > Disabling stv under optimize_size would avoid such code size increment > > > > > and no need to touch ix86_size_cost that has not been tuned for long > > > > > time. > > > > > > > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu{-m32,}, > > > > > > > > > > Ok for master? > > > > > > > > > > gcc/ChangeLog: > > > > > > > > > > PR target/105034 > > > > > * config/i386/i386-features.cc (pass_stv::gate()): Block out > > > > > optimize_size. > > > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > > > PR target/105034 > > > > > * gcc.target/i386/pr105034.c: New test. > > > > > --- > > > > > gcc/config/i386/i386-features.cc | 3 ++- > > > > > gcc/testsuite/gcc.target/i386/pr105034.c | 23 +++++++++++++++++++++++ > > > > > 2 files changed, 25 insertions(+), 1 deletion(-) > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr105034.c > > > > > > > > > > diff --git a/gcc/config/i386/i386-features.cc b/gcc/config/i386/i386-features.cc > > > > > index 6fe41c3c24f..f57281e672f 100644 > > > > > --- a/gcc/config/i386/i386-features.cc > > > > > +++ b/gcc/config/i386/i386-features.cc > > > > > @@ -1911,7 +1911,8 @@ public: > > > > > virtual bool gate (function *) > > > > > { > > > > > return ((!timode_p || TARGET_64BIT) > > > > > - && TARGET_STV && TARGET_SSE2 && optimize > 1); > > > > > + && TARGET_STV && TARGET_SSE2 && optimize > 1 > > > > > + && !optimize_size); > > > > > > > > optimize_function_for_speed ()? > > > > > > > > > } > > > > > > > > > > virtual unsigned int execute (function *) > > > > > diff --git a/gcc/testsuite/gcc.target/i386/pr105034.c b/gcc/testsuite/gcc.target/i386/pr105034.c > > > > > new file mode 100644 > > > > > index 00000000000..d997e26e9ed > > > > > --- /dev/null > > > > > +++ b/gcc/testsuite/gcc.target/i386/pr105034.c > > > > > @@ -0,0 +1,23 @@ > > > > > +/* PR target/105034 */ > > > > > +/* { dg-do compile } */ > > > > > +/* { dg-options "-Os -msse4.1" } */ > > > > > + > > > > > +#define max(a,b) (((a) > (b))? (a) : (b)) > > > > > +#define min(a,b) (((a) < (b))? (a) : (b)) > > > > > + > > > > > +int foo(int x) > > > > > +{ > > > > > + return max(x,0); > > > > > +} > > > > > + > > > > > +int bar(int x) > > > > > +{ > > > > > + return min(x,0); > > > > > +} > > > > > + > > > > > +unsigned int baz(unsigned int x) > > > > > +{ > > > > > + return min(x,1); > > > > > +} > > > > > + > > > > > +/* { dg-final { scan-assembler-not "xmm" } } */ > > > > > -- > > > > > 2.18.1 > > > > >
> As a general comment it would be nicer if the cost metric itself would focus > on size costs when optimizing for size and speed costs when optimizing for > speed so that individual STV opportunities can be enabled/disabled based > on it. Agreed. I think the cost computation should consider insn number under -Os, also for ABS/MIN/MAX it needs more correct model to describe the actual insn count. Thanks for your review. Richard Biener <richard.guenther@gmail.com> 于2022年4月14日周四 16:56写道: > > On Thu, Apr 14, 2022 at 10:31 AM Hongyu Wang <wwwhhhyyy333@gmail.com> wrote: > > > > > > virtual bool gate (function *) > > > > > > please name the parameter ... > > > > > > > { > > > > return ((!timode_p || TARGET_64BIT) > > > > - && TARGET_STV && TARGET_SSE2 && optimize > 1); > > > > + && TARGET_STV && TARGET_SSE2 && optimize > 1 > > > > + && optimize_function_for_speed_p (cfun)); > > > > > > ... and use it here instead of referencing 'cfun' > > > > Updated. Thanks! > > As a general comment it would be nicer if the cost metric itself would focus > on size costs when optimizing for size and speed costs when optimizing for > speed so that individual STV opportunities can be enabled/disabled based > on it. > > At least I see the chance that there will be a case where STV improves > code size that will regress if we simply disable it for -Os. Like when I do > > typedef int v4si __attribute__((vector_size(16))); > > #define min(a,b) ((a)<(b)?(a):(b)) > > v4si foo (v4si a, v4si b) > { > a[0] = min (a[0], b[0]); > return a; > } > > there's a xmm to grp move penalty for scalar code that could go away > (but oddly enough we're not arranging for the use of pminsd here - seems > we're confused about vec_select/vec_merge). > > Richard. > > > gcc/ChangeLog: > > > > PR target/105034 > > * config/i386/i386-features.cc (pass_stv::gate()): Name param > > to fun and add optimize_function_for_speed_p (fun). > > > > gcc/testsuite/ChangeLog: > > > > PR target/105034 > > * gcc.target/i386/pr105034.c: New test. > > --- > > gcc/config/i386/i386-features.cc | 5 +++-- > > gcc/testsuite/gcc.target/i386/pr105034.c | 23 +++++++++++++++++++++++ > > 2 files changed, 26 insertions(+), 2 deletions(-) > > create mode 100644 gcc/testsuite/gcc.target/i386/pr105034.c > > > > diff --git a/gcc/config/i386/i386-features.cc b/gcc/config/i386/i386-features.cc > > index 6fe41c3c24f..26be2986486 100644 > > --- a/gcc/config/i386/i386-features.cc > > +++ b/gcc/config/i386/i386-features.cc > > @@ -1908,10 +1908,11 @@ public: > > {} > > > > /* opt_pass methods: */ > > - virtual bool gate (function *) > > + virtual bool gate (function *fun) > > { > > return ((!timode_p || TARGET_64BIT) > > - && TARGET_STV && TARGET_SSE2 && optimize > 1); > > + && TARGET_STV && TARGET_SSE2 && optimize > 1 > > + && optimize_function_for_speed_p (fun)); > > } > > > > virtual unsigned int execute (function *) > > diff --git a/gcc/testsuite/gcc.target/i386/pr105034.c > > b/gcc/testsuite/gcc.target/i386/pr105034.c > > new file mode 100644 > > index 00000000000..d997e26e9ed > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/i386/pr105034.c > > @@ -0,0 +1,23 @@ > > +/* PR target/105034 */ > > +/* { dg-do compile } */ > > +/* { dg-options "-Os -msse4.1" } */ > > + > > +#define max(a,b) (((a) > (b))? (a) : (b)) > > +#define min(a,b) (((a) < (b))? (a) : (b)) > > + > > +int foo(int x) > > +{ > > + return max(x,0); > > +} > > + > > +int bar(int x) > > +{ > > + return min(x,0); > > +} > > + > > +unsigned int baz(unsigned int x) > > +{ > > + return min(x,1); > > +} > > + > > +/* { dg-final { scan-assembler-not "xmm" } } */ > > -- > > 2.18.1 > > > > > > Richard Biener <richard.guenther@gmail.com> 于2022年4月14日周四 16:06写道: > > > > > > On Thu, Apr 14, 2022 at 9:55 AM Hongyu Wang <wwwhhhyyy333@gmail.com> wrote: > > > > > > > > > > > > > > optimize_function_for_speed ()? > > > > > > > > > > > > > Yes, updated patch with optimize_function_for_speed_p() > > > > > > > > gcc/ChangeLog: > > > > > > > > PR target/105034 > > > > * config/i386/i386-features.cc (pass_stv::gate()): Add > > > > optimize_function_for_speed_p (). > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > PR target/105034 > > > > * gcc.target/i386/pr105034.c: New test. > > > > --- > > > > gcc/config/i386/i386-features.cc | 3 ++- > > > > gcc/testsuite/gcc.target/i386/pr105034.c | 23 +++++++++++++++++++++++ > > > > 2 files changed, 25 insertions(+), 1 deletion(-) > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr105034.c > > > > > > > > diff --git a/gcc/config/i386/i386-features.cc b/gcc/config/i386/i386-features.cc > > > > index 6fe41c3c24f..a49c3aa1525 100644 > > > > --- a/gcc/config/i386/i386-features.cc > > > > +++ b/gcc/config/i386/i386-features.cc > > > > @@ -1911,7 +1911,8 @@ public: > > > > virtual bool gate (function *) > > > > > > please name the parameter ... > > > > > > > { > > > > return ((!timode_p || TARGET_64BIT) > > > > - && TARGET_STV && TARGET_SSE2 && optimize > 1); > > > > + && TARGET_STV && TARGET_SSE2 && optimize > 1 > > > > + && optimize_function_for_speed_p (cfun)); > > > > > > ... and use it here instead of referencing 'cfun' > > > > > > Richard. > > > > > > > } > > > > > > > > virtual unsigned int execute (function *) > > > > diff --git a/gcc/testsuite/gcc.target/i386/pr105034.c > > > > b/gcc/testsuite/gcc.target/i386/pr105034.c > > > > new file mode 100644 > > > > index 00000000000..d997e26e9ed > > > > --- /dev/null > > > > +++ b/gcc/testsuite/gcc.target/i386/pr105034.c > > > > @@ -0,0 +1,23 @@ > > > > +/* PR target/105034 */ > > > > +/* { dg-do compile } */ > > > > +/* { dg-options "-Os -msse4.1" } */ > > > > + > > > > +#define max(a,b) (((a) > (b))? (a) : (b)) > > > > +#define min(a,b) (((a) < (b))? (a) : (b)) > > > > + > > > > +int foo(int x) > > > > +{ > > > > + return max(x,0); > > > > +} > > > > + > > > > +int bar(int x) > > > > +{ > > > > + return min(x,0); > > > > +} > > > > + > > > > +unsigned int baz(unsigned int x) > > > > +{ > > > > + return min(x,1); > > > > +} > > > > + > > > > +/* { dg-final { scan-assembler-not "xmm" } } */ > > > > -- > > > > 2.18.1 > > > > > > > > Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> 于2022年4月14日周四 14:56写道: > > > > > > > > > > On Thu, Apr 14, 2022 at 3:18 AM Hongyu Wang via Gcc-patches > > > > > <gcc-patches@gcc.gnu.org> wrote: > > > > > > > > > > > > Hi, > > > > > > > > > > > > From -Os point of view, stv converts scalar register to vector mode > > > > > > which introduces extra reg conversion and increase instruction size. > > > > > > Disabling stv under optimize_size would avoid such code size increment > > > > > > and no need to touch ix86_size_cost that has not been tuned for long > > > > > > time. > > > > > > > > > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu{-m32,}, > > > > > > > > > > > > Ok for master? > > > > > > > > > > > > gcc/ChangeLog: > > > > > > > > > > > > PR target/105034 > > > > > > * config/i386/i386-features.cc (pass_stv::gate()): Block out > > > > > > optimize_size. > > > > > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > > > > > PR target/105034 > > > > > > * gcc.target/i386/pr105034.c: New test. > > > > > > --- > > > > > > gcc/config/i386/i386-features.cc | 3 ++- > > > > > > gcc/testsuite/gcc.target/i386/pr105034.c | 23 +++++++++++++++++++++++ > > > > > > 2 files changed, 25 insertions(+), 1 deletion(-) > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr105034.c > > > > > > > > > > > > diff --git a/gcc/config/i386/i386-features.cc b/gcc/config/i386/i386-features.cc > > > > > > index 6fe41c3c24f..f57281e672f 100644 > > > > > > --- a/gcc/config/i386/i386-features.cc > > > > > > +++ b/gcc/config/i386/i386-features.cc > > > > > > @@ -1911,7 +1911,8 @@ public: > > > > > > virtual bool gate (function *) > > > > > > { > > > > > > return ((!timode_p || TARGET_64BIT) > > > > > > - && TARGET_STV && TARGET_SSE2 && optimize > 1); > > > > > > + && TARGET_STV && TARGET_SSE2 && optimize > 1 > > > > > > + && !optimize_size); > > > > > > > > > > optimize_function_for_speed ()? > > > > > > > > > > > } > > > > > > > > > > > > virtual unsigned int execute (function *) > > > > > > diff --git a/gcc/testsuite/gcc.target/i386/pr105034.c b/gcc/testsuite/gcc.target/i386/pr105034.c > > > > > > new file mode 100644 > > > > > > index 00000000000..d997e26e9ed > > > > > > --- /dev/null > > > > > > +++ b/gcc/testsuite/gcc.target/i386/pr105034.c > > > > > > @@ -0,0 +1,23 @@ > > > > > > +/* PR target/105034 */ > > > > > > +/* { dg-do compile } */ > > > > > > +/* { dg-options "-Os -msse4.1" } */ > > > > > > + > > > > > > +#define max(a,b) (((a) > (b))? (a) : (b)) > > > > > > +#define min(a,b) (((a) < (b))? (a) : (b)) > > > > > > + > > > > > > +int foo(int x) > > > > > > +{ > > > > > > + return max(x,0); > > > > > > +} > > > > > > + > > > > > > +int bar(int x) > > > > > > +{ > > > > > > + return min(x,0); > > > > > > +} > > > > > > + > > > > > > +unsigned int baz(unsigned int x) > > > > > > +{ > > > > > > + return min(x,1); > > > > > > +} > > > > > > + > > > > > > +/* { dg-final { scan-assembler-not "xmm" } } */ > > > > > > -- > > > > > > 2.18.1 > > > > > >
diff --git a/gcc/config/i386/i386-features.cc b/gcc/config/i386/i386-features.cc index 6fe41c3c24f..f57281e672f 100644 --- a/gcc/config/i386/i386-features.cc +++ b/gcc/config/i386/i386-features.cc @@ -1911,7 +1911,8 @@ public: virtual bool gate (function *) { return ((!timode_p || TARGET_64BIT) - && TARGET_STV && TARGET_SSE2 && optimize > 1); + && TARGET_STV && TARGET_SSE2 && optimize > 1 + && !optimize_size); } virtual unsigned int execute (function *) diff --git a/gcc/testsuite/gcc.target/i386/pr105034.c b/gcc/testsuite/gcc.target/i386/pr105034.c new file mode 100644 index 00000000000..d997e26e9ed --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr105034.c @@ -0,0 +1,23 @@ +/* PR target/105034 */ +/* { dg-do compile } */ +/* { dg-options "-Os -msse4.1" } */ + +#define max(a,b) (((a) > (b))? (a) : (b)) +#define min(a,b) (((a) < (b))? (a) : (b)) + +int foo(int x) +{ + return max(x,0); +} + +int bar(int x) +{ + return min(x,0); +} + +unsigned int baz(unsigned int x) +{ + return min(x,1); +} + +/* { dg-final { scan-assembler-not "xmm" } } */