remove box derefs from codgen

Author: beepster4096

compiler/rustc_codegen_ssa/src/mir/operand.rs

@@ -118,22 +118,20 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
     }
 
     pub fn deref<Cx: LayoutTypeMethods<'tcx>>(self, cx: &Cx) -> PlaceRef<'tcx, V> {
+        if self.layout.ty.is_box() && !self.layout.abi.is_scalar() {
+            bug!("dereferencing non-scalar box ({:?}) in codegen", self.layout.ty);
+        }
+
         let projected_ty = self
             .layout
             .ty
             .builtin_deref(true)
             .unwrap_or_else(|| bug!("deref of non-pointer {:?}", self))
             .ty;
+
         let (llptr, llextra) = match self.val {
             OperandValue::Immediate(llptr) => (llptr, None),
-            OperandValue::Pair(llptr, llextra) => {
-                // if the box's allocator isn't a ZST, then "llextra" is actually the allocator
-                if self.layout.ty.is_box() && !self.layout.field(cx, 1).is_zst() {
-                    (llptr, None)
-                } else {
-                    (llptr, Some(llextra))
-                }
-            }
+            OperandValue::Pair(llptr, llextra) => (llptr, Some(llextra)),
             OperandValue::Ref(..) => bug!("Deref of by-Ref operand {:?}", self),
         };
         let layout = cx.layout_of(projected_ty);

compiler/rustc_codegen_ssa/src/mir/place.rs

@@ -446,35 +446,15 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                         mir::PlaceRef { projection: &place_ref.projection[..elem.0], ..place_ref },
                     );
 
