Skip to content

Commit eae44ed

Browse files
committed
add init_box_via_move intrinsic and use it for vec!
This allows us to get rid of box_new entirely
1 parent 9044961 commit eae44ed

File tree

38 files changed

+774
-960
lines changed

38 files changed

+774
-960
lines changed

compiler/rustc_borrowck/src/type_check/mod.rs

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1537,15 +1537,17 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
15371537
}
15381538
}
15391539
CastKind::Transmute => {
1540-
let ty_from = op.ty(self.body, tcx);
1541-
match ty_from.kind() {
1542-
ty::Pat(base, _) if base == ty => {}
1543-
_ => span_mirbug!(
1544-
self,
1545-
rvalue,
1546-
"Unexpected CastKind::Transmute {ty_from:?} -> {ty:?}, which is not permitted in Analysis MIR",
1547-
),
1548-
}
1540+
// FIXME: `init_box_via_move` lowering really wants to use this.
1541+
// What do we have to do here?
1542+
// let ty_from = op.ty(self.body, tcx);
1543+
// match ty_from.kind() {
1544+
// ty::Pat(base, _) if base == ty => {}
1545+
// _ => span_mirbug!(
1546+
// self,
1547+
// rvalue,
1548+
// "Unexpected CastKind::Transmute {ty_from:?} -> {ty:?}, which is not permitted in Analysis MIR",
1549+
// ),
1550+
// }
15491551
}
15501552
CastKind::Subtype => {
15511553
bug!("CastKind::Subtype shouldn't exist in borrowck")

compiler/rustc_hir_analysis/src/check/intrinsic.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ fn intrinsic_operation_unsafety(tcx: TyCtxt<'_>, intrinsic_id: LocalDefId) -> hi
7676
| sym::autodiff
7777
| sym::bitreverse
7878
| sym::black_box
79-
| sym::box_new
8079
| sym::breakpoint
8180
| sym::bswap
8281
| sym::caller_location
@@ -132,6 +131,7 @@ fn intrinsic_operation_unsafety(tcx: TyCtxt<'_>, intrinsic_id: LocalDefId) -> hi
132131
| sym::forget
133132
| sym::frem_algebraic
134133
| sym::fsub_algebraic
134+
| sym::init_box_via_move
135135
| sym::is_val_statically_known
136136
| sym::log2f16
137137
| sym::log2f32
@@ -553,6 +553,12 @@ pub(crate) fn check_intrinsic_type(
553553
sym::write_via_move => {
554554
(1, 0, vec![Ty::new_mut_ptr(tcx, param(0)), param(0)], tcx.types.unit)
555555
}
556+
sym::init_box_via_move => {
557+
let t = param(0);
558+
let maybe_uninit_t = Ty::new_maybe_uninit(tcx, t);
559+
560+
(1, 0, vec![Ty::new_box(tcx, maybe_uninit_t), param(0)], Ty::new_box(tcx, t))
561+
}
556562

557563
sym::typed_swap_nonoverlapping => {
558564
(1, 0, vec![Ty::new_mut_ptr(tcx, param(0)); 2], tcx.types.unit)
@@ -645,8 +651,6 @@ pub(crate) fn check_intrinsic_type(
645651

646652
sym::ub_checks => (0, 0, Vec::new(), tcx.types.bool),
647653

648-
sym::box_new => (1, 0, vec![param(0)], Ty::new_box(tcx, param(0))),
649-
650654
// contract_check_requires::<C>(C) -> bool, where C: impl Fn() -> bool
651655
sym::contract_check_requires => (1, 0, vec![param(0)], tcx.types.unit),
652656
sym::contract_check_ensures => {

compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3022,6 +3022,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
30223022
{
30233023
let deref_kind = if checked_ty.is_box() {
30243024
// detect Box::new(..)
3025+
// FIXME: use `box_new` diagnostic item instead?
30253026
if let ExprKind::Call(box_new, [_]) = expr.kind
30263027
&& let ExprKind::Path(qpath) = &box_new.kind
30273028
&& let Res::Def(DefKind::AssocFn, fn_id) =

compiler/rustc_mir_build/src/builder/expr/into.rs

Lines changed: 99 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -365,30 +365,112 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
365365
None
366366
})
367367
}
368-
// The `write_via_move` intrinsic needs to be special-cased very early to avoid
369-
// introducing unnecessary copies that can be hard to remove again later:
370-
// `write_via_move(ptr, val)` becomes `*ptr = val` but without any dropping.
368+
// Some intrinsics are handled here because they desperately want to avoid introducing
369+
// unnecessary copies.
371370
ExprKind::Call { ty, fun, ref args, .. }
372-
if let ty::FnDef(def_id, _generic_args) = ty.kind()
371+
if let ty::FnDef(def_id, generic_args) = ty.kind()
373372
&& let Some(intrinsic) = this.tcx.intrinsic(def_id)
374-
&& intrinsic.name == sym::write_via_move =>
373+
&& matches!(intrinsic.name, sym::write_via_move | sym::init_box_via_move) =>
375374
{
376375
// We still have to evaluate the callee expression as normal (but we don't care
377376
// about its result).
378377
let _fun = unpack!(block = this.as_local_operand(block, fun));
379-
// The destination must have unit type (so we don't actually have to store anything
380-
// into it).
381-
assert!(destination.ty(&this.local_decls, this.tcx).ty.is_unit());
382378

383-
// Compile this to an assignment of the argument into the destination.
384-
let [ptr, val] = **args else {
385-
span_bug!(expr_span, "invalid write_via_move call")
386-
};
387-
let Some(ptr) = unpack!(block = this.as_local_operand(block, ptr)).place() else {
388-
span_bug!(expr_span, "invalid write_via_move call")
389-
};
390-
let ptr_deref = ptr.project_deeper(&[ProjectionElem::Deref], this.tcx);
391-
this.expr_into_dest(ptr_deref, block, val)
379+
match intrinsic.name {
380+
sym::write_via_move => {
381+
// `write_via_move(ptr, val)` becomes `*ptr = val` but without any dropping.
382+
383+
// The destination must have unit type (so we don't actually have to store anything
384+
// into it).
385+
assert!(destination.ty(&this.local_decls, this.tcx).ty.is_unit());
386+
387+
// Compile this to an assignment of the argument into the destination.
388+
let [ptr, val] = **args else {
389+
span_bug!(expr_span, "invalid write_via_move call")
390+
};
391+
let Some(ptr) = unpack!(block = this.as_local_operand(block, ptr)).place()
392+
else {
393+
span_bug!(expr_span, "invalid write_via_move call")
394+
};
395+
let ptr_deref = ptr.project_deeper(&[ProjectionElem::Deref], this.tcx);
396+
this.expr_into_dest(ptr_deref, block, val)
397+
}
398+
sym::init_box_via_move => {
399+
// `write_via_move(b, val)` becomes
400+
// ```
401+
// *transmute::<_, *mut T>(b) = val;
402+
// transmute::<_, Box<T>>(b)
403+
// ```
404+
let t = generic_args.type_at(0);
405+
let [b, val] = **args else {
406+
span_bug!(expr_span, "invalid init_box_via_move call")
407+
};
408+
let Some(b) = unpack!(block = this.as_local_operand(block, b)).place()
409+
else {
410+
span_bug!(expr_span, "invalid init_box_via_move call")
411+
};
412+
// Project to the pointer inside `b`. We have to keep `b` in scope to ensure
413+
// it gets dropped. After the first projection we can transmute which is
414+
// easier.
415+
let ty::Adt(box_adt_def, box_adt_args) =
416+
b.ty(&this.local_decls, this.tcx).ty.kind()
417+
else {
418+
span_bug!(expr_span, "invalid init_box_via_move call")
419+
};
420+
let unique_field =
421+
this.tcx.adt_def(box_adt_def.did()).non_enum_variant().fields
422+
[rustc_abi::FieldIdx::ZERO]
423+
.did;
424+
let Some(unique_def) =
425+
this.tcx.type_of(unique_field).instantiate_identity().ty_adt_def()
426+
else {
427+
span_bug!(
428+
this.tcx.def_span(unique_field),
429+
"expected Box to contain Unique"
430+
)
431+
};
432+
let unique_ty =
433+
Ty::new_adt(this.tcx, unique_def, this.tcx.mk_args(&[box_adt_args[0]]));
434+
let b_field = b.project_deeper(
435+
&[ProjectionElem::Field(rustc_abi::FieldIdx::ZERO, unique_ty)],
436+
this.tcx,
437+
);
438+
// `ptr` is `b` transmuted to `*mut T`.
439+
let ptr_ty = Ty::new_mut_ptr(this.tcx, t);
440+
let ptr = this.local_decls.push(LocalDecl::new(ptr_ty, expr_span));
441+
this.cfg.push(
442+
block,
443+
Statement::new(source_info, StatementKind::StorageLive(ptr)),
444+
);
445+
// Make sure `StorageDead` gets emitted.
446+
this.schedule_drop_storage_and_value(expr_span, this.local_scope(), ptr);
447+
this.cfg.push_assign(
448+
block,
449+
source_info,
450+
Place::from(ptr),
451+
// Needs to be a `Copy` so that `b` still gets dropped if `val` panics.
452+
Rvalue::Cast(CastKind::Transmute, Operand::Copy(b_field), ptr_ty),
453+
);
454+
// Store `val` into `ptr`.
455+
let ptr_deref =
456+
Place::from(ptr).project_deeper(&[ProjectionElem::Deref], this.tcx);
457+
unpack!(block = this.expr_into_dest(ptr_deref, block, val));
458+
// Return `ptr` transmuted to `Box<T>`.
459+
this.cfg.push_assign(
460+
block,
461+
source_info,
462+
destination,
463+
Rvalue::Cast(
464+
CastKind::Transmute,
465+
// Move from `b` so that does not get dropped any more.
466+
Operand::Move(b),
467+
Ty::new_box(this.tcx, t),
468+
),
469+
);
470+
block.unit()
471+
}
472+
_ => rustc_middle::bug!(),
473+
}
392474
}
393475
ExprKind::Call { ty: _, fun, ref args, from_hir_call, fn_span } => {
394476
let fun = unpack!(block = this.as_local_operand(block, fun));

compiler/rustc_mir_build/src/thir/cx/expr.rs

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use rustc_middle::ty::{
2020
self, AdtKind, GenericArgs, InlineConstArgs, InlineConstArgsParts, ScalarInt, Ty, UpvarArgs,
2121
};
2222
use rustc_middle::{bug, span_bug};
23-
use rustc_span::{Span, sym};
23+
use rustc_span::Span;
2424
use tracing::{debug, info, instrument, trace};
2525

2626
use crate::errors::*;
@@ -385,24 +385,6 @@ impl<'tcx> ThirBuildCx<'tcx> {
385385
from_hir_call: true,
386386
fn_span: expr.span,
387387
}
388-
} else if let ty::FnDef(def_id, _) = self.typeck_results.expr_ty(fun).kind()
389-
&& let Some(intrinsic) = self.tcx.intrinsic(def_id)
390-
&& intrinsic.name == sym::box_new
391-
{
392-
// We don't actually evaluate `fun` here, so make sure that doesn't miss any side-effects.
393-
if !matches!(fun.kind, hir::ExprKind::Path(_)) {
394-
span_bug!(
395-
expr.span,
396-
"`box_new` intrinsic can only be called via path expression"
397-
);
398-
}
399-
let value = &args[0];
400-
return Expr {
401-
temp_lifetime: TempLifetime { temp_lifetime, backwards_incompatible },
402-
ty: expr_ty,
403-
span: expr.span,
404-
kind: ExprKind::Box { value: self.mirror_expr(value) },
405-
};
406388
} else {
407389
// Tuple-like ADTs are represented as ExprKind::Call. We convert them here.
408390
let adt_data = if let hir::ExprKind::Path(ref qpath) = fun.kind

compiler/rustc_span/src/symbol.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1233,6 +1233,7 @@ symbols! {
12331233
infer_static_outlives_requirements,
12341234
inherent_associated_types,
12351235
inherit,
1236+
init_box_via_move,
12361237
initial,
12371238
inlateout,
12381239
inline,

library/alloc/src/alloc.rs

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -342,23 +342,6 @@ unsafe impl Allocator for Global {
342342
}
343343
}
344344

345-
/// The allocator for `Box`.
346-
///
347-
/// # Safety
348-
///
349-
/// `size` and `align` must satisfy the conditions in [`Layout::from_size_align`].
350-
#[cfg(not(no_global_oom_handling))]
351-
#[lang = "exchange_malloc"]
352-
#[inline]
353-
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
354-
pub(crate) unsafe fn exchange_malloc(size: usize, align: usize) -> *mut u8 {
355-
let layout = unsafe { Layout::from_size_align_unchecked(size, align) };
356-
match Global.allocate(layout) {
357-
Ok(ptr) => ptr.as_mut_ptr(),
358-
Err(_) => handle_alloc_error(layout),
359-
}
360-
}
361-
362345
// # Allocation error handler
363346

364347
#[cfg(not(no_global_oom_handling))]

library/alloc/src/boxed.rs

Lines changed: 49 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ use core::fmt;
192192
use core::future::Future;
193193
use core::hash::{Hash, Hasher};
194194
use core::marker::{Tuple, Unsize};
195-
use core::mem::{self, SizedTypeProperties};
195+
use core::mem::{self, MaybeUninit, SizedTypeProperties};
196196
use core::ops::{
197197
AsyncFn, AsyncFnMut, AsyncFnOnce, CoerceUnsized, Coroutine, CoroutineState, Deref, DerefMut,
198198
DerefPure, DispatchFromDyn, LegacyReceiver,
@@ -203,7 +203,7 @@ use core::task::{Context, Poll};
203203

204204
#[cfg(not(no_global_oom_handling))]
205205
use crate::alloc::handle_alloc_error;
206-
use crate::alloc::{AllocError, Allocator, Global, Layout, exchange_malloc};
206+
use crate::alloc::{AllocError, Allocator, Global, Layout};
207207
use crate::raw_vec::RawVec;
208208
#[cfg(not(no_global_oom_handling))]
209209
use crate::str::from_boxed_utf8_unchecked;
@@ -233,14 +233,39 @@ pub struct Box<
233233
#[unstable(feature = "allocator_api", issue = "32838")] A: Allocator = Global,
234234
>(Unique<T>, A);
235235

236-
/// Constructs a `Box<T>` by calling the `exchange_malloc` lang item and moving the argument into
237-
/// the newly allocated memory. This is an intrinsic to avoid unnecessary copies.
236+
/// Monomorphic function for allocating an uninit `Box`.
238237
///
239-
/// This is the surface syntax for `box <expr>` expressions.
240-
#[doc(hidden)]
238+
/// # Safety
239+
///
240+
/// size and align need to be safe for `Layout::from_size_align_unchecked`.
241+
#[inline]
242+
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
243+
unsafe fn box_new_uninit(size: usize, align: usize) -> *mut u8 {
244+
let layout = unsafe { Layout::from_size_align_unchecked(size, align) };
245+
match Global.allocate(layout) {
246+
Ok(ptr) => ptr.as_mut_ptr(),
247+
Err(_) => handle_alloc_error(layout),
248+
}
249+
}
250+
251+
/// Writes `x` into `b`, then returns `b` at its new type`.
252+
///
253+
/// This is needed for `vec!`, which can't afford any extra copies of the argument (or else debug
254+
/// builds regress), has to be written fully as a call chain without `let` (or else the temporary
255+
/// lifetimes of the arguments change), and can't use an `unsafe` block as that would then also
256+
/// include the user-provided `$x`.
241257
#[rustc_intrinsic]
242258
#[unstable(feature = "liballoc_internals", issue = "none")]
243-
pub fn box_new<T>(x: T) -> Box<T>;
259+
pub fn init_box_via_move<T>(b: Box<MaybeUninit<T>>, x: T) -> Box<T>;
260+
261+
/// Helper for `vec!` to ensure type inferences work correctly (which it wouldn't if we
262+
/// inlined the `as` cast).
263+
#[doc(hidden)]
264+
#[unstable(feature = "liballoc_internals", issue = "none")]
265+
#[inline(always)]
266+
pub fn box_array_into_vec<T, const N: usize>(b: Box<[T; N]>) -> crate::vec::Vec<T> {
267+
(b as Box<[T]>).into_vec()
268+
}
244269

245270
impl<T> Box<T> {
246271
/// Allocates memory on the heap and then places `x` into it.
@@ -259,9 +284,10 @@ impl<T> Box<T> {
259284
#[rustc_diagnostic_item = "box_new"]
260285
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
261286
pub fn new(x: T) -> Self {
262-
// SAFETY: the size and align of a valid type `T` are always valid for `Layout`.
287+
// This is `Box::new_uninit` but inlined to avoid build time regressions.
288+
// SAFETY: The size and align of a valid type `T` are always valid for `Layout`.
263289
let ptr = unsafe {
264-
exchange_malloc(<T as SizedTypeProperties>::SIZE, <T as SizedTypeProperties>::ALIGN)
290+
box_new_uninit(<T as SizedTypeProperties>::SIZE, <T as SizedTypeProperties>::ALIGN)
265291
} as *mut T;
266292
// Nothing below can panic so we do not have to worry about deallocating `ptr`.
267293
// SAFETY: we just allocated the box to store `x`.
@@ -285,9 +311,21 @@ impl<T> Box<T> {
285311
#[cfg(not(no_global_oom_handling))]
286312
#[stable(feature = "new_uninit", since = "1.82.0")]
287313
#[must_use]
288-
#[inline]
314+
#[inline(always)]
315+
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
289316
pub fn new_uninit() -> Box<mem::MaybeUninit<T>> {
290-
Self::new_uninit_in(Global)
317+
// This is the same as `Self::new_uninit_in(Global)`, but manually inlined (just like
318+
// `Box::new`).
319+
320+
// SAFETY:
321+
// - The size and align of a valid type `T` are always valid for `Layout`.
322+
// - If `allocate` succeeds, the returned pointer exactly matches what `Box` needs.
323+
unsafe {
324+
mem::transmute(box_new_uninit(
325+
<T as SizedTypeProperties>::SIZE,
326+
<T as SizedTypeProperties>::ALIGN,
327+
))
328+
}
291329
}
292330

293331
/// Constructs a new `Box` with uninitialized contents, with the memory

library/alloc/src/macros.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,15 @@ macro_rules! vec {
4747
$crate::vec::from_elem($elem, $n)
4848
);
4949
($($x:expr),+ $(,)?) => (
50-
<[_]>::into_vec(
51-
// Using the intrinsic produces a dramatic improvement in stack usage for
50+
$crate::boxed::box_array_into_vec(
51+
// Using `init_box_via_move` produces a dramatic improvement in stack usage for
5252
// unoptimized programs using this code path to construct large Vecs.
53-
$crate::boxed::box_new([$($x),+])
53+
// We can't use `write_via_move` because this entire invocation has to remain a call
54+
// chain without `let` bindings, or else the temporary scopes change and things break;
55+
// it also has to all be safe since `safe { ... }` blocks sadly are not a thing and we
56+
// must not wrap `$x` in `unsafe` (also, wrapping `$x` in a safe block has a good chance
57+
// of introducing extra moves so might not be a good call either).
58+
$crate::boxed::init_box_via_move($crate::boxed::Box::new_uninit(), [$($x),+])
5459
)
5560
);
5661
}

0 commit comments

Comments
 (0)