Skip to content

Commit

Permalink
Auto merge of rust-lang#89030 - nbdd0121:box2, r=jonas-schievink
Browse files Browse the repository at this point in the history
Introduce `Rvalue::ShallowInitBox`

Polished version of rust-lang#88700.

Implements MCP rust-lang/compiler-team#460, and should allow rust-lang#43596 to go forward.

In short, creating an empty box is split from a nullary-op `NullOp::Box` into two steps, first a call to `exchange_malloc`, then a `Rvalue::ShallowInitBox` which transmutes `*mut u8` to a shallow-initialized `Box<T>`. This allows the `exchange_malloc` call to unwind. Details can be found in the MCP.

`NullOp::Box` is not yet removed, purely to make reverting easier in case anything goes wrong as the result of this PR. If revert is needed a reversion of "Use Rvalue::ShallowInitBox for box expression" commit followed by a test bless should be sufficient.

Experiments in rust-lang#88700 showed a very slight compile-time perf regression due to (supposedly) slightly more time spent in LLVM. We could omit unwind edge generation (in non-`oom=panic` case) in box expression MIR construction to restore perf; but I don't think it's necessary since runtime perf isn't affected and perf difference is rather small.
  • Loading branch information
bors committed Sep 25, 2021
2 parents 218a96c + ab64580 commit e9f29a8
Show file tree
Hide file tree
Showing 31 changed files with 468 additions and 227 deletions.
3 changes: 2 additions & 1 deletion compiler/rustc_borrowck/src/invalidation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,8 @@ impl<'cx, 'tcx> InvalidationGenerator<'cx, 'tcx> {
Rvalue::Use(ref operand)
| Rvalue::Repeat(ref operand, _)
| Rvalue::UnaryOp(_ /*un_op*/, ref operand)
| Rvalue::Cast(_ /*cast_kind*/, ref operand, _ /*ty*/) => {
| Rvalue::Cast(_ /*cast_kind*/, ref operand, _ /*ty*/)
| Rvalue::ShallowInitBox(ref operand, _ /*ty*/) => {
self.consume_operand(location, operand)
}

Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_borrowck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1361,7 +1361,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
Rvalue::Use(ref operand)
| Rvalue::Repeat(ref operand, _)
| Rvalue::UnaryOp(_ /*un_op*/, ref operand)
| Rvalue::Cast(_ /*cast_kind*/, ref operand, _ /*ty*/) => {
| Rvalue::Cast(_ /*cast_kind*/, ref operand, _ /*ty*/)
| Rvalue::ShallowInitBox(ref operand, _ /*ty*/) => {
self.consume_operand(location, (operand, span), flow_state)
}

Expand Down
9 changes: 2 additions & 7 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2024,13 +2024,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
}
}

Rvalue::NullaryOp(_, ty) => {
// Even with unsized locals cannot box an unsized value.
if self.unsized_feature_enabled() {
let span = body.source_info(location).span;
self.ensure_place_sized(ty, span);
}

Rvalue::NullaryOp(_, ty) | Rvalue::ShallowInitBox(_, ty) => {
let trait_ref = ty::TraitRef {
def_id: tcx.require_lang_item(LangItem::Sized, Some(self.last_span)),
substs: tcx.mk_substs_trait(ty, &[]),
Expand Down Expand Up @@ -2363,6 +2357,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
| Rvalue::AddressOf(..)
| Rvalue::Len(..)
| Rvalue::Cast(..)
| Rvalue::ShallowInitBox(..)
| Rvalue::BinaryOp(..)
| Rvalue::CheckedBinaryOp(..)
| Rvalue::NullaryOp(..)
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_codegen_cranelift/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,13 @@ fn codegen_stmt<'tcx>(
let len = codegen_array_len(fx, place);
lval.write_cvalue(fx, CValue::by_val(len, usize_layout));
}
Rvalue::ShallowInitBox(ref operand, content_ty) => {
let content_ty = fx.monomorphize(content_ty);
let box_layout = fx.layout_of(fx.tcx.mk_box(content_ty));
let operand = codegen_operand(fx, operand);
let operand = operand.load_scalar(fx);
lval.write_cvalue(fx, CValue::by_val(operand, box_layout));
}
Rvalue::NullaryOp(NullOp::Box, content_ty) => {
let usize_type = fx.clif_type(fx.tcx.types.usize).unwrap();
let content_ty = fx.monomorphize(content_ty);
Expand Down
13 changes: 13 additions & 0 deletions compiler/rustc_codegen_ssa/src/mir/rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,18 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
OperandRef::new_zst(&mut bx, self.cx.layout_of(self.monomorphize(ty)));
(bx, operand)
}
mir::Rvalue::ShallowInitBox(ref operand, content_ty) => {
let operand = self.codegen_operand(&mut bx, operand);
let lloperand = operand.immediate();

let content_ty = self.monomorphize(content_ty);
let box_layout = bx.cx().layout_of(bx.tcx().mk_box(content_ty));
let llty_ptr = bx.cx().backend_type(box_layout);

let val = bx.pointercast(lloperand, llty_ptr);
let operand = OperandRef { val: OperandValue::Immediate(val), layout: box_layout };
(bx, operand)
}
}
}

Expand Down Expand Up @@ -763,6 +775,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
mir::Rvalue::AddressOf(..) |
mir::Rvalue::Len(..) |
mir::Rvalue::Cast(..) | // (*)
mir::Rvalue::ShallowInitBox(..) | // (*)
mir::Rvalue::BinaryOp(..) |
mir::Rvalue::CheckedBinaryOp(..) |
mir::Rvalue::UnaryOp(..) |
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_const_eval/src/interpret/step.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
self.write_scalar(Scalar::from_machine_usize(val, self), &dest)?;
}

