All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Gary Guo" <gary@garyguo.net>
To: "Miguel Ojeda" <ojeda@kernel.org>,
	"Nathan Chancellor" <nathan@kernel.org>
Cc: "Boqun Feng" <boqun@kernel.org>, "Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Tamir Duberstein" <tamird@kernel.org>,
	"Alexandre Courbot" <acourbot@nvidia.com>,
	"Onur Özkan" <work@onurozkan.dev>,
	rust-for-linux@vger.kernel.org,
	"Nick Desaulniers" <ndesaulniers@google.com>,
	"Bill Wendling" <morbo@google.com>,
	"Justin Stitt" <justinstitt@google.com>,
	llvm@lists.linux.dev
Subject: Re: [PATCH v3 1/2] rust: std_vendor: add `likely()`, `unlikely()` and `cold_path()`
Date: Tue, 20 Oct 2026 14:12:41 +0100	[thread overview]
Message-ID: <DM9OWX055OGW.V0IQRYW38QX7@garyguo.net> (raw)
In-Reply-To: <20261016162446.121509-1-ojeda@kernel.org>

On Fri Oct 16, 2026 at 5:24 PM BST, Miguel Ojeda wrote:
> `{un,}likely()` are commonly used macros on the C side -- there are
> more than 25k occurrences in the tree. Rust is likely going to provide
> functions (not macros) for the same purpose: `core::hint::{un,}likely()`.
> Currently they are unstable under `feature(likely_unlikely)` [1].
> 
> For instance, given:
> 
>     fn f1(a: i32) -> i32 {
>         if a < 0 {
>             return 123;
>         }
> 
>         42
>     }
> 
>     fn f2(a: i32) -> i32 {
>         if likely(a < 0) {
>             return 124;
>         }
> 
>         42
>     }
> 
>     fn f3(a: i32) -> i32 {
>         if unlikely(a < 0) {
>             return 125;
>         }
> 
>         42
>     }
> 
> LLVM emits code equivalent to the one it would for similar C functions:
> 
>     f1:
>             test   %edi,%edi
>             mov    $0x7b,%ecx
>             mov    $0x2a,%eax
>             cmovs  %ecx,%eax
>             ret
> 
>     f2:
>             mov    $0x7c,%eax
>             test   %edi,%edi
>         /-- jns    <f2+0xa>
>         |   ret
>         \-> mov    $0x2a,%eax
>             ret
> 
>     f3:
>             mov    $0x2a,%eax
>             test   %edi,%edi
>         /-- js     <f3+0xa>
>         |   ret
>         \-> mov    $0x7d,%eax
>             ret
> 
> Thus vendor (essentially) the `core` library code to provide the same
> API (so that we may use `core`'s directly in the future).
> 
> We do so by implementing `{un,}likely()` like `core`'s `intrinsics` do
> (to save a layer) and based on calling our own `cold_path()` instead of
> using `core`'s (which is stable since 1.95.0 [2][3]).
> 
> It turns out that simply using `#[cold]` to emulate `cold_path()`,
> suggested by Gary [4], works in the cases I tested across all supported
> versions so far, so we can avoid the complexity of having one more Kconfig
> and conditionally enabling the feature in callers (since `cold_path()`
> was added in Rust 1.86.0 [5]).
> 
> We could optionally call `cold_path()` for Rust >= 1.86.0 within our own
> `cold_path()`, but it seems that it is not needed, and anyway since Rust
> 1.98.0 `cold_path()` is itself `#[cold]` so that it works even if the
> MIR inliner can't inline it [6].
> 
> Even if we find that it is not optimal in some cases, it can be improved,
> and meanwhile we will have the means to annotate these cases already.
> 
> Apart from that, the only differences with `core`'s version are the
> usual changes (`pr_*!` calls and their `\n`, imports, attributes, lint
> cleanup), plus a fix for a polarity typo (I sent a fix upstream [7])
> and removal of unneeded code documentation.
> 
> In addition, add them to the prelude (inlined, so that the documentation
> is rendered at least there, until we can use `core`'s).
> 
> Link: https://github.com/rust-lang/rust/issues/151619 [1]
> Link: https://github.com/rust-lang/rust/issues/136873 [2]
> Link: https://github.com/rust-lang/rust/pull/151576 [3]
> Link: https://lore.kernel.org/rust-for-linux/DGA6GUR58QJ7.1XZZ0P4VZNW86@garyguo.net/ [4]
> Link: https://github.com/rust-lang/rust/pull/133695 [5]
> Link: https://github.com/rust-lang/rust/pull/156863 [6]
> Link: https://github.com/rust-lang/rust/pull/164374 [7]
> Assisted-by: LLM
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
> v3: I used Gary's `#[cold]` suggestion, and thus removed the Kconfig.
> I rebased and made sure the code is up to date with `core`, tested a
> bunch of things, compared again with C and the assembly generated, found
> a strange behavior on Clang I will report separately, a polarity typo in
> `core` (fixed here) and inlined the docs in the prelude to get it
> rendered somewhere until we bump the MSRV. I think the potential issue
> I found in v2's discussion may have been with `-Os`, but it happens the
> same way with `core`'s, so it does not matter.
> 
> Alice, Gary: I removed your Reviewed-by's just in case. It is pretty
> similar to last time, but the commit message is reworded, and the commit
> is simpler.
> 
>  rust/kernel/prelude.rs    |   7 ++
>  rust/kernel/std_vendor.rs | 136 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 143 insertions(+)


  parent reply	other threads:[~2026-10-20 13:12 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-16 16:24 [PATCH v3 1/2] rust: std_vendor: add `likely()`, `unlikely()` and `cold_path()` Miguel Ojeda
2026-10-16 16:24 ` [PATCH v3 2/2] rust: bug: hint `warn_on!` conditions as unlikely Miguel Ojeda
2026-10-20 13:08   ` FUJITA Tomonori
2026-10-20 13:12 ` Gary Guo [this message]
2026-10-20 13:14 ` [PATCH v3 1/2] rust: std_vendor: add `likely()`, `unlikely()` and `cold_path()` FUJITA Tomonori

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DM9OWX055OGW.V0IQRYW38QX7@garyguo.net \
    --to=gary@garyguo.net \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=justinstitt@google.com \
    --cc=llvm@lists.linux.dev \
    --cc=lossin@kernel.org \
    --cc=morbo@google.com \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tamird@kernel.org \
    --cc=tmgross@umich.edu \
    --cc=work@onurozkan.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.