Skip to content

Commit e110567

Browse files
committed
Revert "Auto merge of rust-lang#115105 - cjgillot:dest-prop-default, r=oli-obk"
This reverts commit cfb7304, reversing changes made to 91c0823.
1 parent 434999e commit e110567

21 files changed

+661
-373
lines changed

compiler/rustc_mir_transform/src/dest_prop.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation {
160160
// 2. Despite being an overall perf improvement, this still causes a 30% regression in
161161
// keccak. We can temporarily fix this by bounding function size, but in the long term
162162
// we should fix this by being smarter about invalidating analysis results.
163-
sess.mir_opt_level() >= 2
163+
sess.mir_opt_level() >= 3
164164
}
165165

166166
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {

compiler/rustc_mir_transform/src/lib.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ mod match_branches;
8888
mod mentioned_items;
8989
mod multiple_return_terminators;
9090
mod normalize_array_len;
91+
mod nrvo;
9192
mod prettify;
9293
mod promote_consts;
9394
mod ref_prop;
@@ -607,12 +608,13 @@ fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
607608
&jump_threading::JumpThreading,
608609
&early_otherwise_branch::EarlyOtherwiseBranch,
609610
&simplify_comparison_integral::SimplifyComparisonIntegral,
611+
&dest_prop::DestinationPropagation,
610612
&o1(simplify_branches::SimplifyConstCondition::Final),
611613
&o1(remove_noop_landing_pads::RemoveNoopLandingPads),
612614
&o1(simplify::SimplifyCfg::Final),
613615
&copy_prop::CopyProp,
614616
&dead_store_elimination::DeadStoreElimination::Final,
615-
&dest_prop::DestinationPropagation,
617+
&nrvo::RenameReturnPlace,
616618
&simplify::SimplifyLocals::Final,
617619
&multiple_return_terminators::MultipleReturnTerminators,
618620
&deduplicate_blocks::DeduplicateBlocks,
+235
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,235 @@
1+
//! See the docs for [`RenameReturnPlace`].
2+
3+
use rustc_hir::Mutability;
4+
use rustc_index::bit_set::BitSet;
5+
use rustc_middle::bug;
6+
use rustc_middle::mir::visit::{MutVisitor, NonUseContext, PlaceContext, Visitor};
7+
use rustc_middle::mir::{self, BasicBlock, Local, Location};
8+
use rustc_middle::ty::TyCtxt;
9+
10+
use crate::MirPass;
11+
12+
/// This pass looks for MIR that always copies the same local into the return place and eliminates
13+
/// the copy by renaming all uses of that local to `_0`.
14+
///
15+
/// This allows LLVM to perform an optimization similar to the named return value optimization
16+
/// (NRVO) that is guaranteed in C++. This avoids a stack allocation and `memcpy` for the
17+
/// relatively common pattern of allocating a buffer on the stack, mutating it, and returning it by
18+
/// value like so:
19+
///
20+
/// ```rust
21+
/// fn foo(init: fn(&mut [u8; 1024])) -> [u8; 1024] {
22+
/// let mut buf = [0; 1024];
23+
/// init(&mut buf);
24+
/// buf
25+
/// }
26+
/// ```
27+
///
28+
/// For now, this pass is very simple and only capable of eliminating a single copy. A more general
29+
/// version of copy propagation, such as the one based on non-overlapping live ranges in [#47954] and
30+
/// [#71003], could yield even more benefits.
31+
///
32+
/// [#47954]: https://github.com/rust-lang/rust/pull/47954
33+
/// [#71003]: https://github.com/rust-lang/rust/pull/71003
34+
pub struct RenameReturnPlace;
35+
36+
impl<'tcx> MirPass<'tcx> for RenameReturnPlace {
37+
fn is_enabled(&self, sess: &rustc_session::Session) -> bool {
38+
// unsound: #111005
39+
sess.mir_opt_level() > 0 && sess.opts.unstable_opts.unsound_mir_opts
40+
}
41+
42+
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut mir::Body<'tcx>) {
43+
let def_id = body.source.def_id();
44+
let Some(returned_local) = local_eligible_for_nrvo(body) else {
45+
debug!("`{:?}` was ineligible for NRVO", def_id);
46+
return;
47+
};
48+
49+
if !tcx.consider_optimizing(|| format!("RenameReturnPlace {def_id:?}")) {
50+
return;
51+
}
52+
53+
debug!(
54+
"`{:?}` was eligible for NRVO, making {:?} the return place",
55+
def_id, returned_local
56+
);
57+
58+
RenameToReturnPlace { tcx, to_rename: returned_local }.visit_body_preserves_cfg(body);
59+
60+
// Clean up the `NOP`s we inserted for statements made useless by our renaming.
61+
for block_data in body.basic_blocks.as_mut_preserves_cfg() {
62+
block_data.statements.retain(|stmt| stmt.kind != mir::StatementKind::Nop);
63+
}
64+
65+
// Overwrite the debuginfo of `_0` with that of the renamed local.
66+
let (renamed_decl, ret_decl) =
67+
body.local_decls.pick2_mut(returned_local, mir::RETURN_PLACE);
68+
69+
// Sometimes, the return place is assigned a local of a different but coercible type, for
70+
// example `&mut T` instead of `&T`. Overwriting the `LocalInfo` for the return place means
71+
// its type may no longer match the return type of its function. This doesn't cause a
72+
// problem in codegen because these two types are layout-compatible, but may be unexpected.
73+
debug!("_0: {:?} = {:?}: {:?}", ret_decl.ty, returned_local, renamed_decl.ty);
74+
ret_decl.clone_from(renamed_decl);
75+
76+
// The return place is always mutable.
77+
ret_decl.mutability = Mutability::Mut;
78+
}
79+
}
80+
81+
/// MIR that is eligible for the NRVO must fulfill two conditions:
82+
/// 1. The return place must not be read prior to the `Return` terminator.
83+
/// 2. A simple assignment of a whole local to the return place (e.g., `_0 = _1`) must be the
84+
/// only definition of the return place reaching the `Return` terminator.
85+
///
86+
/// If the MIR fulfills both these conditions, this function returns the `Local` that is assigned
87+
/// to the return place along all possible paths through the control-flow graph.
88+
fn local_eligible_for_nrvo(body: &mir::Body<'_>) -> Option<Local> {
89+
if IsReturnPlaceRead::run(body) {
90+
return None;
91+
}
92+
93+
let mut copied_to_return_place = None;
94+
for block in body.basic_blocks.indices() {
95+
// Look for blocks with a `Return` terminator.
96+
if !matches!(body[block].terminator().kind, mir::TerminatorKind::Return) {
97+
continue;
98+
}
99+
100+
// Look for an assignment of a single local to the return place prior to the `Return`.
101+
let returned_local = find_local_assigned_to_return_place(block, body)?;
102+
match body.local_kind(returned_local) {
103+
// FIXME: Can we do this for arguments as well?
104+
mir::LocalKind::Arg => return None,
105+
106+
mir::LocalKind::ReturnPointer => bug!("Return place was assigned to itself?"),
107+
mir::LocalKind::Temp => {}
108+
}
109+
110+
// If multiple different locals are copied to the return place. We can't pick a
111+
// single one to rename.
112+
if copied_to_return_place.is_some_and(|old| old != returned_local) {
113+
return None;
114+
}
115+
116+
copied_to_return_place = Some(returned_local);
117+
}
118+
119+
copied_to_return_place
120+
}
121+
122+
fn find_local_assigned_to_return_place(start: BasicBlock, body: &mir::Body<'_>) -> Option<Local> {
123+
let mut block = start;
124+
let mut seen = BitSet::new_empty(body.basic_blocks.len());
125+
126+
// Iterate as long as `block` has exactly one predecessor that we have not yet visited.
127+
while seen.insert(block) {
128+
trace!("Looking for assignments to `_0` in {:?}", block);
129+
130+
let local = body[block].statements.iter().rev().find_map(as_local_assigned_to_return_place);
131+
if local.is_some() {
132+
return local;
133+
}
134+
135+
match body.basic_blocks.predecessors()[block].as_slice() {
136+
&[pred] => block = pred,
137+
_ => return None,
138+
}
139+
}
140+
141+
None
142+
}
143+
144+
// If this statement is an assignment of an unprojected local to the return place,
145+
// return that local.
146+
fn as_local_assigned_to_return_place(stmt: &mir::Statement<'_>) -> Option<Local> {
147+
if let mir::StatementKind::Assign(box (lhs, rhs)) = &stmt.kind {
148+
if lhs.as_local() == Some(mir::RETURN_PLACE) {
149+
if let mir::Rvalue::Use(mir::Operand::Copy(rhs) | mir::Operand::Move(rhs)) = rhs {
150+
return rhs.as_local();
151+
}
152+
}
153+
}
154+
155+
None
156+
}
157+
158+
struct RenameToReturnPlace<'tcx> {
159+
to_rename: Local,
160+
tcx: TyCtxt<'tcx>,
161+
}
162+
163+
/// Replaces all uses of `self.to_rename` with `_0`.
164+
impl<'tcx> MutVisitor<'tcx> for RenameToReturnPlace<'tcx> {
165+
fn tcx(&self) -> TyCtxt<'tcx> {
166+
self.tcx
167+
}
168+
169+
fn visit_statement(&mut self, stmt: &mut mir::Statement<'tcx>, loc: Location) {
170+
// Remove assignments of the local being replaced to the return place, since it is now the
171+
// return place:
172+
// _0 = _1
173+
if as_local_assigned_to_return_place(stmt) == Some(self.to_rename) {
174+
stmt.kind = mir::StatementKind::Nop;
175+
return;
176+
}
177+
178+
// Remove storage annotations for the local being replaced:
179+
// StorageLive(_1)
180+
if let mir::StatementKind::StorageLive(local) | mir::StatementKind::StorageDead(local) =
181+
stmt.kind
182+
{
183+
if local == self.to_rename {
184+
stmt.kind = mir::StatementKind::Nop;
185+
return;
186+
}
187+
}
188+
189+
self.super_statement(stmt, loc)
190+
}
191+
192+
fn visit_terminator(&mut self, terminator: &mut mir::Terminator<'tcx>, loc: Location) {
193+
// Ignore the implicit "use" of the return place in a `Return` statement.
194+
if let mir::TerminatorKind::Return = terminator.kind {
195+
return;
196+
}
197+
198+
self.super_terminator(terminator, loc);
199+
}
200+
201+
fn visit_local(&mut self, l: &mut Local, ctxt: PlaceContext, _: Location) {
202+
if *l == mir::RETURN_PLACE {
203+
assert_eq!(ctxt, PlaceContext::NonUse(NonUseContext::VarDebugInfo));
204+
} else if *l == self.to_rename {
205+
*l = mir::RETURN_PLACE;
206+
}
207+
}
208+
}
209+
210+
struct IsReturnPlaceRead(bool);
211+
212+
impl IsReturnPlaceRead {
213+
fn run(body: &mir::Body<'_>) -> bool {
214+
let mut vis = IsReturnPlaceRead(false);
215+
vis.visit_body(body);
216+
vis.0
217+
}
218+
}
219+
220+
impl<'tcx> Visitor<'tcx> for IsReturnPlaceRead {
221+
fn visit_local(&mut self, l: Local, ctxt: PlaceContext, _: Location) {
222+
if l == mir::RETURN_PLACE && ctxt.is_use() && !ctxt.is_place_assignment() {
223+
self.0 = true;
224+
}
225+
}
226+
227+
fn visit_terminator(&mut self, terminator: &mir::Terminator<'tcx>, loc: Location) {
228+
// Ignore the implicit "use" of the return place in a `Return` statement.
229+
if let mir::TerminatorKind::Return = terminator.kind {
230+
return;
231+
}
232+
233+
self.super_terminator(terminator, loc);
234+
}
235+
}

