Skip to content

Commit 53792b9

Browse files
borsbors
committed
Auto merge of #96862 - oli-obk:enum_cast_mir, r=RalfJung
Change enum->int casts to not go through MIR casts. follow-up to #96814 this simplifies all backends and even gives LLVM more information about the return value of `Rvalue::Discriminant`, enabling optimizations in more cases.
2 parents 4045ce6 + 82c73af commit 53792b9

File tree

21 files changed

+238
-143
lines changed

21 files changed

+238
-143
lines changed

Diff for: ‎compiler/rustc_codegen_cranelift/src/base.rs

-23
Original file line numberDiff line numberDiff line change
@@ -635,29 +635,6 @@ fn codegen_stmt<'tcx>(
635635
let (ptr, _extra) = operand.load_scalar_pair(fx);
636636
lval.write_cvalue(fx, CValue::by_val(ptr, dest_layout))
637637
}
638-
} else if let ty::Adt(adt_def, _substs) = from_ty.kind() {
639-
// enum -> discriminant value
640-
assert!(adt_def.is_enum());
641-
match to_ty.kind() {
642-
ty::Uint(_) | ty::Int(_) => {}
643-
_ => unreachable!("cast adt {} -> {}", from_ty, to_ty),
644-
}
645-
let to_clif_ty = fx.clif_type(to_ty).unwrap();
646-
647-
let discriminant = crate::discriminant::codegen_get_discriminant(
648-
fx,
649-
operand,
650-
fx.layout_of(operand.layout().ty.discriminant_ty(fx.tcx)),
651-
)
652-
.load_scalar(fx);
653-
654-
let res = crate::cast::clif_intcast(
655-
fx,
656-
discriminant,
657-
to_clif_ty,
658-
to_ty.is_signed(),
659-
);
660-
lval.write_cvalue(fx, CValue::by_val(res, dest_layout));
661638
} else {
662639
let to_clif_ty = fx.clif_type(to_ty).unwrap();
663640
let from = operand.load_scalar(fx);

Diff for: ‎compiler/rustc_codegen_llvm/src/builder.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ use std::ffi::CStr;
2828
use std::iter;
2929
use std::ops::Deref;
3030
use std::ptr;
31-
use tracing::debug;
31+
use tracing::{debug, instrument};
3232

3333
// All Builders must have an llfn associated with them
3434
#[must_use]
@@ -464,15 +464,15 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
464464
}
465465
}
466466

467+
#[instrument(level = "trace", skip(self))]
467468
fn load_operand(&mut self, place: PlaceRef<'tcx, &'ll Value>) -> OperandRef<'tcx, &'ll Value> {
468-
debug!("PlaceRef::load: {:?}", place);
469-
470469
assert_eq!(place.llextra.is_some(), place.layout.is_unsized());
471470

472471
if place.layout.is_zst() {
473472
return OperandRef::new_zst(self, place.layout);
474473
}
475474

475+
#[instrument(level = "trace", skip(bx))]
476476
fn scalar_load_metadata<'a, 'll, 'tcx>(
477477
bx: &mut Builder<'a, 'll, 'tcx>,
478478
load: &'ll Value,

Diff for: ‎compiler/rustc_codegen_llvm/src/llvm/ffi.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1619,7 +1619,7 @@ extern "C" {
16191619
B: &Builder<'a>,
16201620
Val: &'a Value,
16211621
DestTy: &'a Type,
1622-
IsSized: bool,
1622+
IsSigned: bool,
16231623
) -> &'a Value;
16241624

16251625
// Comparisons

Diff for: ‎compiler/rustc_codegen_ssa/src/mir/place.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,7 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
204204
}
205205

206206
/// Obtain the actual discriminant of a value.
207+
#[instrument(level = "trace", skip(bx))]
207208
pub fn codegen_get_discr<Bx: BuilderMethods<'a, 'tcx, Value = V>>(
208209
self,
209210
bx: &mut Bx,
@@ -420,12 +421,12 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
420421
}
421422

422423
impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
424+
#[instrument(level = "trace", skip(self, bx))]
423425
pub fn codegen_place(
424426
&mut self,
425427
bx: &mut Bx,
426428
place_ref: mir::PlaceRef<'tcx>,
427429
) -> PlaceRef<'tcx, Bx::Value> {
428-
debug!("codegen_place(place_ref={:?})", place_ref);
429430
let cx = self.cx;
430431
let tcx = self.cx.tcx();
431432

Diff for: ‎compiler/rustc_codegen_ssa/src/mir/rvalue.rs

