Get rid of all NEON intrinsics stdint.h type hacks
Subsystems (7)
cover Get rid of all NEON intrinsics stdint.h type hacks
From: Ard Biesheuvel <[email protected]>
Recent Clang has become more finicky when it comes to overriding the type macros that the freestanding stdint.h consumes when defining the uintXX_t types, and so it is time to finally get rid of this entirely.
So instead of making stdint.h (which comes from the compiler when -ffreestanding is set on the command line) agree with the kernel's linux/types.h header, avoid the latter entirely in the compilation units that actually use the NEON intrinsics (which are always leaf functions that are not tightly coupled to kernel APIs).
Note that the disagreements are about long vs. long long on arm64, and long vs. int on 32-bit ARM, so never about types that actually have conflicting representations.
Ard Biesheuvel (4):
lib/crc: Don't include <linux/types.h> in NEON intrinsics code
xor: arm/arm64: Move NEON xor_gen() dispatch into the glue code
arm64: Drop stdint.h type overrides from asm/neon-intrinsics.h
ARM: Drop stdint.h type overrides from asm/neon-intrinsics.h
Documentation/arch/arm/kernel_mode_neon.rst | 10 +++-- arch/arm/include/asm/neon-intrinsics.h | 43 +++----------------- arch/arm64/include/asm/neon-intrinsics.h | 24 +++-------- lib/crc/crc64-neon.c | 10 ++--- lib/raid/xor/arm/xor-neon-glue.c | 3 ++ lib/raid/xor/arm/xor-neon.h | 16 +++++++- lib/raid/xor/arm64/xor-eor3.c | 15 ++----- lib/raid/xor/arm64/xor-neon-glue.c | 5 +++ lib/raid/xor/arm64/xor-neon.h | 31 ++++++++++++-- lib/raid/xor/xor-neon.c | 16 ++------ 10 files changed, 78 insertions(+), 95 deletions(-)
Signature · 1 line
2.56.0.313.g2479322e68-goog
PATCH 1/4 lib/crc: Don't include <linux/types.h> in NEON intrinsics code
From: Ard Biesheuvel <[email protected]>
The kernel's definitions of uint64_t and int64_t in <linux/types.h> are based on [unsigned] long long, whereas the ones provided by <stdint.h>, which is included by <arm_neon.h>, are based on [unsigned] long on arm64. On 32-bit ARM, a similar mismatch exists for intptr_t.
Including both into the same compilation unit currently only works because asm/neon-intrinsics.h overrides the predefined macros that the compiler's freestanding <stdint.h> uses to define these types. In order to be able to get rid of those overrides, stop including <linux/types.h> into the CRC64 NEON intrinsics code.
Instead, use <stddef.h> for the definition of size_t, and rely on <asm/neon-intrinsics.h> to provide the u8 and u64 types used in the prototype of crc64_nvme_neon(), which is shared with code that does include <linux/types.h>. Use uint64_t for the arrays of constants that are passed to vld1q_u64(), so that they match its prototype regardless of how uint64_t is defined.
No change in generated code.
--- lib/crc/crc64-neon.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
lib/crc/crc64-neon.c +5 -5
@@ -3,7 +3,7 @@* Accelerated CRC64 (NVMe) using ARM NEON C intrinsics */ -#include <linux/types.h> +#include <stddef.h> #include <asm/neon-intrinsics.h> #include "crc64-neon.h"
@@ -11,11 +11,11 @@u64 crc64_nvme_neon (u64 crc,const u8 *p,size_t len);/* x^191 mod G, x^127 mod G */ -static const u64 fold_consts_val[2 ] = {0xeadc41fd2ba3d420ULL , -0x21e9761e252621acULL }; +static const uint64_t fold_consts_val[2 ] = {0xeadc41fd2ba3d420ULL , +0x21e9761e252621acULL };/* floor(x^127 / G), (G - x^64) / x */ -static const u64 bconsts_val[2 ] = {0x27ecfa329aef9f77ULL , -0x34d926535897936aULL }; +static const uint64_t bconsts_val[2 ] = {0x27ecfa329aef9f77ULL , +0x34d926535897936aULL };u64 crc64_nvme_neon (u64 crc,const u8 *p,size_t len) {
Signature · 1 line
2.56.0.313.g2479322e68-goog
PATCH 2/4 xor: arm/arm64: Move NEON xor_gen() dispatch into the glue code
In reply to Ard BiesheuvelFrom: Ard Biesheuvel <[email protected]>
The NEON intrinsics code in xor-neon.c and arm64/xor-eor3.c includes xor_impl.h for the definition of __DO_XOR_BLOCKS(), which pulls in <linux/types.h> via <linux/init.h> and <linux/minmax.h>. This only works because asm/neon-intrinsics.h overrides the compiler's predefined macros that <stdint.h> uses to define the C99 types, so that they match the kernel's definitions in <linux/types.h>.
So instead, follow the approach taken by LoongArch, and instantiate the dispatch routines using DO_XOR_BLOCKS() in the glue code, which already includes the kernel headers, and does not use NEON intrinsics itself. This requires the NEON routines that handle a fixed number of sources to be exposed to the glue code, which means that the arm64 glue code can use __xor_neon_2() directly for the EOR3 flavor as well, and so the __xor_eor3_2() alias can be dropped.
This makes the NEON intrinsics code independent of the kernel's own headers, which will allow the macro overrides in asm/neon-intrinsics.h to be removed.
--- lib/raid/xor/arm/xor-neon-glue.c | 3 ++ lib/raid/xor/arm/xor-neon.h | 16 ++++++++-- lib/raid/xor/arm64/xor-eor3.c | 15 ++-------- lib/raid/xor/arm64/xor-neon-glue.c | 5 ++++ lib/raid/xor/arm64/xor-neon.h | 31 +++++++++++++++++--- lib/raid/xor/xor-neon.c | 16 +++------- 6 files changed, 56 insertions(+), 30 deletions(-)
lib/raid/xor/arm/xor-neon-glue.c +3 -0
@@ -5,6 +5,9 @@#include "xor_impl.h" #include "xor_arch.h" +DO_XOR_BLOCKS (neon_inner, __xor_neon_2, __xor_neon_3, __xor_neon_4, + __xor_neon_5); +static void xor_gen_neon (void *dest,void **srcs,unsigned int src_cnt,unsigned int bytes) {
lib/raid/xor/arm/xor-neon.h +14 -2
@@ -3,5 +3,17 @@extern struct xor_block_template xor_block_arm4regs;extern struct xor_block_template xor_block_neon; -void xor_gen_neon_inner (void *dest,void **srcs,unsigned int src_cnt, -unsigned int bytes); +void __xor_neon_2 (unsigned long bytes,unsigned long * __restrict p1, +const unsigned long * __restrict p2); +void __xor_neon_3 (unsigned long bytes,unsigned long * __restrict p1, +const unsigned long * __restrict p2, +const unsigned long * __restrict p3); +void __xor_neon_4 (unsigned long bytes,unsigned long * __restrict p1, +const unsigned long * __restrict p2, +const unsigned long * __restrict p3, +const unsigned long * __restrict p4); +void __xor_neon_5 (unsigned long bytes,unsigned long * __restrict p1, +const unsigned long * __restrict p2, +const unsigned long * __restrict p3, +const unsigned long * __restrict p4, +const unsigned long * __restrict p5);
lib/raid/xor/arm64/xor-eor3.c +3 -12
@@ -1,14 +1,8 @@// SPDX-License-Identifier: GPL-2.0-only -#include <linux/cache.h> #include <asm/neon-intrinsics.h> -#include "xor_impl.h" -#include "xor_arch.h" #include "xor-neon.h" -extern void __xor_eor3_2 (unsigned long bytes,unsigned long * __restrict p1, -const unsigned long * __restrict p2); -static inline uint64x2_t eor3 (uint64x2_t p,uint64x2_t q,uint64x2_t r) {uint64x2_t res;
@@ -19,7 +13,7 @@ static inline uint64x2_t eor3(uint64x2_t p, uint64x2_t q, uint64x2_t r)return res; } -static void __xor_eor3_3 (unsigned long bytes,unsigned long * __restrict p1, +void __xor_eor3_3 (unsigned long bytes,unsigned long * __restrict p1,const unsigned long * __restrict p2,const unsigned long * __restrict p3) {
@@ -53,7 +47,7 @@ static void __xor_eor3_3(unsigned long bytes, unsigned long * __restrict p1, }while (--lines >0 ); } -static void __xor_eor3_4 (unsigned long bytes,unsigned long * __restrict p1, +void __xor_eor3_4 (unsigned long bytes,unsigned long * __restrict p1,const unsigned long * __restrict p2,const unsigned long * __restrict p3,const unsigned long * __restrict p4)
@@ -96,7 +90,7 @@ static void __xor_eor3_4(unsigned long bytes, unsigned long * __restrict p1, }while (--lines >0 ); } -static void __xor_eor3_5(unsigned long bytes, unsigned long * __restrict p1, +void __xor_eor3_5(unsigned long bytes, unsigned long * __restrict p1, const unsigned long * __restrict p2, const unsigned long * __restrict p3, const unsigned long * __restrict p4,
@@ -141,6 +135,3 @@ static void __xor_eor3_5(unsigned long bytes, unsigned long * __restrict p1, dp5 +=8 ; }while (--lines >0 ); } - -__DO_XOR_BLOCKS (eor3_inner, __xor_eor3_2, __xor_eor3_3, __xor_eor3_4, - __xor_eor3_5);
lib/raid/xor/arm64/xor-neon-glue.c +5 -0
@@ -9,6 +9,11 @@#include "xor_arch.h" #include "xor-neon.h" +DO_XOR_BLOCKS (neon_inner, __xor_neon_2, __xor_neon_3, __xor_neon_4, + __xor_neon_5); +DO_XOR_BLOCKS (eor3_inner, __xor_neon_2, __xor_eor3_3, __xor_eor3_4, + __xor_eor3_5); +#define XOR_TEMPLATE (_name) \ static void xor_gen_##_name(void *dest, void **srcs, unsigned int src_cnt, \ unsigned int bytes) \
lib/raid/xor/arm64/xor-neon.h +27 -4
@@ -1,6 +1,29 @@/* SPDX-License-Identifier: GPL-2.0-only */ -void xor_gen_neon_inner (void *dest,void **srcs,unsigned int src_cnt, -unsigned int bytes); -void xor_gen_eor3_inner (void *dest,void **srcs,unsigned int src_cnt, -unsigned int bytes); +void __xor_neon_2 (unsigned long bytes,unsigned long * __restrict p1, +const unsigned long * __restrict p2); +void __xor_neon_3 (unsigned long bytes,unsigned long * __restrict p1, +const unsigned long * __restrict p2, +const unsigned long * __restrict p3); +void __xor_neon_4 (unsigned long bytes,unsigned long * __restrict p1, +const unsigned long * __restrict p2, +const unsigned long * __restrict p3, +const unsigned long * __restrict p4); +void __xor_neon_5 (unsigned long bytes,unsigned long * __restrict p1, +const unsigned long * __restrict p2, +const unsigned long * __restrict p3, +const unsigned long * __restrict p4, +const unsigned long * __restrict p5); + +void __xor_eor3_3 (unsigned long bytes,unsigned long * __restrict p1, +const unsigned long * __restrict p2, +const unsigned long * __restrict p3); +void __xor_eor3_4 (unsigned long bytes,unsigned long * __restrict p1, +const unsigned long * __restrict p2, +const unsigned long * __restrict p3, +const unsigned long * __restrict p4); +void __xor_eor3_5 (unsigned long bytes,unsigned long * __restrict p1, +const unsigned long * __restrict p2, +const unsigned long * __restrict p3, +const unsigned long * __restrict p4, +const unsigned long * __restrict p5);
lib/raid/xor/xor-neon.c +4 -12
@@ -4,12 +4,11 @@* Copyright (C) 2018,Tianjin KYLIN Information Technology Co., Ltd. */ -#include "xor_impl.h" #include "xor-neon.h" #include <asm/neon-intrinsics.h> -static void __xor_neon_2 (unsigned long bytes,unsigned long * __restrict p1, +void __xor_neon_2 (unsigned long bytes,unsigned long * __restrict p1,const unsigned long * __restrict p2) {uint64_t *dp1 = (uint64_t *)p1;
@@ -36,7 +35,7 @@ static void __xor_neon_2(unsigned long bytes, unsigned long * __restrict p1, }while (--lines >0 ); } -static void __xor_neon_3 (unsigned long bytes,unsigned long * __restrict p1, +void __xor_neon_3 (unsigned long bytes,unsigned long * __restrict p1,const unsigned long * __restrict p2,const unsigned long * __restrict p3) {
@@ -72,7 +71,7 @@ static void __xor_neon_3(unsigned long bytes, unsigned long * __restrict p1, }while (--lines >0 ); } -static void __xor_neon_4 (unsigned long bytes,unsigned long * __restrict p1, +void __xor_neon_4 (unsigned long bytes,unsigned long * __restrict p1,const unsigned long * __restrict p2,const unsigned long * __restrict p3,const unsigned long * __restrict p4)
@@ -117,7 +116,7 @@ static void __xor_neon_4(unsigned long bytes, unsigned long * __restrict p1, }while (--lines >0 ); } -static void __xor_neon_5(unsigned long bytes, unsigned long * __restrict p1, +void __xor_neon_5(unsigned long bytes, unsigned long * __restrict p1, const unsigned long * __restrict p2, const unsigned long * __restrict p3, const unsigned long * __restrict p4,
@@ -170,10 +169,3 @@ static void __xor_neon_5(unsigned long bytes, unsigned long * __restrict p1, dp5 +=8 ; }while (--lines >0 ); } - -__DO_XOR_BLOCKS (neon_inner, __xor_neon_2, __xor_neon_3, __xor_neon_4, - __xor_neon_5); - -#ifdef CONFIG_ARM64 -extern typeof(__xor_neon_2) __xor_eor3_2 __alias(__xor_neon_2); -#endif
Signature · 1 line
2.56.0.313.g2479322e68-goog
PATCH 3/4 arm64: Drop stdint.h type overrides from asm/neon-intrinsics.h
In reply to Ard BiesheuvelFrom: Ard Biesheuvel <[email protected]>
None of the users of NEON intrinsics include <linux/types.h> anymore, and so there is no longer a need to override __INT64_TYPE__ and __UINT64_TYPE__ in order to make the definitions of int64_t and uint64_t provided by the compiler's freestanding <stdint.h> compatible with the kernel's. So drop the overrides.
This also means that uint64_t now matches the type that Clang uses for 64-bit quantities in the prototypes of its NEON builtins, and so the -Wincompatible-pointer-types warnings that are currently being disabled for all NEON intrinsics code will no longer occur. So drop the pragma that disables them as well.
--- arch/arm64/include/asm/neon-intrinsics.h | 24 ++++---------------- 1 file changed, 5 insertions(+), 19 deletions(-)
arch/arm64/include/asm/neon-intrinsics.h +5 -19
@@ -6,23 +6,13 @@#ifndef __ASM_NEON_INTRINSICS_H#define __ASM_NEON_INTRINSICS_H -#include <asm-generic/int-ll64.h> -/* -* In the kernel, u64/s64 are [un]signed long long, not [un]signed long. -* So by redefining these macros to the former, we can force gcc-stdint.h -* to define uint64_t / in64_t in a compatible manner. +* <arm_neon.h> includes <stdint.h>, which defines some C99 types differently +* from <linux/types.h>, so the latter must not be included into code that uses +* NEON intrinsics. The kernel's u8/u16/u32/u64 types can be used alongside the +* C99 ones, so provide those instead. */ - -#ifdef __INT64_TYPE__ -#undef __INT64_TYPE__ -#define __INT64_TYPE__ long long -#endif - -#ifdef __UINT64_TYPE__ -#undef __UINT64_TYPE__ -#define __UINT64_TYPE__ unsigned long long -#endif +#include <asm-generic/int-ll64.h> /* * genksyms chokes on the ARM NEON instrinsics system header, but we
@@ -33,8 +23,4 @@#include <arm_neon.h> #endif -#ifdef CONFIG_CC_IS_CLANG -#pragma clang diagnostic ignored "-Wincompatible-pointer-types" -#endif -#endif /* __ASM_NEON_INTRINSICS_H */
Signature · 1 line
2.56.0.313.g2479322e68-goog
PATCH 4/4 ARM: Drop stdint.h type overrides from asm/neon-intrinsics.h
In reply to Ard BiesheuvelFrom: Ard Biesheuvel <[email protected]>
None of the users of NEON intrinsics include <linux/types.h> anymore, and so there is no longer a need to override the compiler's predefined macros that are used to define the C99 [u]int32_t and [u]intptr_t types in order to make them compatible with the kernel's definitions. So drop the overrides from asm/neon-intrinsics.h.
Update the documentation accordingly.
--- Documentation/arch/arm/kernel_mode_neon.rst | 10 +++-- arch/arm/include/asm/neon-intrinsics.h | 43 +++----------------- 2 files changed, 12 insertions(+), 41 deletions(-)
Documentation/arch/arm/kernel_mode_neon.rst +7 -3
@@ -121,6 +121,10 @@ observe the following in addition to the rules above: * Compile the unit containing the NEON intrinsics with '-ffreestanding' so GCC uses its builtin version of <stdint.h> (this is a C99 header which the kernel does not supply); -* Do not include <arm_neon.h> directly: instead, include <asm/neon-intrinsics.h>, - which tweaks some macro definitions so that system headers can be included - safely. +* Do not include <arm_neon.h> directly: instead, include <asm/neon-intrinsics.h>; +* Do not include <linux/types.h>, or any other kernel header that includes it, + into the same compilation unit: some of the C99 types it defines (e.g., + intptr_t) are incompatible with the ones defined by <stdint.h>. The kernel's + own u8, u16, u32 and u64 types are provided by <asm/neon-intrinsics.h>, and + can be used safely in the prototypes of functions that are called from code + that does include <linux/types.h>.
arch/arm/include/asm/neon-intrinsics.h +5 -38
51 lines not on this page · raw message
Signature · 1 line
2.56.0.313.g2479322e68-goog
Hi Ard,
Quoted 3 lines · Ard Biesheuvel
Recent Clang has become more finicky when it comes to overriding the type macros that the freestanding
stdint.hconsumes when defining theuintXX_ttypes, and so it is time to finally get rid of this entirely.
Just as information for other folks:
https://github.com/llvm/llvm-project/pull/227722#issuecomment-6065215071
Quoted 5 lines
So instead of making
stdint.h(which comes from the compiler when -ffreestanding is set on the command line) agree with the kernel'slinux/types.hheader, avoid the latter entirely in the compilation units that actually use the NEON intrinsics (which are always leaf functions that are not tightly coupled to kernel APIs).
I had not even considered this as an option, thanks a lot for exploring it and sending this series as a result. I definitely think this is the preferred route forward based on my discussions with clang folks upstream since they were quite unhappy with stdint.h being overridden in this manner.
Quoted 9 lines
Note that the disagreements are about long vs. long long on arm64, and long vs. int on 32-bit ARM, so never about types that actually have conflicting representations.
Ard Biesheuvel (4):
lib/crc: Don't include <linux/types.h> in NEON intrinsics code
xor: arm/arm64: Move NEONxor_gen()dispatch into the glue code
arm64: Dropstdint.htype overrides fromasm/neon-intrinsics.h
ARM: Dropstdint.htype overrides fromasm/neon-intrinsics.h
This passes my arm and arm64 test matrix with LLVM 23.1.4 and a version of LLVM 24 with the breaking change mentioned above. This will be needed in stable but I don't mind taking a look at the backports when the time comes.
Signature · 2 lines
Cheers, Nathan
Quoted 20 lines · Ard Biesheuvel
From: Ard Biesheuvel <[email protected]>
Recent Clang has become more finicky when it comes to overriding the type macros that the freestanding
stdint.hconsumes when defining theuintXX_ttypes, and so it is time to finally get rid of this entirely.So instead of making
stdint.h(which comes from the compiler when -ffreestanding is set on the command line) agree with the kernel'slinux/types.hheader, avoid the latter entirely in the compilation units that actually use the NEON intrinsics (which are always leaf functions that are not tightly coupled to kernel APIs).Note that the disagreements are about long vs. long long on arm64, and long vs. int on 32-bit ARM, so never about types that actually have conflicting representations.
Cc Christoph Hellwig <[email protected]>Cc Eric Biggers <[email protected]>Cc Nathan Chancellor <[email protected]>
Quoted 5 lines
Ard Biesheuvel (4):
lib/crc: Don't include <linux/types.h> in NEON intrinsics code
xor: arm/arm64: Move NEONxor_gen()dispatch into the glue code
arm64: Dropstdint.htype overrides fromasm/neon-intrinsics.h
ARM: Dropstdint.htype overrides fromasm/neon-intrinsics.h
- Eric