Skip to content

Stop putting Assumes in MIR for every enum-as cast #138297

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

Closed
wants to merge 2 commits into from
Closed
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
25 changes: 19 additions & 6 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use rustc_middle::mir::visit::{NonMutatingUseContext, PlaceContext, Visitor};
use rustc_middle::mir::*;
use rustc_middle::traits::query::NoSolution;
use rustc_middle::ty::adjustment::PointerCoercion;
use rustc_middle::ty::cast::CastTy;
use rustc_middle::ty::cast::{CastTy, IntTy};
use rustc_middle::ty::fold::fold_regions;
use rustc_middle::ty::visit::TypeVisitableExt;
use rustc_middle::ty::{
Expand Down Expand Up @@ -2183,11 +2183,24 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
}
}
CastKind::Transmute => {
span_mirbug!(
self,
rvalue,
"Unexpected CastKind::Transmute, which is not permitted in Analysis MIR",
);
let ty_from = op.ty(body, tcx);
let cast_ty_from = CastTy::from_ty(ty_from);
let cast_ty_to = CastTy::from_ty(*ty);
match (cast_ty_from, cast_ty_to) {
(
Some(CastTy::Int(IntTy::CEnum)),
Some(CastTy::Int(IntTy::U(_) | IntTy::I)),
) => (),
_ => {
span_mirbug!(
self,
rvalue,
"Invalid CastKind::Transmute cast {:?} -> {:?}",
ty_from,
ty
)
}
}
}
}
}
Expand Down
155 changes: 69 additions & 86 deletions compiler/rustc_mir_build/src/builder/expr/as_rvalue.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
//! See docs in `build/expr/mod.rs`.

use rustc_abi::{BackendRepr, FieldIdx, Primitive};
use std::assert_matches::assert_matches;

