x86/bitops: Replace __scanbit() with a simpler tzcnt()
PATCH 1/1 x86/bitops: Replace __scanbit() with a simpler tzcnt()
There are 4 remaining users of __scanbit(), and they're capable of dealing with max being BITS_PER_LONG. Switch to a simpler tzcnt() helper, which also removed some alternatives from behind the scenes.
No functional change.
---
CC: Jan Beulich <[email protected]>
CC: Roger Pau Monné <[email protected]>
CC: Teddy Astie <[email protected]>
Supercedes "x86/bitops: Improve code generation for __scanbit()"
Bloat-o-meter shows rather less churn:
add/remove: 1/0 grow/shrink: 9/18 up/down: 244/-279 (-35)
Function old new delta
is_cpu_primary - 88 +88 vmap_size 38 91 +53 get_outstanding_claims 198 214 +16 csched2_unit_insert 413 429 +16 csched2_schedule 5371 5387 +16 __cpu_disable 583 599 +16 memory_add 4036 4049 +13 csched2_alloc_domdata 476 487 +11 numa_init_array 153 161 +8 dump_execstate 221 228 +7 dump_numa 471 469 -2 csched2_res_pick 1351 1349 -2 csched_schedule 3790 3786 -4 fixup_irqs 1655 1650 -5 dump_numa.cold 977 970 -7 cpupool_assign_cpu_locked 410 403 -7 _assign_irq_vector 1571 1564 -7 vmx_sync_exit_bitmap 73 65 -8 numa_emulation 487 479 -8 __find_next_zero_bit 145 137 -8 get_free_buddy 1014 1005 -9 runq_tickle 1753 1737 -16 node_to_scrub 533 517 -16 dom0_compute_nr_pages 1424 1408 -16 sched_init_vcpu 1811 1791 -20 microcode_nmi_callback 300 260 -40 heap_init_late 1659 1611 -48 do_microcode_update 1014 958 -56
Total: Before=2013616, After=2013581, chg -0.00% add/remove: 0/0 grow/shrink: 3/0 up/down: 36/0 (36)
Data old new delta
symbols_names 134797 134817 +20
symbols_sorted_offsets 69368 69376 +8 symbols_offsets 53456 53464 +8
For things like vmap_size() what has happened is that vm_size() has now been inlined.
--- xen/arch/x86/bitops.c | 4 ++-- xen/arch/x86/include/asm/bitops.h | 31 ++++++++++++++++++------------- 2 files changed, 20 insertions(+), 15 deletions(-)
xen/arch/x86/bitops.c +2 -2
@@ -35,7 +35,7 @@ unsigned int __find_next_bit(if ( bit !=0 ) {/* Look for a bit in the first word. */ - set =__scanbit (*p >> bit,BITS_PER_LONG ); + set =tzcnt (*p >> bit);if ( set <BITS_PER_LONG )return (offset + set); offset +=BITS_PER_LONG - bit;
@@ -85,7 +85,7 @@ unsigned int __find_next_zero_bit(if ( bit !=0 ) {/* Look for zero in the first word. */ - set =__scanbit (~*p >> bit,BITS_PER_LONG ); + set =tzcnt (~*p >> bit);if ( set <BITS_PER_LONG )return (offset + set); offset +=BITS_PER_LONG - bit;
xen/arch/x86/include/asm/bitops.h +18 -13
@@ -320,18 +320,23 @@ extern unsigned int __find_first_zero_bit(extern unsigned int __find_next_zero_bit (const unsigned long *addr,unsigned int size,unsigned int offset); -static always_inline unsigned int __scanbit(unsigned long val, unsigned int max) +/* +* Search for the first set bit in @x, returning BITS_PER_LONG if @x was 0. +* See arch_ffs() for safety discussions. +*/ +static always_inline unsigned int tzcnt(unsigned long x) { -if (__builtin_constant_p (max) && max ==BITS_PER_LONG ) -alternative_io ("bsf %[in],%[out]; cmovz %[max],%k[out]" , -"rep; bsf %[in],%[out]" , - X86_FEATURE_BMI1, - [out] "=&r" (val), - [in] "r" (val), [max] "r" (max)); -else - asm ("bsf %1,%0 ; cmovz %2,%k0" - :"=&r" (val) :"r" (val),"r" (max) ); -return (unsigned int )val; +unsigned int r; + +/* +* The destination register still needs pre-loading for older CPUs which +* will execute TZCNT as BSF. +*/ + asm ("tzcnt %[x], %q[r]" + : [r]"=r" (r) + : [x]"rm" (x),"[r]" (BITS_PER_LONG ) ); + +return r; }/**
@@ -358,7 +363,7 @@ static always_inline unsigned int __scanbit(unsigned long val, unsigned int max)if ( o__ >= s__ ) \ r__ = s__; \else if (__builtin_constant_p (size) && s__ <=BITS_PER_LONG ) \ - r__ = o__ +__scanbit (*a__ >> o__, s__); \ + r__ = o__ +tzcnt (*a__ >> o__); \else if (__builtin_constant_p (off) && !o__ ) \ r__ =__find_first_bit (a__, s__); \else \
@@ -390,7 +395,7 @@ static always_inline unsigned int __scanbit(unsigned long val, unsigned int max)if ( o__ >= s__ ) \ r__ = s__; \else if (__builtin_constant_p (size) && s__ <=BITS_PER_LONG ) \ - r__ = o__ +__scanbit (~*a__ >> o__, s__); \ + r__ = o__ +tzcnt (~*a__ >> o__); \else if (__builtin_constant_p (off) && !o__ ) \ r__ =__find_first_zero_bit (a__, s__); \else \
base-commit: 204e45847d5a743b611de989835f651fcfd6ddc0
Signature · 1 line
2.39.5
Quoted 7 lines · Andrew Cooper
There are 4 remaining users of
__scanbit(), and they're capable of dealing with max beingBITS_PER_LONG. Switch to a simplertzcnt()helper, which also removed some alternatives from behind the scenes.No functional change.
Signed-off-by Andrew Cooper <[email protected]>
(with a suggestion below)
Quoted 1 line
Supercedes "x86/bitops: Improve code generation for
__scanbit()"
Quite a bit neater indeed.
Quoted 25 lines
xen/arch/x86/include/asm/bitops.h +7
-11@@ -320,18 +320,23 @@ extern unsigned int __find_first_zero_bit(extern unsigned int __find_next_zero_bit (const unsigned long *addr,unsigned int size,unsigned int offset); -static always_inline unsigned int __scanbit(unsigned long val, unsigned int max) +/* +* Search for the first set bit in @x, returning BITS_PER_LONG if @x was 0. +* See arch_ffs() for safety discussions. +*/ +static always_inline unsigned int tzcnt(unsigned long x) { -if (__builtin_constant_p (max) && max ==BITS_PER_LONG ) -alternative_io ("bsf %[in],%[out]; cmovz %[max],%k[out]" , -"rep; bsf %[in],%[out]" , - X86_FEATURE_BMI1, - [out] "=&r" (val), - [in] "r" (val), [max] "r" (max)); -else - asm ("bsf %1,%0 ; cmovz %2,%k0" - :"=&r" (val) :"r" (val),"r" (max) ); -return (unsigned int )val; +unsigned int r; +
xen/arch/x86/include/asm/bitops.h extern unsigned int __find_first_zero_bit( · line 340
Quoted 10 lines
xen/arch/x86/include/asm/bitops.h +9
-0+/* +* The destination register still needs pre-loading for older CPUs which +* will execute TZCNT as BSF. +*/ + asm ("tzcnt %[x], %q[r]" + : [r]"=r" (r) + : [x]"rm" (x),"[r]" (BITS_PER_LONG ) ); + +return r; }
By having r and the function return type be unsigned int, when the actual insn really is a qword operation, you force call sites to do the zero- extension if they want an unsigned long quantity. When both are unsigned long, no zero-extension needs performing by the compiler. Truncation to unsigned int (or less) won't require any extra insn(s) anyway. While this doesn't look as if it would make a difference for present uses, future ones may benefit.
Jan