Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 86ef320

Browse files
committed
Auto merge of #144347 - scottmcm:ssa-enums-v0, r=WaffleLapkin
No longer need `alloca`s 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. While doing so, rewrite `LocalAnalyzer::process_place` to be non-recursive, solving a 6+ year old FIXME. r? codegen
2 parents 052114f + 6a5c7e0 commit 86ef320

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
@@ -921,9 +921,10 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
921921

922922
match self.locals[place_ref.local] {
923923
LocalRef::Operand(mut o) => {
924-
// Moves out of scalar and scalar pair fields are trivial.
925-
for elem in place_ref.projection.iter() {
926-
match elem {
924+
// We only need to handle the projections that
925+
// `LocalAnalyzer::process_place` let make it here.
926+
for elem in place_ref.projection {
927+
match *elem {
927928
mir::ProjectionElem::Field(f, _) => {
928929
assert!(
929930
!o.layout.ty.is_any_ptr(),
@@ -932,17 +933,18 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
932933
);
933934
o = o.extract_field(self, bx, f.index());
934935
}
935-
mir::ProjectionElem::Index(_)
936-
| mir::ProjectionElem::ConstantIndex { .. } => {
937-
// ZSTs don't require any actual memory access.
938-
// FIXME(eddyb) deduplicate this with the identical
939-
// checks in `codegen_consume` and `extract_field`.
940-
let elem = o.layout.field(bx.cx(), 0);
941-
if elem.is_zst() {
942-
o = OperandRef::zero_sized(elem);
943-
} else {
944-
return None;
945-
}
936+
mir::PlaceElem::Downcast(_, vidx) => {
937+
debug_assert_eq!(
938+
o.layout.variants,
939+
abi::Variants::Single { index: vidx },
940+
);
941+
let layout = o.layout.for_variant(bx.cx(), vidx);
942+
o = OperandRef { val: o.val, layout }
943+
}
944+
mir::PlaceElem::Subtype(subtype_ty) => {
945+
let subtype_ty = self.monomorphize(subtype_ty);
946+
let layout = self.cx.layout_of(subtype_ty);
947+
o = OperandRef { val: o.val, layout }
946948
}
947949
_ => return None,
948950
}
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)

AltStyle によって変換されたページ (->オリジナル) /