ShallowInitBox(ref operand, _) => {
let src = self.eval_operand(operand, None)?;
let v = self.read_immediate(&src)?;
self.write_immediate(*v, &dest)?;
}

Cast(cast_kind, ref operand, cast_ty) => {
let src = self.eval_operand(operand, None)?;
let cast_ty = self.subst_from_current_frame_and_normalize_erasing_regions(cast_ty);
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_const_eval/src/transform/check_consts/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,7 @@ impl Visitor<'tcx> for Checker<'mir, 'tcx> {

Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf, _) => {}
Rvalue::NullaryOp(NullOp::Box, _) => self.check_op(ops::HeapAllocation),
Rvalue::ShallowInitBox(_, _) => {}

Rvalue::UnaryOp(_, ref operand) => {
let ty = operand.ty(self.body, self.tcx);
Expand Down Expand Up @@ -912,6 +913,11 @@ impl Visitor<'tcx> for Checker<'mir, 'tcx> {
return;
}

if Some(callee) == tcx.lang_items().exchange_malloc_fn() {
self.check_op(ops::HeapAllocation);
return;
}

// `async` blocks get lowered to `std::future::from_generator(/* a closure */)`.
let is_async_block = Some(callee) == tcx.lang_items().from_generator_fn();
if is_async_block {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,8 @@ where
Rvalue::Use(operand)
| Rvalue::Repeat(operand, _)
| Rvalue::UnaryOp(_, operand)
| Rvalue::Cast(_, operand, _) => in_operand::<Q, _>(cx, in_local, operand),
| Rvalue::Cast(_, operand, _)
| Rvalue::ShallowInitBox(operand, _) => in_operand::<Q, _>(cx, in_local, operand),

Rvalue::BinaryOp(_, box (lhs, rhs)) | Rvalue::CheckedBinaryOp(_, box (lhs, rhs)) => {
in_operand::<Q, _>(cx, in_local, lhs) || in_operand::<Q, _>(cx, in_local, rhs)
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_const_eval/src/transform/promote_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,8 @@ impl<'tcx> Validator<'_, 'tcx> {
NullOp::AlignOf => {}
},

Rvalue::ShallowInitBox(_, _) => return Err(Unpromotable),

Rvalue::UnaryOp(op, operand) => {
match op {
// These operations can never fail.
Expand Down
10 changes: 10 additions & 0 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2200,6 +2200,12 @@ pub enum Rvalue<'tcx> {
/// that `Foo` has a destructor. These rvalues can be optimized
/// away after type-checking and before lowering.
Aggregate(Box<AggregateKind<'tcx>>, Vec<Operand<'tcx>>),

/// Transmutes a `*mut u8` into shallow-initialized `Box<T>`.
///
/// This is different a normal transmute because dataflow analysis will treat the box
/// as initialized but its content as uninitialized.
ShallowInitBox(Operand<'tcx>, Ty<'tcx>),
}

#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
Expand Down Expand Up @@ -2450,6 +2456,10 @@ impl<'tcx> Debug for Rvalue<'tcx> {
}),
}
}

