[PATCH] csky: bitops: Use the compiler builtin for __fls()

[PATCH] csky: bitops: Use the compiler builtin for __fls()

[Tzu-Ching Yang]

The csky __fls() is a C wrapper, fls(x) - 1, around the ff1 asm
volatile in fls(), so the compiler can neither fold it for a constant
argument nor merge repeated calls. GCC implements __builtin_clzl()
with the same ff1 instruction for CK807, CK810 and CK860; CK610
(abiv1) cannot be built with upstream GCC.

Use the asm-generic builtin __fls() instead. ffs(), __ffs() and fls()
keep their asm: GCC has no ctz pattern for csky, so builtin ffs() and
__ffs() would become calls to __ffssi2() and __ctzsi2(), and a builtin
fls() needs a test for zero that ff1 makes unnecessary by returning 32
for 0.

__fls() now returns unsigned int instead of unsigned long, like the
asm-generic version; both are 32 bits wide on csky. __fls(0) remains
undefined.

With GCC 9 to 16, the text of every defconfig-based vmlinux tested
shrinks by 128 to 320 bytes.

Suggested-by: Arnd Bergmann <arnd@arndb.de>
Link: https://lore.kernel.org/all/affe7da1-41aa-4710-bee0-5e3c2b8a8dfc@app.fastmail.com/
Signed-off-by: Tzu-Ching Yang <shingekinocore@gmail.com>
---
In a test function, __fls(x) takes 4 instructions (return included)
before and after with each GCC and CPU tested, at -O2 and -Os; with a
constant argument, it now folds.

CK610 needs an abiv1 GCC; the only one I could find is the vendor's 6.3,
older than the 8.1 minimum and without a clz pattern for abiv1. There,
__fls() would become a call to __clzsi2(), which lib/clz_ctz.c
implements with the fls() asm, so it would still work; a builtin fls()
would make __clzsi2() call itself.

Testing: 35 defconfig-based kernels for CK807, CK810 and CK860, some
with KUnit or SMP, built before and after with W=1 by kernel.org GCC 9.5
to 16.2 (GCC 8 has no csky port, and GCC 9 to 11 reject -mcpu=ck860): no
new warnings and no calls to __ctzsi2(), __clzsi2() or __ffssi2().
allmodconfig with GCC 12 and 16 compiles the same objects with the same
warnings, but does not reach a vmlinux because of GCC internal compiler
errors, before and after. Under XuanTie's qemu-cskyv2 user mode, the old
and new helpers return the same values for every defined 32-bit input,
built with the kernel's flags by each GCC for each CPU at -O2 and -Os.
On qemu-system-cskyv2 -M virt, the ffs and bitops KUnit tests pass
before and after; test_bitmap passes with GCC 12 to 16 and has the same
one failure with GCC 9 to 11 in both, from a test bug fixed in
linux-next. No csky hardware was available.

Against the linux-next branch of c-sky/csky-linux.git (abb81e5ce7d9);
also applies to next-20261021 and to bitmap-for-next. Full build and
test data on request.

 arch/csky/include/asm/bitops.h | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/arch/csky/include/asm/bitops.h b/arch/csky/include/asm/bitops.h
index 80d67eee6e86..118fb0df5267 100644
--- a/arch/csky/include/asm/bitops.h
+++ b/arch/csky/include/asm/bitops.h
@@ -49,14 +49,7 @@ static __always_inline __attribute_const__ int fls(unsigned int x)
 	return (32 - x);
 }
 
-/*
- * asm-generic/bitops/__fls.h
- */
-static __always_inline __attribute_const__ unsigned long __fls(unsigned long x)
-{
-	return fls(x) - 1;
-}
-
+#include <asm-generic/bitops/builtin-__fls.h>
 #include <asm-generic/bitops/ffz.h>
 #include <asm-generic/bitops/fls64.h>
 

base-commit: abb81e5ce7d995baa41556b8125fa59e28ba3be8
-- 
2.34.1

Re: [PATCH] csky: bitops: Use the compiler builtin for __fls()

[Yury Norov]