-                    // a box with a non-zst allocator should not be directly dereferenced
-                    if cg_base.layout.ty.is_box() && !cg_base.layout.field(cx, 1).is_zst() {
-                        // Extract `Box<T>` -> `Unique<T>` -> `NonNull<T>` -> `*const T`
-                        let ptr =
-                            cg_base.extract_field(bx, 0).extract_field(bx, 0).extract_field(bx, 0);
-
-                        ptr.deref(bx.cx())
-                    } else {
-                        cg_base.deref(bx.cx())
-                    }
+                    cg_base.deref(bx.cx())
                 } else {
                     bug!("using operand local {:?} as place", place_ref);
                 }
             }
         };
         for elem in place_ref.projection[base..].iter() {
             cg_base = match *elem {
-                mir::ProjectionElem::Deref => {
-                    // a box with a non-zst allocator should not be directly dereferenced
-                    if cg_base.layout.ty.is_box() && !cg_base.layout.field(cx, 1).is_zst() {
-                        // Project `Box<T>` -> `Unique<T>` -> `NonNull<T>` -> `*const T`
-                        let ptr =
-                            cg_base.project_field(bx, 0).project_field(bx, 0).project_field(bx, 0);
-
-                        bx.load_operand(ptr).deref(bx.cx())
-                    } else {
-                        bx.load_operand(cg_base).deref(bx.cx())
-                    }
-                }
+                mir::ProjectionElem::Deref => bx.load_operand(cg_base).deref(bx.cx()),
                 mir::ProjectionElem::Field(ref field, _) => {
                     cg_base.project_field(bx, field.index())
                 }

compiler/rustc_const_eval/src/transform/validate.rs

@@ -240,65 +240,79 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
         context: PlaceContext,
         location: Location,
     ) {
-        if let ProjectionElem::Index(index) = elem {
-            let index_ty = self.body.local_decls[index].ty;
-            if index_ty != self.tcx.types.usize {
-                self.fail(location, format!("bad index ({:?} != usize)", index_ty))
+        match elem {
+            ProjectionElem::Index(index) => {
+                let index_ty = self.body.local_decls[index].ty;
+                if index_ty != self.tcx.types.usize {
+                    self.fail(location, format!("bad index ({:?} != usize)", index_ty))
+                }
             }
-        }
-        if let ProjectionElem::Field(f, ty) = elem {
-            let parent = Place { local, projection: self.tcx.intern_place_elems(proj_base) };
-            let parent_ty = parent.ty(&self.body.local_decls, self.tcx);
-            let fail_out_of_bounds = |this: &Self, location| {
-                this.fail(location, format!("Out of bounds field {:?} for {:?}", f, parent_ty));
-            };
-            let check_equal = |this: &Self, location, f_ty| {
-                if !this.mir_assign_valid_types(ty, f_ty) {
-                    this.fail(
+            ProjectionElem::Deref if self.mir_phase >= MirPhase::GeneratorsLowered => {
+                let base_ty = Place::ty_from(local, proj_base, &self.body.local_decls, self.tcx).ty;
+
+                if base_ty.is_box() {
+                    self.fail(
+                        location,
+                        format!("{:?} dereferenced after ElaborateBoxDerefs", base_ty),
+                    )
+                }
+            }
+            ProjectionElem::Field(f, ty) => {
+                let parent = Place { local, projection: self.tcx.intern_place_elems(proj_base) };
+                let parent_ty = parent.ty(&self.body.local_decls, self.tcx);
+                let fail_out_of_bounds = |this: &Self, location| {
+                    this.fail(location, format!("Out of bounds field {:?} for {:?}", f, parent_ty));
+                };
+                let check_equal = |this: &Self, location, f_ty| {
+                    if !this.mir_assign_valid_types(ty, f_ty) {
+                        this.fail(
                         location,
                         format!(
                             "Field projection `{:?}.{:?}` specified type `{:?}`, but actual type is {:?}",
                             parent, f, ty, f_ty
                         )
                     )
-                }
-            };
-            match parent_ty.ty.kind() {
-                ty::Tuple(fields) => {
-                    let Some(f_ty) = fields.get(f.as_usize()) else {
-                        fail_out_of_bounds(self, location);
-                        return;
-                    };
-                    check_equal(self, location, *f_ty);
-                }
-                ty::Adt(adt_def, substs) => {
-                    let var = parent_ty.variant_index.unwrap_or(VariantIdx::from_u32(0));
-                    let Some(field) = adt_def.variant(var).fields.get(f.as_usize()) else {
-                        fail_out_of_bounds(self, location);
-                        return;
-                    };
-                    check_equal(self, location, field.ty(self.tcx, substs));
-                }
-                ty::Closure(_, substs) => {
-                    let substs = substs.as_closure();
-                    let Some(f_ty) = substs.upvar_tys().nth(f.as_usize()) else {
-                        fail_out_of_bounds(self, location);
-                        return;
-                    };
-                    check_equal(self, location, f_ty);
-                }
-                ty::Generator(_, substs, _) => {
-                    let substs = substs.as_generator();
-                    let Some(f_ty) = substs.upvar_tys().nth(f.as_usize()) else {
-                        fail_out_of_bounds(self, location);
-                        return;
-                    };
-                    check_equal(self, location, f_ty);
-                }
-                _ => {
-                    self.fail(location, format!("{:?} does not have fields", parent_ty.ty));
+                    }
+                };
+
+                match parent_ty.ty.kind() {
+                    ty::Tuple(fields) => {
+                        let Some(f_ty) = fields.get(f.as_usize()) else {
+                            fail_out_of_bounds(self, location);
+                            return;
+                        };
+                        check_equal(self, location, *f_ty);
+                    }
+                    ty::Adt(adt_def, substs) => {
+                        let var = parent_ty.variant_index.unwrap_or(VariantIdx::from_u32(0));
+                        let Some(field) = adt_def.variant(var).fields.get(f.as_usize()) else {
+                            fail_out_of_bounds(self, location);
+                            return;
+                        };
+                        check_equal(self, location, field.ty(self.tcx, substs));
+                    }
+                    ty::Closure(_, substs) => {
+                        let substs = substs.as_closure();
+                        let Some(f_ty) = substs.upvar_tys().nth(f.as_usize()) else {
+                            fail_out_of_bounds(self, location);
+                            return;
+                        };
+                        check_equal(self, location, f_ty);
+                    }
+                    ty::Generator(_, substs, _) => {
+                        let substs = substs.as_generator();
+                        let Some(f_ty) = substs.upvar_tys().nth(f.as_usize()) else {
+                            fail_out_of_bounds(self, location);
+                            return;
+                        };
+                        check_equal(self, location, f_ty);
+                    }
+                    _ => {
+                        self.fail(location, format!("{:?} does not have fields", parent_ty.ty));
+                    }
                 }
             }
+            _ => {}
         }
         self.super_projection_elem(local, proj_base, elem, context, location);
     }

compiler/rustc_middle/src/mir/mod.rs

@@ -176,8 +176,9 @@ pub enum MirPhase {
     DropsLowered = 3,
     /// After this projections may only contain deref projections as the first element.
     Derefered = 4,
-    /// Beginning with this phase, the following variant is disallowed:
+    /// Beginning with this phase, the following variants are disallowed:
     /// * [`Rvalue::Aggregate`] for any `AggregateKind` except `Array`
+    /// * [`ProjectionElem::Deref`] of `Box`
     ///
     /// And the following variant is allowed:
     /// * [`StatementKind::SetDiscriminant`]

compiler/rustc_mir_dataflow/src/elaborate_drops.rs

@@ -410,7 +410,18 @@ where
     fn open_drop_for_box(&mut self, adt: ty::AdtDef<'tcx>, substs: SubstsRef<'tcx>) -> BasicBlock {
         debug!("open_drop_for_box({:?}, {:?}, {:?})", self, adt, substs);
 
-        let interior = self.tcx().mk_place_deref(self.place);
+        // drop glue is sent straight to codegen
+        // box cannot be directly dereferenced
+        let unique_ty = adt.non_enum_variant().fields[0].ty(self.tcx(), substs);
+        let nonnull_ty =
+            unique_ty.ty_adt_def().unwrap().non_enum_variant().fields[0].ty(self.tcx(), substs);
+        let ptr_ty = self.tcx().mk_imm_ptr(substs[0].expect_ty());
+
+        let unique_place = self.tcx().mk_place_field(self.place, Field::new(0), unique_ty);
+        let nonnull_place = self.tcx().mk_place_field(unique_place, Field::new(0), nonnull_ty);
+        let ptr_place = self.tcx().mk_place_field(nonnull_place, Field::new(0), ptr_ty);
+        let interior = self.tcx().mk_place_deref(ptr_place);
+
         let interior_path = self.elaborator.deref_subpath(self.path);
 
         let succ = self.box_free_block(adt, substs, self.succ, self.unwind);

compiler/rustc_mir_transform/src/elaborate_box_derefs.rs

@@ -0,0 +1,130 @@
+//! This pass transforms derefs of Box into a deref of the pointer inside Box
+//! Codegen does not allow box to be directly dereferenced
+
+use crate::MirPass;
+use rustc_hir::def_id::DefId;
+use rustc_index::vec::Idx;
+use rustc_middle::mir::patch::MirPatch;
+use rustc_middle::mir::visit::MutVisitor;
+use rustc_middle::mir::*;
+use rustc_middle::ty::subst::Subst;
+use rustc_middle::ty::TyCtxt;
+
+struct ElaborateBoxDerefVistor<'tcx, 'a> {
+    tcx: TyCtxt<'tcx>,
+    unique_did: DefId,
+    nonnull_did: DefId,
+    local_decls: &'a mut LocalDecls<'tcx>,
+    patch: MirPatch<'tcx>,
+}
+
+impl<'tcx, 'a> MutVisitor<'tcx> for ElaborateBoxDerefVistor<'tcx, 'a> {
+    fn tcx(&self) -> TyCtxt<'tcx> {
+        self.tcx
+    }
+
+    fn visit_place(
+        &mut self,
+        place: &mut Place<'tcx>,
+        context: visit::PlaceContext,
+        location: Location,
+    ) {
+        let tcx = self.tcx;
+
+        let base_ty = self.local_decls[place.local].ty;
+
+        // Derefer ensures that derefs are always the first projection
+        if place.projection.first() == Some(&PlaceElem::Deref) && base_ty.is_box() {
+            let source_info = self.local_decls[place.local].source_info;
+
+            let substs = tcx.intern_substs(&[base_ty.boxed_ty().into()]);
+            let unique_ty = tcx.bound_type_of(self.unique_did).subst(tcx, substs);
+            let nonnull_ty = tcx.bound_type_of(self.nonnull_did).subst(tcx, substs);
+            let ptr_ty = tcx.mk_imm_ptr(base_ty.boxed_ty());
+
+            let ptr_local = self.patch.new_temp(ptr_ty, source_info.span);
+            self.local_decls.push(LocalDecl::new(ptr_ty, source_info.span));
+
+            self.patch.add_statement(location, StatementKind::StorageLive(ptr_local));
+
+            self.patch.add_assign(
+                location,
+                Place::from(ptr_local),
+                Rvalue::Use(Operand::Copy(Place::from(place.local).project_deeper(
+                    &[
+                        PlaceElem::Field(Field::new(0), unique_ty),
+                        PlaceElem::Field(Field::new(0), nonnull_ty),
+                        PlaceElem::Field(Field::new(0), ptr_ty),
+                    ],
+                    tcx,
+                ))),
+            );
+
+            place.local = ptr_local;
+
+            self.patch.add_statement(
+                Location { block: location.block, statement_index: location.statement_index + 1 },
+                StatementKind::StorageDead(ptr_local),
+            );
+        }
+
+        self.super_place(place, context, location);
+    }
+}
+
+pub struct ElaborateBoxDerefs;
+
+impl<'tcx> MirPass<'tcx> for ElaborateBoxDerefs {
+    fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
+        if let Some(def_id) = tcx.lang_items().owned_box() {
+            let unique_did = tcx.adt_def(def_id).non_enum_variant().fields[0].did;
+
+            let Some(nonnull_def) = tcx.type_of(unique_did).ty_adt_def() else {
+                span_bug!(tcx.def_span(unique_did), "expected Box to contain Unique")
+            };
+
+            let nonnull_did = nonnull_def.non_enum_variant().fields[0].did;
+
+            let patch = MirPatch::new(body);
+
+            let (basic_blocks, local_decls) = body.basic_blocks_and_local_decls_mut();
+
+            let mut visitor =
+                ElaborateBoxDerefVistor { tcx, unique_did, nonnull_did, local_decls, patch };
+
+            for (block, BasicBlockData { statements, terminator, .. }) in
+                basic_blocks.iter_enumerated_mut()
+            {
+                let mut index = 0;
+                for statement in statements {
+                    let location = Location { block, statement_index: index };
+                    visitor.visit_statement(statement, location);
+                    index += 1;
+                }
+
+                if let Some(terminator) = terminator
+                && !matches!(terminator.kind, TerminatorKind::Yield{..})
+                {
+                    let location = Location { block, statement_index: index };
+                    visitor.visit_terminator(terminator, location);
+                }
+
+                let location = Location { block, statement_index: index };
+                match terminator {
+                    // yielding into a box is handed when lowering generators
+                    Some(Terminator { kind: TerminatorKind::Yield { value, .. }, .. }) => {
+                        visitor.visit_operand(value, location);
+                    }
+                    Some(terminator) => {
+                        visitor.visit_terminator(terminator, location);
+                    }
+                    None => {}
+                }
+            }
+
+            visitor.patch.apply(body);
+        } else {
+            // box is not present, this pass doesn't need to do anything
+        }
+    }
+}