x86: Use DW_EH_PE_indirect|DW_EH_PE_pcrel encodings for -fno-pic code

Message ID 20230201073859.3920910-1-maskray@google.com
State New
Headers
Series x86: Use DW_EH_PE_indirect|DW_EH_PE_pcrel encodings for -fno-pic code |

Commit Message

Fangrui Song Feb. 1, 2023, 7:38 a.m. UTC
  Follow aarch64 and riscv ports by using (global ? DW_EH_PE_indirect : 0)
| DW_EH_PE_pcrel for -fno-pic code.  This avoids a canonical PLT entry
for a personality reference in .eh_frame and a copy relocation for a
typeinfo reference in .gcc_except_table, when the definition is in
libstdc++.so.6.

gcc/

    PR target/108622
    * config/i386/i386.cc (asm_preferred_eh_data_format): Always use the
    PIC encoding. Use DW_EH_PE_indirect if global.
---
 gcc/config/i386/i386.cc | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)
  

Comments

Jakub Jelinek Feb. 1, 2023, 9:01 a.m. UTC | #1
On Tue, Jan 31, 2023 at 11:38:59PM -0800, Fangrui Song via Gcc-patches wrote:
> Follow aarch64 and riscv ports by using (global ? DW_EH_PE_indirect : 0)
> | DW_EH_PE_pcrel for -fno-pic code.  This avoids a canonical PLT entry
> for a personality reference in .eh_frame and a copy relocation for a
> typeinfo reference in .gcc_except_table, when the definition is in
> libstdc++.so.6.
> 
> gcc/
> 
>     PR target/108622
>     * config/i386/i386.cc (asm_preferred_eh_data_format): Always use the
>     PIC encoding. Use DW_EH_PE_indirect if global.

Not a review, just comments:

> diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
> index 3cacf738c4a..b572dd46a62 100644
> --- a/gcc/config/i386/i386.cc
> +++ b/gcc/config/i386/i386.cc
> @@ -22851,22 +22851,13 @@ ix86_stack_protect_fail (void)
>  int
>  asm_preferred_eh_data_format (int code, int global)
>  {
> -  /* PE-COFF is effectively always -fPIC because of the .reloc section.  */
> -  if (flag_pic || TARGET_PECOFF || !ix86_direct_extern_access)
> -    {
> -      int type = DW_EH_PE_sdata8;
> -      if (ptr_mode == SImode
> -	  || ix86_cmodel == CM_SMALL_PIC
> -	  || (ix86_cmodel == CM_MEDIUM_PIC && (global || code)))
> -	type = DW_EH_PE_sdata4;
> -      return (global ? DW_EH_PE_indirect : 0) | DW_EH_PE_pcrel | type;
> -    }
> -
> -  if (ix86_cmodel == CM_SMALL
> -      || (ix86_cmodel == CM_MEDIUM && code))
> -    return DW_EH_PE_udata4;
> -
> -  return DW_EH_PE_absptr;
> +  int type = DW_EH_PE_sdata8;
> +  if (ptr_mode == SImode || ix86_cmodel == CM_SMALL ||
> +      ix86_cmodel == CM_SMALL_PIC ||
> +      (ix86_cmodel == CM_MEDIUM && code) ||
> +      (ix86_cmodel == CM_MEDIUM_PIC && (global || code)))

The above is incorrectly formatted, the GCC Coding Conventions
say || etc. shouldn't be at the end of lines, but rather at the
start of the next ones.

And, while I can understand the rationale for global cases
(though am not sure I agree, as currently the user can choose
by using -mno-direct-extern-access or not using it, the above change
disallows the choice), for !global I miss the point altogether.
In non-PIC (and non-PECOFF maybe, don't know that format well)
code if code references a local symbol, it will never generate
a copy relocation and so DW_EH_PE_udata4 could be just fine and cheaper
to handle on the consumer sides.

	Jakub
  
Jakub Jelinek Feb. 1, 2023, 9:09 a.m. UTC | #2
On Wed, Feb 01, 2023 at 10:01:36AM +0100, Jakub Jelinek via Gcc-patches wrote:
> And, while I can understand the rationale for global cases
> (though am not sure I agree, as currently the user can choose
> by using -mno-direct-extern-access or not using it, the above change
> disallows the choice), for !global I miss the point altogether.
> In non-PIC (and non-PECOFF maybe, don't know that format well)
> code if code references a local symbol, it will never generate
> a copy relocation and so DW_EH_PE_udata4 could be just fine and cheaper
> to handle on the consumer sides.

Plus, e.g. for the typeinfo references, if using the default
-mdirect-extern-access and RTTI is used, there will be copy relocations
anyway and so the .eh_frame growth will be just waste of .got space.

	Jakub
  
Fangrui Song Feb. 2, 2023, 5:50 a.m. UTC | #3
> The above is incorrectly formatted, the GCC Coding Conventions
say || etc. shouldn't be at the end of lines, but rather at the
start of the next ones.

Ack.

> And, while I can understand the rationale for global cases
> (though am not sure I agree, as currently the user can choose
> by using -mno-direct-extern-access or not using it, the above change
> disallows the choice), for !global I miss the point altogether.
> In non-PIC (and non-PECOFF maybe, don't know that format well)
> code if code references a local symbol, it will never generate
> a copy relocation and so DW_EH_PE_udata4 could be just fine and cheaper
> to handle on the consumer sides.

On Wed, Feb 1, 2023 at 1:09 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Wed, Feb 01, 2023 at 10:01:36AM +0100, Jakub Jelinek via Gcc-patches wrote:
> > And, while I can understand the rationale for global cases
> > (though am not sure I agree, as currently the user can choose
> > by using -mno-direct-extern-access or not using it, the above change
> > disallows the choice), for !global I miss the point altogether.
> > In non-PIC (and non-PECOFF maybe, don't know that format well)
> > code if code references a local symbol, it will never generate
> > a copy relocation and so DW_EH_PE_udata4 could be just fine and cheaper
> > to handle on the consumer sides.
>
> Plus, e.g. for the typeinfo references, if using the default
> -mdirect-extern-access and RTTI is used, there will be copy relocations
> anyway and so the .eh_frame growth will be just waste of .got space.
>
>         Jakub
>

There is a copy relocation but the canonical PLT entry for
__gxx_personality_v0 can be removed.
-fno-pic C++ exception code is less common nowadays, so there seems little value
to keep a complex condition (if (flag_pic || TARGET_PECOFF ||
!ix86_direct_extern_access)) to
optimize a bit for the object file size (I think you mean .data space
instead of .got space).
The .eh_frame growth is nearly negligible as well.
  

Patch

diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
index 3cacf738c4a..b572dd46a62 100644
--- a/gcc/config/i386/i386.cc
+++ b/gcc/config/i386/i386.cc
@@ -22851,22 +22851,13 @@  ix86_stack_protect_fail (void)
 int
 asm_preferred_eh_data_format (int code, int global)
 {
-  /* PE-COFF is effectively always -fPIC because of the .reloc section.  */
-  if (flag_pic || TARGET_PECOFF || !ix86_direct_extern_access)
-    {
-      int type = DW_EH_PE_sdata8;
-      if (ptr_mode == SImode
-	  || ix86_cmodel == CM_SMALL_PIC
-	  || (ix86_cmodel == CM_MEDIUM_PIC && (global || code)))
-	type = DW_EH_PE_sdata4;
-      return (global ? DW_EH_PE_indirect : 0) | DW_EH_PE_pcrel | type;
-    }
-
-  if (ix86_cmodel == CM_SMALL
-      || (ix86_cmodel == CM_MEDIUM && code))
-    return DW_EH_PE_udata4;
-
-  return DW_EH_PE_absptr;
+  int type = DW_EH_PE_sdata8;
+  if (ptr_mode == SImode || ix86_cmodel == CM_SMALL ||
+      ix86_cmodel == CM_SMALL_PIC ||
+      (ix86_cmodel == CM_MEDIUM && code) ||
+      (ix86_cmodel == CM_MEDIUM_PIC && (global || code)))
+    type = DW_EH_PE_sdata4;
+  return (global ? DW_EH_PE_indirect : 0) | DW_EH_PE_pcrel | type;
 }
 
 /* Implement targetm.vectorize.builtin_vectorization_cost.  */