Rollup merge of rust-lang#139726 - Amanieu:select_unpredictable_hint, r=dtolnay

Move `select_unpredictable` to the `hint` module

There has been considerable discussion in both the ACP (rust-lang/libs-team#468) and tracking issue (rust-lang#133962) about whether the `bool::select_unpredictable` method should be in `core::hint` instead.

I believe this is the right move for the following reasons:
- The documentation explicitly says that it is a hint, not a codegen guarantee.
- `bool` doesn't have a corresponding `select` method, and I don't think we should be adding one.
- This shouldn't be something that people reach for with auto-completion unless they specifically understand the interactions with branch prediction. Using conditional moves can easily make code *slower* by preventing the CPU from speculating past the condition due to the data dependency.
- Although currently `core::hint` only contains no-ops, this isn't a hard rule (for example `unreachable_unchecked` is a bit of a gray area). The documentation only status that the module contains "hints to compiler that affects how code should be emitted or optimized". This is consistent with what `select_unpredictable` does.

Author: ChrisDenton

library/core/src/bool.rs

@@ -61,52 +61,4 @@ impl bool {
     pub fn then<T, F: FnOnce() -> T>(self, f: F) -> Option<T> {
         if self { Some(f()) } else { None }
     }
-
-    /// Returns either `true_val` or `false_val` depending on the value of
-    /// `self`, with a hint to the compiler that `self` is unlikely
-    /// to be correctly predicted by a CPU’s branch predictor.
-    ///
-    /// This method is functionally equivalent to
-    /// ```ignore (this is just for illustrative purposes)
-    /// fn select_unpredictable<T>(b: bool, true_val: T, false_val: T) -> T {
-    ///     if b { true_val } else { false_val }
-    /// }
-    /// ```
-    /// but might generate different assembly. In particular, on platforms with
-    /// a conditional move or select instruction (like `cmov` on x86 or `csel`
-    /// on ARM) the optimizer might use these instructions to avoid branches,
-    /// which can benefit performance if the branch predictor is struggling
-    /// with predicting `condition`, such as in an implementation of  binary
-    /// search.
-    ///
-    /// Note however that this lowering is not guaranteed (on any platform) and
-    /// should not be relied upon when trying to write constant-time code. Also
-    /// be aware that this lowering might *decrease* performance if `condition`
-    /// is well-predictable. It is advisable to perform benchmarks to tell if
-    /// this function is useful.
-    ///
-    /// # Examples
-    ///
-    /// Distribute values evenly between two buckets:
-    /// ```
-    /// #![feature(select_unpredictable)]
-    ///
-    /// use std::hash::BuildHasher;
-    ///
-    /// fn append<H: BuildHasher>(hasher: &H, v: i32, bucket_one: &mut Vec<i32>, bucket_two: &mut Vec<i32>) {
-    ///     let hash = hasher.hash_one(&v);
-    ///     let bucket = (hash % 2 == 0).select_unpredictable(bucket_one, bucket_two);
-    ///     bucket.push(v);
-    /// }
-    /// # let hasher = std::collections::hash_map::RandomState::new();
-    /// # let mut bucket_one = Vec::new();
-    /// # let mut bucket_two = Vec::new();
-    /// # append(&hasher, 42, &mut bucket_one, &mut bucket_two);
-    /// # assert_eq!(bucket_one.len() + bucket_two.len(), 1);
-    /// ```
-    #[inline(always)]
-    #[unstable(feature = "select_unpredictable", issue = "133962")]
-    pub fn select_unpredictable<T>(self, true_val: T, false_val: T) -> T {
-        crate::intrinsics::select_unpredictable(self, true_val, false_val)
-    }
 }

library/core/src/hint.rs

@@ -734,3 +734,52 @@ pub const fn unlikely(b: bool) -> bool {
 pub const fn cold_path() {
     crate::intrinsics::cold_path()
 }
+
+/// Returns either `true_val` or `false_val` depending on the value of `b`,
+/// with a hint to the compiler that `b` is unlikely to be correctly
+/// predicted by a CPU’s branch predictor.
+///
+/// This method is functionally equivalent to
+/// ```ignore (this is just for illustrative purposes)
+/// fn select_unpredictable<T>(b: bool, true_val: T, false_val: T) -> T {
+///     if b { true_val } else { false_val }
+/// }
+/// ```
+/// but might generate different assembly. In particular, on platforms with
+/// a conditional move or select instruction (like `cmov` on x86 or `csel`
+/// on ARM) the optimizer might use these instructions to avoid branches,
+/// which can benefit performance if the branch predictor is struggling
+/// with predicting `condition`, such as in an implementation of binary
+/// search.
+///
+/// Note however that this lowering is not guaranteed (on any platform) and
+/// should not be relied upon when trying to write constant-time code. Also
+/// be aware that this lowering might *decrease* performance if `condition`
+/// is well-predictable. It is advisable to perform benchmarks to tell if
+/// this function is useful.
+///
+/// # Examples
+///
+/// Distribute values evenly between two buckets:
+/// ```
+/// #![feature(select_unpredictable)]
+///
+/// use std::hash::BuildHasher;
+/// use std::hint;
+///
+/// fn append<H: BuildHasher>(hasher: &H, v: i32, bucket_one: &mut Vec<i32>, bucket_two: &mut Vec<i32>) {
+///     let hash = hasher.hash_one(&v);
+///     let bucket = hint::select_unpredictable(hash % 2 == 0, bucket_one, bucket_two);
+///     bucket.push(v);
+/// }
+/// # let hasher = std::collections::hash_map::RandomState::new();
+/// # let mut bucket_one = Vec::new();
+/// # let mut bucket_two = Vec::new();
+/// # append(&hasher, 42, &mut bucket_one, &mut bucket_two);
+/// # assert_eq!(bucket_one.len() + bucket_two.len(), 1);
+/// ```
+#[inline(always)]
+#[unstable(feature = "select_unpredictable", issue = "133962")]
+pub fn select_unpredictable<T>(b: bool, true_val: T, false_val: T) -> T {
+    crate::intrinsics::select_unpredictable(b, true_val, false_val)
+}

library/core/src/intrinsics/mod.rs

@@ -1326,7 +1326,7 @@ pub const fn unlikely(b: bool) -> bool {
 /// Therefore, implementations must not require the user to uphold
 /// any safety invariants.
 ///
-/// The public form of this instrinsic is [`bool::select_unpredictable`].
+/// The public form of this instrinsic is [`core::hint::select_unpredictable`].
 #[unstable(feature = "core_intrinsics", issue = "none")]
 #[rustc_intrinsic]
 #[rustc_nounwind]

library/core/src/slice/mod.rs

@@ -2822,7 +2822,7 @@ impl<T> [T] {
             // Binary search interacts poorly with branch prediction, so force
             // the compiler to use conditional moves if supported by the target
             // architecture.
-            base = (cmp == Greater).select_unpredictable(base, mid);
+            base = hint::select_unpredictable(cmp == Greater, base, mid);
 
             // This is imprecise in the case where `size` is odd and the
             // comparison returns Greater: the mid element still gets included

library/core/src/slice/sort/shared/smallsort.rs

@@ -2,7 +2,7 @@
 
 use crate::mem::{self, ManuallyDrop, MaybeUninit};
 use crate::slice::sort::shared::FreezeMarker;
-use crate::{intrinsics, ptr, slice};
+use crate::{hint, intrinsics, ptr, slice};
 
 // It's important to differentiate between SMALL_SORT_THRESHOLD performance for
 // small slices and small-sort performance sorting small sub-slices as part of
@@ -408,8 +408,8 @@ where
         // }
 
         // The goal is to generate cmov instructions here.
-        let v_a_swap = should_swap.select_unpredictable(v_b, v_a);
-        let v_b_swap = should_swap.select_unpredictable(v_a, v_b);
+        let v_a_swap = hint::select_unpredictable(should_swap, v_b, v_a);
+        let v_b_swap = hint::select_unpredictable(should_swap, v_a, v_b);
 
         let v_b_swap_tmp = ManuallyDrop::new(ptr::read(v_b_swap));
         ptr::copy(v_a_swap, v_a, 1);
@@ -640,15 +640,15 @@ pub unsafe fn sort4_stable<T, F: FnMut(&T, &T) -> bool>(
         //  1,  1 |  c   b    a         d
         let c3 = is_less(&*c, &*a);
         let c4 = is_less(&*d, &*b);
-        let min = c3.select_unpredictable(c, a);
-        let max = c4.select_unpredictable(b, d);
-        let unknown_left = c3.select_unpredictable(a, c4.select_unpredictable(c, b));
-        let unknown_right = c4.select_unpredictable(d, c3.select_unpredictable(b, c));
+        let min = hint::select_unpredictable(c3, c, a);
+        let max = hint::select_unpredictable(c4, b, d);
+        let unknown_left = hint::select_unpredictable(c3, a, hint::select_unpredictable(c4, c, b));
+        let unknown_right = hint::select_unpredictable(c4, d, hint::select_unpredictable(c3, b, c));
 
         // Sort the last two unknown elements.
         let c5 = is_less(&*unknown_right, &*unknown_left);
-        let lo = c5.select_unpredictable(unknown_right, unknown_left);
-        let hi = c5.select_unpredictable(unknown_left, unknown_right);
+        let lo = hint::select_unpredictable(c5, unknown_right, unknown_left);
+        let hi = hint::select_unpredictable(c5, unknown_left, unknown_right);
 
         ptr::copy_nonoverlapping(min, dst, 1);
         ptr::copy_nonoverlapping(lo, dst.add(1), 1);

tests/codegen/intrinsics/select_unpredictable.rs

@@ -46,27 +46,27 @@ pub fn test_zst(p: bool, a: (), b: ()) -> () {
 pub fn test_int2(p: bool, a: u64, b: u64) -> u64 {
     // CHECK-LABEL: define{{.*}} @test_int2
     // CHECK: select i1 %p, i64 %a, i64 %b, !unpredictable
-    p.select_unpredictable(a, b)
+    core::hint::select_unpredictable(p, a, b)
 }
 
 #[no_mangle]
 pub fn test_pair2(p: bool, a: (u64, u64), b: (u64, u64)) -> (u64, u64) {
     // CHECK-LABEL: define{{.*}} @test_pair2
     // CHECK: select i1 %p, {{.*}}, !unpredictable
-    p.select_unpredictable(a, b)
+    core::hint::select_unpredictable(p, a, b)
 }
 
 #[no_mangle]
 pub fn test_struct2(p: bool, a: Large, b: Large) -> Large {
     // CHECK-LABEL: define{{.*}} @test_struct2
     // CHECK: select i1 %p, {{.*}}, !unpredictable
-    p.select_unpredictable(a, b)
+    core::hint::select_unpredictable(p, a, b)
 }
 
 #[no_mangle]
 pub fn test_zst2(p: bool, a: (), b: ()) -> () {
     // CHECK-LABEL: define{{.*}} @test_zst2
     // CHECK-NEXT: start:
     // CHECK-NEXT: ret void
-    p.select_unpredictable(a, b)
+    core::hint::select_unpredictable(p, a, b)
 }