On Thu, Oct 22, 2026 at 07:59:22PM +0800, Tzu-Ching Yang wrote:
> The csky __fls() is a C wrapper, fls(x) - 1, around the ff1 asm
> volatile in fls(), so the compiler can neither fold it for a constant
> argument nor merge repeated calls. GCC implements __builtin_clzl()
> with the same ff1 instruction for CK807, CK810 and CK860; CK610
> (abiv1) cannot be built with upstream GCC.
> 
> Use the asm-generic builtin __fls() instead. ffs(), __ffs() and fls()
> keep their asm: GCC has no ctz pattern for csky, so builtin ffs() and
> __ffs() would become calls to __ffssi2() and __ctzsi2(), and a builtin
> fls() needs a test for zero that ff1 makes unnecessary by returning 32
> for 0.
> 
> __fls() now returns unsigned int instead of unsigned long, like the
> asm-generic version; both are 32 bits wide on csky. __fls(0) remains
> undefined.
> 
> With GCC 9 to 16, the text of every defconfig-based vmlinux tested
> shrinks by 128 to 320 bytes.
> 
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Link: https://lore.kernel.org/all/affe7da1-41aa-4710-bee0-5e3c2b8a8dfc@app.fastmail.com/
> Signed-off-by: Tzu-Ching Yang <shingekinocore@gmail.com>
> ---
> In a test function, __fls(x) takes 4 instructions (return included)
> before and after with each GCC and CPU tested, at -O2 and -Os; with a
> constant argument, it now folds.
> 
> CK610 needs an abiv1 GCC; the only one I could find is the vendor's 6.3,
> older than the 8.1 minimum and without a clz pattern for abiv1. There,
> __fls() would become a call to __clzsi2(), which lib/clz_ctz.c
> implements with the fls() asm, so it would still work; a builtin fls()
> would make __clzsi2() call itself.
> 
> Testing: 35 defconfig-based kernels for CK807, CK810 and CK860, some
> with KUnit or SMP, built before and after with W=1 by kernel.org GCC 9.5
> to 16.2 (GCC 8 has no csky port, and GCC 9 to 11 reject -mcpu=ck860): no
> new warnings and no calls to __ctzsi2(), __clzsi2() or __ffssi2().
> allmodconfig with GCC 12 and 16 compiles the same objects with the same
> warnings, but does not reach a vmlinux because of GCC internal compiler
> errors, before and after. Under XuanTie's qemu-cskyv2 user mode, the old
> and new helpers return the same values for every defined 32-bit input,
> built with the kernel's flags by each GCC for each CPU at -O2 and -Os.
> On qemu-system-cskyv2 -M virt, the ffs and bitops KUnit tests pass
> before and after; test_bitmap passes with GCC 12 to 16 and has the same
> one failure with GCC 9 to 11 in both, from a test bug fixed in
> linux-next. No csky hardware was available.
> 
> Against the linux-next branch of c-sky/csky-linux.git (abb81e5ce7d9);
> also applies to next-20261021 and to bitmap-for-next. Full build and
> test data on request.

So I didn't understand if you have an access to the hardware or not. I
don't know the story behind using the non-generic version of ffl(),
but it may be for performance reasons. Can you / someone else please
run the lib/find_bit_benchmark.c on real hardware before we move this?

Thanks,
Yury
 
>  arch/csky/include/asm/bitops.h | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/arch/csky/include/asm/bitops.h b/arch/csky/include/asm/bitops.h
> index 80d67eee6e86..118fb0df5267 100644
> --- a/arch/csky/include/asm/bitops.h
> +++ b/arch/csky/include/asm/bitops.h
> @@ -49,14 +49,7 @@ static __always_inline __attribute_const__ int fls(unsigned int x)
>  	return (32 - x);
>  }
>  
> -/*
> - * asm-generic/bitops/__fls.h
> - */
> -static __always_inline __attribute_const__ unsigned long __fls(unsigned long x)
> -{
> -	return fls(x) - 1;
> -}
> -
> +#include <asm-generic/bitops/builtin-__fls.h>
>  #include <asm-generic/bitops/ffz.h>
>  #include <asm-generic/bitops/fls64.h>
>  
> 
> base-commit: abb81e5ce7d995baa41556b8125fa59e28ba3be8
> -- 
> 2.34.1

Re: [PATCH] csky: bitops: Use the compiler builtin for __fls()

