Skip to content

Commit

Permalink
Auto merge of #129027 - erikdesjardins:outofuandme, r=<try>
Browse files Browse the repository at this point in the history 
Stop generating assumes for validity ranges

After #128371, we represent validity ranges as parameter / return value attributes, so we no longer need to use assumes.

r? `@ghost`
  • Loading branch information
@bors
bors committed Aug 13, 2024
2 parents a2e1d15 + 0d514ae commit 3f964f8
Show file tree
Hide file tree
Showing 20 changed files with 60 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 @@ -242,13 +242,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 @@ -264,13 +262,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 @@ -294,12 +290,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 @@ -341,21 +331,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 @@ -370,55 +354,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
1 change: 1 addition & 0 deletions tests/codegen/cast-optimized.rs
View file
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
#![crate_type = "lib"]

// This tests that LLVM can optimize based on the niches in the source or
Expand Down
1 change: 1 addition & 0 deletions tests/codegen/enum/enum-bounds-check-derived-idx.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// This test checks an optimization that is not guaranteed to work. This test case should not block
// a future LLVM update.
//@ compile-flags: -O
//@ min-llvm-version: 19

#![crate_type = "lib"]

Expand Down
1 change: 1 addition & 0 deletions tests/codegen/enum/enum-bounds-check-issue-13926.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// This test checks an optimization that is not guaranteed to work. This test case should not block
// a future LLVM update.
//@ compile-flags: -O
//@ min-llvm-version: 19

#![crate_type = "lib"]

Expand Down
1 change: 1 addition & 0 deletions tests/codegen/enum/enum-bounds-check.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
//@ compile-flags: -O
//@ min-llvm-version: 19

#![crate_type = "lib"]

Expand Down
Loading

0 comments on commit 3f964f8

Please sign in to comment.