| Message ID | 20210620021035.450959-1-hjl.tools@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add TARGET_IFUNC_REF_LOCAL_OK | expand |
On Sat, Jun 19, 2021 at 7:10 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > 1. On some targets, like PowerPC, reference to ifunc function resolver > must be non-local so that compiler will properly emit PLT call. Add > TARGET_IFUNC_REF_LOCAL_OK to allow binding indirect function resolver > locally for targets which don't require special PLT call sequence. > 2. Add ix86_call_use_plt_p to call local ifunc function resolvers via > PLT. > > gcc/ > > PR target/51469 > PR target/83782 > * target.def (ifunc_ref_local_ok): Add a target hook. > * varasm.c (default_binds_local_p_3): Force indirect function > resolver non-local only if targetm.ifunc_ref_local_ok returns > false. > * config/i386/i386-expand.c (ix86_expand_call): Call > ix86_call_use_plt_p to check if PLT should be used. > * config/i386/i386-protos.h (ix86_call_use_plt_p): New. > * config/i386/i386.c (output_pic_addr_const): Call > ix86_call_use_plt_p to check if "@PLT" is needed. > (ix86_call_use_plt_p): New. > (TARGET_IFUNC_REF_LOCAL_OK): New. > * doc/tm.texi.in: Add TARGET_IFUNC_REF_LOCAL_OK. > * doc/tm.texi: Regenerated. > > gcc/testsuite/ > > PR target/51469 > PR target/83782 > * gcc.target/i386/pr83782-1.c: New test. > * gcc.target/i386/pr83782-2.c: Likewise. > --- > gcc/config/i386/i386-expand.c | 2 +- > gcc/config/i386/i386-protos.h | 1 + > gcc/config/i386/i386.c | 25 +++++++++++++++++++++- > gcc/doc/tm.texi | 5 +++++ > gcc/doc/tm.texi.in | 2 ++ > gcc/target.def | 8 +++++++ > gcc/testsuite/gcc.target/i386/pr83782-1.c | 26 +++++++++++++++++++++++ > gcc/testsuite/gcc.target/i386/pr83782-2.c | 26 +++++++++++++++++++++++ > gcc/varasm.c | 3 ++- > 9 files changed, 95 insertions(+), 3 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/i386/pr83782-1.c > create mode 100644 gcc/testsuite/gcc.target/i386/pr83782-2.c > > diff --git a/gcc/config/i386/i386-expand.c b/gcc/config/i386/i386-expand.c > index 8f4e4e4d884..229a765cb40 100644 > --- a/gcc/config/i386/i386-expand.c > +++ b/gcc/config/i386/i386-expand.c > @@ -8184,7 +8184,7 @@ ix86_expand_call (rtx retval, rtx fnaddr, rtx callarg1, > rtx addr = XEXP (fnaddr, 0); > if (flag_pic > && GET_CODE (addr) == SYMBOL_REF > - && !SYMBOL_REF_LOCAL_P (addr)) > + && ix86_call_use_plt_p (addr)) > { > if (flag_plt > && (SYMBOL_REF_DECL (addr) == NULL_TREE > diff --git a/gcc/config/i386/i386-protos.h b/gcc/config/i386/i386-protos.h > index e6ac9390777..2d1cd07a215 100644 > --- a/gcc/config/i386/i386-protos.h > +++ b/gcc/config/i386/i386-protos.h > @@ -152,6 +152,7 @@ extern void ix86_expand_sse_movcc (rtx, rtx, rtx, rtx); > extern void ix86_expand_sse_unpack (rtx, rtx, bool, bool); > extern bool ix86_expand_int_addcc (rtx[]); > extern rtx_insn *ix86_expand_call (rtx, rtx, rtx, rtx, rtx, bool); > +extern bool ix86_call_use_plt_p (rtx); > extern void ix86_split_call_vzeroupper (rtx, rtx); > extern void x86_initialize_trampoline (rtx, rtx, rtx); > extern rtx ix86_zero_extend_to_Pmode (rtx); > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > index 7d0d4143bca..731a516b516 100644 > --- a/gcc/config/i386/i386.c > +++ b/gcc/config/i386/i386.c > @@ -11971,7 +11971,7 @@ output_pic_addr_const (FILE *file, rtx x, int code) > assemble_name (file, name); > } > if (!TARGET_MACHO && !(TARGET_64BIT && TARGET_PECOFF) > - && code == 'P' && ! SYMBOL_REF_LOCAL_P (x)) > + && code == 'P' && ix86_call_use_plt_p (x)) > fputs ("@PLT", file); > break; > > @@ -15678,6 +15678,26 @@ ix86_zero_extend_to_Pmode (rtx exp) > return force_reg (Pmode, convert_to_mode (Pmode, exp, 1)); > } > > +/* Return true if the function is called via PLT. */ > + > +bool > +ix86_call_use_plt_p (rtx call_op) > +{ > + if (SYMBOL_REF_LOCAL_P (call_op)) > + { > + if (SYMBOL_REF_DECL (call_op)) > + { > + /* NB: All ifunc functions must be called via PLT. */ > + cgraph_node *node > + = cgraph_node::get (SYMBOL_REF_DECL (call_op)); > + if (node && node->ifunc_resolver) > + return true; > + } > + return false; > + } > + return true; > +} > + > /* Return true if the function being called was marked with attribute > "noplt" or using -fno-plt and we are compiling for non-PIC. We need > to handle the non-PIC case in the backend because there is no easy > @@ -24016,6 +24036,9 @@ ix86_run_selftests (void) > #define TARGET_GET_MULTILIB_ABI_NAME \ > ix86_get_multilib_abi_name > > +#undef TARGET_IFUNC_REF_LOCAL_OK > +#define TARGET_IFUNC_REF_LOCAL_OK hook_bool_void_true > + > static bool ix86_libc_has_fast_function (int fcode ATTRIBUTE_UNUSED) > { > #ifdef OPTION_GLIBC > diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi > index 2a41ae5fba1..d327af03c80 100644 > --- a/gcc/doc/tm.texi > +++ b/gcc/doc/tm.texi > @@ -12346,6 +12346,11 @@ The support includes the assembler, linker and dynamic linker. > The default value of this hook is based on target's libc. > @end deftypefn > > +@deftypefn {Target Hook} bool TARGET_IFUNC_REF_LOCAL_OK (void) > +Return true if it is OK to reference indirect function resolvers > +locally. The default is to return false. > +@end deftypefn > + > @deftypefn {Target Hook} {unsigned int} TARGET_ATOMIC_ALIGN_FOR_MODE (machine_mode @var{mode}) > If defined, this function returns an appropriate alignment in bits for an > atomic object of machine_mode @var{mode}. If 0 is returned then the > diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in > index f881cdabe9e..cc6f7994cb1 100644 > --- a/gcc/doc/tm.texi.in > +++ b/gcc/doc/tm.texi.in > @@ -8073,6 +8073,8 @@ and the associated definitions of those functions. > > @hook TARGET_HAS_IFUNC_P > > +@hook TARGET_IFUNC_REF_LOCAL_OK > + > @hook TARGET_ATOMIC_ALIGN_FOR_MODE > > @hook TARGET_ATOMIC_ASSIGN_EXPAND_FENV > diff --git a/gcc/target.def b/gcc/target.def > index c009671c583..2c8aa90afac 100644 > --- a/gcc/target.def > +++ b/gcc/target.def > @@ -2990,6 +2990,14 @@ The default value of this hook is based on target's libc.", > bool, (void), > default_has_ifunc_p) > > +/* True if it is OK to reference indirect function resolvers locally. */ > +DEFHOOK > +(ifunc_ref_local_ok, > + "Return true if it is OK to reference indirect function resolvers\n\ > +locally. The default is to return false.", > + bool, (void), > + hook_bool_void_false) > + > /* True if it is OK to do sibling call optimization for the specified > call expression EXP. DECL will be the called function, or NULL if > this is an indirect call. */ > diff --git a/gcc/testsuite/gcc.target/i386/pr83782-1.c b/gcc/testsuite/gcc.target/i386/pr83782-1.c > new file mode 100644 > index 00000000000..f4c7370142a > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr83782-1.c > @@ -0,0 +1,26 @@ > +/* { dg-do compile } */ > +/* { dg-require-ifunc "" } */ > +/* { dg-options "-O2 -fpic" } */ > + > +static void > +my_foo (void) > +{ > +} > + > +static void (*resolve_foo (void)) (void) > +{ > + return my_foo; > +} > + > +extern void foo (void) __attribute__((ifunc("resolve_foo"), visibility("hidden"))); > + > +void * > +bar(void) > +{ > + return foo; > +} > + > +/* { dg-final { scan-assembler {leal[ \t]foo@GOTOFF\(%[^,]*\),[ \t]%eax} { target ia32 } } } */ > +/* { dg-final { scan-assembler {leaq[ \t]foo\(%rip\),[ \t]%rax} { target { ! ia32 } } } } */ > +/* { dg-final { scan-assembler-not "foo@GOT\\\(" { target ia32 } } } */ > +/* { dg-final { scan-assembler-not "foo@GOTPCREL\\\(" { target { ! ia32 } } } } */ > diff --git a/gcc/testsuite/gcc.target/i386/pr83782-2.c b/gcc/testsuite/gcc.target/i386/pr83782-2.c > new file mode 100644 > index 00000000000..6c6528fff46 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr83782-2.c > @@ -0,0 +1,26 @@ > +/* { dg-do compile } */ > +/* { dg-require-ifunc "" } */ > +/* { dg-options "-O2 -fpic" } */ > + > +static void > +my_foo (void) > +{ > +} > + > +static void (*resolve_foo (void)) (void) > +{ > + return my_foo; > +} > + > +static void foo (void) __attribute__((ifunc("resolve_foo"))); > + > +void * > +bar(void) > +{ > + return foo; > +} > + > +/* { dg-final { scan-assembler {leal[ \t]foo@GOTOFF\(%[^,]*\),[ \t]%eax} { target ia32 } } } */ > +/* { dg-final { scan-assembler {leaq[ \t]foo\(%rip\),[ \t]%rax} { target { ! ia32 } } } } */ > +/* { dg-final { scan-assembler-not "foo@GOT\\\(" { target ia32 } } } */ > +/* { dg-final { scan-assembler-not "foo@GOTPCREL\\\(" { target { ! ia32 } } } } */ > diff --git a/gcc/varasm.c b/gcc/varasm.c > index 53cf6dea3f3..0cba7ab0ecd 100644 > --- a/gcc/varasm.c > +++ b/gcc/varasm.c > @@ -7470,7 +7470,8 @@ default_binds_local_p_3 (const_tree exp, bool shlib, bool weak_dominate, > FIXME: We can resolve the weakref case more curefuly by looking at the > weakref alias. */ > if (lookup_attribute ("weakref", DECL_ATTRIBUTES (exp)) > - || (TREE_CODE (exp) == FUNCTION_DECL > + || (!targetm.ifunc_ref_local_ok () > + && TREE_CODE (exp) == FUNCTION_DECL > && cgraph_node::get (exp) > && cgraph_node::get (exp)->ifunc_resolver)) > return false; > -- > 2.31.1 > PING.
On 7/6/2021 5:47 AM, H.J. Lu via Gcc-patches wrote: > On Sat, Jun 19, 2021 at 7:10 PM H.J. Lu <hjl.tools@gmail.com> wrote: >> 1. On some targets, like PowerPC, reference to ifunc function resolver >> must be non-local so that compiler will properly emit PLT call. Add >> TARGET_IFUNC_REF_LOCAL_OK to allow binding indirect function resolver >> locally for targets which don't require special PLT call sequence. >> 2. Add ix86_call_use_plt_p to call local ifunc function resolvers via >> PLT. >> >> gcc/ >> >> PR target/51469 >> PR target/83782 >> * target.def (ifunc_ref_local_ok): Add a target hook. >> * varasm.c (default_binds_local_p_3): Force indirect function >> resolver non-local only if targetm.ifunc_ref_local_ok returns >> false. >> * config/i386/i386-expand.c (ix86_expand_call): Call >> ix86_call_use_plt_p to check if PLT should be used. >> * config/i386/i386-protos.h (ix86_call_use_plt_p): New. >> * config/i386/i386.c (output_pic_addr_const): Call >> ix86_call_use_plt_p to check if "@PLT" is needed. >> (ix86_call_use_plt_p): New. >> (TARGET_IFUNC_REF_LOCAL_OK): New. >> * doc/tm.texi.in: Add TARGET_IFUNC_REF_LOCAL_OK. >> * doc/tm.texi: Regenerated. >> >> gcc/testsuite/ >> >> PR target/51469 >> PR target/83782 >> * gcc.target/i386/pr83782-1.c: New test. >> * gcc.target/i386/pr83782-2.c: Likewise. This is OK. Please re-test (I don't expect problems, but it's an old outstanding patch) & install. jeff
diff --git a/gcc/config/i386/i386-expand.c b/gcc/config/i386/i386-expand.c index 8f4e4e4d884..229a765cb40 100644 --- a/gcc/config/i386/i386-expand.c +++ b/gcc/config/i386/i386-expand.c @@ -8184,7 +8184,7 @@ ix86_expand_call (rtx retval, rtx fnaddr, rtx callarg1, rtx addr = XEXP (fnaddr, 0); if (flag_pic && GET_CODE (addr) == SYMBOL_REF - && !SYMBOL_REF_LOCAL_P (addr)) + && ix86_call_use_plt_p (addr)) { if (flag_plt && (SYMBOL_REF_DECL (addr) == NULL_TREE diff --git a/gcc/config/i386/i386-protos.h b/gcc/config/i386/i386-protos.h index e6ac9390777..2d1cd07a215 100644 --- a/gcc/config/i386/i386-protos.h +++ b/gcc/config/i386/i386-protos.h @@ -152,6 +152,7 @@ extern void ix86_expand_sse_movcc (rtx, rtx, rtx, rtx); extern void ix86_expand_sse_unpack (rtx, rtx, bool, bool); extern bool ix86_expand_int_addcc (rtx[]); extern rtx_insn *ix86_expand_call (rtx, rtx, rtx, rtx, rtx, bool); +extern bool ix86_call_use_plt_p (rtx); extern void ix86_split_call_vzeroupper (rtx, rtx); extern void x86_initialize_trampoline (rtx, rtx, rtx); extern rtx ix86_zero_extend_to_Pmode (rtx); diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 7d0d4143bca..731a516b516 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -11971,7 +11971,7 @@ output_pic_addr_const (FILE *file, rtx x, int code) assemble_name (file, name); } if (!TARGET_MACHO && !(TARGET_64BIT && TARGET_PECOFF) - && code == 'P' && ! SYMBOL_REF_LOCAL_P (x)) + && code == 'P' && ix86_call_use_plt_p (x)) fputs ("@PLT", file); break; @@ -15678,6 +15678,26 @@ ix86_zero_extend_to_Pmode (rtx exp) return force_reg (Pmode, convert_to_mode (Pmode, exp, 1)); } +/* Return true if the function is called via PLT. */ + +bool +ix86_call_use_plt_p (rtx call_op) +{ + if (SYMBOL_REF_LOCAL_P (call_op)) + { + if (SYMBOL_REF_DECL (call_op)) + { + /* NB: All ifunc functions must be called via PLT. */ + cgraph_node *node + = cgraph_node::get (SYMBOL_REF_DECL (call_op)); + if (node && node->ifunc_resolver) + return true; + } + return false; + } + return true; +} + /* Return true if the function being called was marked with attribute "noplt" or using -fno-plt and we are compiling for non-PIC. We need to handle the non-PIC case in the backend because there is no easy @@ -24016,6 +24036,9 @@ ix86_run_selftests (void) #define TARGET_GET_MULTILIB_ABI_NAME \ ix86_get_multilib_abi_name +#undef TARGET_IFUNC_REF_LOCAL_OK +#define TARGET_IFUNC_REF_LOCAL_OK hook_bool_void_true + static bool ix86_libc_has_fast_function (int fcode ATTRIBUTE_UNUSED) { #ifdef OPTION_GLIBC diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi index 2a41ae5fba1..d327af03c80 100644 --- a/gcc/doc/tm.texi +++ b/gcc/doc/tm.texi @@ -12346,6 +12346,11 @@ The support includes the assembler, linker and dynamic linker. The default value of this hook is based on target's libc. @end deftypefn +@deftypefn {Target Hook} bool TARGET_IFUNC_REF_LOCAL_OK (void) +Return true if it is OK to reference indirect function resolvers +locally. The default is to return false. +@end deftypefn + @deftypefn {Target Hook} {unsigned int} TARGET_ATOMIC_ALIGN_FOR_MODE (machine_mode @var{mode}) If defined, this function returns an appropriate alignment in bits for an atomic object of machine_mode @var{mode}. If 0 is returned then the diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in index f881cdabe9e..cc6f7994cb1 100644 --- a/gcc/doc/tm.texi.in +++ b/gcc/doc/tm.texi.in @@ -8073,6 +8073,8 @@ and the associated definitions of those functions. @hook TARGET_HAS_IFUNC_P +@hook TARGET_IFUNC_REF_LOCAL_OK + @hook TARGET_ATOMIC_ALIGN_FOR_MODE @hook TARGET_ATOMIC_ASSIGN_EXPAND_FENV diff --git a/gcc/target.def b/gcc/target.def index c009671c583..2c8aa90afac 100644 --- a/gcc/target.def +++ b/gcc/target.def @@ -2990,6 +2990,14 @@ The default value of this hook is based on target's libc.", bool, (void), default_has_ifunc_p) +/* True if it is OK to reference indirect function resolvers locally. */ +DEFHOOK +(ifunc_ref_local_ok, + "Return true if it is OK to reference indirect function resolvers\n\ +locally. The default is to return false.", + bool, (void), + hook_bool_void_false) + /* True if it is OK to do sibling call optimization for the specified call expression EXP. DECL will be the called function, or NULL if this is an indirect call. */ diff --git a/gcc/testsuite/gcc.target/i386/pr83782-1.c b/gcc/testsuite/gcc.target/i386/pr83782-1.c new file mode 100644 index 00000000000..f4c7370142a --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr83782-1.c @@ -0,0 +1,26 @@ +/* { dg-do compile } */ +/* { dg-require-ifunc "" } */ +/* { dg-options "-O2 -fpic" } */ + +static void +my_foo (void) +{ +} + +static void (*resolve_foo (void)) (void) +{ + return my_foo; +} + +extern void foo (void) __attribute__((ifunc("resolve_foo"), visibility("hidden"))); + +void * +bar(void) +{ + return foo; +} + +/* { dg-final { scan-assembler {leal[ \t]foo@GOTOFF\(%[^,]*\),[ \t]%eax} { target ia32 } } } */ +/* { dg-final { scan-assembler {leaq[ \t]foo\(%rip\),[ \t]%rax} { target { ! ia32 } } } } */ +/* { dg-final { scan-assembler-not "foo@GOT\\\(" { target ia32 } } } */ +/* { dg-final { scan-assembler-not "foo@GOTPCREL\\\(" { target { ! ia32 } } } } */ diff --git a/gcc/testsuite/gcc.target/i386/pr83782-2.c b/gcc/testsuite/gcc.target/i386/pr83782-2.c new file mode 100644 index 00000000000..6c6528fff46 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr83782-2.c @@ -0,0 +1,26 @@ +/* { dg-do compile } */ +/* { dg-require-ifunc "" } */ +/* { dg-options "-O2 -fpic" } */ + +static void +my_foo (void) +{ +} + +static void (*resolve_foo (void)) (void) +{ + return my_foo; +} + +static void foo (void) __attribute__((ifunc("resolve_foo"))); + +void * +bar(void) +{ + return foo; +} + +/* { dg-final { scan-assembler {leal[ \t]foo@GOTOFF\(%[^,]*\),[ \t]%eax} { target ia32 } } } */ +/* { dg-final { scan-assembler {leaq[ \t]foo\(%rip\),[ \t]%rax} { target { ! ia32 } } } } */ +/* { dg-final { scan-assembler-not "foo@GOT\\\(" { target ia32 } } } */ +/* { dg-final { scan-assembler-not "foo@GOTPCREL\\\(" { target { ! ia32 } } } } */ diff --git a/gcc/varasm.c b/gcc/varasm.c index 53cf6dea3f3..0cba7ab0ecd 100644 --- a/gcc/varasm.c +++ b/gcc/varasm.c @@ -7470,7 +7470,8 @@ default_binds_local_p_3 (const_tree exp, bool shlib, bool weak_dominate, FIXME: We can resolve the weakref case more curefuly by looking at the weakref alias. */ if (lookup_attribute ("weakref", DECL_ATTRIBUTES (exp)) - || (TREE_CODE (exp) == FUNCTION_DECL + || (!targetm.ifunc_ref_local_ok () + && TREE_CODE (exp) == FUNCTION_DECL && cgraph_node::get (exp) && cgraph_node::get (exp)->ifunc_resolver)) return false;