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/[email protected]/
 [1]

Reply via email to