Skip to content

Commit

Permalink
Stop generating assumes for validity ranges
Browse files Browse the repository at this point in the history 
Now that LLVM has implemented range attributes on arguments and returns,
and now that we generate such attributes, we no longer need to use
assumes to represent validity ranges.
  • Loading branch information
@erikdesjardins
erikdesjardins committed Aug 12, 2024
1 parent e08b80c commit 42fe613
Show file tree
Hide file tree
Showing 15 changed files with 53 additions and 333 deletions.
69 changes: 4 additions & 65 deletions compiler/rustc_codegen_ssa/src/mir/rvalue.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,13 +240,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 @@ -262,13 +260,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 @@ -292,12 +288,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 @@ -339,21 +329,15 @@ 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 {
use abi::Primitive::*;

assert_eq!(from_scalar.size(self.cx), to_scalar.size(self.cx));

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);

imm = match (from_scalar.primitive(), to_scalar.primitive()) {
(Int(..) | Float(_), Int(..) | Float(_)) => bx.bitcast(imm, to_backend_ty),
(Pointer(..), Pointer(..)) => bx.pointercast(imm, to_backend_ty),
Expand All @@ -368,55 +352,10 @@ 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);
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,
) {
if matches!(self.cx.sess().opts.optimize, OptLevel::No | OptLevel::Less)
// For now, the critical niches are all over `Int`eger values.
// Should floating-point values or pointers ever get more complex
// niches, then this code will probably want to handle them too.
|| !matches!(scalar.primitive(), abi::Primitive::Int(..))
|| scalar.is_always_valid(self.cx)
{
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);
}
}

pub fn codegen_rvalue_unsized(
&mut self,
bx: &mut Bx,
Expand Down
76 changes: 2 additions & 74 deletions compiler/rustc_mir_build/src/build/expr/as_rvalue.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,11 @@ use rustc_middle::mir::interpret::Scalar;
use rustc_middle::mir::*;
use rustc_middle::thir::*;
use rustc_middle::ty::cast::{mir_cast_kind, CastTy};
use rustc_middle::ty::layout::IntegerExt;
use rustc_middle::ty::util::IntTypeExt;
use rustc_middle::ty::{self, Ty, UpvarArgs};
use rustc_span::source_map::Spanned;
use rustc_span::{Span, DUMMY_SP};
use rustc_target::abi::{Abi, FieldIdx, Primitive};
use rustc_target::abi::FieldIdx;
use tracing::debug;

use crate::build::expr::as_place::PlaceBase;
Expand Down Expand Up @@ -201,85 +200,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
{
let discr_ty = adt_def.repr().discr_type().to_ty(this.tcx);
let temp = unpack!(block = this.as_temp(block, scope, source, Mutability::Not));
let layout = this.tcx.layout_of(this.param_env.and(source_expr.ty));
let discr = this.temp(discr_ty, source_expr.span);
this.cfg.push_assign(
block,
source_info,
discr,
Rvalue::Discriminant(temp.into()),
);
let (op, ty) = (Operand::Move(discr), discr_ty);

if let Abi::Scalar(scalar) = layout.unwrap().abi
&& !scalar.is_always_valid(&this.tcx)
&& let Primitive::Int(int_width, _signed) = scalar.primitive()
{
let unsigned_ty = int_width.to_ty(this.tcx, false);
let unsigned_place = this.temp(unsigned_ty, expr_span);
this.cfg.push_assign(
block,
source_info,
unsigned_place,
Rvalue::Cast(CastKind::IntToInt, Operand::Copy(discr), unsigned_ty),
);

let bool_ty = this.tcx.types.bool;
let range = scalar.valid_range(&this.tcx);
let merge_op =
if range.start <= range.end { BinOp::BitAnd } else { BinOp::BitOr };

let mut comparer = |range: u128, bin_op: BinOp| -> Place<'tcx> {
let range_val = Const::from_bits(
this.tcx,
range,
ty::ParamEnv::empty().and(unsigned_ty),
);
let lit_op = this.literal_operand(expr.span, range_val);
let is_bin_op = this.temp(bool_ty, expr_span);
this.cfg.push_assign(
block,
source_info,
is_bin_op,
Rvalue::BinaryOp(
bin_op,
Box::new((Operand::Copy(unsigned_place), lit_op)),
),
);
is_bin_op
};
let assert_place = if range.start == 0 {
comparer(range.end, BinOp::Le)
} else {
let start_place = comparer(range.start, BinOp::Ge);
let end_place = comparer(range.end, BinOp::Le);
let merge_place = this.temp(bool_ty, expr_span);
this.cfg.push_assign(
block,
source_info,
merge_place,
Rvalue::BinaryOp(
merge_op,
Box::new((
Operand::Move(start_place),
Operand::Move(end_place),
)),
),
);
merge_place
};
this.cfg.push(
block,
Statement {
source_info,
kind: StatementKind::Intrinsic(Box::new(
NonDivergingIntrinsic::Assume(Operand::Move(assert_place)),
)),
},
);
}

(op, ty)
(Operand::Move(discr), discr_ty)
} else {
let ty = source_expr.ty;
let source = unpack!(
Expand Down
1 change: 0 additions & 1 deletion src/tools/tidy/src/issues.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3628,7 +3628,6 @@ ui/regions/issue-56537-closure-uses-region-from-container.rs
ui/regions/issue-6157.rs
ui/regions/issue-72051-member-region-hang.rs
ui/regions/issue-78262.rs
ui/repr/issue-83505-repr-simd.rs
ui/resolve/auxiliary/issue-112831-aux.rs
ui/resolve/auxiliary/issue-19452-aux.rs
ui/resolve/auxiliary/issue-21221-3.rs
Expand Down
111 changes: 10 additions & 101 deletions tests/codegen/intrinsics/transmute-niched.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
//@ revisions: OPT DBG
//@ [OPT] compile-flags: -C opt-level=3 -C no-prepopulate-passes
//@ [DBG] compile-flags: -C opt-level=0 -C no-prepopulate-passes
//@ only-64bit (so I don't need to worry about usize)
//@ compile-flags: -C opt-level=3 -C no-prepopulate-passes
#![crate_type = "lib"]

use std::mem::transmute;
Expand All @@ -14,59 +11,27 @@ pub enum SmallEnum {
C = 12,
}

// CHECK-LABEL: @check_to_enum(
// CHECK: noundef range(i8 10, 13) i8 @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
// OPT: call void @llvm.assume(i1 %1)
// DBG-NOT: icmp
// DBG-NOT: assume
// CHECK: ret i8 %x

transmute(x)
}

// CHECK-LABEL: @check_from_enum(
// CHECK: @check_from_enum(i8 noundef 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: ret i8 %x

transmute(x)
}

