No longer need allocas for consuming Result<!, i32> and similar

compiler/rustc_codegen_ssa/src/mir/analyze.rs

@@ -1,12 +1,13 @@
 //! An analysis to determine which locals require allocas and
 //! which do not.
 
+use rustc_abi as abi;
 use rustc_data_structures::graph::dominators::Dominators;
 use rustc_index::bit_set::DenseBitSet;
 use rustc_index::{IndexSlice, IndexVec};
 use rustc_middle::mir::visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor};
 use rustc_middle::mir::{self, DefLocation, Location, TerminatorKind, traversal};
-use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf};
+use rustc_middle::ty::layout::LayoutOf;
 use rustc_middle::{bug, span_bug};
 use tracing::debug;
 
@@ -99,63 +100,75 @@ impl<'a, 'b, 'tcx, Bx: BuilderMethods<'b, 'tcx>> LocalAnalyzer<'a, 'b, 'tcx, Bx>
         context: PlaceContext,
         ___location: Location,
     ) {
-        let cx = self.fx.cx;
-
-        if let Some((place_base, elem)) = place_ref.last_projection() {
-            let mut base_context = if context.is_mutating_use() {
-                PlaceContext::MutatingUse(MutatingUseContext::Projection)
-            } else {
-                PlaceContext::NonMutatingUse(NonMutatingUseContext::Projection)
-            };
-
-            // Allow uses of projections that are ZSTs or from scalar fields.
-            let is_consume = matches!(
-                context,
-                PlaceContext::NonMutatingUse(
-                    NonMutatingUseContext::Copy | NonMutatingUseContext::Move,
-                )
-            );
-            if is_consume {
-                let base_ty = place_base.ty(self.fx.mir, cx.tcx());
-                let base_ty = self.fx.monomorphize(base_ty);
-
-                // ZSTs don't require any actual memory access.
-                let elem_ty = base_ty.projection_ty(cx.tcx(), self.fx.monomorphize(elem)).ty;
-                let span = self.fx.mir.local_decls[place_ref.local].source_info.span;
-                if cx.spanned_layout_of(elem_ty, span).is_zst() {
-                    return;
+        if !place_ref.projection.is_empty() {
+            const COPY_CONTEXT: PlaceContext =
+                PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy);
+
+            // `PlaceElem::Index` is the only variant that can mention other `Local`s,
+            // so check for those up-front before any potential short-circuits.
+            for elem in place_ref.projection {
+                if let mir::PlaceElem::Index(index_local) = *elem {
+                    self.visit_local(index_local, COPY_CONTEXT, ___location);
                 }
+            }
 
-                if let mir::ProjectionElem::Field(..) = elem {
-                    let layout = cx.spanned_layout_of(base_ty.ty, span);
-                    if cx.is_backend_immediate(layout) || cx.is_backend_scalar_pair(layout) {
-                        // Recurse with the same context, instead of `Projection`,
-                        // potentially stopping at non-operand projections,
-                        // which would trigger `not_ssa` on locals.
-                        base_context = context;
-                    }
-                }
+            // If our local is already memory, nothing can make it *more* memory
+            // so we don't need to bother checking the projections further.
+            if self.locals[place_ref.local] == LocalKind::Memory {
+                return;
             }
 
-            if let mir::ProjectionElem::Deref = elem {
-                // Deref projections typically only read the pointer.
-                base_context = PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy);
+            if place_ref.is_indirect_first_projection() {
+                // If this starts with a `Deref`, we only need to record a read of the
+                // pointer being dereferenced, as all the subsequent projections are
+                // working on a place which is always supported. (And because we're
+                // looking at codegen MIR, it can only happen as the first projection.)
+                self.visit_local(place_ref.local, COPY_CONTEXT, ___location);
+                return;
             }
 
-            self.process_place(&place_base, base_context, ___location);
-            // HACK(eddyb) this emulates the old `visit_projection_elem`, this
-            // entire `visit_place`-like `process_place` method should be rewritten,
-            // now that we have moved to the "slice of projections" representation.
-            if let mir::ProjectionElem::Index(local) = elem {
-                self.visit_local(
-                    local,
-                    PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy),
-                    ___location,
-                );
+            if context.is_mutating_use() {
+                // If it's a mutating use it doesn't matter what the projections are,
+                // if there are *any* then we need a place to write. (For example,
+                // `_1 = Foo()` works in SSA but `_2.0 = Foo()` does not.)
+                let mut_projection = PlaceContext::MutatingUse(MutatingUseContext::Projection);
+                self.visit_local(place_ref.local, mut_projection, ___location);
+                return;
             }
-        } else {
-            self.visit_local(place_ref.local, context, ___location);
+
+            // Scan through to ensure the only projections are those which
+            // `FunctionCx::maybe_codegen_consume_direct` can handle.
+            let base_ty = self.fx.monomorphized_place_ty(mir::PlaceRef::from(place_ref.local));
+            let mut layout = self.fx.cx.layout_of(base_ty);
+            for elem in place_ref.projection {
+                layout = match *elem {
+                    mir::PlaceElem::Field(fidx, ..) => layout.field(self.fx.cx, fidx.as_usize()),
+                    mir::PlaceElem::Downcast(_, vidx)
+                        if let abi::Variants::Single { index: single_variant } =
+                            layout.variants
+                            && vidx == single_variant =>
+                    {
+                        layout.for_variant(self.fx.cx, vidx)
+                    }
+                    mir::PlaceElem::Subtype(subtype_ty) => {
+                        let subtype_ty = self.fx.monomorphize(subtype_ty);
+                        self.fx.cx.layout_of(subtype_ty)
+                    }
+                    _ => {
+                        self.locals[place_ref.local] = LocalKind::Memory;
+                        return;
+                    }
+                }
+            }
+            debug_assert!(
+                !self.fx.cx.is_backend_ref(layout),
+                "Post-projection {place_ref:?} layout should be non-Ref, but it's {layout:?}",
+            );
         }
+
+        // Even with supported projections, we still need to have `visit_local`
+        // check for things that can't be done in SSA (like `SharedBorrow`).
+        self.visit_local(place_ref.local, context, ___location);
     }
 }
 

compiler/rustc_codegen_ssa/src/mir/operand.rs

@@ -931,9 +931,10 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
 
         match self.locals[place_ref.local] {
             LocalRef::Operand(mut o) => {
-                // Moves out of scalar and scalar pair fields are trivial.
-                for elem in place_ref.projection.iter() {
-                    match elem {
+                // We only need to handle the projections that
+                // `LocalAnalyzer::process_place` let make it here.
+                for elem in place_ref.projection {
+                    match *elem {
                         mir::ProjectionElem::Field(f, _) => {
                             assert!(
                                 !o.layout.ty.is_any_ptr(),
@@ -942,17 +943,18 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                             );
                             o = o.extract_field(self, bx, f.index());
                         }
-                        mir::ProjectionElem::Index(_)
-                        | mir::ProjectionElem::ConstantIndex { .. } => {
-                            // ZSTs don't require any actual memory access.
-                            // FIXME(eddyb) deduplicate this with the identical
-                            // checks in `codegen_consume` and `extract_field`.
-                            let elem = o.layout.field(bx.cx(), 0);
-                            if elem.is_zst() {
-                                o = OperandRef::zero_sized(elem);
-                            } else {
-                                return None;
-                            }
+                        mir::PlaceElem::Downcast(_, vidx) => {
+                            debug_assert_eq!(
+                                o.layout.variants,
+                                abi::Variants::Single { index: vidx },
+                            );
+                            let layout = o.layout.for_variant(bx.cx(), vidx);
+                            o = OperandRef { val: o.val, layout }
+                        }
+                        mir::PlaceElem::Subtype(subtype_ty) => {
+                            let subtype_ty = self.monomorphize(subtype_ty);
+                            let layout = self.cx.layout_of(subtype_ty);
+                            o = OperandRef { val: o.val, layout }
                         }
                         _ => return None,
                     }

tests/codegen-llvm/enum/enum-transparent-extract.rs

@@ -0,0 +1,31 @@
+//@ compile-flags: -Copt-level=0
+//@ only-64bit
+
+#![crate_type = "lib"]
+
+use std::ops::ControlFlow;
+
+pub enum Never {}
+
+#[no_mangle]
+pub fn make_unmake_result_never(x: i32) -> i32 {
+    // CHECK-LABEL: define i32 @make_unmake_result_never(i32 %x)
+    // CHECK: start:
+    // CHECK-NEXT: ret i32 %x
+
+    let y: Result<i32, Never> = Ok(x);
+    let Ok(z) = y;
+    z
+}
+
+#[no_mangle]
+pub fn extract_control_flow_never(x: ControlFlow<&str, Never>) -> &str {
+    // CHECK-LABEL: define { ptr, i64 } @extract_control_flow_never(ptr align 1 %x.0, i64 %x.1)
+    // CHECK: start:
+    // CHECK-NEXT: %[[P0:.+]] = insertvalue { ptr, i64 } poison, ptr %x.0, 0
+    // CHECK-NEXT: %[[P1:.+]] = insertvalue { ptr, i64 } %[[P0]], i64 %x.1, 1
+    // CHECK-NEXT: ret { ptr, i64 } %[[P1]]
+
+    let ControlFlow::Break(s) = x;
+    s
+}