Skip to content

Commit 6a5c7e0

Browse files
scottmcmscottmcm
committed
No longer need allocas for consuming Result<!, i32> 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.
1 parent a7a1618 commit 6a5c7e0

File tree

3 files changed

+110
-64
lines changed

3 files changed

+110
-64
lines changed

‎compiler/rustc_codegen_ssa/src/mir/analyze.rs

Lines changed: 63 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
//! An analysis to determine which locals require allocas and
22
//! which do not.
33
4+
use rustc_abi as abi;
45
use rustc_data_structures::graph::dominators::Dominators;
56
use rustc_index::bit_set::DenseBitSet;
67
use rustc_index::{IndexSlice, IndexVec};
78
use rustc_middle::mir::visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor};
89
use rustc_middle::mir::{self, DefLocation, Location, TerminatorKind, traversal};
9-
use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf};
10+
use rustc_middle::ty::layout::LayoutOf;
1011
use rustc_middle::{bug, span_bug};
1112
use tracing::debug;
1213

@@ -99,63 +100,75 @@ impl<'a, 'b, 'tcx, Bx: BuilderMethods<'b, 'tcx>> LocalAnalyzer<'a, 'b, 'tcx, Bx>
99100
context: PlaceContext,
100101
location: Location,
101102
) {
102-
let cx = self.fx.cx;
103-
104-
if let Some((place_base, elem)) = place_ref.last_projection() {
105-
let mut base_context = if context.is_mutating_use() {
106-
PlaceContext::MutatingUse(MutatingUseContext::Projection)
107-
} else {
108-
PlaceContext::NonMutatingUse(NonMutatingUseContext::Projection)
109-
};
110-
111-
// Allow uses of projections that are ZSTs or from scalar fields.
112-
let is_consume = matches!(
113-
context,
114-
PlaceContext::NonMutatingUse(
115-
NonMutatingUseContext::Copy | NonMutatingUseContext::Move,
116-
)
117-
);
118-
if is_consume {
119-
let base_ty = place_base.ty(self.fx.mir, cx.tcx());
120-
let base_ty = self.fx.monomorphize(base_ty);
121-
122-
// ZSTs don't require any actual memory access.
123-
let elem_ty = base_ty.projection_ty(cx.tcx(), self.fx.monomorphize(elem)).ty;
124-
let span = self.fx.mir.local_decls[place_ref.local].source_info.span;
125-
if cx.spanned_layout_of(elem_ty, span).is_zst() {
126-
return;
103+
if !place_ref.projection.is_empty() {
104+
const COPY_CONTEXT: PlaceContext =
105+
PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy);
106+
107+
// `PlaceElem::Index` is the only variant that can mention other `Local`s,
108+
// so check for those up-front before any potential short-circuits.
109+
for elem in place_ref.projection {
110+
if let mir::PlaceElem::Index(index_local) = *elem {
111+
self.visit_local(index_local, COPY_CONTEXT, location);
127112
}
113+
}
128114

129-
if let mir::ProjectionElem::Field(..) = elem {
130-
let layout = cx.spanned_layout_of(base_ty.ty, span);
131-
if cx.is_backend_immediate(layout) || cx.is_backend_scalar_pair(layout) {
132-
// Recurse with the same context, instead of `Projection`,
133-
// potentially stopping at non-operand projections,
134-
// which would trigger `not_ssa` on locals.
135-
base_context = context;
136-
}
137-
}
115+
// If our local is already memory, nothing can make it *more* memory
116+
// so we don't need to bother checking the projections further.
117+
if self.locals[place_ref.local] == LocalKind::Memory {
118+
return;
138119
}
139120

140-
if let mir::ProjectionElem::Deref = elem {
141-
// Deref projections typically only read the pointer.
142-
base_context = PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy);
121+
if place_ref.is_indirect_first_projection() {
122+
// If this starts with a `Deref`, we only need to record a read of the
123+
// pointer being dereferenced, as all the subsequent projections are
124+
// working on a place which is always supported. (And because we're
125+
// looking at codegen MIR, it can only happen as the first projection.)
126+
self.visit_local(place_ref.local, COPY_CONTEXT, location);
127+
return;
143128
}
144129

145-
self.process_place(&place_base, base_context, location);
146-
// HACK(eddyb) this emulates the old `visit_projection_elem`, this
147-
// entire `visit_place`-like `process_place` method should be rewritten,
148-
// now that we have moved to the "slice of projections" representation.
149-
if let mir::ProjectionElem::Index(local) = elem {
150-
self.visit_local(
151-
local,
152-
PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy),
153-
location,
154-
);
130+
if context.is_mutating_use() {
131+
// If it's a mutating use it doesn't matter what the projections are,
132+
// if there are *any* then we need a place to write. (For example,
133+
// `_1 = Foo()` works in SSA but `_2.0 = Foo()` does not.)
134+
let mut_projection = PlaceContext::MutatingUse(MutatingUseContext::Projection);
135+
self.visit_local(place_ref.local, mut_projection, location);
136+
return;
155137
}
156-
} else {
157-
self.visit_local(place_ref.local, context, location);
138+
139+
// Scan through to ensure the only projections are those which
140+
// `FunctionCx::maybe_codegen_consume_direct` can handle.
141+
let base_ty = self.fx.monomorphized_place_ty(mir::PlaceRef::from(place_ref.local));
142+
let mut layout = self.fx.cx.layout_of(base_ty);
143+
for elem in place_ref.projection {
144+
layout = match *elem {
145+
mir::PlaceElem::Field(fidx, ..) => layout.field(self.fx.cx, fidx.as_usize()),
146+
mir::PlaceElem::Downcast(_, vidx)
147+
if let abi::Variants::Single { index: single_variant } =
148+
layout.variants
149+
&& vidx == single_variant =>
150+
{
151+
layout.for_variant(self.fx.cx, vidx)
152+
}
153+
mir::PlaceElem::Subtype(subtype_ty) => {
154+
let subtype_ty = self.fx.monomorphize(subtype_ty);
155+
self.fx.cx.layout_of(subtype_ty)
156+
}
157+
_ => {
158+
self.locals[place_ref.local] = LocalKind::Memory;
159+
return;
160+
}
161+
}
162+
}
163+
debug_assert!(
164+
!self.fx.cx.is_backend_ref(layout),
165+
"Post-projection {place_ref:?} layout should be non-Ref, but it's {layout:?}",
166+
);
158167
}
168+
169+
// Even with supported projections, we still need to have `visit_local`
170+
// check for things that can't be done in SSA (like `SharedBorrow`).
171+
self.visit_local(place_ref.local, context, location);
159172
}
160173
}
161174

‎compiler/rustc_codegen_ssa/src/mir/operand.rs

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -931,9 +931,10 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
931931

932932
match self.locals[place_ref.local] {
933933
LocalRef::Operand(mut o) => {
934-
// Moves out of scalar and scalar pair fields are trivial.
935-
for elem in place_ref.projection.iter() {
936-
match elem {
934+
// We only need to handle the projections that
935+
// `LocalAnalyzer::process_place` let make it here.
936+
for elem in place_ref.projection {
937+
match *elem {
937938
mir::ProjectionElem::Field(f, _) => {
938939
assert!(
939940
!o.layout.ty.is_any_ptr(),
@@ -942,17 +943,18 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
942943
);
943944
o = o.extract_field(self, bx, f.index());
944945
}
945-
mir::ProjectionElem::Index(_)
946-
| mir::ProjectionElem::ConstantIndex { .. } => {
947-
// ZSTs don't require any actual memory access.
948-
// FIXME(eddyb) deduplicate this with the identical
949-
// checks in `codegen_consume` and `extract_field`.
950-
let elem = o.layout.field(bx.cx(), 0);
951-
if elem.is_zst() {
952-
o = OperandRef::zero_sized(elem);
953-
} else {
954-
return None;
955-
}
946+
mir::PlaceElem::Downcast(_, vidx) => {
947+
debug_assert_eq!(
948+
o.layout.variants,
949+
abi::Variants::Single { index: vidx },
950+
);
951+
let layout = o.layout.for_variant(bx.cx(), vidx);
952+
o = OperandRef { val: o.val, layout }
953+
}
954+
mir::PlaceElem::Subtype(subtype_ty) => {
955+
let subtype_ty = self.monomorphize(subtype_ty);
956+
let layout = self.cx.layout_of(subtype_ty);
957+
o = OperandRef { val: o.val, layout }
956958
}
957959
_ => return None,
958960
}

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
//@ compile-flags: -Copt-level=0
2+
//@ only-64bit
3+
4+
#![crate_type = "lib"]
5+
6+
use std::ops::ControlFlow;
7+
8+
pub enum Never {}
9+
10+
#[no_mangle]
11+
pub fn make_unmake_result_never(x: i32) -> i32 {
12+
// CHECK-LABEL: define i32 @make_unmake_result_never(i32 %x)
13+
// CHECK: start:
14+
// CHECK-NEXT: ret i32 %x
15+
16+
let y: Result<i32, Never> = Ok(x);
17+
let Ok(z) = y;
18+
z
19+
}
20+
21+
#[no_mangle]
22+
pub fn extract_control_flow_never(x: ControlFlow<&str, Never>) -> &str {
23+
// CHECK-LABEL: define { ptr, i64 } @extract_control_flow_never(ptr align 1 %x.0, i64 %x.1)
24+
// CHECK: start:
25+
// CHECK-NEXT: %[[P0:.+]] = insertvalue { ptr, i64 } poison, ptr %x.0, 0
26+
// CHECK-NEXT: %[[P1:.+]] = insertvalue { ptr, i64 } %[[P0]], i64 %x.1, 1
27+
// CHECK-NEXT: ret { ptr, i64 } %[[P1]]
28+
29+
let ControlFlow::Break(s) = x;
30+
s
31+
}

0 commit comments

Comments
 (0)