Skip to content

Commit 472058a

Browse files
committed
Force LLVM to use CMOV for binary search
Since https://reviews.llvm.org/D118118, LLVM will no longer turn CMOVs into branches if it comes from a `select` marked with an `unpredictable` metadata attribute. This PR introduces `core::intrinsics::select_unpredictable` which emits such a `select` and uses it in the implementation of `binary_search_by`.
1 parent 355efac commit 472058a

File tree

8 files changed

+67
-7
lines changed

8 files changed

+67
-7
lines changed

compiler/rustc_codegen_llvm/src/builder.rs

+10
Original file line numberDiff line numberDiff line change
@@ -1353,6 +1353,16 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
13531353
}
13541354
}
13551355

1356+
pub fn set_unpredictable(&mut self, inst: &'ll Value) {
1357+
unsafe {
1358+
llvm::LLVMSetMetadata(
1359+
inst,
1360+
llvm::MD_unpredictable as c_uint,
1361+
llvm::LLVMMDNodeInContext(self.cx.llcx, ptr::null(), 0),
1362+
);
1363+
}
1364+
}
1365+
13561366
pub fn minnum(&mut self, lhs: &'ll Value, rhs: &'ll Value) -> &'ll Value {
13571367
unsafe { llvm::LLVMRustBuildMinNum(self.llbuilder, lhs, rhs) }
13581368
}

compiler/rustc_codegen_llvm/src/intrinsic.rs

