Re: [PATCH] perf bench: Update arch/x86/lib/mem{cpy,set}_64.S
From: Arnaldo Carvalho de Melo
Date: Mon Nov 16 2020 - 12:02:26 EST
Em Tue, Nov 03, 2020 at 04:56:09PM -0800, Fangrui Song escreveu:
> In memset_64.S, the macros expand to `.weak MEMSET ... .globl MEMSET`
> which will produce a STB_WEAK MEMSET with GNU as but STB_GLOBAL MEMSET
> with LLVM's integrated assembler before LLVM 12. LLVM 12 (since
> https://reviews.llvm.org/D90108) will error on such an overridden symbol
> binding. memcpy_64.S is similar.
>
> Port http://lore.kernel.org/r/20201103012358.168682-1-maskray@xxxxxxxxxx
> ("x86_64: Change .weak to SYM_FUNC_START_WEAK for arch/x86/lib/mem*_64.S")
> to fix the issue. Additionally, port SYM_L_WEAK and SYM_FUNC_START_WEAK
> from include/linux/linkage.h to tools/perf/util/include/linux/linkage.h
Sorry, I noticed this just now and I have done this update already, will
send to Linus soon.
- Arnaldo
> Fixes: 7d7d1bf1d1da ("perf bench: Copy kernel files needed to build mem{cpy,set} x86_64 benchmarks")
> Link: https://lore.kernel.org/r/20201103012358.168682-1-maskray@xxxxxxxxxx
> Signed-off-by: Fangrui Song <maskray@xxxxxxxxxx>
> ---
> tools/arch/x86/lib/memcpy_64.S | 4 +---
> tools/arch/x86/lib/memset_64.S | 4 +---
> tools/perf/util/include/linux/linkage.h | 7 +++++++
> 3 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/tools/arch/x86/lib/memcpy_64.S b/tools/arch/x86/lib/memcpy_64.S
> index 0b5b8ae56bd9..9428f251df0f 100644
> --- a/tools/arch/x86/lib/memcpy_64.S
> +++ b/tools/arch/x86/lib/memcpy_64.S
> @@ -16,8 +16,6 @@
> * to a jmp to memcpy_erms which does the REP; MOVSB mem copy.
> */
>
> -.weak memcpy
> -
> /*
> * memcpy - Copy a memory block.
> *
> @@ -30,7 +28,7 @@
> * rax original destination
> */
> SYM_FUNC_START_ALIAS(__memcpy)
> -SYM_FUNC_START_LOCAL(memcpy)
> +SYM_FUNC_START_WEAK(memcpy)
> ALTERNATIVE_2 "jmp memcpy_orig", "", X86_FEATURE_REP_GOOD, \
> "jmp memcpy_erms", X86_FEATURE_ERMS
>
> diff --git a/tools/arch/x86/lib/memset_64.S b/tools/arch/x86/lib/memset_64.S
> index fd5d25a474b7..1f9b11f9244d 100644
> --- a/tools/arch/x86/lib/memset_64.S
> +++ b/tools/arch/x86/lib/memset_64.S
> @@ -5,8 +5,6 @@
> #include <asm/cpufeatures.h>
> #include <asm/alternative-asm.h>
>
> -.weak memset
> -
> /*
> * ISO C memset - set a memory block to a byte value. This function uses fast
> * string to get better performance than the original function. The code is
> @@ -18,7 +16,7 @@
> *
> * rax original destination
> */
> -SYM_FUNC_START_ALIAS(memset)
> +SYM_FUNC_START_WEAK(memset)
> SYM_FUNC_START(__memset)
> /*
> * Some CPUs support enhanced REP MOVSB/STOSB feature. It is recommended
> diff --git a/tools/perf/util/include/linux/linkage.h b/tools/perf/util/include/linux/linkage.h
> index b8a5159361b4..0e493bf3151b 100644
> --- a/tools/perf/util/include/linux/linkage.h
> +++ b/tools/perf/util/include/linux/linkage.h
> @@ -25,6 +25,7 @@
>
> /* SYM_L_* -- linkage of symbols */
> #define SYM_L_GLOBAL(name) .globl name
> +#define SYM_L_WEAK(name) .weak name
> #define SYM_L_LOCAL(name) /* nothing */
>
> #define ALIGN __ALIGN
> @@ -78,6 +79,12 @@
> SYM_START(name, SYM_L_LOCAL, SYM_A_ALIGN)
> #endif
>
> +/* SYM_FUNC_START_WEAK -- use for weak functions */
> +#ifndef SYM_FUNC_START_WEAK
> +#define SYM_FUNC_START_WEAK(name) \
> + SYM_START(name, SYM_L_WEAK, SYM_A_ALIGN)
> +#endif
> +
> /* SYM_FUNC_END_ALIAS -- the end of LOCAL_ALIASed or ALIASed function */
> #ifndef SYM_FUNC_END_ALIAS
> #define SYM_FUNC_END_ALIAS(name) \
> --
> 2.29.1.341.ge80a0c044ae-goog
>
--
- Arnaldo