Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reexport likely/unlikely in std::hint #133695

Merged
merged 2 commits into from
Jan 20, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 140 additions & 0 deletions library/core/src/hint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -511,3 +511,143 @@ pub const fn black_box<T>(dummy: T) -> T {
pub const fn must_use<T>(value: T) -> T {
value
}

/// Hints to the compiler that a branch condition is likely to be true.
/// Returns the value passed to it.
///
/// It can be used with `if` or boolean `match` expressions.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be expanded a bit? What happens if you use it outside (presumably, nothing)? Can this be used in a && or || expression?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also it would be good to point people to cold_path (for which we just approved the ACP) for general match and if let.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I also export cold_path() in this PR, or create a new PR for that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can do it in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also added a test for cold_path(), but not with idiomatic Rust such as if let ... because this will only work after #133852

///
/// When used outside of a branch condition, it may still work if there is a branch close by, but
/// it is not guaranteed to have any effect.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As with all1 of the functions in the std::hint module, the compiler/processor is allowed to ignore it (ie. it is allowed to do nothing). Perhaps this language should be changed to clarify this.

Footnotes

  1. Except unreachable_unchecked

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reworded

///
/// It can also be applied to parts of expressions, such as `likely(a) && unlikely(b)`, or to
/// compound expressions, such as `likely(a && b)`. When applied to compound expressions, it has
/// the following effect:
/// ```text
/// likely(!a) => !unlikely(a)
/// likely(a && b) => likely(a) && likely(b)
/// likely(a || b) => a || likely(b)
/// ```
///
/// See also the function [`cold_path()`] which may be more appropriate for idiomatic Rust code.
///
/// # Examples
///
/// ```
/// #![feature(likely_unlikely)]
/// use core::hint::likely;
///
/// fn foo(x: i32) {
/// if likely(x > 0) {
/// println!("this branch is likely to be taken");
/// } else {
/// println!("this branch is unlikely to be taken");
/// }
///
/// match likely(x > 0) {
/// true => println!("this branch is likely to be taken"),
/// false => println!("this branch is unlikely to be taken"),
/// }
///
/// // Use outside of a branch condition. This may still work if there is a branch close by,
/// // but it is not guaranteed to have any effect
/// let cond = likely(x != 0);
/// if cond {
/// println!("this branch is likely to be taken");
/// }
/// }
/// ```
///
///
#[unstable(feature = "likely_unlikely", issue = "26179")]
#[rustc_nounwind]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe this attribute is appropriate for public functions.

Suggested change
#[rustc_nounwind]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Attribute removed

#[inline(always)]
pub const fn likely(b: bool) -> bool {
crate::intrinsics::likely(b)
}

/// Hints to the compiler that a branch condition is unlikely to be true.
/// Returns the value passed to it.
///
/// It can be used with `if` or boolean `match` expressions.
///
/// When used outside of a branch condition, it may still work if there is a branch close by, but
/// it is not guaranteed to have any effect.
///
/// It can also be applied to parts of expressions, such as `likely(a) && unlikely(b)`, or to
/// compound expressions, such as `unlikely(a && b)`. When applied to compound expressions, it has
/// the following effect:
/// ```text
/// unlikely(!a) => !likely(a)
/// unlikely(a && b) => a && unlikely(b)
/// unlikely(a || b) => unlikely(a) || unlikely(b)
/// ```
///
/// See also the function [`cold_path()`] which may be more appropriate for idiomatic Rust code.
///
/// # Examples
///
/// ```
/// #![feature(likely_unlikely)]
/// use core::hint::unlikely;
///
/// fn foo(x: i32) {
/// if unlikely(x > 0) {
/// println!("this branch is unlikely to be taken");
/// } else {
/// println!("this branch is likely to be taken");
/// }
///
/// match unlikely(x > 0) {
/// true => println!("this branch is unlikely to be taken"),
/// false => println!("this branch is likely to be taken"),
/// }
///
/// // Use outside of a branch condition. This may still work if there is a branch close by,
/// // but it is not guaranteed to have any effect
/// let cond = unlikely(x != 0);
/// if cond {
/// println!("this branch is likely to be taken");
/// }
/// }
/// ```
#[unstable(feature = "likely_unlikely", issue = "26179")]
#[rustc_nounwind]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe this attribute is appropriate for public functions.

Suggested change
#[rustc_nounwind]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Attribute removed

#[inline(always)]
pub const fn unlikely(b: bool) -> bool {
crate::intrinsics::unlikely(b)
}

/// Hints to the compiler that given path is cold, i.e., unlikely to be taken. The compiler may
/// choose to optimize paths that are not cold at the expense of paths that are cold.
///
/// # Examples
///
/// ```
/// #![feature(cold_path)]
/// use core::hint::cold_path;
///
/// fn foo(x: &[i32]) {
/// if let Some(first) = x.get(0) {
/// // this is the fast path
/// } else {
/// // this path is unlikely
/// cold_path();
/// }
/// }
///
/// fn bar(x: i32) -> i32 {
/// match x {
/// 1 => 10,
/// 2 => 100,
/// 3 => { cold_path(); 1000 }, // this branch is unlikely
/// _ => { cold_path(); 10000 }, // this is also unlikely
/// }
/// }
/// ```
#[unstable(feature = "cold_path", issue = "26179")]
#[rustc_nounwind]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe this attribute is appropriate for public functions.

Suggested change
#[rustc_nounwind]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Attribute removed

#[inline(always)]
pub const fn cold_path() {
crate::intrinsics::cold_path()
}
54 changes: 54 additions & 0 deletions tests/codegen/hint/cold_path.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
//@ compile-flags: -O
#![crate_type = "lib"]
#![feature(cold_path)]

use std::hint::cold_path;

#[inline(never)]
#[no_mangle]
pub fn path_a() {
println!("path a");
}

#[inline(never)]
#[no_mangle]
pub fn path_b() {
println!("path b");
}

#[no_mangle]
pub fn test1(x: bool) {
if x {
path_a();
} else {
cold_path();
path_b();
}

// CHECK-LABEL: @test1(
// CHECK: br i1 %x, label %bb1, label %bb2, !prof ![[NUM:[0-9]+]]
// CHECK: bb2:
// CHECK: path_b
// CHECK: bb1:
// CHECK: path_a
}

#[no_mangle]
pub fn test2(x: i32) {
match x > 0 {
true => path_a(),
false => {
cold_path();
path_b()
}
}

// CHECK-LABEL: @test2(
// CHECK: br i1 %_2, label %bb2, label %bb1, !prof ![[NUM]]
// CHECK: bb1:
// CHECK: path_b
// CHECK: bb2:
// CHECK: path_a
}

// CHECK: ![[NUM]] = !{!"branch_weights", {{(!"expected", )?}}i32 2000, i32 1}
81 changes: 81 additions & 0 deletions tests/codegen/hint/likely.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
//@ compile-flags: -O
#![crate_type = "lib"]
#![feature(likely_unlikely)]

use std::hint::likely;

#[inline(never)]
#[no_mangle]
pub fn path_a() {
println!("path a");
}

#[inline(never)]
#[no_mangle]
pub fn path_b() {
println!("path b");
}

#[no_mangle]
pub fn test1(x: bool) {
if likely(x) {
path_a();
} else {
path_b();
}

// CHECK-LABEL: @test1(
// CHECK: br i1 %x, label %bb2, label %bb3, !prof ![[NUM:[0-9]+]]
// CHECK: bb3:
// CHECK: path_b
// CHECK: bb2:
// CHECK: path_a
}

#[no_mangle]
pub fn test2(x: i32) {
match likely(x > 0) {
true => path_a(),
false => path_b(),
}

// CHECK-LABEL: @test2(
// CHECK: br i1 %_2, label %bb2, label %bb3, !prof ![[NUM]]
// CHECK: bb3:
// CHECK: path_b
// CHECK: bb2:
// CHECK: path_a
}

#[no_mangle]
pub fn test3(x: i8) {
match likely(x < 7) {
true => path_a(),
_ => path_b(),
}

// CHECK-LABEL: @test3(
// CHECK: br i1 %_2, label %bb2, label %bb3, !prof ![[NUM]]
// CHECK: bb3:
// CHECK: path_b
// CHECK: bb2:
// CHECK: path_a
}

#[no_mangle]
pub fn test4(x: u64) {
match likely(x != 33) {
false => path_a(),
_ => path_b(),
}

// CHECK-LABEL: @test4(
// CHECK: br i1 %0, label %bb3, label %bb2, !prof ![[NUM2:[0-9]+]]
// CHECK: bb3:
// CHECK: path_a
// CHECK: bb2:
// CHECK: path_b
}

// CHECK: ![[NUM]] = !{!"branch_weights", {{(!"expected", )?}}i32 2000, i32 1}
// CHECK: ![[NUM2]] = !{!"branch_weights", {{(!"expected", )?}}i32 1, i32 2000}
80 changes: 80 additions & 0 deletions tests/codegen/hint/unlikely.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
//@ compile-flags: -O
#![crate_type = "lib"]
#![feature(likely_unlikely)]

use std::hint::unlikely;

#[inline(never)]
#[no_mangle]
pub fn path_a() {
println!("path a");
}

#[inline(never)]
#[no_mangle]
pub fn path_b() {
println!("path b");
}

#[no_mangle]
pub fn test1(x: bool) {
if unlikely(x) {
path_a();
} else {
path_b();
}

// CHECK-LABEL: @test1(
// CHECK: br i1 %x, label %bb2, label %bb4, !prof ![[NUM:[0-9]+]]
// CHECK: bb4:
// CHECK: path_b
// CHECK: bb2:
// CHECK: path_a
}

#[no_mangle]
pub fn test2(x: i32) {
match unlikely(x > 0) {
true => path_a(),
false => path_b(),
}

// CHECK-LABEL: @test2(
// CHECK: br i1 %_2, label %bb2, label %bb4, !prof ![[NUM]]
// CHECK: bb4:
// CHECK: path_b
// CHECK: bb2:
// CHECK: path_a
}

#[no_mangle]
pub fn test3(x: i8) {
match unlikely(x < 7) {
true => path_a(),
_ => path_b(),
}

// CHECK-LABEL: @test3(
// CHECK: br i1 %_2, label %bb2, label %bb4, !prof ![[NUM]]
// CHECK: bb4:
// CHECK: path_b
// CHECK: bb2:
// CHECK: path_a
}

#[no_mangle]
pub fn test4(x: u64) {
match unlikely(x != 33) {
false => path_a(),
_ => path_b(),
}

// CHECK-LABEL: @test4(
// CHECK: br i1 %0, label %bb4, label %bb2, !prof ![[NUM2:[0-9]+]]
// CHECK: bb4:
// CHECK: path_a
// CHECK: bb2:
// CHECK: path_b
}

// CHECK: ![[NUM]] = !{!"branch_weights", {{(!"expected", )?}}i32 1, i32 2000}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ fn step_forward(_1: u16, _2: usize) -> u16 {
scope 6 (inlined core::num::<impl u16>::checked_add) {
let mut _5: (u16, bool);
let mut _6: bool;
scope 7 (inlined unlikely) {
scope 7 (inlined std::intrinsics::unlikely) {
let _7: ();
}
}
Expand Down Expand Up @@ -55,7 +55,7 @@ fn step_forward(_1: u16, _2: usize) -> u16 {
}

bb3: {
_7 = cold_path() -> [return: bb4, unwind unreachable];
_7 = std::intrinsics::cold_path() -> [return: bb4, unwind unreachable];
}

bb4: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ fn step_forward(_1: u16, _2: usize) -> u16 {
scope 6 (inlined core::num::<impl u16>::checked_add) {
let mut _5: (u16, bool);
let mut _6: bool;
scope 7 (inlined unlikely) {
scope 7 (inlined std::intrinsics::unlikely) {
let _7: ();
}
}
Expand Down Expand Up @@ -55,7 +55,7 @@ fn step_forward(_1: u16, _2: usize) -> u16 {
}

bb3: {
_7 = cold_path() -> [return: bb4, unwind unreachable];
_7 = std::intrinsics::cold_path() -> [return: bb4, unwind unreachable];
}

bb4: {
Expand Down
Loading