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

[Yury Norov <ynorov@nvidia.com>]

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