On Thu, Feb 27, 2025 at 2:53 AM H. Peter Anvin <hpa@xxxxxxxxx> wrote: > On February 26, 2025 10:28:59 PM PST, Eric Biggers <ebiggers@xxxxxxxxxx> wrote: > >On Wed, Feb 26, 2025 at 10:12:47PM -0800, Bill Wendling wrote: > >> For both gcc and clang, crc32 builtins generate better code than the > >> inline asm. GCC improves, removing unneeded "mov" instructions. Clang > >> does the same and unrolls the loops. GCC has no changes on i386, but > >> Clang's code generation is vastly improved, due to Clang's "rm" > >> constraint issue. > >> > >> The number of cycles improved by ~0.1% for GCC and ~1% for Clang, which > >> is expected because of the "rm" issue. However, Clang's performance is > >> better than GCC's by ~1.5%, most likely due to loop unrolling. > >> > >> Link: https://github.com/llvm/llvm-project/issues/20571#issuecomment-2649330009 > >> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx> > >> Cc: Ingo Molnar <mingo@xxxxxxxxxx> > >> Cc: Borislav Petkov <bp@xxxxxxxxx> > >> Cc: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx> > >> Cc: x86@xxxxxxxxxx > >> Cc: "H. Peter Anvin" <hpa@xxxxxxxxx> > >> Cc: Eric Biggers <ebiggers@xxxxxxxxxx> > >> Cc: Ard Biesheuvel <ardb@xxxxxxxxxx> > >> Cc: Nathan Chancellor <nathan@xxxxxxxxxx> > >> Cc: Nick Desaulniers <nick.desaulniers+lkml@xxxxxxxxx> > >> Cc: Justin Stitt <justinstitt@xxxxxxxxxx> > >> Cc: linux-kernel@xxxxxxxxxxxxxxx > >> Cc: linux-crypto@xxxxxxxxxxxxxxx > >> Cc: llvm@xxxxxxxxxxxxxxx > >> Signed-off-by: Bill Wendling <morbo@xxxxxxxxxx> > >> --- > >> arch/x86/Makefile | 3 +++ > >> arch/x86/lib/crc32-glue.c | 8 ++++---- > >> 2 files changed, 7 insertions(+), 4 deletions(-) > > > >Thanks! A couple concerns, though: > > > >> diff --git a/arch/x86/Makefile b/arch/x86/Makefile > >> index 5b773b34768d..241436da1473 100644 > >> --- a/arch/x86/Makefile > >> +++ b/arch/x86/Makefile > >> @@ -114,6 +114,9 @@ else > >> KBUILD_CFLAGS += $(call cc-option,-fcf-protection=none) > >> endif > >> > >> +# Enables the use of CRC32 builtins. > >> +KBUILD_CFLAGS += -mcrc32 > > > >Doesn't this technically allow the compiler to insert CRC32 instructions > >anywhere in arch/x86/ without the needed runtime CPU feature check? Normally > >when using intrinsics it's necessary to limit the scope of the feature > >enablement to match the runtime CPU feature check that is done, e.g. by using > >the target function attribute. > > > >> diff --git a/arch/x86/lib/crc32-glue.c b/arch/x86/lib/crc32-glue.c > >> index 2dd18a886ded..fdb94bff25f4 100644 > >> --- a/arch/x86/lib/crc32-glue.c > >> +++ b/arch/x86/lib/crc32-glue.c > >> @@ -48,9 +48,9 @@ u32 crc32_le_arch(u32 crc, const u8 *p, size_t len) > >> EXPORT_SYMBOL(crc32_le_arch); > >> > >> #ifdef CONFIG_X86_64 > >> -#define CRC32_INST "crc32q %1, %q0" > >> +#define CRC32_INST __builtin_ia32_crc32di > >> #else > >> -#define CRC32_INST "crc32l %1, %0" > >> +#define CRC32_INST __builtin_ia32_crc32si > >> #endif > > > >Do both gcc and clang consider these builtins to be a stable API, or do they > >only guarantee the stability of _mm_crc32_*() from immintrin.h? At least for > >the rest of the SSE and AVX stuff, I thought that only the immintrin.h functions > >are actually considered stable. > > > >- Eric > > There is that... also are there compiler versions that we support that do not have -mcrc32 support? > Checking GCC 5.1.0 and Clang 13.0.1, it seems that both support '-mcrc32'. -bw
