From: "Kewen.Lin" <linkw@linux.ibm.com>
To: GCC Patches <gcc-patches@gcc.gnu.org>
Cc: "H.J. Lu" <hjl.tools@gmail.com>, Jakub Jelinek <jakub@redhat.com>,
Richard Biener <richard.guenther@gmail.com>,
Segher Boessenkool <segher@kernel.crashing.org>,
AlanM <amodra@gmail.com>, Peter Bergner <bergner@linux.ibm.com>,
Richard Sandiford <richard.sandiford@arm.com>,
Jeff Law <law@redhat.com>, David Edelsohn <dje.gcc@gmail.com>
Subject: [PATCH] Adjust the symbol for SECTION_LINK_ORDER linked_to section [PR99889]
Date: Wed, 24 Aug 2022 16:17:07 +0800 [thread overview]
Message-ID: <0558633c-b553-5ef1-aa6f-c76fcf297454@linux.ibm.com> (raw)
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 reply other threads:[~2022-08-24 8:17 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-24 8:17 Kewen.Lin [this message]
2022-09-28 5:41 ` PING^1 " Kewen.Lin
2022-11-10 8:15 ` PING^2 " Kewen.Lin
2022-11-21 14:20 ` 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=0558633c-b553-5ef1-aa6f-c76fcf297454@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=law@redhat.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).