ShallowInitBox(ref place, ref ty) => {
write!(fmt, "ShallowInitBox({:?}, {:?})", place, ty)
}
}
}
}
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_middle/src/mir/tcx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ impl<'tcx> Rvalue<'tcx> {
tcx.mk_generator(did, substs, movability)
}
},
Rvalue::ShallowInitBox(_, ty) => tcx.mk_box(ty),
}
}

Expand All @@ -214,7 +215,9 @@ impl<'tcx> Rvalue<'tcx> {
/// whether its only shallowly initialized (`Rvalue::Box`).
pub fn initialization_state(&self) -> RvalueInitializationState {
match *self {
Rvalue::NullaryOp(NullOp::Box, _) => RvalueInitializationState::Shallow,
Rvalue::NullaryOp(NullOp::Box, _) | Rvalue::ShallowInitBox(_, _) => {
RvalueInitializationState::Shallow
}
_ => RvalueInitializationState::Deep,
}
}
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_middle/src/mir/type_foldable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ impl<'tcx> TypeFoldable<'tcx> for Rvalue<'tcx> {
});
Aggregate(kind, fields.fold_with(folder))
}
ShallowInitBox(op, ty) => ShallowInitBox(op.fold_with(folder), ty.fold_with(folder)),
}
}

Expand Down Expand Up @@ -255,6 +256,10 @@ impl<'tcx> TypeFoldable<'tcx> for Rvalue<'tcx> {
}
fields.visit_with(visitor)
}
ShallowInitBox(ref op, ty) => {
op.visit_with(visitor)?;
ty.visit_with(visitor)
}
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_middle/src/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,11 @@ macro_rules! make_mir_visitor {
self.visit_operand(operand, location);
}
}

Rvalue::ShallowInitBox(operand, ty) => {
self.visit_operand(operand, location);
self.visit_ty(ty, TyContext::Location(location));
}
}
}

Expand Down
55 changes: 53 additions & 2 deletions compiler/rustc_mir_build/src/build/expr/as_rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use rustc_index::vec::Idx;
use crate::build::expr::as_place::PlaceBase;
use crate::build::expr::category::{Category, RvalueFunc};
use crate::build::{BlockAnd, BlockAndExtension, Builder};
use rustc_hir::lang_items::LangItem;
use rustc_middle::middle::region;
use rustc_middle::mir::AssertKind;
use rustc_middle::mir::Place;
Expand Down Expand Up @@ -88,6 +89,56 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}
ExprKind::Box { value } => {
let value = &this.thir[value];
let tcx = this.tcx;

// `exchange_malloc` is unsafe but box is safe, so need a new scope.
let synth_scope = this.new_source_scope(
expr_span,
LintLevel::Inherited,
Some(Safety::BuiltinUnsafe),
);
let synth_info = SourceInfo { span: expr_span, scope: synth_scope };

let size = this.temp(tcx.types.usize, expr_span);
this.cfg.push_assign(
block,
synth_info,
size,
Rvalue::NullaryOp(NullOp::SizeOf, value.ty),
);

let align = this.temp(tcx.types.usize, expr_span);
this.cfg.push_assign(
block,
synth_info,
align,
Rvalue::NullaryOp(NullOp::AlignOf, value.ty),
);

// malloc some memory of suitable size and align:
let exchange_malloc = Operand::function_handle(
tcx,
tcx.require_lang_item(LangItem::ExchangeMalloc, Some(expr_span)),
ty::List::empty(),
expr_span,
);
let storage = this.temp(tcx.mk_mut_ptr(tcx.types.u8), expr_span);
let success = this.cfg.start_new_block();
this.cfg.terminate(
block,
synth_info,
TerminatorKind::Call {
func: exchange_malloc,
args: vec![Operand::Move(size), Operand::Move(align)],
destination: Some((Place::from(storage), success)),
cleanup: None,
from_hir_call: false,
fn_span: expr_span,
},
);
this.diverge_from(block);
block = success;

// The `Box<T>` temporary created here is not a part of the HIR,
// and therefore is not considered during generator auto-trait
// determination. See the comment about `box` at `yield_in_scope`.
Expand All @@ -101,8 +152,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
this.schedule_drop_storage_and_value(expr_span, scope, result);
}

