public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc(refs/users/giulianob/heads/pfe_backport_clean)] Backport rs6000: Avoid -fpatchable-function-entry* regressions on powerpc64 be [PR98125]
@ 2021-10-19 18:40 Giuliano Belinassi
  0 siblings, 0 replies; 4+ messages in thread
From: Giuliano Belinassi @ 2021-10-19 18:40 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:cea6ee90a846468f87f71a592c9761a5136646b7

commit cea6ee90a846468f87f71a592c9761a5136646b7
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Sat Apr 3 10:03:15 2021 +0200

    Backport rs6000: Avoid -fpatchable-function-entry* regressions on powerpc64 be [PR98125]
    
    The SECTION_LINK_ORDER changes broke powerpc64-linux ELFv1.  Seems
    that the assembler/linker relies on the symbol mentioned for the
    "awo" section to be in the same section as the symbols mentioned in
    the relocations in that section (i.e. labels for the patchable area
    in this case).  That is the case for most targets, including powerpc-linux
    32-bit or powerpc64 ELFv2 (that one has -fpatchable-function-entry*
    support broken for other reasons and it doesn't seem to be a regression).
    But it doesn't work on powerpc64-linux ELFv1.
    We emit:
            .section        ".opd","aw"
            .align 3
    _Z3foov:
            .quad   .L._Z3foov,.TOC.@tocbase,0
            .previous
            .type   _Z3foov, @function
    .L._Z3foov:
            .section        __patchable_function_entries,"awo",@progbits,_Z3foov
            .align 3
            .8byte  .LPFE1
            .section        .text._Z3foov,"axG",@progbits,_Z3foov,comdat
    .LPFE1:
            nop
    .LFB0:
            .cfi_startproc
    and because _Z3foov is in the .opd section rather than the function text
    section, it doesn't work.
    
    I'm afraid I don't know what exactly should be done, whether e.g.
    it could use
            .section        __patchable_function_entries,"awo",@progbits,.L._Z3foov
    instead, or whether the linker should be changed to handle it as is, or
    something else.
    
    But because we have a P1 regression that didn't see useful progress over the
    4 months since it has been filed and we don't really have much time, below
    is an attempt to do a targetted reversion of H.J's patch, basically act as
    if HAVE_GAS_SECTION_LINK_ORDER is never true for powerpc64-linux ELFv1,
    but for 32-bit or 64-bit ELFv2 keep working as is.
    This would give us time to resolve it for GCC 12 properly.
    
    2021-04-03  Jakub Jelinek  <jakub@redhat.com>
    
            PR testsuite/98125
            * targhooks.h (default_print_patchable_function_entry_1): Declare.
            * targhooks.c (default_print_patchable_function_entry_1): New function,
            copied from default_print_patchable_function_entry with an added flags
            argument.
            (default_print_patchable_function_entry): Rewritten into a small
            wrapper around default_print_patchable_function_entry_1.
            * config/rs6000/rs6000.c (TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY):
            Redefine.
            (rs6000_print_patchable_function_entry): New function.
    
            * g++.dg/pr93195a.C: Skip on powerpc*-*-* 64-bit.

Diff:
---
 gcc/config/rs6000/rs6000.c      |  8 ++++++--
 gcc/targhooks.c                 | 38 ++++++++++++++++++++++++++------------
 gcc/targhooks.h                 |  3 +++
 gcc/testsuite/g++.dg/pr93195a.C |  1 +
 4 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index af0c7ce1656..ac60d6d39ac 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -1615,6 +1615,10 @@ static const struct attribute_spec rs6000_attribute_table[] =
 #define TARGET_ASM_ASSEMBLE_VISIBILITY rs6000_assemble_visibility
 #endif
 
+#undef TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY
+#define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY \
+  rs6000_print_patchable_function_entry
+
 #undef TARGET_SET_UP_BY_PROLOGUE
 #define TARGET_SET_UP_BY_PROLOGUE rs6000_set_up_by_prologue
 
@@ -27933,8 +27937,8 @@ debug_stack_info (rs6000_stack_t *info)
   fprintf (stderr, "\n");
 }
 
