Message ID | 20220907070842.63722-1-guojiufu@linux.ibm.com |
---|---|
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 51CCC385C32E for <patchwork@sourceware.org>; Wed, 7 Sep 2022 07:09:19 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 51CCC385C32E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1662534559; bh=b10iCegoTTuy7ME3OcCwHTC7kqeAMZp5iBLGO0MSu50=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=DbfUfJpEbmOxvcdT4CoKEhKAvet83LBE7a9ITMJEJ8DYQZWpI5EbBEAB/luZL9FwB MTPLQQ+03dljsNoKQlZrqltvK1NIzKVreu54Pr4JXvtRE3blzT2hVN36ZV0NQ1jV6h C69IqxAWO6QaJrDHpi4Gm8O9y5a34Cnsa1EkqdBk= 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 4442B3858D28; Wed, 7 Sep 2022 07:08:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4442B3858D28 Received: from pps.filterd (m0098420.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 2875kX0x020609; Wed, 7 Sep 2022 07:08:48 GMT Received: from pps.reinject (localhost [127.0.0.1]) by mx0b-001b2d01.pphosted.com (PPS) with ESMTPS id 3jenej2bs9-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 07 Sep 2022 07:08:48 +0000 Received: from m0098420.ppops.net (m0098420.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 2876bRN9009472; Wed, 7 Sep 2022 07:08:48 GMT Received: from ppma03ams.nl.ibm.com (62.31.33a9.ip4.static.sl-reverse.com [169.51.49.98]) by mx0b-001b2d01.pphosted.com (PPS) with ESMTPS id 3jenej2brk-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 07 Sep 2022 07:08:48 +0000 Received: from pps.filterd (ppma03ams.nl.ibm.com [127.0.0.1]) by ppma03ams.nl.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 28776NVW016753; Wed, 7 Sep 2022 07:08:46 GMT Received: from b06cxnps3074.portsmouth.uk.ibm.com (d06relay09.portsmouth.uk.ibm.com [9.149.109.194]) by ppma03ams.nl.ibm.com with ESMTP id 3jbxj8vs1w-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 07 Sep 2022 07:08:46 +0000 Received: from d06av25.portsmouth.uk.ibm.com (d06av25.portsmouth.uk.ibm.com [9.149.105.61]) by b06cxnps3074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 28778id944040494 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 7 Sep 2022 07:08:44 GMT Received: from d06av25.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 0452B11C050; Wed, 7 Sep 2022 07:08:44 +0000 (GMT) Received: from d06av25.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 3AF8811C04A; Wed, 7 Sep 2022 07:08:43 +0000 (GMT) Received: from pike.rch.stglabs.ibm.com (unknown [9.5.12.127]) by d06av25.portsmouth.uk.ibm.com (Postfix) with ESMTP; Wed, 7 Sep 2022 07:08:43 +0000 (GMT) To: gcc-patches@gcc.gnu.org Subject: [PATCH V3] rs6000: cannot_force_const_mem for HIGH code rtx[PR106460] Date: Wed, 7 Sep 2022 15:08:42 +0800 Message-Id: <20220907070842.63722-1-guojiufu@linux.ibm.com> X-Mailer: git-send-email 2.17.1 X-TM-AS-GCONF: 00 X-Proofpoint-GUID: ajG_ln9ToVgF-JB_oixYDdE8n5OxYgnm X-Proofpoint-ORIG-GUID: hePYipqtxOE6LlBOSoRMcFYhuqh6nvLi X-Proofpoint-UnRewURL: 0 URL was un-rewritten MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.528,FMLib:17.11.122.1 definitions=2022-09-07_04,2022-09-06_02,2022-06-22_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 suspectscore=0 adultscore=0 phishscore=0 impostorscore=0 mlxlogscore=999 priorityscore=1501 spamscore=0 malwarescore=0 lowpriorityscore=0 bulkscore=0 clxscore=1015 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2207270000 definitions=main-2209070029 X-Spam-Status: No, score=-12.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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: Jiufu Guo via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Jiufu Guo <guojiufu@linux.ibm.com> Cc: dje.gcc@gmail.com, segher@kernel.crashing.org, linkw@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 |
[V3] rs6000: cannot_force_const_mem for HIGH code rtx[PR106460]
|
|
Commit Message
Jiufu Guo
Sept. 7, 2022, 7:08 a.m. UTC
Hi, As the issue in PR106460, a rtx 'high:DI (symbol_ref:DI ("var_48")' is tried to store into constant pool and ICE occur. But actually, this rtx represents partial address and can not be put into a .rodata section. This patch updates rs6000_cannot_force_const_mem to return true for rtx(s) with HIGH code, because these rtx(s) indicate part of address and are not ok for constant pool. Below are some examples: (high:DI (const:DI (plus:DI (symbol_ref:DI ("xx") (const_int 12 [0xc]))))) (high:DI (symbol_ref:DI ("var_1")..))) This patch updated the previous patch, and drafted an test case which ICE without the patch, and assoicated with one PR. https://gcc.gnu.org/pipermail/gcc-patches/2022-July/597712.html This patch also updated the message for previous patch V2. I would ask help to review this patch one more time. Bootstrap and regtest pass on ppc64 and ppc64le. Is this ok for trunk. BR, Jeff(Jiufu) PR target/106460 gcc/ChangeLog: * config/rs6000/rs6000.cc (rs6000_cannot_force_const_mem): Return true for HIGH code rtx. gcc/testsuite/ChangeLog: * gcc.target/powerpc/pr106460.c: New test. --- gcc/config/rs6000/rs6000.cc | 7 +++++-- gcc/testsuite/gcc.target/powerpc/pr106460.c | 11 +++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gcc.target/powerpc/pr106460.c
Comments
Hi, Gentle ping: https://gcc.gnu.org/pipermail/gcc-patches/2022-September/601190.html BR, Jeff (Jiufu) Jiufu Guo <guojiufu@linux.ibm.com> writes: > Hi, > > As the issue in PR106460, a rtx 'high:DI (symbol_ref:DI ("var_48")' is tried > to store into constant pool and ICE occur. But actually, this rtx represents > partial address and can not be put into a .rodata section. > > This patch updates rs6000_cannot_force_const_mem to return true for rtx(s) with > HIGH code, because these rtx(s) indicate part of address and are not ok for > constant pool. > > Below are some examples: > (high:DI (const:DI (plus:DI (symbol_ref:DI ("xx") (const_int 12 [0xc]))))) > (high:DI (symbol_ref:DI ("var_1")..))) > > This patch updated the previous patch, and drafted an test case which ICE > without the patch, and assoicated with one PR. > https://gcc.gnu.org/pipermail/gcc-patches/2022-July/597712.html > This patch also updated the message for previous patch V2. > > I would ask help to review this patch one more time. > > Bootstrap and regtest pass on ppc64 and ppc64le. > Is this ok for trunk. > > BR, > Jeff(Jiufu) > > PR target/106460 > > gcc/ChangeLog: > > * config/rs6000/rs6000.cc (rs6000_cannot_force_const_mem): Return true > for HIGH code rtx. > > gcc/testsuite/ChangeLog: > > * gcc.target/powerpc/pr106460.c: New test. > --- > gcc/config/rs6000/rs6000.cc | 7 +++++-- > gcc/testsuite/gcc.target/powerpc/pr106460.c | 11 +++++++++++ > 2 files changed, 16 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/powerpc/pr106460.c > > diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc > index 2f3146e56f8..04e3a393147 100644 > --- a/gcc/config/rs6000/rs6000.cc > +++ b/gcc/config/rs6000/rs6000.cc > @@ -9643,8 +9643,11 @@ rs6000_init_stack_protect_guard (void) > static bool > rs6000_cannot_force_const_mem (machine_mode mode ATTRIBUTE_UNUSED, rtx x) > { > - if (GET_CODE (x) == HIGH > - && GET_CODE (XEXP (x, 0)) == UNSPEC) > + /* If GET_CODE (x) is HIGH, the 'X' represets the high part of a symbol_ref. > + It indicates partial address, which can not be put into a constant pool. > + e.g. (high:DI (unspec:DI [(symbol_ref/u:DI ("*.LC0")..) > + (high:DI (symbol_ref:DI ("var")..)). */ > + if (GET_CODE (x) == HIGH) > return true; > > /* A TLS symbol in the TOC cannot contain a sum. */ > diff --git a/gcc/testsuite/gcc.target/powerpc/pr106460.c b/gcc/testsuite/gcc.target/powerpc/pr106460.c > new file mode 100644 > index 00000000000..dfaffcb6e28 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/pr106460.c > @@ -0,0 +1,11 @@ > +/* { dg-options "-O1 -mdejagnu-cpu=power10" } */ > + > +/* (high:DI (symbol_ref:DI ("var_48")..))) should not cause ICE. */ > +extern short var_48; > +void > +foo (double *r) > +{ > + if (var_48) > + *r = 1234.5678; > +} > +
Hi Jeff, on 2022/9/7 15:08, Jiufu Guo via Gcc-patches wrote: > Hi, > > As the issue in PR106460, a rtx 'high:DI (symbol_ref:DI ("var_48")' is tried > to store into constant pool and ICE occur. But actually, this rtx represents > partial address and can not be put into a .rodata section. > > This patch updates rs6000_cannot_force_const_mem to return true for rtx(s) with > HIGH code, because these rtx(s) indicate part of address and are not ok for > constant pool. > > Below are some examples: > (high:DI (const:DI (plus:DI (symbol_ref:DI ("xx") (const_int 12 [0xc]))))) > (high:DI (symbol_ref:DI ("var_1")..))) > > This patch updated the previous patch, and drafted an test case which ICE > without the patch, and assoicated with one PR. > https://gcc.gnu.org/pipermail/gcc-patches/2022-July/597712.html > This patch also updated the message for previous patch V2. > > I would ask help to review this patch one more time. > > Bootstrap and regtest pass on ppc64 and ppc64le. > Is this ok for trunk. > > BR, > Jeff(Jiufu) > > PR target/106460 > > gcc/ChangeLog: > > * config/rs6000/rs6000.cc (rs6000_cannot_force_const_mem): Return true > for HIGH code rtx. > > gcc/testsuite/ChangeLog: > > * gcc.target/powerpc/pr106460.c: New test. > --- > gcc/config/rs6000/rs6000.cc | 7 +++++-- > gcc/testsuite/gcc.target/powerpc/pr106460.c | 11 +++++++++++ > 2 files changed, 16 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/powerpc/pr106460.c > > diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc > index 2f3146e56f8..04e3a393147 100644 > --- a/gcc/config/rs6000/rs6000.cc > +++ b/gcc/config/rs6000/rs6000.cc > @@ -9643,8 +9643,11 @@ rs6000_init_stack_protect_guard (void) > static bool > rs6000_cannot_force_const_mem (machine_mode mode ATTRIBUTE_UNUSED, rtx x) > { > - if (GET_CODE (x) == HIGH > - && GET_CODE (XEXP (x, 0)) == UNSPEC) > + /* If GET_CODE (x) is HIGH, the 'X' represets the high part of a symbol_ref. > + It indicates partial address, which can not be put into a constant pool. > + e.g. (high:DI (unspec:DI [(symbol_ref/u:DI ("*.LC0")..) > + (high:DI (symbol_ref:DI ("var")..)). */ Nit: Maybe it's good to align these two "(high:DI ... ? > + if (GET_CODE (x) == HIGH) > return true; > > /* A TLS symbol in the TOC cannot contain a sum. */ > diff --git a/gcc/testsuite/gcc.target/powerpc/pr106460.c b/gcc/testsuite/gcc.target/powerpc/pr106460.c > new file mode 100644 > index 00000000000..dfaffcb6e28 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/pr106460.c > @@ -0,0 +1,11 @@ Need a power10_ok effective target here. /* { dg-require-effective-target power10_ok } */ > +/* { dg-options "-O1 -mdejagnu-cpu=power10" } */ Nit: As Segher's review on one of my patches, O2 is preferred against O1 if it still works for this issue. The point is to avoid some related optimization (routines or passes) to be disabled at O1 one day and this becomes ineffective. BR, Kewen
Hi, "Kewen.Lin" <linkw@linux.ibm.com> writes: > Hi Jeff, > > on 2022/9/7 15:08, Jiufu Guo via Gcc-patches wrote: >> Hi, >> >> As the issue in PR106460, a rtx 'high:DI (symbol_ref:DI ("var_48")' is tried >> to store into constant pool and ICE occur. But actually, this rtx represents >> partial address and can not be put into a .rodata section. >> >> This patch updates rs6000_cannot_force_const_mem to return true for rtx(s) with >> HIGH code, because these rtx(s) indicate part of address and are not ok for >> constant pool. >> >> Below are some examples: >> (high:DI (const:DI (plus:DI (symbol_ref:DI ("xx") (const_int 12 [0xc]))))) >> (high:DI (symbol_ref:DI ("var_1")..))) >> >> This patch updated the previous patch, and drafted an test case which ICE >> without the patch, and assoicated with one PR. >> https://gcc.gnu.org/pipermail/gcc-patches/2022-July/597712.html >> This patch also updated the message for previous patch V2. >> >> I would ask help to review this patch one more time. >> >> Bootstrap and regtest pass on ppc64 and ppc64le. >> Is this ok for trunk. >> >> BR, >> Jeff(Jiufu) >> >> PR target/106460 >> >> gcc/ChangeLog: >> >> * config/rs6000/rs6000.cc (rs6000_cannot_force_const_mem): Return true >> for HIGH code rtx. >> >> gcc/testsuite/ChangeLog: >> >> * gcc.target/powerpc/pr106460.c: New test. >> --- >> gcc/config/rs6000/rs6000.cc | 7 +++++-- >> gcc/testsuite/gcc.target/powerpc/pr106460.c | 11 +++++++++++ >> 2 files changed, 16 insertions(+), 2 deletions(-) >> create mode 100644 gcc/testsuite/gcc.target/powerpc/pr106460.c >> >> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc >> index 2f3146e56f8..04e3a393147 100644 >> --- a/gcc/config/rs6000/rs6000.cc >> +++ b/gcc/config/rs6000/rs6000.cc >> @@ -9643,8 +9643,11 @@ rs6000_init_stack_protect_guard (void) >> static bool >> rs6000_cannot_force_const_mem (machine_mode mode ATTRIBUTE_UNUSED, rtx x) >> { >> - if (GET_CODE (x) == HIGH >> - && GET_CODE (XEXP (x, 0)) == UNSPEC) >> + /* If GET_CODE (x) is HIGH, the 'X' represets the high part of a symbol_ref. >> + It indicates partial address, which can not be put into a constant pool. >> + e.g. (high:DI (unspec:DI [(symbol_ref/u:DI ("*.LC0")..) >> + (high:DI (symbol_ref:DI ("var")..)). */ > > Nit: Maybe it's good to align these two "(high:DI ... ? OK, thanks! > >> + if (GET_CODE (x) == HIGH) >> return true; >> >> /* A TLS symbol in the TOC cannot contain a sum. */ >> diff --git a/gcc/testsuite/gcc.target/powerpc/pr106460.c b/gcc/testsuite/gcc.target/powerpc/pr106460.c >> new file mode 100644 >> index 00000000000..dfaffcb6e28 >> --- /dev/null >> +++ b/gcc/testsuite/gcc.target/powerpc/pr106460.c >> @@ -0,0 +1,11 @@ > > Need a power10_ok effective target here. > > /* { dg-require-effective-target power10_ok } */ OK, will add this. > >> +/* { dg-options "-O1 -mdejagnu-cpu=power10" } */ > > Nit: As Segher's review on one of my patches, O2 is preferred against O1 if it > still works for this issue. The point is to avoid some related optimization > (routines or passes) to be disabled at O1 one day and this becomes ineffective. > Yeap. While, for this case, the ICE is not reproduciable with -O2. So, -O1 is used here. BR, Jeff (Jiufu) > BR, > Kewen
diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc index 2f3146e56f8..04e3a393147 100644 --- a/gcc/config/rs6000/rs6000.cc +++ b/gcc/config/rs6000/rs6000.cc @@ -9643,8 +9643,11 @@ rs6000_init_stack_protect_guard (void) static bool rs6000_cannot_force_const_mem (machine_mode mode ATTRIBUTE_UNUSED, rtx x) { - if (GET_CODE (x) == HIGH - && GET_CODE (XEXP (x, 0)) == UNSPEC) + /* If GET_CODE (x) is HIGH, the 'X' represets the high part of a symbol_ref. + It indicates partial address, which can not be put into a constant pool. + e.g. (high:DI (unspec:DI [(symbol_ref/u:DI ("*.LC0")..) + (high:DI (symbol_ref:DI ("var")..)). */ + if (GET_CODE (x) == HIGH) return true; /* A TLS symbol in the TOC cannot contain a sum. */ diff --git a/gcc/testsuite/gcc.target/powerpc/pr106460.c b/gcc/testsuite/gcc.target/powerpc/pr106460.c new file mode 100644 index 00000000000..dfaffcb6e28 --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/pr106460.c @@ -0,0 +1,11 @@ +/* { dg-options "-O1 -mdejagnu-cpu=power10" } */ + +/* (high:DI (symbol_ref:DI ("var_48")..))) should not cause ICE. */ +extern short var_48; +void +foo (double *r) +{ + if (var_48) + *r = 1234.5678; +} +