// malloc some memory of suitable type (thus far, uninitialized):
let box_ = Rvalue::NullaryOp(NullOp::Box, value.ty);
// Transmute `*mut u8` to the box (thus far, uninitialized):
let box_ = Rvalue::ShallowInitBox(Operand::Move(Place::from(storage)), value.ty);
this.cfg.push_assign(block, source_info, Place::from(result), box_);

// initialize the box contents:
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ where
}

mir::Rvalue::Cast(..)
| mir::Rvalue::ShallowInitBox(..)
| mir::Rvalue::Use(..)
| mir::Rvalue::ThreadLocalRef(..)
| mir::Rvalue::Repeat(..)
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_dataflow/src/move_paths/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,7 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> {
Rvalue::Use(ref operand)
| Rvalue::Repeat(ref operand, _)
| Rvalue::Cast(_, ref operand, _)
| Rvalue::ShallowInitBox(ref operand, _)
| Rvalue::UnaryOp(_, ref operand) => self.gather_operand(operand),
Rvalue::BinaryOp(ref _binop, box (ref lhs, ref rhs))
| Rvalue::CheckedBinaryOp(ref _binop, box (ref lhs, ref rhs)) => {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_transform/src/const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -723,6 +723,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
| Rvalue::Repeat(..)
| Rvalue::Len(..)
| Rvalue::Cast(..)
| Rvalue::ShallowInitBox(..)
| Rvalue::Discriminant(..)
| Rvalue::NullaryOp(..) => {}
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_transform/src/dest_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -967,6 +967,7 @@ impl<'tcx> Visitor<'tcx> for BorrowCollector {
}

Rvalue::Cast(..)
| Rvalue::ShallowInitBox(..)
| Rvalue::Use(..)
| Rvalue::Repeat(..)
| Rvalue::Len(..)
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_mir_transform/src/separate_const_switch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ fn is_likely_const<'tcx>(mut tracked_place: Place<'tcx>, block: &BasicBlockData<
| Rvalue::AddressOf(_, _)
| Rvalue::Cast(_, Operand::Constant(_), _)
| Rvalue::NullaryOp(_, _)
| Rvalue::ShallowInitBox(_, _)
| Rvalue::UnaryOp(_, Operand::Constant(_)) => return true,

// These rvalues make things ambiguous
Expand Down Expand Up @@ -301,6 +302,7 @@ fn find_determining_place<'tcx>(
| Rvalue::ThreadLocalRef(_)
| Rvalue::AddressOf(_, _)
| Rvalue::NullaryOp(_, _)
| Rvalue::ShallowInitBox(_, _)
| Rvalue::UnaryOp(_, Operand::Constant(_))
| Rvalue::Cast(_, Operand::Constant(_), _)
=> return None,
Expand Down
1 change: 0 additions & 1 deletion library/alloc/src/alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,6 @@ unsafe impl Allocator for Global {
}

/// The allocator for unique pointers.
// This function must not unwind. If it does, MIR codegen will fail.
#[cfg(all(not(no_global_oom_handling), not(test)))]
#[lang = "exchange_malloc"]
#[inline]
Expand Down
Loading

0 comments on commit e9f29a8

Please sign in to comment.