[Tzu-Ching Yang]

On Thu, Oct 22, 2026 at 03:41:07PM -0400, Yury Norov wrote:
> So I didn't understand if you have an access to the hardware or not. I
> don't know the story behind using the non-generic version of ffl(),
> but it may be for performance reasons. Can you / someone else please
> run the lib/find_bit_benchmark.c on real hardware before we move this?

No, I have no csky hardware; all run-time tests were under XuanTie's
QEMU.

The ff1 asm is in fls(), which the patch keeps. __fls() has been a C
wrapper around it, fls(x) - 1, since the 2018 port (735ee005c371), with
no reason recorded. The builtin compiles to the same ff1: 4
instructions before and after with every GCC 9 to 16 and CPU tested,
so __fls() itself is neither faster nor slower. The gain is elsewhere:
the builtin folds constant arguments and merges repeated calls, which
saves 128 to 320 bytes of text. It is the csky part of the conversions
Arnd suggested in the ffs()/fls() thread (the Link: in the patch).

In lib/find_bit_benchmark.c only find_last_bit() reaches __fls(); the
other six tests use the unchanged __ffs() asm. _find_last_bit() is 27
instructions with one ff1 before and after with GCC 10 to 16 (28 -> 27
with GCC 9), and under QEMU with -icount it executes the same number of
instructions per call before and after with GCC 12 and 16, one fewer
with GCC 9. QEMU has no pipeline model, so a real core could still
differ.

Guo Ren, or anyone with a CK8xx board: could you run the benchmark
(CONFIG_FIND_BIT_BENCHMARK=y) before and after? I'm happy to wait.

Thanks,
Tzu-Ching

Re: [PATCH] csky: bitops: Use the compiler builtin for __fls()

[Yury Norov]

On Fri, Oct 23, 2026 at 10:08:06PM +0800, Tzu-Ching Yang wrote:
> On Thu, Oct 22, 2026 at 03:41:07PM -0400, Yury Norov wrote:
> > So I didn't understand if you have an access to the hardware or not. I
> > don't know the story behind using the non-generic version of ffl(),
> > but it may be for performance reasons. Can you / someone else please
> > run the lib/find_bit_benchmark.c on real hardware before we move this?
> 
> No, I have no csky hardware; all run-time tests were under XuanTie's
> QEMU.
> 
> The ff1 asm is in fls(), which the patch keeps. __fls() has been a C
> wrapper around it, fls(x) - 1, since the 2018 port (735ee005c371), with
> no reason recorded. The builtin compiles to the same ff1: 4
> instructions before and after with every GCC 9 to 16 and CPU tested,
> so __fls() itself is neither faster nor slower. The gain is elsewhere:
> the builtin folds constant arguments and merges repeated calls, which
> saves 128 to 320 bytes of text. It is the csky part of the conversions
> Arnd suggested in the ffs()/fls() thread (the Link: in the patch).
> 
> In lib/find_bit_benchmark.c only find_last_bit() reaches __fls(); the
> other six tests use the unchanged __ffs() asm. _find_last_bit() is 27
> instructions with one ff1 before and after with GCC 10 to 16 (28 -> 27
> with GCC 9), and under QEMU with -icount it executes the same number of
> instructions per call before and after with GCC 12 and 16, one fewer
> with GCC 9. QEMU has no pipeline model, so a real core could still
> differ.
> 
> Guo Ren, or anyone with a CK8xx board: could you run the benchmark
> (CONFIG_FIND_BIT_BENCHMARK=y) before and after? I'm happy to wait.

While waiting happy, can you print the bloat-o-meter results, and a
couple listings of the code generation, including compile- and runtime
cases.

Thanks,
Yury

Re: [PATCH] csky: bitops: Use the compiler builtin for __fls()

[Tzu-Ching Yang]

On Fri, Oct 23, 2026 at 11:45:35AM -0400, Yury Norov wrote:
> While waiting happy, can you print the bloat-o-meter results, and a
> couple listings of the code generation, including compile- and runtime
> cases.

scripts/bloat-o-meter -t on csky defconfig (CK807) vmlinux, GCC 16.2:

