Skip to content

Commit f135989

Browse files
make sure that retagging body arguments stays the first instruction
1 parent 7cc2dd8 commit f135989

6 files changed

+125
-77
lines changed

compiler/rustc_mir_transform/src/coroutine/relocate_upvars.rs

Lines changed: 44 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@
102102
103103
use std::borrow::Cow;
104104

105+
use itertools::Itertools;
105106
use rustc_abi::FieldIdx;
106107
use rustc_index::bit_set::DenseBitSet;
107108
use rustc_index::{IndexSlice, IndexVec};
@@ -261,6 +262,28 @@ impl RelocateUpvars {
261262
| ty::InstanceKind::FnPtrAddrShim(..) => unreachable!(),
262263
};
263264

265+
// HACK: in case `AddRetag` is already run, we have one `Retag` at the body entrance
266+
// so we need to make sure that first `Retag` is run.
267+
let last_retag = body.basic_blocks[START_BLOCK]
268+
.statements
269+
.iter()
270+
.find_position(|stmt| !matches!(stmt.kind, StatementKind::Retag(_, _)));
271+
let retagged_start_block = if let Some((index, _)) = last_retag {
272+
let span = body.span;
273+
let bbs = body.basic_blocks_mut();
274+
let stmts = bbs[START_BLOCK].statements.split_off(index);
275+
let terminator = bbs[START_BLOCK].terminator.take();
276+
let split_start_block = bbs.push(BasicBlockData::new_stmts(stmts, terminator, false));
277+
bbs[START_BLOCK].statements.shrink_to_fit();
278+
bbs[START_BLOCK].terminator = Some(Terminator {
279+
kind: TerminatorKind::Goto { target: split_start_block },
280+
source_info: SourceInfo::outermost(span),
281+
});
282+
Some(split_start_block)
283+
} else {
284+
None
285+
};
286+
264287
if let Some(mir_dumper) = MirDumper::new(tcx, "RelocateUpvars", body) {
265288
mir_dumper.set_disambiguator(&"before").dump_mir(body);
266289
}
@@ -308,7 +331,7 @@ impl RelocateUpvars {
308331
SubstituteUpvarVisitor { tcx, mappings: &substitution_mapping }.visit_body(body);
309332

310333
rewrite_drop_coroutine_struct(tcx, body, &substitution_mapping);
311-
insert_substitution_prologue(body, &substitution_mapping);
334+
insert_substitution_prologue(body, retagged_start_block, &substitution_mapping);
312335
patch_missing_storage_deads(tcx, body, &substitution_mapping);
313336
hydrate_var_debug_info(body, &substitution_mapping);
314337
if let Some(mir_dumper) = MirDumper::new(tcx, "RelocateUpvars", body) {
@@ -445,10 +468,11 @@ fn rewrite_drop_coroutine_struct<'tcx>(
445468

446469
fn insert_substitution_prologue<'tcx>(
447470
body: &mut Body<'tcx>,
471+
retagged_start_block: Option<BasicBlock>,
448472
substitution_mapping: &IndexSlice<FieldIdx, UpvarSubstitution<'tcx>>,
449473
) {
450474
let mut patch = MirPatch::new(body);
451-
let mut stmts = Vec::with_capacity(2 * substitution_mapping.len());
475+
let mut stmts = Vec::with_capacity(5 * substitution_mapping.len());
452476
for &UpvarSubstitution { local, reloc, upvar_place, span, name: _ } in substitution_mapping {
453477
// For each upvar-local _$i
454478
let source_info = SourceInfo::outermost(span);
@@ -481,22 +505,30 @@ fn insert_substitution_prologue<'tcx>(
481505
let source_info = SourceInfo::outermost(body.span);
482506
let prologue = patch.new_block(BasicBlockData::new_stmts(
483507
stmts,
484-
Some(Terminator { source_info, kind: TerminatorKind::Goto { target: START_BLOCK } }),
508+
Some(Terminator {
509+
source_info,
510+
kind: TerminatorKind::Goto { target: retagged_start_block.unwrap_or(START_BLOCK) },
511+
}),
485512
false,
486513
));
487514
patch.apply(body);
488515

489516
// Manually patch so that prologue is the new entry-point
490-
let preds = body.basic_blocks.predecessors()[START_BLOCK].clone();
491-
let basic_blocks = body.basic_blocks.as_mut();
492-
for pred in preds {
493-
basic_blocks[pred].terminator_mut().successors_mut(|target| {
494-
if *target == START_BLOCK {
495-
*target = prologue;
496-
}
497-
});
517+
if retagged_start_block.is_some() {
518+
let basic_blocks = body.basic_blocks.as_mut();
519+
basic_blocks[START_BLOCK].terminator_mut().successors_mut(|target| *target = prologue);
520+
} else {
521+
let preds = body.basic_blocks.predecessors()[START_BLOCK].clone();
522+
let basic_blocks = body.basic_blocks.as_mut();
523+
for pred in preds {
524+
basic_blocks[pred].terminator_mut().successors_mut(|target| {
525+
if *target == START_BLOCK {
526+
*target = prologue;
527+
}
528+
});
529+
}
530+
basic_blocks.swap(START_BLOCK, prologue);
498531
}
499-
basic_blocks.swap(START_BLOCK, prologue);
500532
}
501533

502534
/// Occasionally there are upvar locals left without `StorageDead` because

tests/mir-opt/coroutine_relocate_upvars.main-{closure#0}.RelocateUpvars.after.panic-abort.mir

Lines changed: 29 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,7 @@ yields ()
1616
let _10: &mut std::string::String;
1717

1818
bb0: {
19-
StorageLive(_9);
20-
StorageLive(_10);
21-
_10 = move (_1.0: &mut std::string::String);
22-
_9 = move _10;
23-
StorageDead(_10);
24-
goto -> bb18;
19+
goto -> bb19;
2520
}
2621

2722
bb1: {
@@ -47,7 +42,7 @@ yields ()
4742
StorageDead(_5);
4843
StorageDead(_4);
4944
_0 = const ();
50-
goto -> bb13;
45+
goto -> bb14;
5146
}
5247

5348
bb5: {
@@ -61,55 +56,64 @@ yields ()
6156
}
6257

6358
bb7: {
64-
goto -> bb17;
59+
goto -> bb18;
6560
}
6661

6762
bb8: {
6863
coroutine_drop;
6964
}
7065

71-
bb9 (cleanup): {
72-
unreachable;
66+
bb9: {
67+
StorageLive(_3);
68+
_3 = String::new() -> [return: bb1, unwind unreachable];
7369
}
7470

7571
bb10 (cleanup): {
76-
StorageDead(_9);
77-
goto -> bb9;
72+
unreachable;
7873
}
7974

8075
bb11 (cleanup): {
76+
StorageDead(_9);
8177
goto -> bb10;
8278
}
8379

84-
bb12: {
80+
bb12 (cleanup): {
81+
goto -> bb11;
82+
}
83+
84+
bb13: {
8585
StorageDead(_9);
8686
goto -> bb5;
8787
}
8888

89-
bb13: {
90-
goto -> bb12;
89+
bb14: {
90+
goto -> bb13;
9191
}
9292

93-
bb14 (cleanup): {
93+
bb15 (cleanup): {
9494
StorageDead(_9);
95-
goto -> bb9;
95+
goto -> bb10;
9696
}
9797

98-
bb15 (cleanup): {
99-
goto -> bb14;
98+
bb16 (cleanup): {
99+
goto -> bb15;
100100
}
101101

102-
bb16: {
102+
bb17: {
103103
StorageDead(_9);
104104
goto -> bb8;
105105
}
106106

107-
bb17: {
108-
goto -> bb16;
107+
bb18: {
108+
goto -> bb17;
109109
}
110110

111-
bb18: {
112-
StorageLive(_3);
113-
_3 = String::new() -> [return: bb1, unwind unreachable];
111+
bb19: {
112+
StorageLive(_9);
113+
StorageLive(_10);
114+
_10 = move (_1.0: &mut std::string::String);
115+
_9 = move _10;
116+
StorageDead(_10);
117+
goto -> bb9;
114118
}
115119
}

tests/mir-opt/coroutine_relocate_upvars.main-{closure#0}.RelocateUpvars.after.panic-unwind.mir

Lines changed: 32 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,7 @@ yields ()
1616
let _10: &mut std::string::String;
1717

1818
bb0: {
19-
StorageLive(_9);
20-
StorageLive(_10);
21-
_10 = move (_1.0: &mut std::string::String);
22-
_9 = move _10;
23-
StorageDead(_10);
24-
goto -> bb23;
19+
goto -> bb24;
2520
}
2621

2722
bb1: {
@@ -53,7 +48,7 @@ yields ()
5348
StorageDead(_5);
5449
StorageDead(_4);
5550
_0 = const ();
56-
goto -> bb15;
51+
goto -> bb16;
5752
}
5853

5954
bb6: {
@@ -67,7 +62,7 @@ yields ()
6762
}
6863

6964
bb8: {
70-
goto -> bb19;
65+
goto -> bb20;
7166
}
7267

7368
bb9: {
@@ -76,64 +71,73 @@ yields ()
7671

7772
bb10 (cleanup): {
7873
StorageDead(_3);
79-
goto -> bb22;
74+
goto -> bb23;
8075
}
8176

8277
bb11 (cleanup): {
8378
resume;
8479
}
8580

86-
bb12 (cleanup): {
81+
bb12: {
82+
StorageLive(_3);
83+
_3 = String::new() -> [return: bb1, unwind: bb10];
84+
}
85+
86+
bb13 (cleanup): {
8787
StorageDead(_9);
8888
goto -> bb11;
8989
}
9090

91-
bb13 (cleanup): {
92-
goto -> bb12;
91+
bb14 (cleanup): {
92+
goto -> bb13;
9393
}
9494

95-
bb14: {
95+
bb15: {
9696
StorageDead(_9);
9797
goto -> bb6;
9898
}
9999

100-
bb15: {
101-
goto -> bb14;
100+
bb16: {
101+
goto -> bb15;
102102
}
103103

104-
bb16 (cleanup): {
104+
bb17 (cleanup): {
105105
StorageDead(_9);
106106
goto -> bb11;
107107
}
108108

109-
bb17 (cleanup): {
110-
goto -> bb16;
109+
bb18 (cleanup): {
110+
goto -> bb17;
111111
}
112112

113-
bb18: {
113+
bb19: {
114114
StorageDead(_9);
115115
goto -> bb9;
116116
}
117117

118-
bb19: {
119-
goto -> bb18;
118+
bb20: {
119+
goto -> bb19;
120120
}
121121

122-
bb20 (cleanup): {
122+
bb21 (cleanup): {
123123
terminate(cleanup);
124124
}
125125

126-
bb21 (cleanup): {
126+
bb22 (cleanup): {
127127
StorageDead(_9);
128128
goto -> bb11;
129129
}
130130

131-
bb22 (cleanup): {
132-
goto -> bb21;
131+
bb23 (cleanup): {
132+
goto -> bb22;
133133
}
134134

135-
bb23: {
136-
StorageLive(_3);
137-
_3 = String::new() -> [return: bb1, unwind: bb10];
135+
bb24: {
136+
StorageLive(_9);
137+
StorageLive(_10);
138+
_10 = move (_1.0: &mut std::string::String);
139+
_9 = move _10;
140+
StorageDead(_10);
141+
goto -> bb12;
138142
}
139143
}

tests/mir-opt/coroutine_relocate_upvars.main-{closure#0}.RelocateUpvars.before.panic-abort.mir

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,7 @@ yields ()
1313
let mut _8: &mut std::string::String;
1414

1515
bb0: {
16-
StorageLive(_3);
17-
_3 = String::new() -> [return: bb1, unwind unreachable];
16+
goto -> bb9;
1817
}
1918

2019
bb1: {
@@ -60,4 +59,9 @@ yields ()
6059
bb8: {
6160
coroutine_drop;
6261
}
62+
63+
bb9: {
64+
StorageLive(_3);
65+
_3 = String::new() -> [return: bb1, unwind unreachable];
66+
}
6367
}

tests/mir-opt/coroutine_relocate_upvars.main-{closure#0}.RelocateUpvars.before.panic-unwind.mir

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,7 @@ yields ()
1313
let mut _8: &mut std::string::String;
1414

1515
bb0: {
16-
StorageLive(_3);
17-
_3 = String::new() -> [return: bb1, unwind: bb10];
16+
goto -> bb12;
1817
}
1918

2019
bb1: {
@@ -75,4 +74,9 @@ yields ()
7574
bb11 (cleanup): {
7675
resume;
7776
}
77+
78+
bb12: {
79+
StorageLive(_3);
80+
_3 = String::new() -> [return: bb1, unwind: bb10];
81+
}
7882
}

0 commit comments

Comments
 (0)