| Message ID | 20200323020844.17064-6-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | x86: remove always-defined CONFIG_AS_* options | expand |
On Sun, Mar 22, 2020 at 8:10 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > diff --git a/lib/raid6/algos.c b/lib/raid6/algos.c > index bf1b4765c8f6..77457ea5a239 100644 > --- a/lib/raid6/algos.c > +++ b/lib/raid6/algos.c > @@ -103,9 +103,7 @@ const struct raid6_recov_calls *const raid6_recov_algos[] = { > #ifdef CONFIG_AS_AVX2 > &raid6_recov_avx2, > #endif > -#ifdef CONFIG_AS_SSSE3 > &raid6_recov_ssse3, > -#endif > #ifdef CONFIG_S390 > &raid6_recov_s390xc, > #endif algos.c is compiled on all platforms, so you'll need to ifdef that x86 section where SSSE3 is no longer guarding it. The pattern in the rest of the file, if you want to follow it, is "#if defined(__x86_64__) && !defined(__arch_um__)". That seems ugly and like there are better ways, but in the interest of uniformity and a lack of desire to rewrite all the raid6 code, I went with that in this cleanup: https://git.zx2c4.com/linux-dev/commit/?h=jd/kconfig-assembler-support&id=512a00ddebbe5294a88487dcf1dc845cf56703d9
On Tue, Mar 24, 2020 at 3:06 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > On Sun, Mar 22, 2020 at 8:10 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > diff --git a/lib/raid6/algos.c b/lib/raid6/algos.c > > index bf1b4765c8f6..77457ea5a239 100644 > > --- a/lib/raid6/algos.c > > +++ b/lib/raid6/algos.c > > @@ -103,9 +103,7 @@ const struct raid6_recov_calls *const raid6_recov_algos[] = { > > #ifdef CONFIG_AS_AVX2 > > &raid6_recov_avx2, > > #endif > > -#ifdef CONFIG_AS_SSSE3 > > &raid6_recov_ssse3, > > -#endif > > #ifdef CONFIG_S390 > > &raid6_recov_s390xc, > > #endif > > algos.c is compiled on all platforms, so you'll need to ifdef that x86 > section where SSSE3 is no longer guarding it. The pattern in the rest > of the file, if you want to follow it, is "#if defined(__x86_64__) && > !defined(__arch_um__)". That seems ugly and like there are better > ways, but in the interest of uniformity and a lack of desire to > rewrite all the raid6 code, I went with that in this cleanup: > > https://git.zx2c4.com/linux-dev/commit/?h=jd/kconfig-assembler-support&id=512a00ddebbe5294a88487dcf1dc845cf56703d9 Thanks for the pointer, but I think guarding with CONFIG_X86 makes more sense. raid6_recov_ssse3 is defined in lib/raid6/recov_ssse3.c, which is guarded by like this: raid6_pq-$(CONFIG_X86) += recov_ssse3.o recov_avx2.o mmx.o sse1.o sse2.o avx2.o avx512.o recov_avx512.o Indeed, #if defined(__x86_64__) && !defined(__arch_um__) is ugly. I wonder why the code was written like that. I rather want to check a single CONFIG option. Please see the attached patch. > -- > You received this message because you are subscribed to the Google Groups "Clang Built Linux" group. > To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/CAHmME9p3LAnrUMmcGPEUFqY5vOASe8MVk4%3DpzqFRj3E9C-bM%2BQ%40mail.gmail.com.
On Mon, Mar 23, 2020 at 2:45 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Tue, Mar 24, 2020 at 3:06 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > > > On Sun, Mar 22, 2020 at 8:10 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > diff --git a/lib/raid6/algos.c b/lib/raid6/algos.c > > > index bf1b4765c8f6..77457ea5a239 100644 > > > --- a/lib/raid6/algos.c > > > +++ b/lib/raid6/algos.c > > > @@ -103,9 +103,7 @@ const struct raid6_recov_calls *const raid6_recov_algos[] = { > > > #ifdef CONFIG_AS_AVX2 > > > &raid6_recov_avx2, > > > #endif > > > -#ifdef CONFIG_AS_SSSE3 > > > &raid6_recov_ssse3, > > > -#endif > > > #ifdef CONFIG_S390 > > > &raid6_recov_s390xc, > > > #endif > > > > algos.c is compiled on all platforms, so you'll need to ifdef that x86 > > section where SSSE3 is no longer guarding it. The pattern in the rest > > of the file, if you want to follow it, is "#if defined(__x86_64__) && > > !defined(__arch_um__)". That seems ugly and like there are better > > ways, but in the interest of uniformity and a lack of desire to > > rewrite all the raid6 code, I went with that in this cleanup: > > > > https://git.zx2c4.com/linux-dev/commit/?h=jd/kconfig-assembler-support&id=512a00ddebbe5294a88487dcf1dc845cf56703d9 > > > Thanks for the pointer, > but I think guarding with CONFIG_X86 makes more sense. > > raid6_recov_ssse3 is defined in lib/raid6/recov_ssse3.c, > which is guarded by like this: > > raid6_pq-$(CONFIG_X86) += recov_ssse3.o recov_avx2.o mmx.o sse1.o > sse2.o avx2.o avx512.o recov_avx512.o > > > Indeed, > > #if defined(__x86_64__) && !defined(__arch_um__) > > is ugly. > > > I wonder why the code was written like that. > > I rather want to check a single CONFIG option. > Please see the attached patch. Seems better indeed. Looks like you've cleaned up multiple cases. Now if you could only tell me what is wrong with my series... "Your series does not work correctly. I will comment why later." I've been at the edge of my seat, Fermat's last theorem style. :) By the way, it looks like 5.7 will be raising the minimum binutils to 2.23: https://lore.kernel.org/lkml/20200316160259.GN26126@zn.tnic/ In light of this, I'll place another patch on top of my branch handling that transition. Jason
On Mon, Mar 23, 2020 at 2:48 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > By the way, it looks like 5.7 will be raising the minimum binutils to > 2.23: https://lore.kernel.org/lkml/20200316160259.GN26126@zn.tnic/ In > light of this, I'll place another patch on top of my branch handling > that transition. That now lives at the top of the usual branch: https://git.zx2c4.com/linux-dev/log/?h=jd/kconfig-assembler-support
diff --git a/arch/x86/Makefile b/arch/x86/Makefile index e4a062313bb0..94f89612e024 100644 --- a/arch/x86/Makefile +++ b/arch/x86/Makefile @@ -178,7 +178,6 @@ ifeq ($(ACCUMULATE_OUTGOING_ARGS), 1) endif # does binutils support specific instructions? -asinstr += $(call as-instr,pshufb %xmm0$(comma)%xmm0,-DCONFIG_AS_SSSE3=1) avx_instr := $(call as-instr,vxorps %ymm0$(comma)%ymm1$(comma)%ymm2,-DCONFIG_AS_AVX=1) avx2_instr :=$(call as-instr,vpbroadcastb %xmm0$(comma)%ymm1,-DCONFIG_AS_AVX2=1) avx512_instr :=$(call as-instr,vpmovm2b %k1$(comma)%zmm5,-DCONFIG_AS_AVX512=1) @@ -186,8 +185,8 @@ sha1_ni_instr :=$(call as-instr,sha1msg1 %xmm0$(comma)%xmm1,-DCONFIG_AS_SHA1_NI= sha256_ni_instr :=$(call as-instr,sha256msg1 %xmm0$(comma)%xmm1,-DCONFIG_AS_SHA256_NI=1) adx_instr := $(call as-instr,adox %r10$(comma)%r10,-DCONFIG_AS_ADX=1) -KBUILD_AFLAGS += $(asinstr) $(avx_instr) $(avx2_instr) $(avx512_instr) $(sha1_ni_instr) $(sha256_ni_instr) $(adx_instr) -KBUILD_CFLAGS += $(asinstr) $(avx_instr) $(avx2_instr) $(avx512_instr) $(sha1_ni_instr) $(sha256_ni_instr) $(adx_instr) +KBUILD_AFLAGS += $(avx_instr) $(avx2_instr) $(avx512_instr) $(sha1_ni_instr) $(sha256_ni_instr) $(adx_instr) +KBUILD_CFLAGS += $(avx_instr) $(avx2_instr) $(avx512_instr) $(sha1_ni_instr) $(sha256_ni_instr) $(adx_instr) KBUILD_LDFLAGS := -m elf_$(UTS_MACHINE) diff --git a/arch/x86/crypto/blake2s-core.S b/arch/x86/crypto/blake2s-core.S index 24910b766bdd..2ca79974f819 100644 --- a/arch/x86/crypto/blake2s-core.S +++ b/arch/x86/crypto/blake2s-core.S @@ -46,7 +46,6 @@ SIGMA2: #endif /* CONFIG_AS_AVX512 */ .text -#ifdef CONFIG_AS_SSSE3 SYM_FUNC_START(blake2s_compress_ssse3) testq %rdx,%rdx je .Lendofloop @@ -174,7 +173,6 @@ SYM_FUNC_START(blake2s_compress_ssse3) .Lendofloop: ret SYM_FUNC_END(blake2s_compress_ssse3) -#endif /* CONFIG_AS_SSSE3 */ #ifdef CONFIG_AS_AVX512 SYM_FUNC_START(blake2s_compress_avx512) diff --git a/lib/raid6/algos.c b/lib/raid6/algos.c index bf1b4765c8f6..77457ea5a239 100644 --- a/lib/raid6/algos.c +++ b/lib/raid6/algos.c @@ -103,9 +103,7 @@ const struct raid6_recov_calls *const raid6_recov_algos[] = { #ifdef CONFIG_AS_AVX2 &raid6_recov_avx2, #endif -#ifdef CONFIG_AS_SSSE3 &raid6_recov_ssse3, -#endif #ifdef CONFIG_S390 &raid6_recov_s390xc, #endif diff --git a/lib/raid6/recov_ssse3.c b/lib/raid6/recov_ssse3.c index 1de97d2405d0..4bfa3c6b60de 100644 --- a/lib/raid6/recov_ssse3.c +++ b/lib/raid6/recov_ssse3.c @@ -3,8 +3,6 @@ * Copyright (C) 2012 Intel Corporation */ -#ifdef CONFIG_AS_SSSE3 - #include <linux/raid/pq.h> #include "x86.h" @@ -328,7 +326,3 @@ const struct raid6_recov_calls raid6_recov_ssse3 = { #endif .priority = 1, }; - -#else -#warning "your version of binutils lacks SSSE3 support" -#endif diff --git a/lib/raid6/test/Makefile b/lib/raid6/test/Makefile index 3ab8720aa2f8..79777645cac9 100644 --- a/lib/raid6/test/Makefile +++ b/lib/raid6/test/Makefile @@ -34,9 +34,6 @@ endif ifeq ($(IS_X86),yes) OBJS += mmx.o sse1.o sse2.o avx2.o recov_ssse3.o recov_avx2.o avx512.o recov_avx512.o - CFLAGS += $(shell echo "pshufb %xmm0, %xmm0" | \ - gcc -c -x assembler - >&/dev/null && \ - rm ./-.o && echo -DCONFIG_AS_SSSE3=1) CFLAGS += $(shell echo "vpbroadcastb %xmm0, %ymm1" | \ gcc -c -x assembler - >&/dev/null && \ rm ./-.o && echo -DCONFIG_AS_AVX2=1)
CONFIG_AS_SSSE3 was introduced by commit 75aaf4c3e6a4 ("x86/raid6: correctly check for assembler capabilities"). We raise the minimal supported binutils version from time to time. The last bump was commit 1fb12b35e5ff ("kbuild: Raise the minimum required binutils version to 2.21"). I confirmed the code in $(call as-instr,...) can be assembled by the binutils 2.21 assembler and also by LLVM integrated assembler. Remove CONFIG_AS_SSSE3, which is always defined. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- arch/x86/Makefile | 5 ++--- arch/x86/crypto/blake2s-core.S | 2 -- lib/raid6/algos.c | 2 -- lib/raid6/recov_ssse3.c | 6 ------ lib/raid6/test/Makefile | 3 --- 5 files changed, 2 insertions(+), 16 deletions(-)