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"
> } } */
> --
