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