From 1e7933a575ed8af4a64dd5089c2f6912da66dd79 Mon Sep 17 00:00:00 2001 From: I Hsin Cheng Date: Wed, 26 Feb 2025 14:56:23 +0800 Subject: uapi: Revert "bitops: avoid integer overflow in GENMASK(_ULL)" This patch reverts 'commit c32ee3d9abd2("bitops: avoid integer overflow in GENMASK(_ULL)")'. The code generation can be shrink by over 1KB by reverting this commit. Originally the commit claimed that clang would emit warnings using the implementation at that time. The patch was applied and tested against numerous compilers, including gcc-13, gcc-12, gcc-11 cross-compiler, clang-17, clang-18 and clang-19. Various warning levels were set (-W=0, -W=1, -W=2) and CONFIG_WERROR disabled to complete the compilation. The results show that no compilation errors or warnings were generated due to the patch. The results of code size reduction are summarized in the following table. The code size changes for clang are all zero across different versions, so they're not listed in the table. For NR_CPUS=64 on x86_64. ---------------------------------------------- | | gcc-13 | gcc-12 | gcc-11 | ---------------------------------------------- | old | 22438085 | 22453915 | 22302033 | ---------------------------------------------- | new | 22436816 | 22452913 | 22300826 | ---------------------------------------------- | new - old | -1269 | -1002 | -1207 | ---------------------------------------------- For NR_CPUS=1024 on x86_64. ---------------------------------------------- | | gcc-13 | gcc-12 | gcc-11 | ---------------------------------------------- | old | 22493682 | 22509812 | 22357661 | ---------------------------------------------- | new | 22493230 | 22509487 | 22357250 | ---------------------------------------------- | new - old | -452 | -325 | -411 | ---------------------------------------------- For arm64 architecture, gcc cross-compiler was used and QEMU was utilized to execute a VM for a CPU-heavy workload to ensure no side effects and that functionalities remained correct. The test even demonstrated a positive result in terms of code size reduction: * Before: 31660668 * After: 31658724 * Difference (After - Before): -1944 An analysis of multiple functions compiled with gcc-13 on x86_64 was performed. In summary, the patch elimates one negation in almost every use case. However, negative effects may occur in some cases, such as the generation of additional "mov" instruction or increased register usage. The use of "~_UL(0) << (l)" may even result in the allocations of "%r*" registers instead of "%e*" registers (which are 32-bit registers) because the compiler cannot assume that the higher bits are zero. Yury: We limit GENMASK() usage with the const_true(l > h) condition, and most of users just call it with constant parameters. For those, the actual implementation of the macro doesn't matter, and since it triggered clang warnings back then, it was reasonable to workaround the warnings on the kernel side. Now that some find_bit() functions call GENMASK() with runtime parameters (although the const_true() condition holds), this ended up hurting the generated code, as I Hsin discovered. This is especially bad because it hurts small_const_nbits() optimization, where people are most concerned about generated code quality. So, revert it to the original version for good. Signed-off-by: I Hsin Cheng Signed-off-by: Yury Norov --- include/uapi/linux/bits.h | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'include/uapi/linux') diff --git a/include/uapi/linux/bits.h b/include/uapi/linux/bits.h index 5ee30f882736..682b406e1067 100644 --- a/include/uapi/linux/bits.h +++ b/include/uapi/linux/bits.h @@ -4,13 +4,9 @@ #ifndef _UAPI_LINUX_BITS_H #define _UAPI_LINUX_BITS_H -#define __GENMASK(h, l) \ - (((~_UL(0)) - (_UL(1) << (l)) + 1) & \ - (~_UL(0) >> (__BITS_PER_LONG - 1 - (h)))) +#define __GENMASK(h, l) (((~_UL(0)) << (l)) & (~_UL(0) >> (BITS_PER_LONG - 1 - (h)))) -#define __GENMASK_ULL(h, l) \ - (((~_ULL(0)) - (_ULL(1) << (l)) + 1) & \ - (~_ULL(0) >> (__BITS_PER_LONG_LONG - 1 - (h)))) +#define __GENMASK_ULL(h, l) (((~_ULL(0)) << (l)) & (~_ULL(0) >> (BITS_PER_LONG_LONG - 1 - (h)))) #define __GENMASK_U128(h, l) \ ((_BIT128((h)) << 1) - (_BIT128(l))) -- cgit v1.2.3 From 0312e94abe484b9ee58c32d2f8ba177e04955b35 Mon Sep 17 00:00:00 2001 From: Vincent Mailhol Date: Wed, 5 Mar 2025 23:59:32 +0900 Subject: treewide: fix typo 'unsigned __init128' -> 'unsigned __int128' "int" was misspelled as "init" the code comments in the bits.h and const.h files. Fix the typo. CC: Andy Shevchenko Signed-off-by: Vincent Mailhol Signed-off-by: Yury Norov --- include/linux/bits.h | 2 +- include/uapi/linux/const.h | 2 +- tools/include/linux/bits.h | 2 +- tools/include/uapi/linux/const.h | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) (limited to 'include/uapi/linux') diff --git a/include/linux/bits.h b/include/linux/bits.h index 61a75d3f294b..14fd0ca9a6cd 100644 --- a/include/linux/bits.h +++ b/include/linux/bits.h @@ -40,7 +40,7 @@ * Missing asm support * * __GENMASK_U128() depends on _BIT128() which would not work - * in the asm code, as it shifts an 'unsigned __init128' data + * in the asm code, as it shifts an 'unsigned __int128' data * type instead of direct representation of 128 bit constants * such as long and unsigned long. The fundamental problem is * that a 128 bit constant will get silently truncated by the diff --git a/include/uapi/linux/const.h b/include/uapi/linux/const.h index e16be0d37746..b8f629ef135f 100644 --- a/include/uapi/linux/const.h +++ b/include/uapi/linux/const.h @@ -33,7 +33,7 @@ * Missing asm support * * __BIT128() would not work in the asm code, as it shifts an - * 'unsigned __init128' data type as direct representation of + * 'unsigned __int128' data type as direct representation of * 128 bit constants is not supported in the gcc compiler, as * they get silently truncated. * diff --git a/tools/include/linux/bits.h b/tools/include/linux/bits.h index 60044b608817..8de2914e6510 100644 --- a/tools/include/linux/bits.h +++ b/tools/include/linux/bits.h @@ -41,7 +41,7 @@ * Missing asm support * * __GENMASK_U128() depends on _BIT128() which would not work - * in the asm code, as it shifts an 'unsigned __init128' data + * in the asm code, as it shifts an 'unsigned __int128' data * type instead of direct representation of 128 bit constants * such as long and unsigned long. The fundamental problem is * that a 128 bit constant will get silently truncated by the diff --git a/tools/include/uapi/linux/const.h b/tools/include/uapi/linux/const.h index e16be0d37746..b8f629ef135f 100644 --- a/tools/include/uapi/linux/const.h +++ b/tools/include/uapi/linux/const.h @@ -33,7 +33,7 @@ * Missing asm support * * __BIT128() would not work in the asm code, as it shifts an - * 'unsigned __init128' data type as direct representation of + * 'unsigned __int128' data type as direct representation of * 128 bit constants is not supported in the gcc compiler, as * they get silently truncated. * -- cgit v1.2.3