tests/mir-opt/dest-prop/union.main.DestinationPropagation.panic-abort.diff

+1-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
bb0: {
1919
StorageLive(_1);
2020
StorageLive(_2);
21-
nop;
21+
_2 = const 1_u32;
2222
_1 = Un { us: const 1_u32 };
2323
StorageDead(_2);
2424
StorageLive(_3);

tests/mir-opt/dest-prop/union.main.DestinationPropagation.panic-unwind.diff

+1-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
bb0: {
1919
StorageLive(_1);
2020
StorageLive(_2);
21-
nop;
21+
_2 = const 1_u32;
2222
_1 = Un { us: const 1_u32 };
2323
StorageDead(_2);
2424
StorageLive(_3);

tests/mir-opt/dest-prop/nrvo_miscompile_111005.rs renamed to tests/mir-opt/nrvo_miscompile_111005.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
// skip-filecheck
22
// This is a miscompilation, #111005 to track
33

4-
//@ test-mir-pass: DestinationPropagation
4+
//@ test-mir-pass: RenameReturnPlace
55

66
#![feature(custom_mir, core_intrinsics)]
77
extern crate core;
88
use core::intrinsics::mir::*;
99

10-
// EMIT_MIR nrvo_miscompile_111005.wrong.DestinationPropagation.diff
10+
// EMIT_MIR nrvo_miscompile_111005.wrong.RenameReturnPlace.diff
1111
#[custom_mir(dialect = "runtime", phase = "initial")]
1212
pub fn wrong(arg: char) -> char {
1313
mir!({

tests/mir-opt/dest-prop/nrvo_miscompile_111005.wrong.DestinationPropagation.diff renamed to tests/mir-opt/nrvo_miscompile_111005.wrong.RenameReturnPlace.diff

+3-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
- // MIR for `wrong` before DestinationPropagation
2-
+ // MIR for `wrong` after DestinationPropagation
1+
- // MIR for `wrong` before RenameReturnPlace
2+
+ // MIR for `wrong` after RenameReturnPlace
33

44
fn wrong(_1: char) -> char {
55
let mut _0: char;
@@ -9,9 +9,8 @@
99
- _2 = _1;
1010
- _0 = _2;
1111
- _2 = const 'b';
12-
+ nop;
1312
+ _0 = _1;
14-
+ _1 = const 'b';
13+
+ _0 = const 'b';
1514
return;
1615
}
1716
}

tests/mir-opt/dest-prop/nrvo_borrowed.nrvo.DestinationPropagation.panic-abort.diff renamed to tests/mir-opt/nrvo_simple.nrvo.RenameReturnPlace.panic-abort.diff

+15-16
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
- // MIR for `nrvo` before DestinationPropagation
2-
+ // MIR for `nrvo` after DestinationPropagation
1+
- // MIR for `nrvo` before RenameReturnPlace
2+
+ // MIR for `nrvo` after RenameReturnPlace
33

44
fn nrvo(_1: for<'a> fn(&'a mut [u8; 1024])) -> [u8; 1024] {
55
debug init => _1;
@@ -10,33 +10,32 @@
1010
let mut _5: &mut [u8; 1024];
1111
let mut _6: &mut [u8; 1024];
1212
scope 1 {
13-
debug buf => _2;
13+
- debug buf => _2;
14+
+ debug buf => _0;
1415
}
1516

1617
bb0: {
17-
StorageLive(_2);
18-
_2 = [const 0_u8; 1024];
18+
- StorageLive(_2);
19+
- _2 = [const 0_u8; 1024];
20+
+ _0 = [const 0_u8; 1024];
1921
StorageLive(_3);
20-
- StorageLive(_4);
21-
- _4 = _1;
22-
+ nop;
23-
+ nop;
22+
StorageLive(_4);
23+
_4 = _1;
2424
StorageLive(_5);
2525
StorageLive(_6);
26-
_6 = &mut _2;
26+
- _6 = &mut _2;
27+
+ _6 = &mut _0;
2728
_5 = &mut (*_6);
28-
- _3 = move _4(move _5) -> [return: bb1, unwind unreachable];
29-
+ _3 = move _1(move _5) -> [return: bb1, unwind unreachable];
29+
_3 = move _4(move _5) -> [return: bb1, unwind unreachable];
3030
}
3131

3232
bb1: {
3333
StorageDead(_5);
34-
- StorageDead(_4);
35-
+ nop;
34+
StorageDead(_4);
3635
StorageDead(_6);
3736
StorageDead(_3);
38-
_0 = _2;
39-
StorageDead(_2);
37+
- _0 = _2;
38+
- StorageDead(_2);
4039
return;
4140
}
4241
}

0 commit comments

Comments
 (0)