-rtx
-rs6000_return_addr (int count, rtx frame)
+enum rtx_code
+rs6000_reverse_condition (machine_mode mode, enum rtx_code code)
 {
   /* We can't use get_hard_reg_initial_val for LR when count == 0 if LR
      is trashed by the prologue, as it is for PIC on ABI_V4 and Darwin.  */
diff --git a/gcc/targhooks.c b/gcc/targhooks.c
index 8aa610f5cde..958ec4ba6ff 100644
--- a/gcc/targhooks.c
+++ b/gcc/targhooks.c
@@ -1610,17 +1610,15 @@ default_compare_by_pieces_branch_ratio (machine_mode)
   return 1;
 }
 
-/* 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.  */
+/* Helper for default_print_patchable_function_entry and other
+   print_patchable_function_entry hook implementations.  */
 
 void
-default_print_patchable_function_entry (FILE *file,
-					unsigned HOST_WIDE_INT patch_area_size,
-					bool record_p)
+default_print_patchable_function_entry_1 (FILE *file,
+					  unsigned HOST_WIDE_INT
+					  patch_area_size,
+					  bool record_p,
+					  unsigned int flags)
 {
   const char *nop_templ = 0;
   int code_num;
@@ -1642,9 +1640,6 @@ default_print_patchable_function_entry (FILE *file,
       patch_area_number++;
       ASM_GENERATE_INTERNAL_LABEL (buf, "LPFE", patch_area_number);
 
-      unsigned int flags = SECTION_WRITE | SECTION_RELRO;
-      if (HAVE_GAS_SECTION_LINK_ORDER)
-	flags |= SECTION_LINK_ORDER;
       switch_to_section (get_section ("__patchable_function_entries",
 				      flags, current_function_decl));
       assemble_align (POINTER_SIZE);
@@ -1661,6 +1656,25 @@ default_print_patchable_function_entry (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 6206fe20823..7b6d2fb9138 100644
--- a/gcc/targhooks.h
+++ b/gcc/targhooks.h
@@ -203,6 +203,9 @@ 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 26d265da74e..b14f1b3e341 100644
--- a/gcc/testsuite/g++.dg/pr93195a.C
+++ b/gcc/testsuite/g++.dg/pr93195a.C
@@ -1,4 +1,5 @@
 /* { 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*-*-* } } */


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [gcc(refs/users/giulianob/heads/pfe_backport_clean)] Backport rs6000: Avoid -fpatchable-function-entry* regressions on powerpc64 be [PR98125]
@ 2021-10-21 16:46 Giuliano Belinassi
  0 siblings, 0 replies; 4+ messages in thread
From: Giuliano Belinassi @ 2021-10-21 16:46 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:22254ceca4c5dcbabc39b636e5f417a152b0d45d

commit 22254ceca4c5dcbabc39b636e5f417a152b0d45d
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Sat Apr 3 10:03:15 2021 +0200

    Backport rs6000: Avoid -fpatchable-function-entry* regressions on powerpc64 be [PR98125]
    
    The SECTION_LINK_ORDER changes broke powerpc64-linux ELFv1.  Seems
    that the assembler/linker relies on the symbol mentioned for the
    "awo" section to be in the same section as the symbols mentioned in
    the relocations in that section (i.e. labels for the patchable area
    in this case).  That is the case for most targets, including powerpc-linux
    32-bit or powerpc64 ELFv2 (that one has -fpatchable-function-entry*
    support broken for other reasons and it doesn't seem to be a regression).
    But it doesn't work on powerpc64-linux ELFv1.
    We emit:
            .section        ".opd","aw"
            .align 3
    _Z3foov:
            .quad   .L._Z3foov,.TOC.@tocbase,0
            .previous
            .type   _Z3foov, @function
    .L._Z3foov:
            .section        __patchable_function_entries,"awo",@progbits,_Z3foov
            .align 3
            .8byte  .LPFE1
            .section        .text._Z3foov,"axG",@progbits,_Z3foov,comdat
    .LPFE1:
            nop
    .LFB0:
            .cfi_startproc
    and because _Z3foov is in the .opd section rather than the function text
    section, it doesn't work.
    
    I'm afraid I don't know what exactly should be done, whether e.g.
    it could use
            .section        __patchable_function_entries,"awo",@progbits,.L._Z3foov
    instead, or whether the linker should be changed to handle it as is, or
    something else.
    
    But because we have a P1 regression that didn't see useful progress over the
    4 months since it has been filed and we don't really have much time, below
    is an attempt to do a targetted reversion of H.J's patch, basically act as
    if HAVE_GAS_SECTION_LINK_ORDER is never true for powerpc64-linux ELFv1,
    but for 32-bit or 64-bit ELFv2 keep working as is.
    This would give us time to resolve it for GCC 12 properly.
    
    2021-10-21  Giuliano Belinassi  <gbelinassi@suse.de>
    
    Backport from mainline
            2021-04-03  Jakub Jelinek  <jakub@redhat.com>
    
            PR testsuite/98125
            * targhooks.h (default_print_patchable_function_entry_1): Declare.
            * targhooks.c (default_print_patchable_function_entry_1): New function,
            copied from default_print_patchable_function_entry with an added flags
            argument.
            (default_print_patchable_function_entry): Rewritten into a small
            wrapper around default_print_patchable_function_entry_1.
            * config/rs6000/rs6000.c (TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY):
            Redefine.
            (rs6000_print_patchable_function_entry): New function.
    
            * g++.dg/pr93195a.C: Skip on powerpc*-*-* 64-bit.

Diff:
---
 gcc/config/rs6000/rs6000.c      | 29 +++++++++++++++++++++++++++++
 gcc/targhooks.c                 | 38 ++++++++++++++++++++++++++------------
 gcc/targhooks.h                 |  3 +++
 gcc/testsuite/g++.dg/pr93195a.C |  1 +
 4 files changed, 59 insertions(+), 12 deletions(-)

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index af0c7ce1656..d0e3ba29c7d 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -1615,6 +1615,10 @@ static const struct attribute_spec rs6000_attribute_table[] =
 #define TARGET_ASM_ASSEMBLE_VISIBILITY rs6000_assemble_visibility
 #endif
 
+#undef TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY
+#define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY \
+  rs6000_print_patchable_function_entry
+
 #undef TARGET_SET_UP_BY_PROLOGUE
 #define TARGET_SET_UP_BY_PROLOGUE rs6000_set_up_by_prologue
 
@@ -24446,6 +24450,31 @@ rs6000_assemble_visibility (tree decl, int vis)
     default_assemble_visibility (decl, vis);
 }
 #endif
+\f
+/* 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
+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);
+}
+
 \f
 enum rtx_code
 rs6000_reverse_condition (machine_mode mode, enum rtx_code code)
diff --git a/gcc/targhooks.c b/gcc/targhooks.c
index 8aa610f5cde..958ec4ba6ff 100644
--- a/gcc/targhooks.c
+++ b/gcc/targhooks.c
@@ -1610,17 +1610,15 @@ default_compare_by_pieces_branch_ratio (machine_mode)
   return 1;
 }
 
-/* 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.  */
+/* Helper for default_print_patchable_function_entry and other
+   print_patchable_function_entry hook implementations.  */
 
 void
-default_print_patchable_function_entry (FILE *file,
-					unsigned HOST_WIDE_INT patch_area_size,
-					bool record_p)
+default_print_patchable_function_entry_1 (FILE *file,
+					  unsigned HOST_WIDE_INT
+					  patch_area_size,
+					  bool record_p,
+					  unsigned int flags)
 {
   const char *nop_templ = 0;
   int code_num;
@@ -1642,9 +1640,6 @@ default_print_patchable_function_entry (FILE *file,
       patch_area_number++;
       ASM_GENERATE_INTERNAL_LABEL (buf, "LPFE", patch_area_number);
 
-      unsigned int flags = SECTION_WRITE | SECTION_RELRO;
-      if (HAVE_GAS_SECTION_LINK_ORDER)
-	flags |= SECTION_LINK_ORDER;
       switch_to_section (get_section ("__patchable_function_entries",
 				      flags, current_function_decl));
       assemble_align (POINTER_SIZE);
@@ -1661,6 +1656,25 @@ default_print_patchable_function_entry (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 6206fe20823..7b6d2fb9138 100644
--- a/gcc/targhooks.h
+++ b/gcc/targhooks.h
@@ -203,6 +203,9 @@ 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 26d265da74e..b14f1b3e341 100644
--- a/gcc/testsuite/g++.dg/pr93195a.C
+++ b/gcc/testsuite/g++.dg/pr93195a.C
@@ -1,4 +1,5 @@
 /* { 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*-*-* } } */


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [gcc(refs/users/giulianob/heads/pfe_backport_clean)] Backport rs6000: Avoid -fpatchable-function-entry* regressions on powerpc64 be [PR98125]
@ 2021-10-21 14:49 Giuliano Belinassi
  0 siblings, 0 replies; 4+ messages in thread
From: Giuliano Belinassi @ 2021-10-21 14:49 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:ca5d90d929e92de3f0ced9204233f92aed68c0f8

commit ca5d90d929e92de3f0ced9204233f92aed68c0f8
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Sat Apr 3 10:03:15 2021 +0200

    Backport rs6000: Avoid -fpatchable-function-entry* regressions on powerpc64 be [PR98125]
    
    The SECTION_LINK_ORDER changes broke powerpc64-linux ELFv1.  Seems
    that the assembler/linker relies on the symbol mentioned for the
    "awo" section to be in the same section as the symbols mentioned in
    the relocations in that section (i.e. labels for the patchable area
    in this case).  That is the case for most targets, including powerpc-linux
    32-bit or powerpc64 ELFv2 (that one has -fpatchable-function-entry*
    support broken for other reasons and it doesn't seem to be a regression).
    But it doesn't work on powerpc64-linux ELFv1.
    We emit:
            .section        ".opd","aw"
            .align 3
    _Z3foov:
            .quad   .L._Z3foov,.TOC.@tocbase,0
            .previous
            .type   _Z3foov, @function
    .L._Z3foov:
            .section        __patchable_function_entries,"awo",@progbits,_Z3foov
            .align 3
            .8byte  .LPFE1
            .section        .text._Z3foov,"axG",@progbits,_Z3foov,comdat
    .LPFE1:
            nop
    .LFB0:
            .cfi_startproc
    and because _Z3foov is in the .opd section rather than the function text
    section, it doesn't work.
    
    I'm afraid I don't know what exactly should be done, whether e.g.
    it could use
            .section        __patchable_function_entries,"awo",@progbits,.L._Z3foov
    instead, or whether the linker should be changed to handle it as is, or
    something else.
    
    But because we have a P1 regression that didn't see useful progress over the
    4 months since it has been filed and we don't really have much time, below
    is an attempt to do a targetted reversion of H.J's patch, basically act as
    if HAVE_GAS_SECTION_LINK_ORDER is never true for powerpc64-linux ELFv1,
    but for 32-bit or 64-bit ELFv2 keep working as is.
    This would give us time to resolve it for GCC 12 properly.
    
    2021-04-03  Jakub Jelinek  <jakub@redhat.com>
    
            PR testsuite/98125
            * targhooks.h (default_print_patchable_function_entry_1): Declare.
            * targhooks.c (default_print_patchable_function_entry_1): New function,
            copied from default_print_patchable_function_entry with an added flags
            argument.
            (default_print_patchable_function_entry): Rewritten into a small
            wrapper around default_print_patchable_function_entry_1.
            * config/rs6000/rs6000.c (TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY):
            Redefine.
            (rs6000_print_patchable_function_entry): New function.
    
            * g++.dg/pr93195a.C: Skip on powerpc*-*-* 64-bit.

Diff:
---
 gcc/config/rs6000/rs6000.c      |  8 ++++++--
 gcc/targhooks.c                 | 38 ++++++++++++++++++++++++++------------
 gcc/targhooks.h                 |  3 +++
 gcc/testsuite/g++.dg/pr93195a.C |  1 +
 4 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index af0c7ce1656..ac60d6d39ac 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -1615,6 +1615,10 @@ static const struct attribute_spec rs6000_attribute_table[] =
 #define TARGET_ASM_ASSEMBLE_VISIBILITY rs6000_assemble_visibility
 #endif
 
+#undef TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY
+#define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY \
+  rs6000_print_patchable_function_entry
+
 #undef TARGET_SET_UP_BY_PROLOGUE
 #define TARGET_SET_UP_BY_PROLOGUE rs6000_set_up_by_prologue
 
@@ -27933,8 +27937,8 @@ debug_stack_info (rs6000_stack_t *info)
   fprintf (stderr, "\n");
 }
 
-rtx
-rs6000_return_addr (int count, rtx frame)
+enum rtx_code
+rs6000_reverse_condition (machine_mode mode, enum rtx_code code)
 {
   /* We can't use get_hard_reg_initial_val for LR when count == 0 if LR
      is trashed by the prologue, as it is for PIC on ABI_V4 and Darwin.  */
diff --git a/gcc/targhooks.c b/gcc/targhooks.c
index 8aa610f5cde..958ec4ba6ff 100644
--- a/gcc/targhooks.c
+++ b/gcc/targhooks.c
@@ -1610,17 +1610,15 @@ default_compare_by_pieces_branch_ratio (machine_mode)
   return 1;
 }
 
-/* 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.  */
+/* Helper for default_print_patchable_function_entry and other
+   print_patchable_function_entry hook implementations.  */
 
 void
-default_print_patchable_function_entry (FILE *file,
-					unsigned HOST_WIDE_INT patch_area_size,
-					bool record_p)
+default_print_patchable_function_entry_1 (FILE *file,
+					  unsigned HOST_WIDE_INT
+					  patch_area_size,
+					  bool record_p,
+					  unsigned int flags)
 {
   const char *nop_templ = 0;
   int code_num;
@@ -1642,9 +1640,6 @@ default_print_patchable_function_entry (FILE *file,
       patch_area_number++;
       ASM_GENERATE_INTERNAL_LABEL (buf, "LPFE", patch_area_number);
 
-      unsigned int flags = SECTION_WRITE | SECTION_RELRO;
-      if (HAVE_GAS_SECTION_LINK_ORDER)
-	flags |= SECTION_LINK_ORDER;
       switch_to_section (get_section ("__patchable_function_entries",
 				      flags, current_function_decl));
       assemble_align (POINTER_SIZE);
@@ -1661,6 +1656,25 @@ default_print_patchable_function_entry (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 6206fe20823..7b6d2fb9138 100644
--- a/gcc/targhooks.h
+++ b/gcc/targhooks.h
@@ -203,6 +203,9 @@ 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 26d265da74e..b14f1b3e341 100644
--- a/gcc/testsuite/g++.dg/pr93195a.C
+++ b/gcc/testsuite/g++.dg/pr93195a.C
@@ -1,4 +1,5 @@
 /* { 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*-*-* } } */


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [gcc(refs/users/giulianob/heads/pfe_backport_clean)] Backport rs6000: Avoid -fpatchable-function-entry* regressions on powerpc64 be [PR98125]
@ 2021-10-19 18:13 Giuliano Belinassi
  0 siblings, 0 replies; 4+ messages in thread
From: Giuliano Belinassi @ 2021-10-19 18:13 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:0d2553b51ba8572fb10221835023f20f5ec6b05e

commit 0d2553b51ba8572fb10221835023f20f5ec6b05e
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Sat Apr 3 10:03:15 2021 +0200

    Backport rs6000: Avoid -fpatchable-function-entry* regressions on powerpc64 be [PR98125]
    
    The SECTION_LINK_ORDER changes broke powerpc64-linux ELFv1.  Seems
    that the assembler/linker relies on the symbol mentioned for the
    "awo" section to be in the same section as the symbols mentioned in
    the relocations in that section (i.e. labels for the patchable area
    in this case).  That is the case for most targets, including powerpc-linux
    32-bit or powerpc64 ELFv2 (that one has -fpatchable-function-entry*
    support broken for other reasons and it doesn't seem to be a regression).
    But it doesn't work on powerpc64-linux ELFv1.
    We emit:
            .section        ".opd","aw"
            .align 3
    _Z3foov:
            .quad   .L._Z3foov,.TOC.@tocbase,0
            .previous
            .type   _Z3foov, @function
    .L._Z3foov:
            .section        __patchable_function_entries,"awo",@progbits,_Z3foov
            .align 3
            .8byte  .LPFE1
            .section        .text._Z3foov,"axG",@progbits,_Z3foov,comdat
    .LPFE1:
            nop
    .LFB0:
            .cfi_startproc
    and because _Z3foov is in the .opd section rather than the function text
    section, it doesn't work.
    
    I'm afraid I don't know what exactly should be done, whether e.g.
    it could use
            .section        __patchable_function_entries,"awo",@progbits,.L._Z3foov
    instead, or whether the linker should be changed to handle it as is, or
    something else.
    
    But because we have a P1 regression that didn't see useful progress over the
    4 months since it has been filed and we don't really have much time, below
    is an attempt to do a targetted reversion of H.J's patch, basically act as
    if HAVE_GAS_SECTION_LINK_ORDER is never true for powerpc64-linux ELFv1,
    but for 32-bit or 64-bit ELFv2 keep working as is.
    This would give us time to resolve it for GCC 12 properly.
    
    2021-04-03  Jakub Jelinek  <jakub@redhat.com>
    
            PR testsuite/98125
            * targhooks.h (default_print_patchable_function_entry_1): Declare.
            * targhooks.c (default_print_patchable_function_entry_1): New function,
            copied from default_print_patchable_function_entry with an added flags
            argument.
            (default_print_patchable_function_entry): Rewritten into a small
            wrapper around default_print_patchable_function_entry_1.
            * config/rs6000/rs6000.c (TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY):
            Redefine.
            (rs6000_print_patchable_function_entry): New function.
    
            * g++.dg/pr93195a.C: Skip on powerpc*-*-* 64-bit.

Diff:
---
 gcc/config/rs6000/rs6000.c      |  8 ++++++--
 gcc/targhooks.c                 | 38 ++++++++++++++++++++++++++------------
 gcc/targhooks.h                 |  3 +++
 gcc/testsuite/g++.dg/pr93195a.C |  1 +
 4 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index af0c7ce1656..ac60d6d39ac 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -1615,6 +1615,10 @@ static const struct attribute_spec rs6000_attribute_table[] =
 #define TARGET_ASM_ASSEMBLE_VISIBILITY rs6000_assemble_visibility
 #endif
 
+#undef TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY
+#define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY \
+  rs6000_print_patchable_function_entry
+
 #undef TARGET_SET_UP_BY_PROLOGUE
 #define TARGET_SET_UP_BY_PROLOGUE rs6000_set_up_by_prologue
 
@@ -27933,8 +27937,8 @@ debug_stack_info (rs6000_stack_t *info)
   fprintf (stderr, "\n");
 }
 
-rtx
-rs6000_return_addr (int count, rtx frame)
+enum rtx_code
+rs6000_reverse_condition (machine_mode mode, enum rtx_code code)
 {
   /* We can't use get_hard_reg_initial_val for LR when count == 0 if LR
      is trashed by the prologue, as it is for PIC on ABI_V4 and Darwin.  */
diff --git a/gcc/targhooks.c b/gcc/targhooks.c
index 8aa610f5cde..958ec4ba6ff 100644
--- a/gcc/targhooks.c
+++ b/gcc/targhooks.c
@@ -1610,17 +1610,15 @@ default_compare_by_pieces_branch_ratio (machine_mode)
   return 1;
 }
 
-/* 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.  */
+/* Helper for default_print_patchable_function_entry and other
+   print_patchable_function_entry hook implementations.  */
 
 void
-default_print_patchable_function_entry (FILE *file,
-					unsigned HOST_WIDE_INT patch_area_size,
-					bool record_p)
+default_print_patchable_function_entry_1 (FILE *file,
+					  unsigned HOST_WIDE_INT
+					  patch_area_size,
+					  bool record_p,
+					  unsigned int flags)
 {
   const char *nop_templ = 0;
   int code_num;
@@ -1642,9 +1640,6 @@ default_print_patchable_function_entry (FILE *file,
       patch_area_number++;
       ASM_GENERATE_INTERNAL_LABEL (buf, "LPFE", patch_area_number);
 
-      unsigned int flags = SECTION_WRITE | SECTION_RELRO;
-      if (HAVE_GAS_SECTION_LINK_ORDER)
-	flags |= SECTION_LINK_ORDER;
       switch_to_section (get_section ("__patchable_function_entries",
 				      flags, current_function_decl));
       assemble_align (POINTER_SIZE);
@@ -1661,6 +1656,25 @@ default_print_patchable_function_entry (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 6206fe20823..7b6d2fb9138 100644
--- a/gcc/targhooks.h
+++ b/gcc/targhooks.h
@@ -203,6 +203,9 @@ 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 26d265da74e..b14f1b3e341 100644
--- a/gcc/testsuite/g++.dg/pr93195a.C
+++ b/gcc/testsuite/g++.dg/pr93195a.C
@@ -1,4 +1,5 @@
 /* { 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*-*-* } } */


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-10-21 16:46 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-19 18:40 [gcc(refs/users/giulianob/heads/pfe_backport_clean)] Backport rs6000: Avoid -fpatchable-function-entry* regressions on powerpc64 be [PR98125] Giuliano Belinassi
  -- strict thread matches above, loose matches on Subject: below --
2021-10-21 16:46 Giuliano Belinassi
2021-10-21 14:49 Giuliano Belinassi
2021-10-19 18:13 Giuliano Belinassi

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).