Stabilize core::hint::cold_path

[tgross35]

cold_path has been around unstably for a while and is a rather useful tool to have. It does what it is supposed to and there are no known remaining issues, so stabilize it here (including const).

Newly stable API:

// in core::hint
pub const fn cold_path();

I have opted to exclude likely and unlikely for now since they have had some concerns about ease of use that cold_path doesn't suffer from. cold_path is also significantly more flexible; in addition to working with boolean if conditions, it can be used in match arms, if let, closures, and other control flow blocks. likely and unlikely are also possible to implement in user code via cold_path, if desired.

Closes: #136873 (tracking issue)

---

There has been some design and implementation work for making #[cold] function in more places, such as if arms, match arms, and closure bodies. Considering a stable cold_path will cover all of these usecases, it does not seem worth pursuing a more powerful #[cold] as an alternative way to do the same thing. If the lang team agrees, then:

Closes: #26179
Closes: #120193

Cc @rust-lang/lang
Cc @rust-lang/libs-api
Cc @rust-lang/wg-const-eval

[rustbot]

Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter
gets adapted for the changes, if necessary.

cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr

[rustbot]

r? @jhpratt

rustbot has assigned @jhpratt.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[tgross35]

Cc also @x17jiri since you did a lot of the work here.

[jhpratt]

Trivial stabilization; r=me with completed FCP.

Anecdotally, I do find likely and unlikely useful, as it's a more "up-front" hint in a conditional statement, but I understand the desire to not stabilize them all.

likely and unlikely are also possible to implement in user code

Technically, so can cold_path. I've been doing it for a little while now: https://github.com/time-rs/time/blob/14da652cc731cbe86afbc5b99442f3a0cd747e60/time-core/src/hint.rs

[RalfJung]

Cc @nikic

There has been some design and implementation work for making #[cold] function in more places, such as if arms, match arms, and closure bodies. Considering a stable cold_path will cover all of these usecases, it does not seem worth pursuing a more powerful #[cold] as an alternative way to do the same thing. If the lang team agrees, then:

Those tracking issues track features that actually exist unstably in the compiler, or do they not? If they do, please don't close them without removing the implementation (which IMO should be a separate PR).

[tgross35]

Those tracking issues track features that actually exist unstably in the compiler, or do they not? If they do, please don't close them without removing the implementation (which IMO should be a separate PR).

There isn't anything linked at #26179 and that issue number doesn't show up in the source, and it doesn't seem like we have any special handling for it based on diagnostics for closures. I was thinking we had this at some point too but perhaps it got ripped out.

We do parse the attribute in places we definitely shouldn't (below builds on stable without issue):

fn foo(a: u32) {}

match 10 {
    #[cold] _ => {}
}
#[cold] if true {}
foo(#[cold] 10);

But that isn't unique to cold, this works too

match 10 {
    #[unsafe(no_mangle)] _ => {}
}
#[unsafe(no_mangle)] if true {}
foo(#[unsafe(no_mangle)] 10);

[saethlin]

Does stabilizing this impact design space for hinting that there are multiple paths with different coldness? The current lowering just produces a magic branch weight, but it could be possible to pass through the weight you want.

[hanna-kruppe]

Does stabilizing this impact design space for hinting that there are multiple paths with different coldness? The current lowering just produces a magic branch weight, but it could be possible to pass through the weight you want.

I'm skeptical that exposing any extra knobs along those lines will ever be useful, especially across compiler upgrades (if you see any performance benefit from it, it's likely from the thirty-steps-removed perturbation on heuristic in very late backend passes like regalloc and machine block placement). In any case, I don't see a plausible design that would be significantly impacted by cold_path, other than the simplicity of cold_path making all other alternatives look less attractive by comparison:

• Wanting manual control over the branch weights is a very niche need¹ even relative to cold_path, which itself is a fairly advanced feature (but very useful when applicable). So I don't think it's plausible that we'll end up saying, e.g., "just use hint::branch_weight::<hint::DEFAULT_BRANCH_WEIGHT>() if you want the default weights" instead of providing cold_path(), that would optimize for the vastly less common case.
• In branch weight terms, if you want to set the weight for one branch you really need to set the weights for all outcomes of the control flow construct, since branch weights are relative to the sum of weights of the out-edges from the predecessor block. In contrast, cold_path only makes a binary classification (cold or not) so it's fine to mark up only some paths and leave the rest implicit.
• Clang and GCC traditionally side-step the "lining up branch weights" problem with the __builtin_expect[_with_probability] framing (associating the hint with the condition, not with the successor) and accepting a constant probability in [0.0, 1.0] rather than an integer weight. Even that ends up as the best design for exposing extra control over branch weights, the reasons why cold_path turned out preferable to likely/unlikely for Rust still hold, so it seems unlikely we'd want to throw away cold_path() in that timeline.

So I think it's expected and fine if a hypothetical feature that gives more control over the branch weights would end up as an entirely different interface from cold_path. The possibility of this isn't a mark against cold_path, if we're otherwise happy with cold_path.

Footnotes

1. Multiple levels of coldness is already possible with cold_path in many cases, since LLVM derives block frequencies from CFG paths. See https://rust.godbolt.org/z/rabxfqMGn for a simple example of a second cold_path() in an already-cold path having an effect (I didn't find a good way to show the block frequency directly). So controlling branch weights is really only useful if you have either a branch with 3+ successors (not if) where it's important to distingush at least three levels of weights for these edges, or if you for some cursed reason must fudge the branch weights across several different branches to influence the overall block frequencies. ↩

[tgross35]

A small bit of prior discussion about probabilities came up on the bit tracking issue, starting around #26179 (comment). I don't think there has been much interest or design work since but ageed with what Hanna said, that having cold_path doesn't block future possibilities here.

[BurntSushi]

@rfcbot fcp merge

[rust-rfcbot]

Error encounted:
Provided team `` is invalid

[BurntSushi]

@rfcbot fcp merge libs-api,lang

[rust-rfcbot]

Team member @BurntSushi has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @nikomatsakis
• @scottmcm
• @the8472
• @tmandry
• @traviscross

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

[traviscross]

I have opted to exclude likely and unlikely for now since they have had some concerns about ease of use that cold_path doesn't suffer from....

Tracking issue: #136873

@tgross35: As a tracking matter, I'd suggest moving likely and unlikely to a new tracking issue so that #136873 is just for cold_path and can be closed and milestoned when this is merged (or the opposite; forking cold_path to a separate tracking issue).

likely and unlikely are also possible to implement in user code via cold_path, if desired.

As a documentation matter, I'd suggest demonstrating this in the docs for cold_path.

Does stabilizing this impact design space for hinting that there are multiple paths with different coldness? The current lowering just produces a magic branch weight, but it could be possible to pass through the weight you want.

I would like to see us later add some manner of core::hint::branch_prob<const P: ..>() or branch_weight or whatnot. I don't see that as being in tension with adding this small convenience now.

@rfcbot reviewed

[tgross35]

The tracking issue and documentation updates are reasonable. Split the issues, updates are in #151612.