add/remove: 0/0 grow/shrink: 0/3 up/down: 0/-196 (-196)
Function                                     old     new   delta
fib6_add                                    1634    1630      -4
build_sched_domains                         3948    3916     -32
skb_try_coalesce                            1766    1606    -160
Total: Before=4621444, After=4621248, chg -0.00%

The same with GCC 12.5:

add/remove: 0/0 grow/shrink: 7/6 up/down: 94/-330 (-236)
Function                                     old     new   delta
__netdev_alloc_skb                           304     338     +34
skb_ts_finish                                 34      54     +20
skb_copy_bits                                608     628     +20
skb_release_data                             354     362      +8
kfree_skb_list_reason                        280     286      +6
fib6_add                                    1632    1636      +4
ipv6_get_saddr_eval                          424     426      +2
_find_last_bit                                86      84      -2
build_sched_domains                         4292    4280     -12
skb_store_bits                               622     606     -16
skb_abort_seq_read                            54      34     -20
__build_skb                                  116      72     -44
skb_try_coalesce                            1560    1324    -236
Total: Before=4333762, After=4333526, chg -0.01%

In both builds only skb_try_coalesce and build_sched_domains change
in instruction count (GCC 16: 524 -> 466 and 1240 -> 1228; GCC 12:
476 -> 413 and 1333 -> 1327). The other lines are not new code. Most
are literal-pool words (the constants that lrw loads) and their
2-byte alignment that moved into or out of a function's range:
__build_skb has 24 instructions and 11 pool words before, 24 and 0
after. fib6_add, ipv6_get_saddr_eval and _find_last_bit each call
__fls() once at run time; they keep their instruction count, and one
instruction changes between its 16- and 32-bit encoding (fib6_add
also gains 2 bytes of pool alignment).

Over the 35 defconfig-based builds (CK807, CK810, CK860, some with
KUnit or SMP, GCC 9 to 16) the bloat-o-meter total is between -118
and -318 bytes in every one. The 128 to 320 bytes in the changelog
are the .text change, which also counts the padding between
functions.

Listings: a standalone file with the old and the new helpers from
arch/csky/include/asm/bitops.h, built by GCC 16.2 with the kernel's
flags, -O2 and -mcpu=ck807, plus -fno-ipa-icf so that GCC keeps the
old and the new copies apart; objdump -d with the addresses dropped.
The constant listing is the same with GCC 9 to 16.

Run-time argument: 4 instructions before and after with GCC 9 to 16;
with GCC 11 to 16 both are this sequence:

	unsigned long f(unsigned long x) { return __fls(x); }

	ff1     r0, r0
	movi    r3, 31
	subu    r0, r3, r0
	rts

(GCC 9 and 10 put the movi first for the builtin; GCC 9 also writes
the asm version as ff1/nor/addi.)

Constant argument. PP_DMA_INDEX_SHIFT in include/linux/mm.h is
1 + __fls(PP_SIGNATURE - POISON_POINTER_DELTA), which is
1 + __fls(0x40):

	unsigned int f(void) { return 1 + __fls(0x40); }

	before:
	movi    r3, 64
	ff1     r3, r3
	movi    r0, 32
	subu    r0, r3
	rts

	after:
	movi    r0, 7
	rts

This is where skb_try_coalesce shrinks: netmem_is_pp(), inlined into
it through skb_pp_frag_ref(), uses that shift, and the function goes
from 12 ff1 to 2 with GCC 12 and 16. The 2 that stay are
__ffs(PAGE_OFFSET) from the same macros; __ffs() keeps its asm
(brev; ff1). build_sched_domains also has one ff1 fewer.

Thanks,
Tzu-Ching

Re: [PATCH] csky: bitops: Use the compiler builtin for __fls()

[Yury Norov]

