On Fri, 16 Oct 2026 19:12:42 +0200 Miguel Ojeda <[email protected]> wrote:
> Hi Nathan, Steve, Masami, Arnd, all, Hi Miguel, > > I was updating my old patch to add `likely()` and friends to the Rust > side of the kernel [1] and I noticed something odd when comparing with > the C side with Clang: > > https://godbolt.org/z/6Kendxzzd > > Under `CONFIG_TRACE_BRANCH_PROFILING`, the way we use `__builtin_expect` > with a temporary variable makes Clang ignore the hint (GCC is fine). > > So essentially: > > #define likely(x) __builtin_expect(!!(x), 1) > #define unlikely(x) __builtin_expect(!!(x), 0) > > is fine for both compilers, but: > > #define __branch_check__(x, expect, is_constant) ({ \ > long ______r; \ > ... > ______r = __builtin_expect(!!(x), expect); \ > ... > ______r; \ > }) > > #define likely(x) (__branch_check__(x, 1, __builtin_constant_p(x))) > #define unlikely(x) (__branch_check__(x, 0, __builtin_constant_p(x))) > > is not. Now, if we do move the `__builtin_expect` outside like: > > #define __branch_check__(x, expect, is_constant) ({ \ > long ______r; \ > ... > ______r = !!(x); \ > ... > ______r; \ > }) > > #define likely(x) __builtin_expect(__branch_check__(x, 1, > __builtin_constant_p(x)), 1) > #define unlikely(x) __builtin_expect(__branch_check__(x, 0, > __builtin_constant_p(x)), 0) > > then it works with Clang again. > > Asking an LLM to sample a few object files to see the effect, it does > seem like it works as expected, e.g. `kernel/signal.o` goes from 1 to > 125 branches carrying weights and 39 of its 355 functions change codegen > (675 of ~3800 across the dozen objects sampled). > > It doesn't mean it is better of course -- I didn't benchmark it, but it > means it is passing the information to the compiler that we expected. > If it works, I'm fine with changing it. The branch profile is *not* optimized ;-) Hmm, which reminds me, I'll be enabling it again on my server to see where things are wrong soon. -- Steve
