Message ID | 7672daad-b86e-df92-bf9a-f3f4e489d5e3@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 822C8393A418 for <patchwork@sourceware.org>; Wed, 12 Jan 2022 12:22:58 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 822C8393A418 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1641990178; bh=pmbBUOOOH7vsWS8eip7ei+NkFESQzuhD7KDTIOu8Tiw=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=XIASLi6cQ6MQLkkxBp+8XmCCyKwd+taB2IcsOcb7We/ZsPKtVRHrHIrc6U0JW80kH XX2EHEMjsQ7W9Kw/jd+2sTHgkg1B8wISWY/04rNfyx684xvJ/ZpQPIMdPgvSolRRjC kg0XQOoEUvmMtAjIjCHeHg2kI9Cws2VT9Q+T12SM= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mx0b-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by sourceware.org (Postfix) with ESMTPS id C9B153858416 for <gcc-patches@gcc.gnu.org>; Wed, 12 Jan 2022 12:22:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C9B153858416 Received: from pps.filterd (m0098421.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.1.2/8.16.1.2) with SMTP id 20CB8ZAj029601; Wed, 12 Jan 2022 12:22:28 GMT Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com with ESMTP id 3dhuugx2f6-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 12 Jan 2022 12:22:28 +0000 Received: from m0098421.ppops.net (m0098421.ppops.net [127.0.0.1]) by pps.reinject (8.16.0.43/8.16.0.43) with SMTP id 20CCJcpd022228; Wed, 12 Jan 2022 12:22:27 GMT Received: from ppma06ams.nl.ibm.com (66.31.33a9.ip4.static.sl-reverse.com [169.51.49.102]) by mx0a-001b2d01.pphosted.com with ESMTP id 3dhuugx2e8-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 12 Jan 2022 12:22:27 +0000 Received: from pps.filterd (ppma06ams.nl.ibm.com [127.0.0.1]) by ppma06ams.nl.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 20CCC2Sq025975; Wed, 12 Jan 2022 12:22:25 GMT Received: from b06cxnps4075.portsmouth.uk.ibm.com (d06relay12.portsmouth.uk.ibm.com [9.149.109.197]) by ppma06ams.nl.ibm.com with ESMTP id 3df1vjbgv1-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 12 Jan 2022 12:22:25 +0000 Received: from d06av24.portsmouth.uk.ibm.com (d06av24.portsmouth.uk.ibm.com [9.149.105.60]) by b06cxnps4075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 20CCMNjP17367400 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 12 Jan 2022 12:22:23 GMT Received: from d06av24.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 3F85842066; Wed, 12 Jan 2022 12:22:23 +0000 (GMT) Received: from d06av24.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id BE57A4205C; Wed, 12 Jan 2022 12:22:21 +0000 (GMT) Received: from [9.197.228.140] (unknown [9.197.228.140]) by d06av24.portsmouth.uk.ibm.com (Postfix) with ESMTP; Wed, 12 Jan 2022 12:22:21 +0000 (GMT) Message-ID: <7672daad-b86e-df92-bf9a-f3f4e489d5e3@linux.ibm.com> Date: Wed, 12 Jan 2022 20:22:19 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.4.1 Content-Language: en-US To: gcc-patches <gcc-patches@gcc.gnu.org> Subject: [PATCH, rs6000] Enable absolute jump table by default Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-ORIG-GUID: 4PPBCsBLkMIkGRSwRiiDrJl3N7fZX7p4 X-Proofpoint-GUID: BFhOGoaIo8SwbcSq0lQi1T4fVhUjvM0d X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.790,Hydra:6.0.425,FMLib:17.11.62.513 definitions=2022-01-12_04,2022-01-11_01,2021-12-02_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 phishscore=0 adultscore=0 suspectscore=0 clxscore=1015 bulkscore=0 spamscore=0 priorityscore=1501 mlxscore=0 mlxlogscore=863 malwarescore=0 lowpriorityscore=0 impostorscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2110150000 definitions=main-2201120079 X-Spam-Status: No, score=-12.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP 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: HAO CHEN GUI via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: HAO CHEN GUI <guihaoc@linux.ibm.com> Cc: Bill Schmidt <wschmidt@linux.ibm.com>, David <dje.gcc@gmail.com>, Segher Boessenkool <segher@kernel.crashing.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 |
[rs6000] Enable absolute jump table by default
|
|
Commit Message
HAO CHEN GUI
Jan. 12, 2022, 12:22 p.m. UTC
Hi, This patch enables absolute jump table by default on rs6000. The relative jump tables are used when it's explicit set by "rs6000_relative_jumptables", or jump tables are placed in text section but global relocation is required. Bootstrapped and tested on powerpc64-linux BE and LE with no regressions. Is this okay for trunk? Any recommendations? Thanks a lot. ChangeLog 2022-01-12 Haochen Gui <guihaoc@linux.ibm.com> gcc/ * config/rs6000/linux64.h (JUMP_TABLES_IN_TEXT_SECTION): Define. * config/rs6000/rs6000.c (rs6000_gen_pic_addr_diff_vec): Return true when relative jump table is explicit required or jump tables have to be put in text section but global relocation is also required. * config/rs6000/rs6000.opt (rs6000_relative_jumptables): Disable. patch.diff
Comments
On Wed, Jan 12, 2022 at 7:22 AM HAO CHEN GUI <guihaoc@linux.ibm.com> wrote: > > Hi, > This patch enables absolute jump table by default on rs6000. The relative jump tables are used when > it's explicit set by "rs6000_relative_jumptables", > or jump tables are placed in text section but global relocation is required. > > Bootstrapped and tested on powerpc64-linux BE and LE with no regressions. Is this okay for trunk? > Any recommendations? Thanks a lot. > > ChangeLog > 2022-01-12 Haochen Gui <guihaoc@linux.ibm.com> > > gcc/ > * config/rs6000/linux64.h (JUMP_TABLES_IN_TEXT_SECTION): Define. > * config/rs6000/rs6000.c (rs6000_gen_pic_addr_diff_vec): Return > true when relative jump table is explicit required or jump tables have > to be put in text section but global relocation is also required. > * config/rs6000/rs6000.opt (rs6000_relative_jumptables): Disable. > > patch.diff > diff --git a/gcc/config/rs6000/linux64.h b/gcc/config/rs6000/linux64.h > index d617f346f81..2e257c60f8c 100644 > --- a/gcc/config/rs6000/linux64.h > +++ b/gcc/config/rs6000/linux64.h > @@ -239,7 +239,7 @@ extern int dot_symbols; > > /* Indicate that jump tables go in the text section. */ > #undef JUMP_TABLES_IN_TEXT_SECTION > -#define JUMP_TABLES_IN_TEXT_SECTION TARGET_64BIT > +#define JUMP_TABLES_IN_TEXT_SECTION 0 > > /* The linux ppc64 ABI isn't explicit on whether aggregates smaller > than a doubleword should be padded upward or downward. You could > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c > index 319182e94d9..9fba893a27a 100644 > --- a/gcc/config/rs6000/rs6000.c > +++ b/gcc/config/rs6000/rs6000.c > @@ -28465,7 +28465,9 @@ rs6000_emit_xxspltidp_v2df (rtx dst, long value) > static bool > rs6000_gen_pic_addr_diff_vec (void) > { > - return rs6000_relative_jumptables; > + return rs6000_relative_jumptables > + || (JUMP_TABLES_IN_TEXT_SECTION > + && targetm.asm_out.reloc_rw_mask () == 3); > } This seems like contorted logic and overriding the rs6000_relative_jumptables option change. The later part of the patch overrides rs6000_relative_jumptables for all rs6000 configurations, and then changes this one use of rs6000_relative_jumptables to add more logic to revert to the old meaning for some targets. What about all of the other uses of rs6000_relative_jumptables in the target? What about rs6000.md? I highly doubt that this patch is correct. Why not override rs6000_relative_jumptables for PPC64 Linux instead of changing its value globally and then trying to fix it up? Thanks, David > > void > diff --git a/gcc/config/rs6000/rs6000.opt b/gcc/config/rs6000/rs6000.opt > index c2a77182a9e..75e3fa86829 100644 > --- a/gcc/config/rs6000/rs6000.opt > +++ b/gcc/config/rs6000/rs6000.opt > @@ -630,7 +630,7 @@ Target Mask(MMA) Var(rs6000_isa_flags) > Generate (do not generate) MMA instructions. > > mrelative-jumptables > -Target Undocumented Var(rs6000_relative_jumptables) Init(1) Save > +Target Undocumented Var(rs6000_relative_jumptables) Init(0) Save > > mrop-protect > Target Var(rs6000_rop_protect) Init(0)
Hi David, On 12/1/2022 下午 10:44, David Edelsohn wrote: > On Wed, Jan 12, 2022 at 7:22 AM HAO CHEN GUI <guihaoc@linux.ibm.com> wrote: >> >> Hi, >> This patch enables absolute jump table by default on rs6000. The relative jump tables are used when >> it's explicit set by "rs6000_relative_jumptables", >> or jump tables are placed in text section but global relocation is required. >> >> Bootstrapped and tested on powerpc64-linux BE and LE with no regressions. Is this okay for trunk? >> Any recommendations? Thanks a lot. >> >> ChangeLog >> 2022-01-12 Haochen Gui <guihaoc@linux.ibm.com> >> >> gcc/ >> * config/rs6000/linux64.h (JUMP_TABLES_IN_TEXT_SECTION): Define. >> * config/rs6000/rs6000.c (rs6000_gen_pic_addr_diff_vec): Return >> true when relative jump table is explicit required or jump tables have >> to be put in text section but global relocation is also required. >> * config/rs6000/rs6000.opt (rs6000_relative_jumptables): Disable. >> >> patch.diff >> diff --git a/gcc/config/rs6000/linux64.h b/gcc/config/rs6000/linux64.h >> index d617f346f81..2e257c60f8c 100644 >> --- a/gcc/config/rs6000/linux64.h >> +++ b/gcc/config/rs6000/linux64.h >> @@ -239,7 +239,7 @@ extern int dot_symbols; >> >> /* Indicate that jump tables go in the text section. */ >> #undef JUMP_TABLES_IN_TEXT_SECTION >> -#define JUMP_TABLES_IN_TEXT_SECTION TARGET_64BIT >> +#define JUMP_TABLES_IN_TEXT_SECTION 0 >> >> /* The linux ppc64 ABI isn't explicit on whether aggregates smaller >> than a doubleword should be padded upward or downward. You could >> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c >> index 319182e94d9..9fba893a27a 100644 >> --- a/gcc/config/rs6000/rs6000.c >> +++ b/gcc/config/rs6000/rs6000.c >> @@ -28465,7 +28465,9 @@ rs6000_emit_xxspltidp_v2df (rtx dst, long value) >> static bool >> rs6000_gen_pic_addr_diff_vec (void) >> { >> - return rs6000_relative_jumptables; >> + return rs6000_relative_jumptables >> + || (JUMP_TABLES_IN_TEXT_SECTION >> + && targetm.asm_out.reloc_rw_mask () == 3); >> } > > This seems like contorted logic and overriding the > rs6000_relative_jumptables option change. The later part of the patch > overrides rs6000_relative_jumptables for all rs6000 configurations, > and then changes this one use of rs6000_relative_jumptables to add > more logic to revert to the old meaning for some targets. > > What about all of the other uses of rs6000_relative_jumptables in the > target? What about rs6000.md? > > I highly doubt that this patch is correct. > > Why not override rs6000_relative_jumptables for PPC64 Linux instead of > changing its value globally and then trying to fix it up? > > Thanks, David Thanks for your comments. In this patch, I tried to enable absolute jump table on all rs6000 targets. For PPC64 Linux, it supports RELRO section (e.g. .data.rel.ro.local) as "JUMP_TABLES_IN_TEXT_SECTION" is set to 0. So, absolute jump tables could be placed in RELRO section whatever global relocation is required or not. The absolute jump table can't be placed in text section when global relocation is required as text section is not writable. So for other rs6000 targets, absolute jump table can't be used if the target doesn't support RELRO and global relocation is required also. Looking forward to your advice. Thanks again. > >> >> void >> diff --git a/gcc/config/rs6000/rs6000.opt b/gcc/config/rs6000/rs6000.opt >> index c2a77182a9e..75e3fa86829 100644 >> --- a/gcc/config/rs6000/rs6000.opt >> +++ b/gcc/config/rs6000/rs6000.opt >> @@ -630,7 +630,7 @@ Target Mask(MMA) Var(rs6000_isa_flags) >> Generate (do not generate) MMA instructions. >> >> mrelative-jumptables >> -Target Undocumented Var(rs6000_relative_jumptables) Init(1) Save >> +Target Undocumented Var(rs6000_relative_jumptables) Init(0) Save >> >> mrop-protect >> Target Var(rs6000_rop_protect) Init(0)
On Wed, Jan 12, 2022 at 8:40 PM HAO CHEN GUI <guihaoc@linux.ibm.com> wrote: > > Hi David, > > On 12/1/2022 下午 10:44, David Edelsohn wrote: > > On Wed, Jan 12, 2022 at 7:22 AM HAO CHEN GUI <guihaoc@linux.ibm.com> wrote: > >> > >> Hi, > >> This patch enables absolute jump table by default on rs6000. The relative jump tables are used when > >> it's explicit set by "rs6000_relative_jumptables", > >> or jump tables are placed in text section but global relocation is required. > >> > >> Bootstrapped and tested on powerpc64-linux BE and LE with no regressions. Is this okay for trunk? > >> Any recommendations? Thanks a lot. > >> > >> ChangeLog > >> 2022-01-12 Haochen Gui <guihaoc@linux.ibm.com> > >> > >> gcc/ > >> * config/rs6000/linux64.h (JUMP_TABLES_IN_TEXT_SECTION): Define. > >> * config/rs6000/rs6000.c (rs6000_gen_pic_addr_diff_vec): Return > >> true when relative jump table is explicit required or jump tables have > >> to be put in text section but global relocation is also required. > >> * config/rs6000/rs6000.opt (rs6000_relative_jumptables): Disable. > >> > >> patch.diff > >> diff --git a/gcc/config/rs6000/linux64.h b/gcc/config/rs6000/linux64.h > >> index d617f346f81..2e257c60f8c 100644 > >> --- a/gcc/config/rs6000/linux64.h > >> +++ b/gcc/config/rs6000/linux64.h > >> @@ -239,7 +239,7 @@ extern int dot_symbols; > >> > >> /* Indicate that jump tables go in the text section. */ > >> #undef JUMP_TABLES_IN_TEXT_SECTION > >> -#define JUMP_TABLES_IN_TEXT_SECTION TARGET_64BIT > >> +#define JUMP_TABLES_IN_TEXT_SECTION 0 > >> > >> /* The linux ppc64 ABI isn't explicit on whether aggregates smaller > >> than a doubleword should be padded upward or downward. You could > >> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c > >> index 319182e94d9..9fba893a27a 100644 > >> --- a/gcc/config/rs6000/rs6000.c > >> +++ b/gcc/config/rs6000/rs6000.c > >> @@ -28465,7 +28465,9 @@ rs6000_emit_xxspltidp_v2df (rtx dst, long value) > >> static bool > >> rs6000_gen_pic_addr_diff_vec (void) > >> { > >> - return rs6000_relative_jumptables; > >> + return rs6000_relative_jumptables > >> + || (JUMP_TABLES_IN_TEXT_SECTION > >> + && targetm.asm_out.reloc_rw_mask () == 3); > >> } > > > > This seems like contorted logic and overriding the > > rs6000_relative_jumptables option change. The later part of the patch > > overrides rs6000_relative_jumptables for all rs6000 configurations, > > and then changes this one use of rs6000_relative_jumptables to add > > more logic to revert to the old meaning for some targets. > > > > What about all of the other uses of rs6000_relative_jumptables in the > > target? What about rs6000.md? > > > > I highly doubt that this patch is correct. > > > > Why not override rs6000_relative_jumptables for PPC64 Linux instead of > > changing its value globally and then trying to fix it up? > > > > Thanks, David > Thanks for your comments. > > In this patch, I tried to enable absolute jump table on all rs6000 targets. > For PPC64 Linux, it supports RELRO section (e.g. .data.rel.ro.local) as > "JUMP_TABLES_IN_TEXT_SECTION" is set to 0. So, absolute jump tables could be placed > in RELRO section whatever global relocation is required or not. The absolute jump > table can't be placed in text section when global relocation is required as text > section is not writable. So for other rs6000 targets, absolute jump table can't be > used if the target doesn't support RELRO and global relocation is required also. > > Looking forward to your advice. Thanks again. Why not override rs6000_relative_jumptables in rs6000_option_override_internal() for PPC64 Linux subtarget instead of changing the default value and then trying to fix the behavior for other configurations in rs6000_gen_pic_addr_diff_vec()? Or override it in SUBSUBTARGET_OVERRIDE_OPTIONS in linux64.h, or whichever header file(s) is appropriate for the subtarget variants. Your initial patch also changed rs6000_gen_pic_addr_diff_vec but didn't address the use of rs6000_relative_jumptables in the definition of CASE_VECTOR_MODE in rs6000.h. That cannot have been correct. At least without the change to the default value of rs6000_relative_jumptables you don't need to add kludges to all of the places where that variable is used for other subtarget variants of the rs6000 target. Thanks, David
diff --git a/gcc/config/rs6000/linux64.h b/gcc/config/rs6000/linux64.h index d617f346f81..2e257c60f8c 100644 --- a/gcc/config/rs6000/linux64.h +++ b/gcc/config/rs6000/linux64.h @@ -239,7 +239,7 @@ extern int dot_symbols; /* Indicate that jump tables go in the text section. */ #undef JUMP_TABLES_IN_TEXT_SECTION -#define JUMP_TABLES_IN_TEXT_SECTION TARGET_64BIT +#define JUMP_TABLES_IN_TEXT_SECTION 0 /* The linux ppc64 ABI isn't explicit on whether aggregates smaller than a doubleword should be padded upward or downward. You could diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index 319182e94d9..9fba893a27a 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -28465,7 +28465,9 @@ rs6000_emit_xxspltidp_v2df (rtx dst, long value) static bool rs6000_gen_pic_addr_diff_vec (void) { - return rs6000_relative_jumptables; + return rs6000_relative_jumptables + || (JUMP_TABLES_IN_TEXT_SECTION + && targetm.asm_out.reloc_rw_mask () == 3); } void diff --git a/gcc/config/rs6000/rs6000.opt b/gcc/config/rs6000/rs6000.opt index c2a77182a9e..75e3fa86829 100644 --- a/gcc/config/rs6000/rs6000.opt +++ b/gcc/config/rs6000/rs6000.opt @@ -630,7 +630,7 @@ Target Mask(MMA) Var(rs6000_isa_flags) Generate (do not generate) MMA instructions. mrelative-jumptables -Target Undocumented Var(rs6000_relative_jumptables) Init(1) Save +Target Undocumented Var(rs6000_relative_jumptables) Init(0) Save mrop-protect Target Var(rs6000_rop_protect) Init(0)