use rustc_abi::{BackendRepr, FieldIdx, TagEncoding, Variants};
use rustc_hir::lang_items::LangItem;
use rustc_index::{Idx, IndexVec};
use rustc_middle::bug;
Expand All @@ -9,8 +11,8 @@ use rustc_middle::mir::interpret::Scalar;
use rustc_middle::mir::*;
use rustc_middle::thir::*;
use rustc_middle::ty::cast::{CastTy, mir_cast_kind};
use rustc_middle::ty::layout::IntegerExt;
use rustc_middle::ty::util::IntTypeExt;
use rustc_middle::ty::layout::PrimitiveExt;
use rustc_middle::ty::util::{Discr, IntTypeExt};
use rustc_middle::ty::{self, Ty, UpvarArgs};
use rustc_span::source_map::Spanned;
use rustc_span::{DUMMY_SP, Span};
Expand Down Expand Up @@ -197,97 +199,78 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
ExprKind::Cast { source } => {
let source_expr = &this.thir[source];

// Casting an enum to an integer is equivalent to computing the discriminant and casting the
// discriminant. Previously every backend had to repeat the logic for this operation. Now we
// create all the steps directly in MIR with operations all backends need to support anyway.
let (source, ty) = if let ty::Adt(adt_def, ..) = source_expr.ty.kind()
&& adt_def.is_enum()
{
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.typing_env().as_query_input(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 BackendRepr::Scalar(scalar) = layout.unwrap().backend_repr
&& !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> {
// We can use `ty::TypingEnv::fully_monomorphized()` here
// as we only need it to compute the layout of a primitive.
let range_val = Const::from_bits(
this.tcx,
range,
ty::TypingEnv::fully_monomorphized(),
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(
let layout = this
.tcx
.layout_of(this.typing_env().as_query_input(source_expr.ty))
.unwrap();
match layout.variants {
// Uninhabited enum, so we're in dead code.
Variants::Empty => {
let false_lit = this.zero_literal(expr_span, this.tcx.types.bool);
this.cfg.push(
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),
Statement {
source_info,
kind: StatementKind::Intrinsic(Box::new(
NonDivergingIntrinsic::Assume(false_lit),
)),
),
},
);
merge_place
};
this.cfg.push(
block,
Statement {
source_info,
kind: StatementKind::Intrinsic(Box::new(
NonDivergingIntrinsic::Assume(Operand::Move(assert_place)),
)),
},
);
// We still need to emit *something*, to keep the following MIR legal,
// so give a zero of the type the cast was asking for.
(this.zero_literal(expr_span, expr.ty), expr.ty)
}
// Only one legal variant, so we can just look up its
// discriminant directly and return it as a constant.
// (In the discriminant's type, not the cast-to type,
// to avoid worrying about truncation or extension.)
Variants::Single { index } => {
let Discr { val, ty } =
adt_def.discriminant_for_variant(this.tcx, index);
let val = Const::from_bits(this.tcx, val, this.typing_env(), ty);
(this.literal_operand(expr_span, val), ty)
}
// Casting an enum to an integer is only supported for enums which
// have no fields, so we can transmute the stored tag.
// This used to emit `Assume`s into the MIR, but that bloated it,
// so now we re-use the `Transmute` checks that backends ought to
// support anyway for polymorphic MIR cases.
Variants::Multiple { tag, ref tag_encoding, .. } => {
assert_matches!(tag_encoding, TagEncoding::Direct);
if let BackendRepr::Scalar(repr) = layout.backend_repr {
assert_eq!(repr, tag);
let tag_ty = tag.primitive().to_ty(this.tcx);
let tag = this.temp(tag_ty, expr_span);
this.cfg.push_assign(
block,
source_info,
tag,
Rvalue::Cast(
CastKind::Transmute,
Operand::Move(Place::from(temp)),
tag_ty,
),
);
(Operand::Move(tag), tag_ty)
} else {
// FIXME: One `Transmute` works union-style (so it can truncate
// the padding down to just the tag) we can remove this fallback.
let discr_ty = adt_def.repr().discr_type().to_ty(this.tcx);
let discr = this.temp(discr_ty, source_expr.span);
this.cfg.push_assign(
block,
source_info,
discr,
Rvalue::Discriminant(temp.into()),
);
(Operand::Move(discr), discr_ty)
}
}
}

(op, ty)
} else {
let ty = source_expr.ty;
let source = unpack!(
Expand Down
48 changes: 19 additions & 29 deletions compiler/rustc_mir_transform/src/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1303,37 +1303,27 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
}
}
CastKind::Transmute => {
if let MirPhase::Runtime(..) = self.body.phase {
// Unlike `mem::transmute`, a MIR `Transmute` is well-formed
// for any two `Sized` types, just potentially UB to run.

if !self
.tcx
.normalize_erasing_regions(self.typing_env, op_ty)
.is_sized(self.tcx, self.typing_env)
{
self.fail(
location,
format!("Cannot transmute from non-`Sized` type {op_ty:?}"),
);
}
if !self
.tcx
.normalize_erasing_regions(self.typing_env, *target_type)
.is_sized(self.tcx, self.typing_env)
{
self.fail(
location,
format!("Cannot transmute to non-`Sized` type {target_type:?}"),
);
}
} else {
// Unlike `mem::transmute`, a MIR `Transmute` is well-formed
// for any two `Sized` types, just potentially UB to run.

if !self
.tcx
.normalize_erasing_regions(self.typing_env, op_ty)
.is_sized(self.tcx, self.typing_env)
{
self.fail(
location,
format!(
"Transmute is not supported in non-runtime phase {:?}.",
self.body.phase
),
format!("Cannot transmute from non-`Sized` type {op_ty:?}"),
);
}
if !self
.tcx
.normalize_erasing_regions(self.typing_env, *target_type)
.is_sized(self.tcx, self.typing_env)
{
self.fail(
location,
format!("Cannot transmute to non-`Sized` type {target_type:?}"),
);
}
}
Expand Down
2 changes: 1 addition & 1 deletion library/core/src/cmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ impl Ordering {
#[inline]
const fn as_raw(self) -> i8 {
// FIXME(const-hack): just use `PartialOrd` against `Equal` once that's const
crate::intrinsics::discriminant_value(&self)
self as i8
}

/// Returns `true` if the ordering is the `Equal` variant.
Expand Down
9 changes: 2 additions & 7 deletions tests/mir-opt/building/enum_cast.bar.built.after.mir
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,12 @@ fn bar(_1: Bar) -> usize {
debug bar => _1;
let mut _0: usize;
let _2: Bar;
let mut _3: isize;
let mut _4: u8;
let mut _5: bool;
let mut _3: u8;

bb0: {
StorageLive(_2);
_2 = move _1;
_3 = discriminant(_2);
_4 = copy _3 as u8 (IntToInt);
_5 = Le(copy _4, const 1_u8);
assume(move _5);
_3 = move _2 as u8 (Transmute);
_0 = move _3 as usize (IntToInt);
StorageDead(_2);
return;
Expand Down
7 changes: 1 addition & 6 deletions tests/mir-opt/building/enum_cast.boo.built.after.mir
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,11 @@ fn boo(_1: Boo) -> usize {
let mut _0: usize;
let _2: Boo;
let mut _3: u8;
let mut _4: u8;
let mut _5: bool;

bb0: {
StorageLive(_2);
_2 = move _1;
_3 = discriminant(_2);
_4 = copy _3 as u8 (IntToInt);
_5 = Le(copy _4, const 1_u8);
assume(move _5);
_3 = move _2 as u8 (Transmute);
_0 = move _3 as usize (IntToInt);
StorageDead(_2);
return;
Expand Down
Loading
Loading