Skip to content

Emit fewer assumes now that we have range metadata on parameters #135610

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Emit fewer assumes now that we have range metadata on parameters #135610

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
53e3939
Emit fewer `assume`s now that we have `range` metadata on parameters
scottmcm Jan 17, 2025
7fb2b90
Emit range assumes as a single `icmp`
scottmcm Jan 17, 2025
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
71 changes: 20 additions & 51 deletions compiler/rustc_codegen_ssa/src/mir/rvalue.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,13 +257,11 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
if let OperandValueKind::Immediate(to_scalar) = cast_kind
&& from_scalar.size(self.cx) == to_scalar.size(self.cx)
{
let from_backend_ty = bx.backend_type(operand.layout);
let to_backend_ty = bx.backend_type(cast);
Some(OperandValue::Immediate(self.transmute_immediate(
bx,
imm,
from_scalar,
from_backend_ty,
to_scalar,
to_backend_ty,
)))
Expand All @@ -279,13 +277,11 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
&& in_a.size(self.cx) == out_a.size(self.cx)
&& in_b.size(self.cx) == out_b.size(self.cx)
{
let in_a_ibty = bx.scalar_pair_element_backend_type(operand.layout, 0, false);
let in_b_ibty = bx.scalar_pair_element_backend_type(operand.layout, 1, false);
let out_a_ibty = bx.scalar_pair_element_backend_type(cast, 0, false);
let out_b_ibty = bx.scalar_pair_element_backend_type(cast, 1, false);
Some(OperandValue::Pair(
self.transmute_immediate(bx, imm_a, in_a, in_a_ibty, out_a, out_a_ibty),
self.transmute_immediate(bx, imm_b, in_b, in_b_ibty, out_b, out_b_ibty),
self.transmute_immediate(bx, imm_a, in_a, out_a, out_a_ibty),
self.transmute_immediate(bx, imm_b, in_b, out_b, out_b_ibty),
))
} else {
None
Expand All @@ -309,12 +305,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
) -> Option<Bx::Value> {
use abi::Primitive::*;

// When scalars are passed by value, there's no metadata recording their
// valid ranges. For example, `char`s are passed as just `i32`, with no
// way for LLVM to know that they're 0x10FFFF at most. Thus we assume
// the range of the input value too, not just the output range.
self.assume_scalar_range(bx, imm, from_scalar, from_backend_ty);

imm = match (from_scalar.primitive(), to_scalar.primitive()) {
(Int(_, is_signed), Int(..)) => bx.intcast(imm, to_backend_ty, is_signed),
(Float(_), Float(_)) => {
Expand Down Expand Up @@ -356,7 +346,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
bx: &mut Bx,
mut imm: Bx::Value,
from_scalar: abi::Scalar,
from_backend_ty: Bx::Type,
to_scalar: abi::Scalar,
to_backend_ty: Bx::Type,
) -> Bx::Value {
Expand All @@ -365,11 +354,13 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
use abi::Primitive::*;
imm = bx.from_immediate(imm);

// When scalars are passed by value, there's no metadata recording their
// valid ranges. For example, `char`s are passed as just `i32`, with no
// way for LLVM to know that they're 0x10FFFF at most. Thus we assume
// the range of the input value too, not just the output range.
self.assume_scalar_range(bx, imm, from_scalar, from_backend_ty);
// We used to `assume` the `from_scalar` here too, but that's no longer needed
// because if we have a scalar, we must already know its range. Either
// 1) It's a parameter with `range` parameter metadata,
// 2) It's something we `load`ed with `!range` metadata, or
// 3) It's something we transmuted and already `assume`d the range.
// And thus in all those cases another `assume` is just wasteful.
// (Case 1 didn't used to be covered, and thus the `assume` was needed.)

imm = match (from_scalar.primitive(), to_scalar.primitive()) {
(Int(..) | Float(_), Int(..) | Float(_)) => bx.bitcast(imm, to_backend_ty),
Expand All @@ -389,18 +380,19 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
bx.bitcast(int_imm, to_backend_ty)
}
};
self.assume_scalar_range(bx, imm, to_scalar, to_backend_ty);

// This `assume` remains important for cases like
// transmute::<u32, NonZeroU32>(x) == 0
// since it's never passed to something with parameter metadata (especially
// after MIR inlining) so the only way to tell the backend about the
// constraint that the `transmute` introduced is to `assume` it.
self.assume_scalar_range(bx, imm, to_scalar);

imm = bx.to_immediate_scalar(imm, to_scalar);
imm
}

fn assume_scalar_range(
&self,
bx: &mut Bx,
imm: Bx::Value,
scalar: abi::Scalar,
backend_ty: Bx::Type,
) {
fn assume_scalar_range(&self, bx: &mut Bx, imm: Bx::Value, scalar: abi::Scalar) {
if matches!(self.cx.sess().opts.optimize, OptLevel::No)
// For now, the critical niches are all over `Int`eger values.
// Should floating-point values or pointers ever get more complex
Expand All @@ -411,31 +403,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
return;
}

let abi::WrappingRange { start, end } = scalar.valid_range(self.cx);

if start <= end {
if start > 0 {
let low = bx.const_uint_big(backend_ty, start);
let cmp = bx.icmp(IntPredicate::IntUGE, imm, low);
bx.assume(cmp);
}

let type_max = scalar.size(self.cx).unsigned_int_max();
if end < type_max {
let high = bx.const_uint_big(backend_ty, end);
let cmp = bx.icmp(IntPredicate::IntULE, imm, high);
bx.assume(cmp);
}
} else {
let low = bx.const_uint_big(backend_ty, start);
let cmp_low = bx.icmp(IntPredicate::IntUGE, imm, low);

let high = bx.const_uint_big(backend_ty, end);
let cmp_high = bx.icmp(IntPredicate::IntULE, imm, high);

let or = bx.or(cmp_low, cmp_high);
bx.assume(or);
}
let range = scalar.valid_range(self.cx);
bx.assume_integer_range(imm, range);
}

pub(crate) fn codegen_rvalue_unsized(
Expand Down
22 changes: 22 additions & 0 deletions compiler/rustc_codegen_ssa/src/traits/builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,28 @@ pub trait BuilderMethods<'a, 'tcx>:
dest: PlaceRef<'tcx, Self::Value>,
);

/// Emits an `assume` that the integer value `imm` is contained in `range`.
///
/// This *always* emits the assumption, so you probably want to check the
/// optimization level and `Scalar::is_always_valid` before calling it.
fn assume_integer_range(&mut self, imm: Self::Value, range: WrappingRange) {
let WrappingRange { start, end } = range;
let backend_ty = self.cx().val_ty(imm);

// Perhaps one day we'll be able to use assume operand bundles for this,
// but for now this encoding with a single icmp+assume is best per
// <https://github.com/llvm/llvm-project/issues/123278#issuecomment-2597440158>
let shifted = if start == 0 {
imm
} else {
let low = self.const_uint_big(backend_ty, start);
self.sub(imm, low)
};
let width = self.const_uint_big(backend_ty, u128::wrapping_sub(end, start));
let cmp = self.icmp(IntPredicate::IntULE, shifted, width);
self.assume(cmp);
}

fn range_metadata(&mut self, load: Self::Value, range: WrappingRange);
fn nonnull_metadata(&mut self, load: Self::Value);

Expand Down
1 change: 1 addition & 0 deletions tests/codegen/cast-optimized.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
//@ compile-flags: -O -Z merge-functions=disabled
//@ min-llvm-version: 19 (these optimizations depend on range parameter metadata)
#![crate_type = "lib"]

// This tests that LLVM can optimize based on the niches in the source or
Expand Down
122 changes: 59 additions & 63 deletions tests/codegen/intrinsics/transmute-niched.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//@ revisions: OPT DBG
//@ [OPT] compile-flags: -C opt-level=3 -C no-prepopulate-passes
//@ [OPT] min-llvm-version: 19 (for range parameter metadata)
//@ [DBG] compile-flags: -C opt-level=0 -C no-prepopulate-passes
//@ only-64bit (so I don't need to worry about usize)
#![crate_type = "lib"]
Expand All @@ -17,26 +18,24 @@ pub enum SmallEnum {
// CHECK-LABEL: @check_to_enum(
#[no_mangle]
pub unsafe fn check_to_enum(x: i8) -> SmallEnum {
// OPT: %0 = icmp uge i8 %x, 10
// OPT: call void @llvm.assume(i1 %0)
// OPT: %1 = icmp ule i8 %x, 12
// CHECK-NOT: icmp
// CHECK-NOT: assume
// OPT: %0 = sub i8 %x, 10
// OPT: %1 = icmp ule i8 %0, 2
// OPT: call void @llvm.assume(i1 %1)
// DBG-NOT: icmp
// DBG-NOT: assume
// CHECK-NOT: icmp
// CHECK-NOT: assume
// CHECK: ret i8 %x

transmute(x)
}

// CHECK-LABEL: @check_from_enum(
// OPT-SAME: range(i8 10, 13){{.+}}%x
#[no_mangle]
pub unsafe fn check_from_enum(x: SmallEnum) -> i8 {
// OPT: %0 = icmp uge i8 %x, 10
// OPT: call void @llvm.assume(i1 %0)
// OPT: %1 = icmp ule i8 %x, 12
// OPT: call void @llvm.assume(i1 %1)
// DBG-NOT: icmp
// DBG-NOT: assume
// CHECK-NOT: icmp
// CHECK-NOT: assume
// CHECK: ret i8 %x

transmute(x)
Expand All @@ -45,26 +44,24 @@ pub unsafe fn check_from_enum(x: SmallEnum) -> i8 {
// CHECK-LABEL: @check_to_ordering(
#[no_mangle]
pub unsafe fn check_to_ordering(x: u8) -> std::cmp::Ordering {
// OPT: %0 = icmp uge i8 %x, -1
// OPT: %1 = icmp ule i8 %x, 1
// OPT: %2 = or i1 %0, %1
// OPT: call void @llvm.assume(i1 %2)
// DBG-NOT: icmp
// DBG-NOT: assume
// CHECK-NOT: icmp
// CHECK-NOT: assume
// OPT: %0 = sub i8 %x, -1
// OPT: %1 = icmp ule i8 %0, 2
// OPT: call void @llvm.assume(i1 %1)
// CHECK-NOT: icmp
// CHECK-NOT: assume
// CHECK: ret i8 %x

transmute(x)
}

// CHECK-LABEL: @check_from_ordering(
// OPT-SAME: range(i8 -1, 2){{.+}}%x
#[no_mangle]
pub unsafe fn check_from_ordering(x: std::cmp::Ordering) -> u8 {
// OPT: %0 = icmp uge i8 %x, -1
// OPT: %1 = icmp ule i8 %x, 1
// OPT: %2 = or i1 %0, %1
// OPT: call void @llvm.assume(i1 %2)
// DBG-NOT: icmp
// DBG-NOT: assume
// CHECK-NOT: icmp
// CHECK-NOT: assume
// CHECK: ret i8 %x

transmute(x)
Expand Down Expand Up @@ -96,50 +93,50 @@ pub enum Minus100ToPlus100 {
}

// CHECK-LABEL: @check_enum_from_char(
// OPT-SAME: range(i32 0, 1114112){{.+}}%x
#[no_mangle]
pub unsafe fn check_enum_from_char(x: char) -> Minus100ToPlus100 {
// OPT: %0 = icmp ule i32 %x, 1114111
// OPT: call void @llvm.assume(i1 %0)
// OPT: %1 = icmp uge i32 %x, -100
// OPT: %2 = icmp ule i32 %x, 100
// OPT: %3 = or i1 %1, %2
// OPT: call void @llvm.assume(i1 %3)
// DBG-NOT: icmp
// DBG-NOT: assume
// CHECK-NOT: icmp
// CHECK-NOT: assume
// OPT: %0 = sub i32 %x, -100
// OPT: %1 = icmp ule i32 %0, 200
// OPT: call void @llvm.assume(i1 %1)
// CHECK-NOT: icmp
// CHECK-NOT: assume
// CHECK: ret i32 %x

transmute(x)
}

// CHECK-LABEL: @check_enum_to_char(
// OPT-SAME: range(i32 -100, 101){{.+}}%x
#[no_mangle]
pub unsafe fn check_enum_to_char(x: Minus100ToPlus100) -> char {
// OPT: %0 = icmp uge i32 %x, -100
// OPT: %1 = icmp ule i32 %x, 100
// OPT: %2 = or i1 %0, %1
// OPT: call void @llvm.assume(i1 %2)
// OPT: %3 = icmp ule i32 %x, 1114111
// OPT: call void @llvm.assume(i1 %3)
// DBG-NOT: icmp
// DBG-NOT: assume
// CHECK-NOT: icmp
// CHECK-NOT: assume
// OPT: %0 = icmp ule i32 %x, 1114111
// OPT: call void @llvm.assume(i1 %0)
// CHECK-NOT: icmp
// CHECK-NOT: assume
// CHECK: ret i32 %x

transmute(x)
}

// CHECK-LABEL: @check_swap_pair(
// OPT-SAME: range(i32 0, 1114112){{.+}}%x.0
// OPT-SAME: range(i32 1, 0){{.+}}%x.1
#[no_mangle]
pub unsafe fn check_swap_pair(x: (char, NonZero<u32>)) -> (NonZero<u32>, char) {
// OPT: %0 = icmp ule i32 %x.0, 1114111
// OPT: call void @llvm.assume(i1 %0)
// OPT: %1 = icmp uge i32 %x.0, 1
// CHECK-NOT: icmp
// CHECK-NOT: assume
// OPT: %0 = sub i32 %x.0, 1
// OPT: %1 = icmp ule i32 %0, -2
// OPT: call void @llvm.assume(i1 %1)
// OPT: %2 = icmp uge i32 %x.1, 1
// OPT: %2 = icmp ule i32 %x.1, 1114111
// OPT: call void @llvm.assume(i1 %2)
// OPT: %3 = icmp ule i32 %x.1, 1114111
// OPT: call void @llvm.assume(i1 %3)
// DBG-NOT: icmp
// DBG-NOT: assume
// CHECK-NOT: icmp
// CHECK-NOT: assume
// CHECK: %[[P1:.+]] = insertvalue { i32, i32 } poison, i32 %x.0, 0
// CHECK: %[[P2:.+]] = insertvalue { i32, i32 } %[[P1]], i32 %x.1, 1
// CHECK: ret { i32, i32 } %[[P2]]
Expand All @@ -148,34 +145,33 @@ pub unsafe fn check_swap_pair(x: (char, NonZero<u32>)) -> (NonZero<u32>, char) {
}

// CHECK-LABEL: @check_bool_from_ordering(
// OPT-SAME: range(i8 -1, 2){{.+}}%x
#[no_mangle]
pub unsafe fn check_bool_from_ordering(x: std::cmp::Ordering) -> bool {
// OPT: %0 = icmp uge i8 %x, -1
// OPT: %1 = icmp ule i8 %x, 1
// OPT: %2 = or i1 %0, %1
// OPT: call void @llvm.assume(i1 %2)
// OPT: %3 = icmp ule i8 %x, 1
// OPT: call void @llvm.assume(i1 %3)
// DBG-NOT: icmp
// DBG-NOT: assume
// CHECK-NOT: icmp
// CHECK-NOT: assume
// OPT: %0 = icmp ule i8 %x, 1
// OPT: call void @llvm.assume(i1 %0)
// CHECK-NOT: icmp
// CHECK-NOT: assume
// CHECK: %[[R:.+]] = trunc i8 %x to i1
// CHECK: ret i1 %[[R]]

transmute(x)
}

// CHECK-LABEL: @check_bool_to_ordering(
// OPT-SAME: i1 {{.+}} %x
#[no_mangle]
pub unsafe fn check_bool_to_ordering(x: bool) -> std::cmp::Ordering {
// CHECK: %_0 = zext i1 %x to i8
// OPT: %0 = icmp ule i8 %_0, 1
// OPT: call void @llvm.assume(i1 %0)
// OPT: %1 = icmp uge i8 %_0, -1
// OPT: %2 = icmp ule i8 %_0, 1
// OPT: %3 = or i1 %1, %2
// OPT: call void @llvm.assume(i1 %3)
// DBG-NOT: icmp
// DBG-NOT: assume
// CHECK-NOT: icmp
// CHECK-NOT: assume
// OPT: %0 = sub i8 %_0, -1
// OPT: %1 = icmp ule i8 %0, 2
// OPT: call void @llvm.assume(i1 %1)
// CHECK-NOT: icmp
// CHECK-NOT: assume
// CHECK: ret i8 %_0

transmute(x)
Expand Down
1 change: 1 addition & 0 deletions tests/codegen/issues/issue-119422.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
//! for `NonZero` integer types.
//!
//@ compile-flags: -O --edition=2021 -Zmerge-functions=disabled
//@ min-llvm-version: 19 (for range parameter metadata)
//@ only-64bit (because the LLVM type of i64 for usize shows up)
#![crate_type = "lib"]

Expand Down
Loading
Loading