Skip to content

Commit 0769865

Browse files
committed
rewrite scope drop to be iterative
while I'm at it, remove the "extra caching" that I was doing for no good reason except laziness. Basically before I was caching at each scope in the chain, but there's not really a reason to do that, since the cached entry point at level N is always equal to the last cached exit point from level N-1.
1 parent a276e75 commit 0769865

File tree

2 files changed

+39
-32
lines changed

2 files changed

+39
-32
lines changed

src/librustc_mir/build/mod.rs

+5
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ pub struct Builder<'a, 'tcx: 'a> {
4747
var_indices: FnvHashMap<ast::NodeId, u32>,
4848
temp_decls: Vec<TempDecl<'tcx>>,
4949
unit_temp: Option<Lvalue<'tcx>>,
50+
51+
// cached block with a RESUME terminator; we create this at the
52+
// first panic
53+
cached_resume_block: Option<BasicBlock>,
5054
}
5155

5256
struct CFG<'tcx> {
@@ -175,6 +179,7 @@ pub fn construct<'a,'tcx>(hir: Cx<'a,'tcx>,
175179
var_decls: vec![],
176180
var_indices: FnvHashMap(),
177181
unit_temp: None,
182+
cached_resume_block: None,
178183
};
179184

180185
assert_eq!(builder.cfg.start_new_block(), START_BLOCK);

src/librustc_mir/build/scope.rs

+34-32
Original file line numberDiff line numberDiff line change
@@ -456,21 +456,41 @@ impl<'a,'tcx> Builder<'a,'tcx> {
456456
/// See module comment for more details. None indicates there’s no
457457
/// cleanup to do at this point.
458458
pub fn diverge_cleanup(&mut self) -> Option<BasicBlock> {
459-
if self.scopes.is_empty() {
459+
if self.scopes.iter().all(|scope| scope.drops.is_empty() && scope.free.is_none()) {
460460
return None;
461461
}
462+
assert!(!self.scopes.is_empty()); // or `all` above would be true
463+
462464
let unit_temp = self.get_unit_temp();
463-
let Builder { ref mut hir, ref mut cfg, ref mut scopes, .. } = *self;
464-
465-
// Given an array of scopes, we generate these from the outermost scope to the innermost
466-
// one. Thus for array [S0, S1, S2] with corresponding cleanup blocks [B0, B1, B2], we will
467-
// generate B0 <- B1 <- B2 in left-to-right order. Control flow of the generated blocks
468-
// always ends up at a block with the Resume terminator.
469-
if scopes.iter().any(|scope| !scope.drops.is_empty() || scope.free.is_some()) {
470-
Some(build_diverge_scope(hir.tcx(), self.fn_span, cfg, &unit_temp, scopes))
465+
let Builder { ref mut hir, ref mut cfg, ref mut scopes,
466+
ref mut cached_resume_block, .. } = *self;
467+
468+
// Build up the drops in **reverse** order. The end result will
469+
// look like:
470+
//
471+
// scopes[n] -> scopes[n-1] -> ... -> scopes[0]
472+
//
473+
// However, we build this in **reverse order**. That is, we
474+
// process scopes[0], then scopes[1], etc, pointing each one at
475+
// the result generates from the one before. Along the way, we
476+
// store caches. If everything is cached, we'll just walk right
477+
// to left reading the cached results but never created anything.
478+
479+
// To start, create the resume terminator.
480+
let mut target = if let Some(target) = *cached_resume_block {
481+
target
471482
} else {
472-
None
483+
let resumeblk = cfg.start_new_cleanup_block();
484+
cfg.terminate(resumeblk, scopes[0].id, self.fn_span, TerminatorKind::Resume);
485+
*cached_resume_block = Some(resumeblk);
486+
resumeblk
487+
};
488+
489+
for scope in scopes {
490+
target = build_diverge_scope(hir.tcx(), cfg, &unit_temp, scope, target);
473491
}
492+
493+
Some(target)
474494
}
475495

476496
/// Utility function for *non*-scope code to build their own drops
@@ -640,43 +660,25 @@ fn build_scope_drops<'tcx>(cfg: &mut CFG<'tcx>,
640660
}
641661

642662
fn build_diverge_scope<'tcx>(tcx: &TyCtxt<'tcx>,
643-
fn_span: Span,
644663
cfg: &mut CFG<'tcx>,
645664
unit_temp: &Lvalue<'tcx>,
646-
scopes: &mut [Scope<'tcx>])
665+
scope: &mut Scope<'tcx>,
666+
mut target: BasicBlock)
647667
-> BasicBlock
648668
{
649-
assert!(scopes.len() >= 1);
650-
651669
// Build up the drops in **reverse** order. The end result will
652670
// look like:
653671
//
654-
// [drops[n]] -...-> [drops[0]] -> [Free] -> [scopes[..n-1]]
672+
// [drops[n]] -...-> [drops[0]] -> [Free] -> [target]
655673
// | |
656674
// +------------------------------------+
657-
// code for scopes[n]
675+
// code for scope
658676
//
659677
// The code in this function reads from right to left. At each
660678
// point, we check for cached blocks representing the
661679
// remainder. If everything is cached, we'll just walk right to
662680
// left reading the cached results but never created anything.
663681

664-
// To start, translate scopes[1..].
665-
let (scope, earlier_scopes) = scopes.split_last_mut().unwrap();
666-
let mut target = if let Some(cached_block) = scope.cached_block {
667-
cached_block
668-
} else if earlier_scopes.is_empty() {
669-
// Diverging from the root scope creates a RESUME terminator.
670-
// FIXME what span to use here?
671-
let resumeblk = cfg.start_new_cleanup_block();
672-
cfg.terminate(resumeblk, scope.id, fn_span, TerminatorKind::Resume);
673-
resumeblk
674-
} else {
675-
// Diverging from any other scope chains up to the previous scope.
676-
build_diverge_scope(tcx, fn_span, cfg, unit_temp, earlier_scopes)
677-
};
678-
scope.cached_block = Some(target);
679-
680682
// Next, build up any free.
681683
if let Some(ref mut free_data) = scope.free {
682684
target = if let Some(cached_block) = free_data.cached_block {

0 commit comments

Comments
 (0)