+9-72
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,15 @@ use rustc_middle::ty::cast::{CastTy, IntTy};
1212
use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf};
1313
use rustc_middle::ty::{self, adjustment::PointerCast, Instance, Ty, TyCtxt};
1414
use rustc_span::source_map::{Span, DUMMY_SP};
15-
use rustc_target::abi::{Abi, Int, Variants};
1615

1716
impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
17+
#[instrument(level = "trace", skip(self, bx))]
1818
pub fn codegen_rvalue(
1919
&mut self,
2020
mut bx: Bx,
2121
dest: PlaceRef<'tcx, Bx::Value>,
2222
rvalue: &mir::Rvalue<'tcx>,
2323
) -> Bx {
24-
debug!("codegen_rvalue(dest.llval={:?}, rvalue={:?})", dest.llval, rvalue);
25-
2624
match *rvalue {
2725
mir::Rvalue::Use(ref operand) => {
2826
let cg_operand = self.codegen_operand(&mut bx, operand);
@@ -285,74 +283,12 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
285283
CastTy::from_ty(operand.layout.ty).expect("bad input type for cast");
286284
let r_t_out = CastTy::from_ty(cast.ty).expect("bad output type for cast");
287285
let ll_t_in = bx.cx().immediate_backend_type(operand.layout);
288-
match operand.layout.variants {
289-
Variants::Single { index } => {
290-
if let Some(discr) =
291-
operand.layout.ty.discriminant_for_variant(bx.tcx(), index)
292-
{
293-
let discr_layout = bx.cx().layout_of(discr.ty);
294-
let discr_t = bx.cx().immediate_backend_type(discr_layout);
295-
let discr_val = bx.cx().const_uint_big(discr_t, discr.val);
296-
let discr_val =
297-
bx.intcast(discr_val, ll_t_out, discr.ty.is_signed());
298-
299-
return (
300-
bx,
301-
OperandRef {
302-
val: OperandValue::Immediate(discr_val),
303-
layout: cast,
304-
},
305-
);
306-
}
307-
}
308-
Variants::Multiple { .. } => {}
309-
}
310286
let llval = operand.immediate();
311287

312-
let mut signed = false;
313-
if let Abi::Scalar(scalar) = operand.layout.abi {
314-
if let Int(_, s) = scalar.primitive() {
315-
// We use `i1` for bytes that are always `0` or `1`,
316-
// e.g., `#[repr(i8)] enum E { A, B }`, but we can't
317-
// let LLVM interpret the `i1` as signed, because
318-
// then `i1 1` (i.e., E::B) is effectively `i8 -1`.
319-
signed = !scalar.is_bool() && s;
320-
321-
if !scalar.is_always_valid(bx.cx())
322-
&& scalar.valid_range(bx.cx()).end
323-
>= scalar.valid_range(bx.cx()).start
324-
{
325-
// We want `table[e as usize ± k]` to not
326-
// have bound checks, and this is the most
327-
// convenient place to put the `assume`s.
328-
if scalar.valid_range(bx.cx()).start > 0 {
329-
let enum_value_lower_bound = bx.cx().const_uint_big(
330-
ll_t_in,
331-
scalar.valid_range(bx.cx()).start,
332-
);
333-
let cmp_start = bx.icmp(
334-
IntPredicate::IntUGE,
335-
llval,
336-
enum_value_lower_bound,
337-
);
338-
bx.assume(cmp_start);
339-
}
340-
341-
let enum_value_upper_bound = bx
342-
.cx()
343-
.const_uint_big(ll_t_in, scalar.valid_range(bx.cx()).end);
344-
let cmp_end = bx.icmp(
345-
IntPredicate::IntULE,
346-
llval,
347-
enum_value_upper_bound,
348-
);
349-
bx.assume(cmp_end);
350-
}
351-
}
352-
}
353-
354288
let newval = match (r_t_in, r_t_out) {
355-
(CastTy::Int(_), CastTy::Int(_)) => bx.intcast(llval, ll_t_out, signed),
289+
(CastTy::Int(i), CastTy::Int(_)) => {
290+
bx.intcast(llval, ll_t_out, i.is_signed())
291+
}
356292
(CastTy::Float, CastTy::Float) => {
357293
let srcsz = bx.cx().float_width(ll_t_in);
358294
let dstsz = bx.cx().float_width(ll_t_out);
@@ -364,8 +300,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
364300
llval
365301
}
366302
}
367-
(CastTy::Int(_), CastTy::Float) => {
368-
if signed {
303+
(CastTy::Int(i), CastTy::Float) => {
304+
if i.is_signed() {
369305
bx.sitofp(llval, ll_t_out)
370306
} else {
371307
bx.uitofp(llval, ll_t_out)
@@ -374,8 +310,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
374310
(CastTy::Ptr(_) | CastTy::FnPtr, CastTy::Ptr(_)) => {
375311
bx.pointercast(llval, ll_t_out)
376312
}
377-
(CastTy::Int(_), CastTy::Ptr(_)) => {
378-
let usize_llval = bx.intcast(llval, bx.cx().type_isize(), signed);
313+
(CastTy::Int(i), CastTy::Ptr(_)) => {
314+
let usize_llval =
315+
bx.intcast(llval, bx.cx().type_isize(), i.is_signed());
379316
bx.inttoptr(usize_llval, ll_t_out)
380317
}
381318
(CastTy::Float, CastTy::Int(IntTy::I)) => {

Diff for: ‎compiler/rustc_codegen_ssa/src/mir/statement.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,8 @@ use crate::traits::BuilderMethods;
66
use crate::traits::*;
77

88
impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
9+
#[instrument(level = "debug", skip(self, bx))]
910
pub fn codegen_statement(&mut self, mut bx: Bx, statement: &mir::Statement<'tcx>) -> Bx {
10-
debug!("codegen_statement(statement={:?})", statement);
11-
1211
self.set_debug_loc(&mut bx, statement.source_info);
1312
match statement.kind {
1413
mir::StatementKind::Assign(box (ref place, ref rvalue)) => {

Diff for: ‎compiler/rustc_const_eval/src/interpret/cast.rs

+2-23
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use rustc_middle::mir::CastKind;
88
use rustc_middle::ty::adjustment::PointerCast;
99
use rustc_middle::ty::layout::{IntegerExt, LayoutOf, TyAndLayout};
1010
use rustc_middle::ty::{self, FloatTy, Ty, TypeAndMut};
11-
use rustc_target::abi::{Integer, Variants};
11+
use rustc_target::abi::Integer;
1212
use rustc_type_ir::sty::TyKind::*;
1313

1414
use super::{
@@ -128,12 +128,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
128128
Float(FloatTy::F64) => {
129129
return Ok(self.cast_from_float(src.to_scalar()?.to_f64()?, cast_ty).into());
130130
}
131-
// The rest is integer/pointer-"like", including fn ptr casts and casts from enums that
132-
// are represented as integers.
131+
// The rest is integer/pointer-"like", including fn ptr casts
133132
_ => assert!(
134133
src.layout.ty.is_bool()
135134
|| src.layout.ty.is_char()
136-
|| src.layout.ty.is_enum()
137135
|| src.layout.ty.is_integral()
138136
|| src.layout.ty.is_any_ptr(),
139137
"Unexpected cast from type {:?}",
@@ -143,25 +141,6 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
143141

144142
// # First handle non-scalar source values.
145143

146-
// Handle cast from a ZST enum (0 or 1 variants).
147-
match src.layout.variants {
148-
Variants::Single { index } => {
149-
if src.layout.abi.is_uninhabited() {
150-
// This is dead code, because an uninhabited enum is UB to
151-
// instantiate.
152-
throw_ub!(Unreachable);
153-
}
154-
if let Some(discr) = src.layout.ty.discriminant_for_variant(*self.tcx, index) {
155-
assert!(src.layout.is_zst());
156-
let discr_layout = self.layout_of(discr.ty)?;
157-
158-
let scalar = Scalar::from_uint(discr.val, discr_layout.layout.size());
159-
return Ok(self.cast_from_int_like(scalar, discr_layout, cast_ty)?.into());
160-
}
161-
}
162-
Variants::Multiple { .. } => {}
163-
}
164-
165144
// Handle casting any ptr to raw ptr (might be a fat ptr).
166145
if src.layout.ty.is_any_ptr() && cast_ty.is_unsafe_ptr() {
167146
let dest_layout = self.layout_of(cast_ty)?;

Diff for: ‎compiler/rustc_const_eval/src/transform/validate.rs

+28-4
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@ use rustc_middle::mir::interpret::Scalar;
77
use rustc_middle::mir::visit::NonUseContext::VarDebugInfo;
88
use rustc_middle::mir::visit::{PlaceContext, Visitor};
99
use rustc_middle::mir::{
10-
traversal, AggregateKind, BasicBlock, BinOp, Body, BorrowKind, Local, Location, MirPass,
11-
MirPhase, Operand, Place, PlaceElem, PlaceRef, ProjectionElem, Rvalue, SourceScope, Statement,
12-
StatementKind, Terminator, TerminatorKind, UnOp, START_BLOCK,
10+
traversal, AggregateKind, BasicBlock, BinOp, Body, BorrowKind, CastKind, Local, Location,
11+
MirPass, MirPhase, Operand, Place, PlaceElem, PlaceRef, ProjectionElem, Rvalue, SourceScope,
12+
Statement, StatementKind, Terminator, TerminatorKind, UnOp, START_BLOCK,
1313
};
1414
use rustc_middle::ty::fold::BottomUpFolder;
1515
use rustc_middle::ty::{self, InstanceDef, ParamEnv, Ty, TyCtxt, TypeFoldable};
@@ -361,6 +361,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
361361
);
362362
}
363363
}
364+
Rvalue::Ref(..) => {}
364365
Rvalue::Len(p) => {
365366
let pty = p.ty(&self.body.local_decls, self.tcx).ty;
366367
check_kinds!(
@@ -503,7 +504,30 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
503504
let a = operand.ty(&self.body.local_decls, self.tcx);
504505
check_kinds!(a, "Cannot shallow init type {:?}", ty::RawPtr(..));
505506
}
506-
_ => {}
507+
Rvalue::Cast(kind, operand, target_type) => {
508+
match kind {
509+
CastKind::Misc => {
510+
let op_ty = operand.ty(self.body, self.tcx);
511+
if op_ty.is_enum() {
512+
self.fail(
513+
location,
514+
format!(
515+
"enum -> int casts should go through `Rvalue::Discriminant`: {operand:?}:{op_ty} as {target_type}",
516+
),
517+
);
518+
}
519+
}
520+
// Nothing to check here
521+
CastKind::PointerFromExposedAddress
522+
| CastKind::PointerExposeAddress
523+
| CastKind::Pointer(_) => {}
524+
}
525+
}
526+
Rvalue::Repeat(_, _)
527+
| Rvalue::ThreadLocalRef(_)
528+
| Rvalue::AddressOf(_, _)
529+
| Rvalue::NullaryOp(_, _)
530+
| Rvalue::Discriminant(_) => {}
507531
}
508532
self.super_rvalue(rvalue, location);
509533
}

Diff for: ‎compiler/rustc_middle/src/ty/cast.rs

+6
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,12 @@ pub enum IntTy {
1515
Char,
1616
}
1717

18+
impl IntTy {
19+
pub fn is_signed(self) -> bool {
20+
matches!(self, Self::I)
21+
}
22+
}
23+
1824
// Valid types for the result of a non-coercion cast
1925
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
2026
pub enum CastTy<'tcx> {

Diff for: ‎compiler/rustc_mir_build/src/build/expr/as_rvalue.rs

+25-4
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
//! See docs in `build/expr/mod.rs`.
22
33
use rustc_index::vec::Idx;
4+
use rustc_middle::ty::util::IntTypeExt;
45

56
use crate::build::expr::as_place::PlaceBase;
67
use crate::build::expr::category::{Category, RvalueFunc};
@@ -190,7 +191,30 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
190191
}
191192
ExprKind::Cast { source } => {
192193
let source = &this.thir[source];
193-
let from_ty = CastTy::from_ty(source.ty);
194+
195+
// Casting an enum to an integer is equivalent to computing the discriminant and casting the
196+
// discriminant. Previously every backend had to repeat the logic for this operation. Now we
197+
// create all the steps directly in MIR with operations all backends need to support anyway.
198+
let (source, ty) = if let ty::Adt(adt_def, ..) = source.ty.kind() && adt_def.is_enum() {
199+
let discr_ty = adt_def.repr().discr_type().to_ty(this.tcx);
200+
let place = unpack!(block = this.as_place(block, source));
201+
let discr = this.temp(discr_ty, source.span);
202+
this.cfg.push_assign(
203+
block,
204+
source_info,
205+
discr,
206+
Rvalue::Discriminant(place),
207+
);
208+
209+
(Operand::Move(discr), discr_ty)
210+
} else {
211+
let ty = source.ty;
212+
let source = unpack!(
213+
block = this.as_operand(block, scope, source, None, NeedsTemporary::No)
214+
);
215+
(source, ty)
216+
};
217+
let from_ty = CastTy::from_ty(ty);
194218
let cast_ty = CastTy::from_ty(expr.ty);
195219
let cast_kind = match (from_ty, cast_ty) {
196220
(Some(CastTy::Ptr(_) | CastTy::FnPtr), Some(CastTy::Int(_))) => {
@@ -201,9 +225,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
201225
}
202226
(_, _) => CastKind::Misc,
203227
};
204-
let source = unpack!(
205-
block = this.as_operand(block, scope, source, None, NeedsTemporary::No)
206-
);
207228
block.and(Rvalue::Cast(cast_kind, source, expr.ty))
208229
}
209230
ExprKind::Pointer { cast, source } => {

0 commit comments

Comments
 (0)