// CHECK-LABEL: @check_to_ordering(
// CHECK: noundef range(i8 -1, 2) i8 @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: ret i8 %x

transmute(x)
}

// CHECK-LABEL: @check_from_ordering(
// CHECK: @check_from_ordering(i8 noundef 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: ret i8 %x

transmute(x)
}

Expand Down Expand Up @@ -95,88 +60,32 @@ pub enum Minus100ToPlus100 {
U = 100,
}

// CHECK-LABEL: @check_enum_from_char(
// CHECK: noundef range(i32 -100, 101) i32 @check_enum_from_char(i32 noundef 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: ret i32 %x

transmute(x)
}

// CHECK-LABEL: @check_enum_to_char(
// CHECK: noundef range(i32 0, 1114112) i32 @check_enum_to_char(i32 noundef 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: ret i32 %x

transmute(x)
}

// CHECK-LABEL: @check_swap_pair(
// CHECK: @check_swap_pair(i32 noundef range(i32 0, 1114112) %x.0, i32 noundef 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
// OPT: call void @llvm.assume(i1 %1)
// OPT: %2 = icmp uge i32 %x.1, 1
// 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: %[[P1:.+]] = insertvalue { i32, i32 } poison, i32 %x.0, 0
// CHECK: %[[P2:.+]] = insertvalue { i32, i32 } %[[P1]], i32 %x.1, 1
// CHECK: ret { i32, i32 } %[[P2]]

transmute(x)
}

// CHECK-LABEL: @check_bool_from_ordering(
// CHECK: @check_bool_from_ordering(i8 noundef 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: %[[R:.+]] = trunc i8 %x to i1
// CHECK: ret i1 %[[R]]

transmute(x)
}

// CHECK-LABEL: @check_bool_to_ordering(
// CHECK: noundef range(i8 -1, 2) i8 @check_bool_to_ordering(
#[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: ret i8 %_0

transmute(x)
}
Loading

0 comments on commit 42fe613

Please sign in to comment.