Wilco Dijkstra <wilco.dijks...@arm.com> writes:
> Add a new option -mdirect-extern-access similar to other targets. This
> removes
> GOT indirections on external symbols with -fPIE, resulting in significantly
> better code quality. With -fPIC it only affects protected symbols, allowing
> for more efficient shared libraries which can be linked with standard PIE
> binaries (this is what LLVM does by default, so this improves interoperability
> with LLVM). This patch doesn't affect ABI, but in the future GCC and LLVM
> should converge to using the same ABI.
Can you go into more detail about:
Use :option:`-mdirect-extern-access` either in shared libraries or in
executables, but not in both. Protected symbols used both in a shared
library and executable may cause linker errors or fail to work correctly
If this is LLVM's default for PIC (and by assumption shared libraries),
is it then invalid to use -mdirect-extern-access for any PIEs that
are linked against those shared libraries and use protected symbols
from those libraries? How would a user know that one of the shared
libraries they're linking against was built in this way?
It looks like the main difference between this implementation and
the x86 one is that x86 allows direct accesses to common symbols.
What's the reason for not doing that for AArch64? Does it not work,
is it a false optimisation (i.e. pessimisation), or did it not seem
important now that -fno-common is the default?
Thanks,
Richard
>
> Regress and bootstrap pass, OK for commit?
>
> gcc/
> * config/aarch64/aarch64.cc (aarch64_binds_local_p): New function.
> (aarch64_symbol_binds_local_p): Refactor, support direct extern
> access.
> * config/aarch64/aarch64-linux.h (TARGET_BINDS_LOCAL_P):
> Use aarch64_binds_local_p.
> * config/aarch64/aarch64-freebsd.h (TARGET_BINDS_LOCAL_P): Likewise.
> * config/aarch64/aarch64-protos.h: Add aarch64_binds_local_p.
> * doc/gcc/gcc-command-options/option-summary.rst: Add
> -mdirect-extern-access.
> *
> doc/gcc/gcc-command-options/machine-dependent-options/aarch64-options.rst:
> Add description of -mdirect-extern-access.
>
> gcc/testsuite/
> * gcc.target/aarch64/pr66912-2.c: New test.
>
> ---
>
> diff --git a/gcc/config/aarch64/aarch64-freebsd.h
> b/gcc/config/aarch64/aarch64-freebsd.h
> index
> 13beb3781b61afd82d767884f3c16ff8eead09cc..20bc0f48e484686cd3754613bf20bb3521079d48
> 100644
> --- a/gcc/config/aarch64/aarch64-freebsd.h
> +++ b/gcc/config/aarch64/aarch64-freebsd.h
> @@ -71,7 +71,7 @@
> strong definitions in dependent shared libraries, will resolve
> to COPY relocated symbol in the executable. See PR65780. */
> #undef TARGET_BINDS_LOCAL_P
> -#define TARGET_BINDS_LOCAL_P default_binds_local_p_2
> +#define TARGET_BINDS_LOCAL_P aarch64_binds_local_p
>
> /* Use the AAPCS type for wchar_t, override the one from
> config/freebsd.h. */
> diff --git a/gcc/config/aarch64/aarch64-linux.h
> b/gcc/config/aarch64/aarch64-linux.h
> index
> 5e4553d79f5053f2da0eb381e0805f47aec964ae..6c962402155d60b82610d4f65af5182d6faa47ad
> 100644
> --- a/gcc/config/aarch64/aarch64-linux.h
> +++ b/gcc/config/aarch64/aarch64-linux.h
> @@ -70,7 +70,7 @@
> strong definitions in dependent shared libraries, will resolve
> to COPY relocated symbol in the executable. See PR65780. */
> #undef TARGET_BINDS_LOCAL_P
> -#define TARGET_BINDS_LOCAL_P default_binds_local_p_2
> +#define TARGET_BINDS_LOCAL_P aarch64_binds_local_p
>
> /* Define this to be nonzero if static stack checking is supported. */
> #define STACK_CHECK_STATIC_BUILTIN 1
> diff --git a/gcc/config/aarch64/aarch64-protos.h
> b/gcc/config/aarch64/aarch64-protos.h
> index
> 238820581c5ee7617f8eed1df2cf5418b1127e19..fac754f78c1d7606ba90e1034820a62466b96b63
> 100644
> --- a/gcc/config/aarch64/aarch64-protos.h
> +++ b/gcc/config/aarch64/aarch64-protos.h
> @@ -1072,5 +1072,6 @@ const char *aarch64_sls_barrier (int);
> const char *aarch64_indirect_call_asm (rtx);
> extern bool aarch64_harden_sls_retbr_p (void);
> extern bool aarch64_harden_sls_blr_p (void);
> +extern bool aarch64_binds_local_p (const_tree);
>
> #endif /* GCC_AARCH64_PROTOS_H */
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index
> d1f979ebcf80333d957f8ad8631deef47dc693a5..ab4c42c34da5b15f6739c9b0a7ebaafda9488f2d
> 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -19185,9 +19185,29 @@ aarch64_tlsdesc_abi_id ()
> static bool
> aarch64_symbol_binds_local_p (const_rtx x)
> {
> - return (SYMBOL_REF_DECL (x)
> - ? targetm.binds_local_p (SYMBOL_REF_DECL (x))
> - : SYMBOL_REF_LOCAL_P (x));
> + if (!SYMBOL_REF_DECL (x))
> + return SYMBOL_REF_LOCAL_P (x);
> +
> + if (targetm.binds_local_p (SYMBOL_REF_DECL (x)))
> + return true;
> +
> + /* In PIE binaries avoid a GOT indirection on non-weak data symbols if
> + aarch64_direct_extern_access is true. */
> + if (flag_pie && aarch64_direct_extern_access && !SYMBOL_REF_WEAK (x)
> + && !SYMBOL_REF_FUNCTION_P (x))
> + return true;
> +
> + return false;
> +}
> +
> +/* Implement TARGET_BINDS_LOCAL_P hook. */
> +
> +bool
> +aarch64_binds_local_p (const_tree exp)
> +{
> + /* Protected symbols are local if aarch64_direct_extern_access is true. */
> + return default_binds_local_p_3 (exp, flag_shlib != 0, true,
> + !aarch64_direct_extern_access, !flag_pic);
> }
>
> /* Return true if SYMBOL_REF X is thread local */
> diff --git a/gcc/config/aarch64/aarch64.opt b/gcc/config/aarch64/aarch64.opt
> index
> b89b20450710592101b93f4f3b5dc33d152d1eb6..6251a36b544a03955361b445c9f5dfad3740eea8
> 100644
> --- a/gcc/config/aarch64/aarch64.opt
> +++ b/gcc/config/aarch64/aarch64.opt
> @@ -299,3 +299,7 @@ Constant memset size in bytes from which to start using
> MOPS sequence.
> -param=aarch64-vect-unroll-limit=
> Target Joined UInteger Var(aarch64_vect_unroll_limit) Init(4) Param
> Limit how much the autovectorizer may unroll a loop.
> +
> +mdirect-extern-access
> +Target Var(aarch64_direct_extern_access) Init(0)
> +Do not indirect accesses to external symbols via the GOT.
> diff --git
> a/gcc/doc/gcc/gcc-command-options/machine-dependent-options/aarch64-options.rst
>
> b/gcc/doc/gcc/gcc-command-options/machine-dependent-options/aarch64-options.rst
> index
> c2b23a6ee97ef2b7c74119f22c1d3e3d85385f4d..599c37fe299dc142d25d2133a4cd0b861e34fd01
> 100644
> ---
> a/gcc/doc/gcc/gcc-command-options/machine-dependent-options/aarch64-options.rst
> +++
> b/gcc/doc/gcc/gcc-command-options/machine-dependent-options/aarch64-options.rst
> @@ -389,6 +389,20 @@ These options are defined for AArch64 implementations:
> The default is :samp:`-msve-vector-bits=scalable`, which produces
> vector-length agnostic code.
>
> +.. option:: -mdirect-extern-access, -mno-direct-extern-access
> +
> + Use direct accesses for external data symbols. It avoids a GOT indirection
> + on all external data symbols with :option:`-fpie` or :option:`-fPIE`.
> This is
> + useful for executables linked with :option:`-static` or
> :option:`-static-pie`.
> + With :option:`-fpic` or :option:`-fPIC`, it only affects accesses to
> protected
> + data symbols. It has no effect on non-position independent code. The
> default
> + is :option:`-mno-direct-extern-access`.
> +
> + .. warning::
> +
> + Use :option:`-mdirect-extern-access` either in shared libraries or in
> + executables, but not in both. Protected symbols used both in a shared
> + library and executable may cause linker errors or fail to work correctly.
>
> .. _aarch64-feature-modifiers:
>
> diff --git a/gcc/doc/gcc/gcc-command-options/option-summary.rst
> b/gcc/doc/gcc/gcc-command-options/option-summary.rst
> index
> d068f98feac27d95f1402a530a78b553d623d2e9..dbc9b45ae1db12737aca3a6fd246b88a0e9467c2
> 100644
> --- a/gcc/doc/gcc/gcc-command-options/option-summary.rst
> +++ b/gcc/doc/gcc/gcc-command-options/option-summary.rst
> @@ -634,7 +634,8 @@ in the following sections.
> :option:`-moverride=string` :option:`-mverbose-cost-dump` |gol|
> :option:`-mstack-protector-guard=guard`
> :option:`-mstack-protector-guard-reg=sysreg` |gol|
> :option:`-mstack-protector-guard-offset=offset`
> :option:`-mtrack-speculation` |gol|
> - :option:`-moutline-atomics`
> + :option:`-moutline-atomics` |gol|
> + :option:`-mdirect-extern-access`
>
> *Adapteva Epiphany Options*
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr66912-2.c
> b/gcc/testsuite/gcc.target/aarch64/pr66912-2.c
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..58e6e1f37116bff77015a7321890ece30c9e6a5c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/pr66912-2.c
> @@ -0,0 +1,41 @@
> +/* { dg-do compile { target *-*-linux* } } */
> +/* { dg-options "-O2 -fpic" } */
> +/* { dg-require-effective-target fpic } */
> +
> +__attribute__((visibility("protected")))
> +int n_common;
> +
> +__attribute__((weak, visibility("protected")))
> +int n_weak_common;
> +
> +__attribute__((visibility("protected")))
> +int n_init = -1;
> +
> +__attribute__((weak, visibility("protected")))
> +int n_weak_init = -1;
> +
> +int
> +f1 ()
> +{
> + return n_common;
> +}
> +
> +int
> +f2 ()
> +{
> + return n_weak_common;
> +}
> +
> +int
> +f3 ()
> +{
> + return n_init;
> +}
> +
> +int
> +f4 ()
> +{
> + return n_weak_init;
> +}
> +
> +/* { dg-final { scan-assembler-times ":got" 0 } } */
