From 6a5c7e04156125cb8c1ff78eae95e6ff076f51ed Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Tue, 22 Jul 2025 23:23:02 -0700 Subject: [PATCH] No longer need `alloca`s for consuming `Result` and similar In optimized builds GVN gets rid of these already, but in `opt-level=0` we actually make `alloca`s for this, which particularly impacts `?`-style things that use actually-only-one-variant types like this. --- compiler/rustc_codegen_ssa/src/mir/analyze.rs | 113 ++++++++++-------- compiler/rustc_codegen_ssa/src/mir/operand.rs | 30 ++--- .../enum/enum-transparent-extract.rs | 31 +++++ 3 files changed, 110 insertions(+), 64 deletions(-) create mode 100644 tests/codegen-llvm/enum/enum-transparent-extract.rs diff --git a/compiler/rustc_codegen_ssa/src/mir/analyze.rs b/compiler/rustc_codegen_ssa/src/mir/analyze.rs index 6d6465dd798b5..26470cb922d07 100644 --- a/compiler/rustc_codegen_ssa/src/mir/analyze.rs +++ b/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); } } diff --git a/compiler/rustc_codegen_ssa/src/mir/operand.rs b/compiler/rustc_codegen_ssa/src/mir/operand.rs index 06bedaaa4a27e..b5010874713f8 100644 --- a/compiler/rustc_codegen_ssa/src/mir/operand.rs +++ b/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, } diff --git a/tests/codegen-llvm/enum/enum-transparent-extract.rs b/tests/codegen-llvm/enum/enum-transparent-extract.rs new file mode 100644 index 0000000000000..c5efb8d472b0f --- /dev/null +++ b/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 = 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 +}