[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