From: "Kewen.Lin" <linkw@linux.ibm.com>
To: GCC Patches <gcc-patches@gcc.gnu.org>
Cc: Jakub Jelinek <jakub@redhat.com>,
Segher Boessenkool <segher@kernel.crashing.org>,
AlanM <amodra@gmail.com>,
Richard Sandiford <richard.sandiford@arm.com>,
Peter Bergner <bergner@linux.ibm.com>,
jlaw@ventanamicro.com, David Edelsohn <dje.gcc@gmail.com>,
Richard Biener <richard.guenther@gmail.com>,
"H.J. Lu" <hjl.tools@gmail.com>
Subject: PING^2 [PATCH] Adjust the symbol for SECTION_LINK_ORDER linked_to section [PR99889]
Date: Thu, 10 Nov 2022 16:15:43 +0800 [thread overview]
Message-ID: <15b488a5-1f5e-c24e-be12-f402b0dcdb5e@linux.ibm.com> (raw)
In-Reply-To: <52ca56ad-af0f-598f-4ccf-aed61fce67b4@linux.ibm.com>
Hi,
Gentle ping: https://gcc.gnu.org/pipermail/gcc-patches/2022-August/600190.html
Any comments are highly appreciated.
BR,
Kewen
on 2022/9/28 13:41, Kewen.Lin via Gcc-patches wrote:
> Hi,
>
> Gentle ping: https://gcc.gnu.org/pipermail/gcc-patches/2022-August/600190.html
>
> BR,
> Kewen
>
> on 2022/8/24 16:17, Kewen.Lin via Gcc-patches wrote:
>> Hi,
>>
>> As discussed in PR98125, -fpatchable-function-entry with
>> SECTION_LINK_ORDER support doesn't work well on powerpc64
>> ELFv1 because the filled "Symbol" in
>>
>> .section name,"flags"o,@type,Symbol
>>
>> sits in .opd section instead of in the function_section
>> like .text or named .text*.
>>
>> Since we already generates one label LPFE* which sits in
>> function_section of current_function_decl, this patch is
>> to reuse it as the symbol for the linked_to section. It
>> avoids the above ABI specific issue when using the symbol
>> concluded from current_function_decl.
>>
>> Besides, with this support some previous workarounds for
>> powerpc64 ELFv1 can be reverted.
>>
>> btw, rs6000_print_patchable_function_entry can be dropped
>> but there is another rs6000 patch which needs this rs6000
>> specific hook rs6000_print_patchable_function_entry, not
>> sure which one gets landed first, so just leave it here.
>>
>> Bootstrapped and regtested on below:
>>
>> 1) powerpc64-linux-gnu P8 with default binutils 2.27
>> and latest binutils 2.39.
>> 2) powerpc64le-linux-gnu P9 (default binutils 2.30).
>> 3) powerpc64le-linux-gnu P10 (default binutils 2.30).
>> 4) x86_64-redhat-linux with default binutils 2.30
>> and latest binutils 2.39.
>> 5) aarch64-linux-gnu with default binutils 2.30
>> and latest binutils 2.39.
>>
>> Is it ok for trunk?
>>
>> BR,
>> Kewen
>> -----
>>
>> PR target/99889
>>
>> gcc/ChangeLog:
>>
>> * config/rs6000/rs6000.cc (rs6000_print_patchable_function_entry):
>> Adjust to call function default_print_patchable_function_entry.
>> * targhooks.cc (default_print_patchable_function_entry_1): Remove and
>> move the flags preparation ...
>> (default_print_patchable_function_entry): ... here, adjust to use
>> current_function_funcdef_no for label no.
>> * targhooks.h (default_print_patchable_function_entry_1): Remove.
>> * varasm.cc (default_elf_asm_named_section): Adjust code for
>> __patchable_function_entries section support with LPFE label.
>>
>> gcc/testsuite/ChangeLog:
>>
>> * g++.dg/pr93195a.C: Remove the skip on powerpc*-*-* 64-bit.
>> * gcc.target/aarch64/pr92424-2.c: Adjust LPFE1 with LPFE0.
>> * gcc.target/aarch64/pr92424-3.c: Likewise.
>> * gcc.target/i386/pr93492-2.c: Likewise.
>> * gcc.target/i386/pr93492-3.c: Likewise.
>> * gcc.target/i386/pr93492-4.c: Likewise.
>> * gcc.target/i386/pr93492-5.c: Likewise.
>> ---
>> gcc/config/rs6000/rs6000.cc | 13 +-----
>> gcc/varasm.cc | 15 ++++---
>> gcc/targhooks.cc | 45 +++++++-------------
>> gcc/targhooks.h | 3 --
>> gcc/testsuite/g++.dg/pr93195a.C | 1 -
>> gcc/testsuite/gcc.target/aarch64/pr92424-2.c | 4 +-
>> gcc/testsuite/gcc.target/aarch64/pr92424-3.c | 4 +-
>> gcc/testsuite/gcc.target/i386/pr93492-2.c | 4 +-
>> gcc/testsuite/gcc.target/i386/pr93492-3.c | 4 +-
>> gcc/testsuite/gcc.target/i386/pr93492-4.c | 4 +-
>> gcc/testsuite/gcc.target/i386/pr93492-5.c | 4 +-
>> 11 files changed, 40 insertions(+), 61 deletions(-)
>>
>> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
>> index df491bee2ea..dba28b8e647 100644
>> --- a/gcc/config/rs6000/rs6000.cc
>> +++ b/gcc/config/rs6000/rs6000.cc
>> @@ -14771,18 +14771,9 @@ rs6000_print_patchable_function_entry (FILE *file,
>> unsigned HOST_WIDE_INT patch_area_size,
>> bool record_p)
>> {
>> - unsigned int flags = SECTION_WRITE | SECTION_RELRO;
>> - /* When .opd section is emitted, the function symbol
>> - default_print_patchable_function_entry_1 is emitted into the .opd section
>> - while the patchable area is emitted into the function section.
>> - Don't use SECTION_LINK_ORDER in that case. */
>> - if (!(TARGET_64BIT && DEFAULT_ABI != ABI_ELFv2)
>> - && HAVE_GAS_SECTION_LINK_ORDER)
>> - flags |= SECTION_LINK_ORDER;
>> - default_print_patchable_function_entry_1 (file, patch_area_size, record_p,
>> - flags);
>> + default_print_patchable_function_entry (file, patch_area_size, record_p);
>> }
>> -
>>
>> +
>> enum rtx_code
>> rs6000_reverse_condition (machine_mode mode, enum rtx_code code)
>> {
>> diff --git a/gcc/varasm.cc b/gcc/varasm.cc
>> index 4db8506b106..d4de6e164ee 100644
>> --- a/gcc/varasm.cc
>> +++ b/gcc/varasm.cc
>> @@ -6906,11 +6906,16 @@ default_elf_asm_named_section (const char *name, unsigned int flags,
>> fprintf (asm_out_file, ",%d", flags & SECTION_ENTSIZE);
>> if (flags & SECTION_LINK_ORDER)
>> {
>> - tree id = DECL_ASSEMBLER_NAME (decl);
>> - ultimate_transparent_alias_target (&id);
>> - const char *name = IDENTIFIER_POINTER (id);
>> - name = targetm.strip_name_encoding (name);
>> - fprintf (asm_out_file, ",%s", name);
>> + /* For now, only section "__patchable_function_entries"
>> + adopts flag SECTION_LINK_ORDER, internal label LPFE*
>> + was emitted in default_print_patchable_function_entry,
>> + just place it here for linked_to section. */
>> + gcc_assert (!strcmp (name, "__patchable_function_entries"));
>> + fprintf (asm_out_file, ",");
>> + char buf[256];
>> + ASM_GENERATE_INTERNAL_LABEL (buf, "LPFE",
>> + current_function_funcdef_no);
>> + assemble_name_raw (asm_out_file, buf);
>> }
>> if (HAVE_COMDAT_GROUP && (flags & SECTION_LINKONCE))
>> {
>> diff --git a/gcc/targhooks.cc b/gcc/targhooks.cc
>> index b15ae19bcb6..e80caecc418 100644
>> --- a/gcc/targhooks.cc
>> +++ b/gcc/targhooks.cc
>> @@ -1979,15 +1979,17 @@ default_compare_by_pieces_branch_ratio (machine_mode)
>> return 1;
>> }
>>
>> -/* Helper for default_print_patchable_function_entry and other
>> - print_patchable_function_entry hook implementations. */
>> +/* Write PATCH_AREA_SIZE NOPs into the asm outfile FILE around a function
>> + entry. If RECORD_P is true and the target supports named sections,
>> + the location of the NOPs will be recorded in a special object section
>> + called "__patchable_function_entries". This routine may be called
>> + twice per function to put NOPs before and after the function
>> + entry. */
>>
>> void
>> -default_print_patchable_function_entry_1 (FILE *file,
>> - unsigned HOST_WIDE_INT
>> - patch_area_size,
>> - bool record_p,
>> - unsigned int flags)
>> +default_print_patchable_function_entry (FILE *file,
>> + unsigned HOST_WIDE_INT patch_area_size,
>> + bool record_p)
>> {
>> const char *nop_templ = 0;
>> int code_num;
>> @@ -2001,13 +2003,17 @@ default_print_patchable_function_entry_1 (FILE *file,
>> if (record_p && targetm_common.have_named_sections)
>> {
>> char buf[256];
>> - static int patch_area_number;
>> section *previous_section = in_section;
>> const char *asm_op = integer_asm_op (POINTER_SIZE_UNITS, false);
>>
>> gcc_assert (asm_op != NULL);
>> - patch_area_number++;
>> - ASM_GENERATE_INTERNAL_LABEL (buf, "LPFE", patch_area_number);
>> + /* If SECTION_LINK_ORDER is supported, this internal label will
>> + be filled as the symbol for linked_to section. */
>> + ASM_GENERATE_INTERNAL_LABEL (buf, "LPFE", current_function_funcdef_no);
>> +
>> + unsigned int flags = SECTION_WRITE | SECTION_RELRO;
>> + if (HAVE_GAS_SECTION_LINK_ORDER)
>> + flags |= SECTION_LINK_ORDER;
>>
>> section *sect = get_section ("__patchable_function_entries",
>> flags, current_function_decl);
>> @@ -2029,25 +2035,6 @@ default_print_patchable_function_entry_1 (FILE *file,
>> output_asm_insn (nop_templ, NULL);
>> }
>>
>> -/* Write PATCH_AREA_SIZE NOPs into the asm outfile FILE around a function
>> - entry. If RECORD_P is true and the target supports named sections,
>> - the location of the NOPs will be recorded in a special object section
>> - called "__patchable_function_entries". This routine may be called
>> - twice per function to put NOPs before and after the function
>> - entry. */
>> -
>> -void
>> -default_print_patchable_function_entry (FILE *file,
>> - unsigned HOST_WIDE_INT patch_area_size,
>> - bool record_p)
>> -{
>> - unsigned int flags = SECTION_WRITE | SECTION_RELRO;
>> - if (HAVE_GAS_SECTION_LINK_ORDER)
>> - flags |= SECTION_LINK_ORDER;
>> - default_print_patchable_function_entry_1 (file, patch_area_size, record_p,
>> - flags);
>> -}
>> -
>> bool
>> default_profile_before_prologue (void)
>> {
>> diff --git a/gcc/targhooks.h b/gcc/targhooks.h
>> index ecce55ebe79..5c1216fad0b 100644
>> --- a/gcc/targhooks.h
>> +++ b/gcc/targhooks.h
>> @@ -229,9 +229,6 @@ extern bool default_use_by_pieces_infrastructure_p (unsigned HOST_WIDE_INT,
>> bool);
>> extern int default_compare_by_pieces_branch_ratio (machine_mode);
>>
>> -extern void default_print_patchable_function_entry_1 (FILE *,
>> - unsigned HOST_WIDE_INT,
>> - bool, unsigned int);
>> extern void default_print_patchable_function_entry (FILE *,
>> unsigned HOST_WIDE_INT,
>> bool);
>> diff --git a/gcc/testsuite/g++.dg/pr93195a.C b/gcc/testsuite/g++.dg/pr93195a.C
>> index b14f1b3e341..26d265da74e 100644
>> --- a/gcc/testsuite/g++.dg/pr93195a.C
>> +++ b/gcc/testsuite/g++.dg/pr93195a.C
>> @@ -1,5 +1,4 @@
>> /* { dg-do link { target { ! { nvptx*-*-* visium-*-* } } } } */
>> -/* { dg-skip-if "not supported" { { powerpc*-*-* } && lp64 } } */
>> // { dg-require-effective-target o_flag_in_section }
>> /* { dg-options "-O0 -fpatchable-function-entry=1" } */
>> /* { dg-additional-options "-fno-pie" { target sparc*-*-* } } */
>> diff --git a/gcc/testsuite/gcc.target/aarch64/pr92424-2.c b/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
>> index 0e75657a153..12465213aef 100644
>> --- a/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
>> +++ b/gcc/testsuite/gcc.target/aarch64/pr92424-2.c
>> @@ -1,7 +1,7 @@
>> /* { dg-do "compile" } */
>> /* { dg-options "-O1" } */
>>
>> -/* Test the placement of the .LPFE1 label. */
>> +/* Test the placement of the .LPFE0 label. */
>>
>> void
>> __attribute__ ((target("branch-protection=bti"),
>> @@ -9,4 +9,4 @@ __attribute__ ((target("branch-protection=bti"),
>> f10_bti ()
>> {
>> }
>> -/* { dg-final { scan-assembler "f10_bti:\n\thint\t34 // bti c\n.*\.LPFE1:\n\tnop\n.*\tret\n" } } */
>> +/* { dg-final { scan-assembler "f10_bti:\n\thint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\tret\n" } } */
>> diff --git a/gcc/testsuite/gcc.target/aarch64/pr92424-3.c b/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
>> index 0a1f74d4096..2c6a73789f0 100644
>> --- a/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
>> +++ b/gcc/testsuite/gcc.target/aarch64/pr92424-3.c
>> @@ -1,7 +1,7 @@
>> /* { dg-do "compile" } */
>> /* { dg-options "-O1" } */
>>
>> -/* Test the placement of the .LPFE1 label. */
>> +/* Test the placement of the .LPFE0 label. */
>>
>> void
>> __attribute__ ((target("branch-protection=bti+pac-ret+leaf"),
>> @@ -9,4 +9,4 @@ __attribute__ ((target("branch-protection=bti+pac-ret+leaf"),
>> f10_pac ()
>> {
>> }
>> -/* { dg-final { scan-assembler "f10_pac:\n\thint\t34 // bti c\n.*\.LPFE1:\n\tnop\n.*\thint\t25 // paciasp\n.*\thint\t29 // autiasp\n.*\tret\n" } } */
>> +/* { dg-final { scan-assembler "f10_pac:\n\thint\t34 // bti c\n.*\.LPFE0:\n\tnop\n.*\thint\t25 // paciasp\n.*\thint\t29 // autiasp\n.*\tret\n" } } */
>> diff --git a/gcc/testsuite/gcc.target/i386/pr93492-2.c b/gcc/testsuite/gcc.target/i386/pr93492-2.c
>> index 3d67095fd10..ede8c2077b7 100644
>> --- a/gcc/testsuite/gcc.target/i386/pr93492-2.c
>> +++ b/gcc/testsuite/gcc.target/i386/pr93492-2.c
>> @@ -1,7 +1,7 @@
>> /* { dg-do "compile" { target *-*-linux* } } */
>> /* { dg-options "-O1 -fcf-protection -mmanual-endbr -fasynchronous-unwind-tables" } */
>>
>> -/* Test the placement of the .LPFE1 label. */
>> +/* Test the placement of the .LPFE0 label. */
>>
>> void
>> __attribute__ ((cf_check,patchable_function_entry (1, 0)))
>> @@ -9,4 +9,4 @@ f10_endbr (void)
>> {
>> }
>>
>> -/* { dg-final { scan-assembler "\t\.cfi_startproc\n\tendbr(32|64)\n.*\.LPFE1:\n\tnop\n\tret\n" } } */
>> +/* { dg-final { scan-assembler "\t\.cfi_startproc\n\tendbr(32|64)\n.*\.LPFE0:\n\tnop\n\tret\n" } } */
>> diff --git a/gcc/testsuite/gcc.target/i386/pr93492-3.c b/gcc/testsuite/gcc.target/i386/pr93492-3.c
>> index a625c927f4f..b68da30bd36 100644
>> --- a/gcc/testsuite/gcc.target/i386/pr93492-3.c
>> +++ b/gcc/testsuite/gcc.target/i386/pr93492-3.c
>> @@ -2,7 +2,7 @@
>> /* { dg-require-effective-target mfentry } */
>> /* { dg-options "-O1 -fcf-protection -mmanual-endbr -mfentry -pg -fasynchronous-unwind-tables" } */
>>
>> -/* Test the placement of the .LPFE1 label. */
>> +/* Test the placement of the .LPFE0 label. */
>>
>> void
>> __attribute__ ((cf_check,patchable_function_entry (1, 0)))
>> @@ -10,4 +10,4 @@ f10_endbr (void)
>> {
>> }
>>
>> -/* { dg-final { scan-assembler "\t\.cfi_startproc\n\tendbr(32|64)\n.*\.LPFE1:\n\tnop\n1:\tcall\t\[^\n\]*__fentry__\[^\n\]*\n\tret\n" } } */
>> +/* { dg-final { scan-assembler "\t\.cfi_startproc\n\tendbr(32|64)\n.*\.LPFE0:\n\tnop\n1:\tcall\t\[^\n\]*__fentry__\[^\n\]*\n\tret\n" } } */
>> diff --git a/gcc/testsuite/gcc.target/i386/pr93492-4.c b/gcc/testsuite/gcc.target/i386/pr93492-4.c
>> index 8f205c345b8..c73034a4624 100644
>> --- a/gcc/testsuite/gcc.target/i386/pr93492-4.c
>> +++ b/gcc/testsuite/gcc.target/i386/pr93492-4.c
>> @@ -1,11 +1,11 @@
>> /* { dg-do "compile" { target *-*-linux* } } */
>> /* { dg-options "-O1 -fpatchable-function-entry=1 -fasynchronous-unwind-tables" } */
>>
>> -/* Test the placement of the .LPFE1 label. */
>> +/* Test the placement of the .LPFE0 label. */
>>
>> void
>> foo (void)
>> {
>> }
>>
>> -/* { dg-final { scan-assembler "\t\.cfi_startproc\n.*\.LPFE1:\n\tnop\n\tret\n" } } */
>> +/* { dg-final { scan-assembler "\t\.cfi_startproc\n.*\.LPFE0:\n\tnop\n\tret\n" } } */
>> diff --git a/gcc/testsuite/gcc.target/i386/pr93492-5.c b/gcc/testsuite/gcc.target/i386/pr93492-5.c
>> index 1ca5ba1fac1..ee9849ae852 100644
>> --- a/gcc/testsuite/gcc.target/i386/pr93492-5.c
>> +++ b/gcc/testsuite/gcc.target/i386/pr93492-5.c
>> @@ -2,11 +2,11 @@
>> /* { dg-options "-O1 -fpatchable-function-entry=1 -mfentry -pg -fasynchronous-unwind-tables" } */
>> /* { dg-additional-options "-fno-PIE" { target ia32 } } */
>>
>> -/* Test the placement of the .LPFE1 label. */
>> +/* Test the placement of the .LPFE0 label. */
>>
>> void
>> foo (void)
>> {
>> }
>>
>> -/* { dg-final { scan-assembler "\t\.cfi_startproc\n.*\.LPFE1:\n\tnop\n1:\tcall\t\[^\n\]*__fentry__\[^\n\]*\n\tret\n" } } */
>> +/* { dg-final { scan-assembler "\t\.cfi_startproc\n.*\.LPFE0:\n\tnop\n1:\tcall\t\[^\n\]*__fentry__\[^\n\]*\n\tret\n" } } */
>> --
>
next prev parent reply other threads:[~2022-11-10 8:15 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-24 8:17 Kewen.Lin
2022-09-28 5:41 ` PING^1 " Kewen.Lin
2022-11-10 8:15 ` Kewen.Lin [this message]
2022-11-21 14:20 ` PING^2 " Richard Sandiford
2022-11-22 2:58 ` Kewen.Lin
2022-11-22 16:08 ` Richard Sandiford
2022-11-25 3:26 ` Kewen.Lin
2023-07-19 6:33 ` Fangrui Song
2023-07-19 8:49 ` Kewen.Lin
2022-09-29 20:31 ` Segher Boessenkool
2022-09-30 12:47 ` Kewen.Lin
2022-09-30 17:47 ` Segher Boessenkool
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=15b488a5-1f5e-c24e-be12-f402b0dcdb5e@linux.ibm.com \
--to=linkw@linux.ibm.com \
--cc=amodra@gmail.com \
--cc=bergner@linux.ibm.com \
--cc=dje.gcc@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=hjl.tools@gmail.com \
--cc=jakub@redhat.com \
--cc=jlaw@ventanamicro.com \
--cc=richard.guenther@gmail.com \
--cc=richard.sandiford@arm.com \
--cc=segher@kernel.crashing.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).