Skip to content

Commit d7df5bd

Browse files
committed
Auto merge of rust-lang#140464 - oli-obk:successors-mut-perf, r=petrochenkov
Use a closure instead of three chained iterators Fixes the perf regression from rust-lang#123948 That PR had chained a third option to the iterator which apparently didn't optimize well
2 parents 5fe04cb + 9193dfe commit d7df5bd

File tree

7 files changed

+61
-76
lines changed

7 files changed

+61
-76
lines changed

compiler/rustc_middle/src/mir/terminator.rs

+48-55
Original file line numberDiff line numberDiff line change
@@ -437,8 +437,8 @@ impl<'tcx> Terminator<'tcx> {
437437
}
438438

439439
#[inline]
440-
pub fn successors_mut(&mut self) -> SuccessorsMut<'_> {
441-
self.kind.successors_mut()
440+
pub fn successors_mut<'a>(&'a mut self, f: impl FnMut(&'a mut BasicBlock)) {
441+
self.kind.successors_mut(f)
442442
}
443443

444444
#[inline]
@@ -486,7 +486,6 @@ pub use helper::*;
486486
mod helper {
487487
use super::*;
488488
pub type Successors<'a> = impl DoubleEndedIterator<Item = BasicBlock> + 'a;
489-
pub type SuccessorsMut<'a> = impl DoubleEndedIterator<Item = &'a mut BasicBlock> + 'a;
490489

491490
impl SwitchTargets {
492491
/// Like [`SwitchTargets::target_for_value`], but returning the same type as
@@ -560,69 +559,63 @@ mod helper {
560559
}
561560

562561
#[inline]
563-
#[define_opaque(SuccessorsMut)]
564-
pub fn successors_mut(&mut self) -> SuccessorsMut<'_> {
562+
pub fn successors_mut<'a>(&'a mut self, mut f: impl FnMut(&'a mut BasicBlock)) {
565563
use self::TerminatorKind::*;
566-
match *self {
567-
// 3-successors for async drop: target, unwind, dropline (parent coroutine drop)
568-
Drop {
569-
target: ref mut t,
570-
unwind: UnwindAction::Cleanup(ref mut u),
571-
drop: Some(ref mut d),
572-
..
573-
} => slice::from_mut(t).into_iter().chain(Some(u).into_iter().chain(Some(d))),
574-
// 2-successors
575-
Call {
576-
target: Some(ref mut t), unwind: UnwindAction::Cleanup(ref mut u), ..
564+
match self {
565+
Drop { target, unwind, drop, .. } => {
566+
f(target);
567+
if let UnwindAction::Cleanup(u) = unwind {
568+
f(u)
569+
}
570+
if let Some(d) = drop {
571+
f(d)
572+
}
573+
}
574+
Call { target, unwind, .. } => {
575+
if let Some(target) = target {
576+
f(target);
577+
}
578+
if let UnwindAction::Cleanup(u) = unwind {
579+
f(u)
580+
}
577581
}
578-
| Yield { resume: ref mut t, drop: Some(ref mut u), .. }
579-
| Drop {
580-
target: ref mut t,
581-
unwind: UnwindAction::Cleanup(ref mut u),
582-
drop: None,
583-
..
582+
Yield { resume, drop, .. } => {
583+
f(resume);
584+
if let Some(d) = drop {
585+
f(d)
586+
}
584587
}
585-
| Drop { target: ref mut t, unwind: _, drop: Some(ref mut u), .. }
586-
| Assert { target: ref mut t, unwind: UnwindAction::Cleanup(ref mut u), .. }
587-
| FalseUnwind {
588-
real_target: ref mut t,
589-
unwind: UnwindAction::Cleanup(ref mut u),
590-
} => slice::from_mut(t).into_iter().chain(Some(u).into_iter().chain(None)),
591-
// single successor
592-
Goto { target: ref mut t }
593-
| Call { target: None, unwind: UnwindAction::Cleanup(ref mut t), .. }
594-
| Call { target: Some(ref mut t), unwind: _, .. }
595-
| Yield { resume: ref mut t, drop: None, .. }
596-
| Drop { target: ref mut t, unwind: _, .. }
597-
| Assert { target: ref mut t, unwind: _, .. }
598-
| FalseUnwind { real_target: ref mut t, unwind: _ } => {
599-
slice::from_mut(t).into_iter().chain(None.into_iter().chain(None))
588+
Assert { target, unwind, .. } | FalseUnwind { real_target: target, unwind } => {
589+
f(target);
590+
if let UnwindAction::Cleanup(u) = unwind {
591+
f(u)
592+
}
593+
}
594+
Goto { target } => {
595+
f(target);
600596
}
601-
// No successors
602597
UnwindResume
603598
| UnwindTerminate(_)
604599
| CoroutineDrop
605600
| Return
606601
| Unreachable
607-
| TailCall { .. }
608-
| Call { target: None, unwind: _, .. } => {
609-
(&mut []).into_iter().chain(None.into_iter().chain(None))
610-
}
611-
// Multiple successors
612-
InlineAsm { ref mut targets, unwind: UnwindAction::Cleanup(ref mut u), .. } => {
613-
targets.iter_mut().chain(Some(u).into_iter().chain(None))
614-
}
615-
InlineAsm { ref mut targets, unwind: _, .. } => {
616-
targets.iter_mut().chain(None.into_iter().chain(None))
602+
| TailCall { .. } => {}
603+
InlineAsm { targets, unwind, .. } => {
604+
for target in targets {
605+
f(target);
606+
}
607+
if let UnwindAction::Cleanup(u) = unwind {
608+
f(u)
609+
}
617610
}
618-
SwitchInt { ref mut targets, .. } => {
619-
targets.targets.iter_mut().chain(None.into_iter().chain(None))
611+
SwitchInt { targets, .. } => {
612+
for target in &mut targets.targets {
613+
f(target);
614+
}
620615
}
621-
// FalseEdge
622-
FalseEdge { ref mut real_target, ref mut imaginary_target } => {
623-
slice::from_mut(real_target)
624-
.into_iter()
625-
.chain(Some(imaginary_target).into_iter().chain(None))
616+
FalseEdge { real_target, imaginary_target } => {
617+
f(real_target);
618+
f(imaginary_target);
626619
}
627620
}
628621
}

compiler/rustc_mir_transform/src/coroutine.rs

+2-4
Original file line numberDiff line numberDiff line change
@@ -1064,10 +1064,8 @@ fn insert_switch<'tcx>(
10641064
},
10651065
);
10661066

1067-
let blocks = body.basic_blocks_mut().iter_mut();
1068-
1069-
for target in blocks.flat_map(|b| b.terminator_mut().successors_mut()) {
1070-
*target += 1;
1067+
for b in body.basic_blocks_mut().iter_mut() {
1068+
b.terminator_mut().successors_mut(|target| *target += 1);
10711069
}
10721070
}
10731071

compiler/rustc_mir_transform/src/jump_threading.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -757,12 +757,12 @@ impl OpportunitySet {
757757

758758
// Replace `succ` by `new_succ` where it appears.
759759
let mut num_edges = 0;
760-
for s in basic_blocks[current].terminator_mut().successors_mut() {
760+
basic_blocks[current].terminator_mut().successors_mut(|s| {
761761
if *s == succ {
762762
*s = new_succ;
763763
num_edges += 1;
764764
}
765-
}
765+
});
766766

767767
// Update predecessors with the new block.
768768
let _new_succ = self.predecessors.push(num_edges);

compiler/rustc_mir_transform/src/prettify.rs

+1-3
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,7 @@ impl<'tcx> MutVisitor<'tcx> for BasicBlockUpdater<'tcx> {
115115
}
116116

117117
fn visit_terminator(&mut self, terminator: &mut Terminator<'tcx>, _location: Location) {
118-
for succ in terminator.successors_mut() {
119-
*succ = self.map[*succ];
120-
}
118+
terminator.successors_mut(|succ| *succ = self.map[*succ]);
121119
}
122120
}
123121

compiler/rustc_mir_transform/src/remove_noop_landing_pads.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,13 @@ impl<'tcx> crate::MirPass<'tcx> for RemoveNoopLandingPads {
5858
}
5959
}
6060

61-
for target in body[bb].terminator_mut().successors_mut() {
61+
body[bb].terminator_mut().successors_mut(|target| {
6262
if *target != resume_block && nop_landing_pads.contains(*target) {
6363
debug!(" folding noop jump to {:?} to resume block", target);
6464
*target = resume_block;
6565
jumps_folded += 1;
6666
}
67-
}
67+
});
6868

6969
let is_nop_landing_pad = self.is_nop_landing_pad(bb, body, &nop_landing_pads);
7070
if is_nop_landing_pad {

compiler/rustc_mir_transform/src/shim.rs

+3-4
Original file line numberDiff line numberDiff line change
@@ -298,10 +298,9 @@ fn local_decls_for_sig<'tcx>(
298298
fn dropee_emit_retag<'tcx>(
299299
tcx: TyCtxt<'tcx>,
300300
body: &mut Body<'tcx>,
301-
dropee_ptr: Place<'tcx>,
301+
mut dropee_ptr: Place<'tcx>,
302302
span: Span,
303303
) -> Place<'tcx> {
304-
let mut dropee_ptr = dropee_ptr;
305304
if tcx.sess.opts.unstable_opts.mir_emit_retag {
306305
let source_info = SourceInfo::outermost(span);
307306
// We want to treat the function argument as if it was passed by `&mut`. As such, we
@@ -365,8 +364,8 @@ fn build_drop_shim<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, ty: Option<Ty<'tcx>>)
365364
new_body(source, blocks, local_decls_for_sig(&sig, span), sig.inputs().len(), span);
366365

367366
// The first argument (index 0), but add 1 for the return value.
368-
let mut dropee_ptr = Place::from(Local::new(1 + 0));
369-
dropee_ptr = dropee_emit_retag(tcx, &mut body, dropee_ptr, span);
367+
let dropee_ptr = Place::from(Local::new(1 + 0));
368+
let dropee_ptr = dropee_emit_retag(tcx, &mut body, dropee_ptr, span);
370369

371370
if ty.is_some() {
372371
let patch = {

compiler/rustc_mir_transform/src/simplify.rs

+3-6
Original file line numberDiff line numberDiff line change
@@ -147,9 +147,8 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> {
147147
let mut terminator =
148148
self.basic_blocks[bb].terminator.take().expect("invalid terminator state");
149149

150-
for successor in terminator.successors_mut() {
151-
self.collapse_goto_chain(successor, &mut changed);
152-
}
150+
terminator
151+
.successors_mut(|successor| self.collapse_goto_chain(successor, &mut changed));
153152

154153
let mut inner_changed = true;
155154
merged_blocks.clear();
@@ -375,9 +374,7 @@ pub(super) fn remove_dead_blocks(body: &mut Body<'_>) {
375374
}
376375

377376
for block in basic_blocks {
378-
for target in block.terminator_mut().successors_mut() {
379-
*target = replacements[target.index()];
380-
}
377+
block.terminator_mut().successors_mut(|target| *target = replacements[target.index()]);
381378
}
382379
}
383380

0 commit comments

Comments
 (0)