+8
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,14 @@ impl<'ll, 'tcx> IntrinsicCallMethods<'tcx> for Builder<'_, 'll, 'tcx> {
203203
}
204204
sym::unlikely => self
205205
.call_intrinsic("llvm.expect.i1", &[args[0].immediate(), self.const_bool(false)]),
206+
sym::select_unpredictable => {
207+
let cond = args[0].immediate();
208+
let true_val = args[1].immediate();
209+
let false_val = args[2].immediate();
210+
let result = self.select(cond, true_val, false_val);
211+
self.set_unpredictable(&result);
212+
result
213+
}
206214
sym::catch_unwind => {
207215
catch_unwind_intrinsic(
208216
self,

compiler/rustc_codegen_llvm/src/llvm/ffi.rs

+1
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,7 @@ pub enum MetadataType {
428428
MD_nontemporal = 9,
429429
MD_mem_parallel_loop_access = 10,
430430
MD_nonnull = 11,
431+
MD_unpredictable = 15,
431432
MD_align = 17,
432433
MD_type = 19,
433434
MD_vcall_visibility = 28,

compiler/rustc_hir_analysis/src/check/intrinsic.rs

+2
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ pub fn intrinsic_operation_unsafety(tcx: TyCtxt<'_>, intrinsic_id: LocalDefId) -
119119
| sym::type_id
120120
| sym::likely
121121
| sym::unlikely
122+
| sym::select_unpredictable
122123
| sym::ptr_guaranteed_cmp
123124
| sym::minnumf16
124125
| sym::minnumf32
@@ -487,6 +488,7 @@ pub fn check_intrinsic_type(
487488
sym::assume => (0, 0, vec![tcx.types.bool], tcx.types.unit),
488489
sym::likely => (0, 0, vec![tcx.types.bool], tcx.types.bool),
489490
sym::unlikely => (0, 0, vec![tcx.types.bool], tcx.types.bool),
491+
sym::select_unpredictable => (1, 0, vec![tcx.types.bool, param(0), param(0)], param(0)),
490492

491493
sym::read_via_copy => (1, 0, vec![Ty::new_imm_ptr(tcx, param(0))], param(0)),
492494
sym::write_via_move => {

compiler/rustc_span/src/symbol.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1701,6 +1701,7 @@ symbols! {
17011701
saturating_add,
17021702
saturating_div,
17031703
saturating_sub,
1704+
select_unpredictable,
17041705
self_in_typedefs,
17051706
self_struct_ctor,
17061707
semitransparent,

library/core/src/intrinsics.rs

+28
Original file line numberDiff line numberDiff line change
@@ -1010,6 +1010,34 @@ pub const fn unlikely(b: bool) -> bool {
10101010
b
10111011
}
10121012

1013+
/// Returns either `true_val` or `false_val` depending on condition `b` with a
1014+
/// hint to the compiler that this condition is unlikely to be correctly
1015+
/// predicted by a CPU's branch predictor (e.g. a binary search).
1016+
///
1017+
/// This is otherwise functionally equivalent to `if b { true_val } else { false_val }`.
1018+
///
1019+
/// Note that, unlike most intrinsics, this is safe to call;
1020+
/// it does not require an `unsafe` block.
1021+
/// Therefore, implementations must not require the user to uphold
1022+
/// any safety invariants.
1023+
///
1024+
/// This intrinsic does not have a stable counterpart.
1025+
#[cfg(not(bootstrap))]
1026+
#[unstable(feature = "core_intrinsics", issue = "none")]
1027+
#[rustc_intrinsic]
1028+
#[rustc_nounwind]
1029+
#[miri::intrinsic_fallback_is_spec]
1030+
#[inline]
1031+
pub fn select_unpredictable<T>(b: bool, true_val: T, false_val: T) -> T {
1032+
if b { true_val } else { false_val }
1033+
}
1034+
1035+
#[cfg(bootstrap)]
1036+
#[inline]
1037+
pub fn select_unpredictable<T>(b: bool, true_val: T, false_val: T) -> T {
1038+
if b { true_val } else { false_val }
1039+
}
1040+
10131041
extern "rust-intrinsic" {
10141042
/// Executes a breakpoint trap, for inspection by a debugger.
10151043
///

library/core/src/slice/mod.rs

+6-7
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
use crate::cmp::Ordering::{self, Equal, Greater, Less};
1010
use crate::fmt;
1111
use crate::hint;
12-
use crate::intrinsics::{exact_div, unchecked_sub};
12+
use crate::intrinsics::{exact_div, select_unpredictable, unchecked_sub};
1313
use crate::mem::{self, SizedTypeProperties};
1414
use crate::num::NonZero;
1515
use crate::ops::{Bound, OneSidedRange, Range, RangeBounds};
@@ -2803,12 +2803,11 @@ impl<T> [T] {
28032803
// we have `left + size/2 < self.len()`, and this is in-bounds.
28042804
let cmp = f(unsafe { self.get_unchecked(mid) });
28052805

2806-
// This control flow produces conditional moves, which results in
2807-
// fewer branches and instructions than if/else or matching on
2808-
// cmp::Ordering.
2809-
// This is x86 asm for u8: https://rust.godbolt.org/z/698eYffTx.
2810-
left = if cmp == Less { mid + 1 } else { left };
2811-
right = if cmp == Greater { mid } else { right };
2806+
// Binary search interacts poorly with branch prediction, so force
2807+
// the compiler to use conditional moves if supported by the target
2808+
// architecture.
2809+
left = select_unpredictable(cmp == Less, mid + 1, left);
2810+
right = select_unpredictable(cmp == Greater, mid, right);
28122811
if cmp == Equal {
28132812
// SAFETY: same as the `get_unchecked` above
28142813
unsafe { hint::assert_unchecked(mid < self.len()) };
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
//@ compile-flags: -O
2+
3+
#![feature(core_intrinsics)]
4+
#![crate_type = "lib"]
5+
6+
#[no_mangle]
7+
pub fn foo(p: bool, a: u64, b: u64) -> u64 {
8+
// CHECK-LABEL: define{{.*}}i64 @foo
9+
// CHECK: select i1 %p, i64 %a, i64 %b, !unpredictable
10+
core::intrinsics::select_unpredictable(p, a, b)
11+
}

0 commit comments

Comments
 (0)