Messages in this thread Patch in this message | | | From | Vincent Mailhol <> | Subject | [PATCH v2 1/2] x86/asm/bitops: Replace __fls() by its generic builtin implementation | Date | Fri, 25 Nov 2022 11:33:29 +0900 |
| |
Below snippet:
#include <linux/bitops.h>
unsigned int foo(unsigned long word) { return __fls(word); }
produces this on GCC 12.1.0:
0000000000000000 <foo>: 0: f3 0f 1e fa endbr64 4: e8 00 00 00 00 call 9 <foo+0x9> 9: 53 push %rbx a: 48 89 fb mov %rdi,%rbx d: e8 00 00 00 00 call 12 <foo+0x12> 12: 48 0f bd c3 bsr %rbx,%rax 16: 5b pop %rbx 17: 31 ff xor %edi,%edi 19: e9 00 00 00 00 jmp 1e <foo+0x1e>
and that on clang 14.0.6:
0000000000000000 <foo>: 0: f3 0f 1e fa endbr64 4: e8 00 00 00 00 call 9 <foo+0x9> 9: 53 push %rbx a: 50 push %rax b: 48 89 fb mov %rdi,%rbx e: e8 00 00 00 00 call 13 <foo+0x13> 13: 48 89 1c 24 mov %rbx,(%rsp) 17: 48 0f bd 04 24 bsr (%rsp),%rax 1c: 48 83 c4 08 add $0x8,%rsp 20: 5b pop %rbx 21: c3 ret
The implementation from <asm-generic/bitops/builtin-__fls.h> [1] produces the exact same code on GCC and below code on clang:
0000000000000000 <foo>: 0: f3 0f 1e fa endbr64 4: e8 00 00 00 00 call 9 <foo+0x9> 9: 53 push %rbx a: 48 89 fb mov %rdi,%rbx d: e8 00 00 00 00 call 12 <foo+0x12> 12: 48 0f bd c3 bsr %rbx,%rax 16: 5b pop %rbx 17: c3 ret
The builtin implementation is better for two reasons:
1/ it saves two instructions on clang (a push and a stack pointer decrement).
2/ when used on constant expressions, the compiler is only able to fold the builtin version (c.f. [2]).
For those two reasons, replace the assembly implementation by its builtin counterpart.
[1] https://elixir.bootlin.com/linux/v6.0/source/include/asm-generic/bitops/builtin-__fls.h [2] commit 146034fed6ee ("x86/asm/bitops: Use __builtin_ffs() to evaluate constant expressions")
CC: Borislav Petkov <bp@suse.de> CC: Yury Norov <yury.norov@gmail.com> Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> --- arch/x86/include/asm/bitops.h | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/arch/x86/include/asm/bitops.h b/arch/x86/include/asm/bitops.h index 2edf68475fec..a31453d7686d 100644 --- a/arch/x86/include/asm/bitops.h +++ b/arch/x86/include/asm/bitops.h @@ -285,19 +285,7 @@ static __always_inline unsigned long variable_ffz(unsigned long word) (unsigned long)__builtin_ctzl(~word) : \ variable_ffz(word)) -/* - * __fls: find last set bit in word - * @word: The word to search - * - * Undefined if no set bit exists, so code should check against 0 first. - */ -static __always_inline unsigned long __fls(unsigned long word) -{ - asm("bsr %1,%0" - : "=r" (word) - : "rm" (word)); - return word; -} +#include <asm-generic/bitops/builtin-__fls.h> #undef ADDR -- 2.37.4
| |