Message ID | YgM0jwK4niPUtP3m@toto.the-meissners.org |
---|---|
State | New |
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 986723858D28 for <patchwork@sourceware.org>; Wed, 9 Feb 2022 03:27:47 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 986723858D28 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1644377267; bh=7dRIlkj/ydd0PEd2VE/IXB9f8vv8DlPXL8XkkqqPvKY=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=jT8UoSLm2y7A7LlKdOOD3FFFqBSrbeGmwcole6930sisWrYSilLmUYpeRjLYbsJsQ ORo7U4NsSYjwhu8urn42sjsY24btRrwucpUDzp2H2oFnwtdG3U47+x7ZWfYF+0At/+ l+1M9bBGwD1maARIs/ny+oWN60MuMw3FesVM8ha0= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mx0a-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by sourceware.org (Postfix) with ESMTPS id 53A143858D28 for <gcc-patches@gcc.gnu.org>; Wed, 9 Feb 2022 03:27:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 53A143858D28 Received: from pps.filterd (m0098414.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.1.2/8.16.1.2) with SMTP id 2191f09Y024401; Wed, 9 Feb 2022 03:27:16 GMT Received: from pps.reinject (localhost [127.0.0.1]) by mx0b-001b2d01.pphosted.com with ESMTP id 3e3fm5cyu9-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 09 Feb 2022 03:27:16 +0000 Received: from m0098414.ppops.net (m0098414.ppops.net [127.0.0.1]) by pps.reinject (8.16.0.43/8.16.0.43) with SMTP id 2193MDn4003262; Wed, 9 Feb 2022 03:27:15 GMT Received: from ppma04dal.us.ibm.com (7a.29.35a9.ip4.static.sl-reverse.com [169.53.41.122]) by mx0b-001b2d01.pphosted.com with ESMTP id 3e3fm5cyu5-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 09 Feb 2022 03:27:15 +0000 Received: from pps.filterd (ppma04dal.us.ibm.com [127.0.0.1]) by ppma04dal.us.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 2193OIbW005575; Wed, 9 Feb 2022 03:27:15 GMT Received: from b03cxnp08026.gho.boulder.ibm.com (b03cxnp08026.gho.boulder.ibm.com [9.17.130.18]) by ppma04dal.us.ibm.com with ESMTP id 3e1gvbsckw-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 09 Feb 2022 03:27:15 +0000 Received: from b03ledav002.gho.boulder.ibm.com (b03ledav002.gho.boulder.ibm.com [9.17.130.233]) by b03cxnp08026.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 2193RD4U34668824 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 9 Feb 2022 03:27:13 GMT Received: from b03ledav002.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 80B1A136051; Wed, 9 Feb 2022 03:27:13 +0000 (GMT) Received: from b03ledav002.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id F1201136053; Wed, 9 Feb 2022 03:27:12 +0000 (GMT) Received: from toto.the-meissners.org (unknown [9.77.136.59]) by b03ledav002.gho.boulder.ibm.com (Postfix) with ESMTPS; Wed, 9 Feb 2022 03:27:12 +0000 (GMT) Date: Tue, 8 Feb 2022 22:27:11 -0500 To: gcc-patches@gcc.gnu.org, Michael Meissner <meissner@linux.ibm.com>, Segher Boessenkool <segher@kernel.crashing.org>, David Edelsohn <dje.gcc@gmail.com>, Bill Schmidt <wschmidt@linux.ibm.com>, Peter Bergner <bergner@linux.ibm.com>, Will Schmidt <will_schmidt@vnet.ibm.com> Subject: [PATCH] PR target/102059 Fix inline of target specific functions Message-ID: <YgM0jwK4niPUtP3m@toto.the-meissners.org> Mail-Followup-To: Michael Meissner <meissner@linux.ibm.com>, gcc-patches@gcc.gnu.org, Segher Boessenkool <segher@kernel.crashing.org>, David Edelsohn <dje.gcc@gmail.com>, Bill Schmidt <wschmidt@linux.ibm.com>, Peter Bergner <bergner@linux.ibm.com>, Will Schmidt <will_schmidt@vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-TM-AS-GCONF: 00 X-Proofpoint-ORIG-GUID: 3uT2cjEeNP2kjawGYO8UyYhFQvbO5WAT X-Proofpoint-GUID: T2b9ZpJwS2bpXBOBkpYjnT57hfGWUSB3 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.816,Hydra:6.0.425,FMLib:17.11.62.513 definitions=2022-02-09_01,2022-02-07_02,2021-12-02_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 mlxscore=0 malwarescore=0 spamscore=0 bulkscore=0 impostorscore=0 priorityscore=1501 phishscore=0 mlxlogscore=999 clxscore=1015 adultscore=0 lowpriorityscore=0 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2201110000 definitions=main-2202090023 X-Spam-Status: No, score=-10.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, GIT_PATCH_0, KAM_MANYTO, KAM_SHORT, RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, 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: Michael Meissner via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Michael Meissner <meissner@linux.ibm.com> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
PR target/102059 Fix inline of target specific functions
|
|
Commit Message
Michael Meissner
Feb. 9, 2022, 3:27 a.m. UTC
Reset -mpower8-fusion for power9 inlining power8 functions, PR 102059. This patch is an attempt to make a much simpler patch to fix PR target/102059 than the previous patch. It just fixes the issue that if a function is specifically declared as a power8 function, you can't inline in functions that are specified with power9 or power10 options. The issue is -mpower8-fusion is cleared when you use -mcpu=power9 or -mcpu=power10. When I wrote the code for controlling which function can inline other functions, -mpower8-fusion was set for -mcpu=power9 (-mcpu=power10 was not an option at that time). This patch fixes this particular problem. Perhaps -mpower8-fusion should go away in the GCC 13 time frame. This patch just goes in and resets the fusion bit for testing inlines. I have built a bootstrapped little endian compiler on power9 and the tests had no regressions. I have built a bootstrapped big endian compiler on power8 and I tested both 32-bit and 64-bit builds, and there were no regressions. Can I install this into the trunk and back port it into GCC 11 after a burn-in period? 2022-02-08 Michael Meissner <meissner@linux.ibm.com> gcc/ PR target/102059 * config/rs6000/rs6000.cc (rs6000_can_inline_p): Don't test for power8-fusion if the caller was power9 or power10. gcc/testsuite/ PR target/102059 * gcc.target/powerpc/pr102059-4.c: New test. --- gcc/config/rs6000/rs6000.cc | 8 ++++++++ gcc/testsuite/gcc.target/powerpc/pr102059-4.c | 20 +++++++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 gcc/testsuite/gcc.target/powerpc/pr102059-4.c
Comments
Hi Michael, on 2022/2/9 上午11:27, Michael Meissner via Gcc-patches wrote: > Reset -mpower8-fusion for power9 inlining power8 functions, PR 102059. > > This patch is an attempt to make a much simpler patch to fix PR target/102059 > than the previous patch. > > It just fixes the issue that if a function is specifically declared as a power8 > function, you can't inline in functions that are specified with power9 or > power10 options. > > The issue is -mpower8-fusion is cleared when you use -mcpu=power9 or > -mcpu=power10. When I wrote the code for controlling which function can inline > other functions, -mpower8-fusion was set for -mcpu=power9 (-mcpu=power10 was > not an option at that time). This patch fixes this particular problem. > > Perhaps -mpower8-fusion should go away in the GCC 13 time frame. This patch > just goes in and resets the fusion bit for testing inlines. > > I have built a bootstrapped little endian compiler on power9 and the tests had > no regressions. > > I have built a bootstrapped big endian compiler on power8 and I tested both > 32-bit and 64-bit builds, and there were no regressions. > > Can I install this into the trunk and back port it into GCC 11 after a burn-in > period? > Thanks for the patch! I guess we also need this for GCC 10 as: $cat htm.c __attribute__((always_inline)) int foo(int *b) { *b += 10; return *b; } #pragma GCC target "cpu=power10,htm" int bar(int* a){ *a = foo(a); return 0; } /opt/at14.0/bin/gcc -flto -S htm.c -mcpu=power8 htm.c:1:36: warning: ‘always_inline’ function might not be inlinable [-Wattributes] 1 | __attribute__((always_inline)) int foo(int *b) { | ^~~ htm.c: In function ‘bar’: htm.c:1:36: error: inlining failed in call to ‘always_inline’ ‘foo’: target specific option mismatch htm.c:8:8: note: called from here 8 | *a = foo(a); | ^~~~~~ Besides, as I noted in the PR, with this fix we can safely remove the option "-mno-power8-fusion" in gcc/testsuite/gcc.dg/lto/pr102059-1_0.c, which has the coverage for lto (though I didn't test ;-)). BR, Kewen > 2022-02-08 Michael Meissner <meissner@linux.ibm.com> > > gcc/ > > PR target/102059 > * config/rs6000/rs6000.cc (rs6000_can_inline_p): Don't test for > power8-fusion if the caller was power9 or power10. > > gcc/testsuite/ > PR target/102059 > * gcc.target/powerpc/pr102059-4.c: New test. > --- > gcc/config/rs6000/rs6000.cc | 8 ++++++++ > gcc/testsuite/gcc.target/powerpc/pr102059-4.c | 20 +++++++++++++++++++ > 2 files changed, 28 insertions(+) > create mode 100644 gcc/testsuite/gcc.target/powerpc/pr102059-4.c > > diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc > index eaba9a2d698..e2d94421ae9 100644 > --- a/gcc/config/rs6000/rs6000.cc > +++ b/gcc/config/rs6000/rs6000.cc > @@ -25278,6 +25278,14 @@ rs6000_can_inline_p (tree caller, tree callee) > callee_isa &= ~OPTION_MASK_HTM; > explicit_isa &= ~OPTION_MASK_HTM; > } > + > + /* Power9 and power10 do not set power8-fusion. If the callee was > + explicitly compiled for power8, and the caller was power9 or > + power10, ignore the power8-fusion bits if it was set by > + default. */ > + if ((caller_isa & OPTION_MASK_P8_FUSION) == 0 > + && (explicit_isa & OPTION_MASK_P8_FUSION) == 0) > + callee_isa &= ~OPTION_MASK_P8_FUSION; > } > > /* The callee's options must be a subset of the caller's options, i.e. > diff --git a/gcc/testsuite/gcc.target/powerpc/pr102059-4.c b/gcc/testsuite/gcc.target/powerpc/pr102059-4.c > new file mode 100644 > index 00000000000..627c6f820c7 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/pr102059-4.c > @@ -0,0 +1,20 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -mdejagnu-cpu=power10 -Wno-attributes" } */ > + > +/* Verify that power10 can explicity include functions compiled for power8. > + The issue is -mcpu=power8 enables -mpower8-fusion, but -mcpu=power9 or > + -mcpu=power10 do not set power8-fusion by default. */ > + > +static inline int __attribute__ ((always_inline,target("cpu=power8"))) > +foo (int *b) > +{ > + *b += 10; > + return *b; > +} > + > +int > +bar (int *a) > +{ > + *a = foo (a); > + return 0; > +} >
On Wed, Feb 09, 2022 at 04:56:13PM +0800, Kewen.Lin wrote: > Hi Michael, > > on 2022/2/9 上午11:27, Michael Meissner via Gcc-patches wrote: > > Reset -mpower8-fusion for power9 inlining power8 functions, PR 102059. > > > > This patch is an attempt to make a much simpler patch to fix PR target/102059 > > than the previous patch. > > > > It just fixes the issue that if a function is specifically declared as a power8 > > function, you can't inline in functions that are specified with power9 or > > power10 options. > > > > The issue is -mpower8-fusion is cleared when you use -mcpu=power9 or > > -mcpu=power10. When I wrote the code for controlling which function can inline > > other functions, -mpower8-fusion was set for -mcpu=power9 (-mcpu=power10 was > > not an option at that time). This patch fixes this particular problem. > > > > Perhaps -mpower8-fusion should go away in the GCC 13 time frame. This patch > > just goes in and resets the fusion bit for testing inlines. > > > > I have built a bootstrapped little endian compiler on power9 and the tests had > > no regressions. > > > > I have built a bootstrapped big endian compiler on power8 and I tested both > > 32-bit and 64-bit builds, and there were no regressions. > > > > Can I install this into the trunk and back port it into GCC 11 after a burn-in > > period? > > > > Thanks for the patch! I guess we also need this for GCC 10 as: Gcc 10 backport is doable once it is installed in the trunk. Since a lot of my work is on IEEE 128-bit or Power10, I don't always think of GCC 10 backports, but in this case, the patch is rather simple. Thanks for the reminder.
Ping patch for PR target/102059 to ignore implicit -mpower8-fusion that prevents a function targeting power9 or power10 from inlining a function that declared it needed power8 via attribute/pragma target. While we likely should revist this in GCC 13, this patch is fairly minimal in that it fixes the specific problem Eigen ran into. It will need to be back ported to GCC 11 and 10 as well. | Date: Tue, 8 Feb 2022 22:27:11 -0500 | From: Michael Meissner <meissner@linux.ibm.com> | Subject: [PATCH] PR target/102059 Fix inline of target specific functions | Message-ID: <YgM0jwK4niPUtP3m@toto.the-meissners.org>
On Fri, Feb 11, 2022 at 12:53:07PM -0500, Michael Meissner wrote: > Ping patch for PR target/102059 to ignore implicit -mpower8-fusion that > prevents a function targeting power9 or power10 from inlining a function that > declared it needed power8 via attribute/pragma target. Can we just disable any effect from this flag, instead? It should just be implied by -mcpu=power8, and be impossible to be enabled otherwise (or disabled!) Segher
On Tue, Mar 08, 2022 at 11:28:03AM -0600, Segher Boessenkool wrote: > On Fri, Feb 11, 2022 at 12:53:07PM -0500, Michael Meissner wrote: > > Ping patch for PR target/102059 to ignore implicit -mpower8-fusion that > > prevents a function targeting power9 or power10 from inlining a function that > > declared it needed power8 via attribute/pragma target. > > Can we just disable any effect from this flag, instead? It should just > be implied by -mcpu=power8, and be impossible to be enabled otherwise > (or disabled!) Yes, I can do that. We should also do the same solution for power10 fusion. What I propose is to set a regular variable with the results of the -mpower8-fusion option. This option would be true by default. Then change TARGET_POWER8_FUSION to be a macro that tests whether the current tuning CPU (not CPU we are compiling code for) to check if we are tuning for a power8. I would likely then remove any TARGET_POWER8 in places that test for TARGET_POWER8_FUSION. And similarly for TARGET_POWER10_FUSION. Note, I haven't looked at Pat's latest changes for power10 fusion. Is this acceptable?
diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc index eaba9a2d698..e2d94421ae9 100644 --- a/gcc/config/rs6000/rs6000.cc +++ b/gcc/config/rs6000/rs6000.cc @@ -25278,6 +25278,14 @@ rs6000_can_inline_p (tree caller, tree callee) callee_isa &= ~OPTION_MASK_HTM; explicit_isa &= ~OPTION_MASK_HTM; } + + /* Power9 and power10 do not set power8-fusion. If the callee was + explicitly compiled for power8, and the caller was power9 or + power10, ignore the power8-fusion bits if it was set by + default. */ + if ((caller_isa & OPTION_MASK_P8_FUSION) == 0 + && (explicit_isa & OPTION_MASK_P8_FUSION) == 0) + callee_isa &= ~OPTION_MASK_P8_FUSION; } /* The callee's options must be a subset of the caller's options, i.e. diff --git a/gcc/testsuite/gcc.target/powerpc/pr102059-4.c b/gcc/testsuite/gcc.target/powerpc/pr102059-4.c new file mode 100644 index 00000000000..627c6f820c7 --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/pr102059-4.c @@ -0,0 +1,20 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -mdejagnu-cpu=power10 -Wno-attributes" } */ + +/* Verify that power10 can explicity include functions compiled for power8. + The issue is -mcpu=power8 enables -mpower8-fusion, but -mcpu=power9 or + -mcpu=power10 do not set power8-fusion by default. */ + +static inline int __attribute__ ((always_inline,target("cpu=power8"))) +foo (int *b) +{ + *b += 10; + return *b; +} + +int +bar (int *a) +{ + *a = foo (a); + return 0; +}