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

Add select_unpredictable to force LLVM to use CMOV #128250

Merged
merged 1 commit into from
Jul 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
10 changes: 10 additions & 0 deletions compiler/rustc_codegen_llvm/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1353,6 +1353,16 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
}
}

pub fn set_unpredictable(&mut self, inst: &'ll Value) {
unsafe {
llvm::LLVMSetMetadata(
inst,
llvm::MD_unpredictable as c_uint,
llvm::LLVMMDNodeInContext(self.cx.llcx, ptr::null(), 0),
);
}
}

pub fn minnum(&mut self, lhs: &'ll Value, rhs: &'ll Value) -> &'ll Value {
unsafe { llvm::LLVMRustBuildMinNum(self.llbuilder, lhs, rhs) }
}
Expand Down
31 changes: 30 additions & 1 deletion compiler/rustc_codegen_llvm/src/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use rustc_codegen_ssa::base::{compare_simd_types, wants_msvc_seh, wants_wasm_eh}
use rustc_codegen_ssa::common::{IntPredicate, TypeKind};
use rustc_codegen_ssa::errors::{ExpectedPointerMutability, InvalidMonomorphization};
use rustc_codegen_ssa::mir::operand::{OperandRef, OperandValue};
use rustc_codegen_ssa::mir::place::PlaceRef;
use rustc_codegen_ssa::mir::place::{PlaceRef, PlaceValue};
use rustc_codegen_ssa::traits::*;
use rustc_hir as hir;
use rustc_middle::mir::BinOp;
Expand Down Expand Up @@ -203,6 +203,35 @@ impl<'ll, 'tcx> IntrinsicCallMethods<'tcx> for Builder<'_, 'll, 'tcx> {
}
sym::unlikely => self
.call_intrinsic("llvm.expect.i1", &[args[0].immediate(), self.const_bool(false)]),
sym::select_unpredictable => {
let cond = args[0].immediate();
assert_eq!(args[1].layout, args[2].layout);
let select = |bx: &mut Self, true_val, false_val| {
let result = bx.select(cond, true_val, false_val);
bx.set_unpredictable(&result);
result
};
match (args[1].val, args[2].val) {
(OperandValue::Ref(true_val), OperandValue::Ref(false_val)) => {
assert!(true_val.llextra.is_none());
assert!(false_val.llextra.is_none());
assert_eq!(true_val.align, false_val.align);
let ptr = select(self, true_val.llval, false_val.llval);
let selected =
OperandValue::Ref(PlaceValue::new_sized(ptr, true_val.align));
selected.store(self, result);
return Ok(());
}
Amanieu marked this conversation as resolved.
Show resolved Hide resolved
(OperandValue::Immediate(_), OperandValue::Immediate(_))
| (OperandValue::Pair(_, _), OperandValue::Pair(_, _)) => {
let true_val = args[1].immediate_or_packed_pair(self);
let false_val = args[2].immediate_or_packed_pair(self);
select(self, true_val, false_val)
}
(OperandValue::ZeroSized, OperandValue::ZeroSized) => return Ok(()),
_ => span_bug!(span, "Incompatible OperandValue for select_unpredictable"),
}
}
sym::catch_unwind => {
catch_unwind_intrinsic(
self,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_llvm/src/llvm/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,7 @@ pub enum MetadataType {
MD_nontemporal = 9,
MD_mem_parallel_loop_access = 10,
MD_nonnull = 11,
MD_unpredictable = 15,
MD_align = 17,
MD_type = 19,
MD_vcall_visibility = 28,
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_hir_analysis/src/check/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ pub fn intrinsic_operation_unsafety(tcx: TyCtxt<'_>, intrinsic_id: LocalDefId) -
| sym::type_id
| sym::likely
| sym::unlikely
| sym::select_unpredictable
| sym::ptr_guaranteed_cmp
| sym::minnumf16
| sym::minnumf32
Expand Down Expand Up @@ -487,6 +488,7 @@ pub fn check_intrinsic_type(
sym::assume => (0, 0, vec![tcx.types.bool], tcx.types.unit),
sym::likely => (0, 0, vec![tcx.types.bool], tcx.types.bool),
sym::unlikely => (0, 0, vec![tcx.types.bool], tcx.types.bool),
sym::select_unpredictable => (1, 0, vec![tcx.types.bool, param(0), param(0)], param(0)),

sym::read_via_copy => (1, 0, vec![Ty::new_imm_ptr(tcx, param(0))], param(0)),
sym::write_via_move => {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1701,6 +1701,7 @@ symbols! {
saturating_add,
saturating_div,
saturating_sub,
select_unpredictable,
self_in_typedefs,
self_struct_ctor,
semitransparent,
Expand Down
28 changes: 28 additions & 0 deletions library/core/src/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1010,6 +1010,34 @@ pub const fn unlikely(b: bool) -> bool {
b
}

/// Returns either `true_val` or `false_val` depending on condition `b` with a
/// hint to the compiler that this condition is unlikely to be correctly
/// predicted by a CPU's branch predictor (e.g. a binary search).
///
/// This is otherwise functionally equivalent to `if b { true_val } else { false_val }`.
///
/// Note that, unlike most intrinsics, this is safe to call;
/// it does not require an `unsafe` block.
/// Therefore, implementations must not require the user to uphold
/// any safety invariants.
///
/// This intrinsic does not have a stable counterpart.
#[cfg(not(bootstrap))]
#[unstable(feature = "core_intrinsics", issue = "none")]
#[rustc_intrinsic]
#[rustc_nounwind]
#[miri::intrinsic_fallback_is_spec]
Copy link
Member

Choose a reason for hiding this comment

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

Please ping @rust-lang/miri when using this attribute, so that we can have a look at the spec. :)

#[inline]
pub fn select_unpredictable<T>(b: bool, true_val: T, false_val: T) -> T {
if b { true_val } else { false_val }
}

#[cfg(bootstrap)]
#[inline]
pub fn select_unpredictable<T>(b: bool, true_val: T, false_val: T) -> T {
if b { true_val } else { false_val }
}

extern "rust-intrinsic" {
/// Executes a breakpoint trap, for inspection by a debugger.
///
Expand Down
35 changes: 35 additions & 0 deletions tests/codegen/intrinsics/select_unpredictable.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
//@ compile-flags: -O

#![feature(core_intrinsics)]
#![crate_type = "lib"]

#[no_mangle]
pub fn test_int(p: bool, a: u64, b: u64) -> u64 {
// CHECK-LABEL: define{{.*}} @test_int
// CHECK: select i1 %p, i64 %a, i64 %b, !unpredictable
core::intrinsics::select_unpredictable(p, a, b)
}

#[no_mangle]
pub fn test_pair(p: bool, a: (u64, u64), b: (u64, u64)) -> (u64, u64) {
// CHECK-LABEL: define{{.*}} @test_pair
// CHECK: select i1 %p, {{.*}}, !unpredictable
core::intrinsics::select_unpredictable(p, a, b)
}

struct Large {
e: [u64; 100],
}

#[no_mangle]
pub fn test_struct(p: bool, a: Large, b: Large) -> Large {
// CHECK-LABEL: define{{.*}} @test_struct
// CHECK: select i1 %p, {{.*}}, !unpredictable
core::intrinsics::select_unpredictable(p, a, b)
}

#[no_mangle]
pub fn test_zst(p: bool, a: (), b: ()) -> () {
// CHECK-LABEL: define{{.*}} @test_zst
core::intrinsics::select_unpredictable(p, a, b)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe?

Suggested change
core::intrinsics::select_unpredictable(p, a, b)
core::intrinsics::select_unpredictable(p, a, b)
// CHECK-NOT: select i1
// CHECK: ret void

Otherwise, I don't know what we are supposed to be testing.

}
Loading