On Tue, Oct 27, 2026 at 02:23:19PM +0800, Tzu-Ching Yang wrote:
> On Fri, Oct 23, 2026 at 11:45:35AM -0400, Yury Norov wrote:
> > While waiting happy, can you print the bloat-o-meter results, and a
> > couple listings of the code generation, including compile- and runtime
> > cases.
> 
> scripts/bloat-o-meter -t on csky defconfig (CK807) vmlinux, GCC 16.2:
> 
> add/remove: 0/0 grow/shrink: 0/3 up/down: 0/-196 (-196)
> Function                                     old     new   delta
> fib6_add                                    1634    1630      -4
> build_sched_domains                         3948    3916     -32
> skb_try_coalesce                            1766    1606    -160
> Total: Before=4621444, After=4621248, chg -0.00%
> 
> The same with GCC 12.5:
> 
> add/remove: 0/0 grow/shrink: 7/6 up/down: 94/-330 (-236)
> Function                                     old     new   delta
> __netdev_alloc_skb                           304     338     +34
> skb_ts_finish                                 34      54     +20
> skb_copy_bits                                608     628     +20
> skb_release_data                             354     362      +8
> kfree_skb_list_reason                        280     286      +6
> fib6_add                                    1632    1636      +4
> ipv6_get_saddr_eval                          424     426      +2
> _find_last_bit                                86      84      -2
> build_sched_domains                         4292    4280     -12
> skb_store_bits                               622     606     -16
> skb_abort_seq_read                            54      34     -20
> __build_skb                                  116      72     -44
> skb_try_coalesce                            1560    1324    -236
> Total: Before=4333762, After=4333526, chg -0.01%

I wonder why the older GCC works better... Can you make sure its the
same kernel version and the config? 

Anyways the numbers show that the switch to the generic implementation
is worth to try. I've added the numbers to the commit message and
pushed in bitmap-for-next for testing.

Conditionally on the (so far, missing) csky engineers feedback.

Thanks,
Yury
 
> In both builds only skb_try_coalesce and build_sched_domains change
> in instruction count (GCC 16: 524 -> 466 and 1240 -> 1228; GCC 12:
> 476 -> 413 and 1333 -> 1327). The other lines are not new code. Most
> are literal-pool words (the constants that lrw loads) and their
> 2-byte alignment that moved into or out of a function's range:
> __build_skb has 24 instructions and 11 pool words before, 24 and 0
> after. fib6_add, ipv6_get_saddr_eval and _find_last_bit each call
> __fls() once at run time; they keep their instruction count, and one
> instruction changes between its 16- and 32-bit encoding (fib6_add
> also gains 2 bytes of pool alignment).
> 
> Over the 35 defconfig-based builds (CK807, CK810, CK860, some with
> KUnit or SMP, GCC 9 to 16) the bloat-o-meter total is between -118
> and -318 bytes in every one. The 128 to 320 bytes in the changelog
> are the .text change, which also counts the padding between
> functions.
> 
> Listings: a standalone file with the old and the new helpers from
> arch/csky/include/asm/bitops.h, built by GCC 16.2 with the kernel's
> flags, -O2 and -mcpu=ck807, plus -fno-ipa-icf so that GCC keeps the
> old and the new copies apart; objdump -d with the addresses dropped.
> The constant listing is the same with GCC 9 to 16.
> 
> Run-time argument: 4 instructions before and after with GCC 9 to 16;
> with GCC 11 to 16 both are this sequence:
> 
> 	unsigned long f(unsigned long x) { return __fls(x); }
> 
> 	ff1     r0, r0
> 	movi    r3, 31
> 	subu    r0, r3, r0
> 	rts
> 
> (GCC 9 and 10 put the movi first for the builtin; GCC 9 also writes
> the asm version as ff1/nor/addi.)
> 
> Constant argument. PP_DMA_INDEX_SHIFT in include/linux/mm.h is
> 1 + __fls(PP_SIGNATURE - POISON_POINTER_DELTA), which is
> 1 + __fls(0x40):
> 
> 	unsigned int f(void) { return 1 + __fls(0x40); }
> 
> 	before:
> 	movi    r3, 64
> 	ff1     r3, r3
> 	movi    r0, 32
> 	subu    r0, r3
> 	rts
> 
> 	after:
> 	movi    r0, 7
> 	rts
> 
> This is where skb_try_coalesce shrinks: netmem_is_pp(), inlined into
> it through skb_pp_frag_ref(), uses that shift, and the function goes
> from 12 ff1 to 2 with GCC 12 and 16. The 2 that stay are
> __ffs(PAGE_OFFSET) from the same macros; __ffs() keeps its asm
> (brev; ff1). build_sched_domains also has one ff1 fewer.
> 
> Thanks,
> Tzu-Ching