mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* `likely` not applied with Clang and `CONFIG_TRACE_BRANCH_PROFILING`
@ 2026-10-16 17:12 Miguel Ojeda
  2026-10-16 17:46 ` Steven Rostedt
  0 siblings, 1 reply; 2+ messages in thread
From: Miguel Ojeda @ 2026-10-16 17:12 UTC (permalink / raw)
  To: Nathan Chancellor, Steven Rostedt, Masami Hiramatsu
  Cc: Nick Desaulniers, Bill Wendling, Justin Stitt, llvm,
	Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
	Arnd Bergmann

Hi Nathan, Steve, Masami, Arnd, all,

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.

Cheers,
Miguel

Link: https://lore.kernel.org/rust-for-linux/20261016162446.121509-1-ojeda@kernel.org/ [1]

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: `likely` not applied with Clang and `CONFIG_TRACE_BRANCH_PROFILING`
  2026-10-16 17:12 `likely` not applied with Clang and `CONFIG_TRACE_BRANCH_PROFILING` Miguel Ojeda
@ 2026-10-16 17:46 ` Steven Rostedt
  0 siblings, 0 replies; 2+ messages in thread
From: Steven Rostedt @ 2026-10-16 17:46 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: Nathan Chancellor, Masami Hiramatsu, Nick Desaulniers,
	Bill Wendling, Justin Stitt, llvm, Mathieu Desnoyers,
	linux-kernel, linux-trace-kernel, Arnd Bergmann

On Fri, 16 Oct 2026 19:12:42 +0200
Miguel Ojeda <ojeda@kernel.org> 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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-10-16 17:46 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-16 17:12 `likely` not applied with Clang and `CONFIG_TRACE_BRANCH_PROFILING` Miguel Ojeda
2026-10-16 17:46 ` Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®