From: Niko Matsakis Date: Wed, 23 Mar 2016 00:39:29 +0000 (-0400) Subject: rewrite drop code X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=cb04e495dc7a34cc708246dad7bb8b844216fd3e;p=rust.git rewrite drop code This was triggered by me wanting to address a use of DUMMY_SP, but actually I'm not sure what would be a better span -- I guess the span for the function as a whole. --- diff --git a/src/librustc_mir/build/expr/into.rs b/src/librustc_mir/build/expr/into.rs index 9992e904dea..4c9e2c0c5fa 100644 --- a/src/librustc_mir/build/expr/into.rs +++ b/src/librustc_mir/build/expr/into.rs @@ -256,7 +256,8 @@ pub fn into_expr(&mut self, block = match value { Some(value) => unpack!(this.into(&Lvalue::ReturnPointer, block, value)), None => { - this.cfg.push_assign_unit(block, scope_id, expr_span, &Lvalue::ReturnPointer); + this.cfg.push_assign_unit(block, scope_id, + expr_span, &Lvalue::ReturnPointer); block } }; diff --git a/src/librustc_mir/build/matches/test.rs b/src/librustc_mir/build/matches/test.rs index 3e374c07b46..3211e5849a0 100644 --- a/src/librustc_mir/build/matches/test.rs +++ b/src/librustc_mir/build/matches/test.rs @@ -272,7 +272,8 @@ pub fn perform_test(&mut self, let (actual, result) = (self.temp(usize_ty), self.temp(bool_ty)); // actual = len(lvalue) - self.cfg.push_assign(block, scope_id, test.span, &actual, Rvalue::Len(lvalue.clone())); + self.cfg.push_assign(block, scope_id, test.span, + &actual, Rvalue::Len(lvalue.clone())); // expected = let expected = self.push_usize(block, scope_id, test.span, len); diff --git a/src/librustc_mir/build/scope.rs b/src/librustc_mir/build/scope.rs index e7066ff083e..5aeaef06b89 100644 --- a/src/librustc_mir/build/scope.rs +++ b/src/librustc_mir/build/scope.rs @@ -113,12 +113,20 @@ pub struct Scope<'tcx> { // This is expected to go away once `box EXPR` becomes a sugar for placement protocol and gets // desugared in some earlier stage. free: Option>, + + /// The cached block for the cleanups-on-diverge path. This block + /// contains a block that will just do a RESUME to an appropriate + /// place. This block does not execute any of the drops or free: + /// each of those has their own cached-blocks, which will branch + /// to this point. + cached_block: Option } struct DropData<'tcx> { span: Span, value: Lvalue<'tcx>, // NB: per-drop “cache” is necessary for the build_scope_drops function below. + /// The cached block for the cleanups-on-diverge path. This block contains code to run the /// current drop and all the preceding drops (i.e. those having lower index in Drop’s /// Scope drop array) @@ -127,10 +135,13 @@ struct DropData<'tcx> { struct FreeData<'tcx> { span: Span, + /// Lvalue containing the allocated box. value: Lvalue<'tcx>, + /// type of item for which the box was allocated for (i.e. the T in Box). item_ty: Ty<'tcx>, + /// The cached block containing code to run the free. The block will also execute all the drops /// in the scope. cached_block: Option @@ -155,6 +166,7 @@ impl<'tcx> Scope<'tcx> { /// Should always be run for all inner scopes when a drop is pushed into some scope enclosing a /// larger extent of code. fn invalidate_cache(&mut self) { + self.cached_block = None; for dropdata in &mut self.drops { dropdata.cached_block = None; } @@ -234,7 +246,8 @@ pub fn push_scope(&mut self, extent: CodeExtent, entry: BasicBlock) -> ScopeId { id: id, extent: extent, drops: vec![], - free: None + free: None, + cached_block: None, }); self.scope_auxiliary.push(ScopeAuxiliary { extent: extent, @@ -288,7 +301,7 @@ pub fn exit_scope(&mut self, block)); if let Some(ref free_data) = scope.free { let next = self.cfg.start_new_block(); - let free = build_free(self.hir.tcx(), tmp.clone(), free_data, next); + let free = build_free(self.hir.tcx(), &tmp, free_data, next); self.cfg.terminate(block, scope.id, span, free); block = next; } @@ -419,20 +432,16 @@ pub fn diverge_cleanup(&mut self) -> Option { } let unit_temp = self.get_unit_temp(); let Builder { ref mut hir, ref mut cfg, ref mut scopes, .. } = *self; - let mut next_block = None; // Given an array of scopes, we generate these from the outermost scope to the innermost // one. Thus for array [S0, S1, S2] with corresponding cleanup blocks [B0, B1, B2], we will // generate B0 <- B1 <- B2 in left-to-right order. Control flow of the generated blocks // always ends up at a block with the Resume terminator. - for scope in scopes.iter_mut().filter(|s| !s.drops.is_empty() || s.free.is_some()) { - next_block = Some(build_diverge_scope(hir.tcx(), - cfg, - unit_temp.clone(), - scope, - next_block)); + if scopes.iter().any(|scope| !scope.drops.is_empty() || scope.free.is_some()) { + Some(build_diverge_scope(hir.tcx(), cfg, &unit_temp, scopes)) + } else { + None } - scopes.iter().rev().flat_map(|x| x.cached_block()).next() } /// Utility function for *non*-scope code to build their own drops @@ -525,8 +534,9 @@ pub fn panic(&mut self, block: BasicBlock, msg: &'static str, span: Span) { // FIXME: We should have this as a constant, rather than a stack variable (to not pollute // icache with cold branch code), however to achieve that we either have to rely on rvalue // promotion or have some way, in MIR, to create constants. - self.cfg.push_assign(block, scope_id, span, &tuple, // tuple = (message_arg, file_arg, line_arg); + self.cfg.push_assign(block, scope_id, span, &tuple, // [1] Rvalue::Aggregate(AggregateKind::Tuple, elems)); + // [1] tuple = (message_arg, file_arg, line_arg); // FIXME: is this region really correct here? self.cfg.push_assign(block, scope_id, span, &tuple_ref, // tuple_ref = &tuple; Rvalue::Ref(region, BorrowKind::Shared, tuple)); @@ -602,58 +612,42 @@ fn build_scope_drops<'tcx>(cfg: &mut CFG<'tcx>, fn build_diverge_scope<'tcx>(tcx: &TyCtxt<'tcx>, cfg: &mut CFG<'tcx>, - unit_temp: Lvalue<'tcx>, - scope: &mut Scope<'tcx>, - target: Option) - -> BasicBlock { - debug_assert!(!scope.drops.is_empty() || scope.free.is_some()); - - // First, we build the drops, iterating the drops array in reverse. We do that so that as soon - // as we find a `cached_block`, we know that we’re finished and don’t need to do anything else. - let mut previous = None; - let mut last_drop_block = None; - for drop_data in scope.drops.iter_mut().rev() { - if let Some(cached_block) = drop_data.cached_block { - if let Some((previous_block, previous_span, previous_value)) = previous { - cfg.terminate(previous_block, - scope.id, - previous_span, - TerminatorKind::Drop { - value: previous_value, - target: cached_block, - unwind: None - }); - return last_drop_block.unwrap(); - } else { - return cached_block; - } - } else { - let block = cfg.start_new_cleanup_block(); - drop_data.cached_block = Some(block); - if let Some((previous_block, previous_span, previous_value)) = previous { - cfg.terminate(previous_block, - scope.id, - previous_span, - TerminatorKind::Drop { - value: previous_value, - target: block, - unwind: None - }); - } else { - last_drop_block = Some(block); - } - previous = Some((block, drop_data.span, drop_data.value.clone())); - } - } - - // Prepare the end target for this chain. - let mut target = target.unwrap_or_else(||{ - let b = cfg.start_new_cleanup_block(); - cfg.terminate(b, scope.id, DUMMY_SP, TerminatorKind::Resume); // TODO - b - }); + unit_temp: &Lvalue<'tcx>, + scopes: &mut [Scope<'tcx>]) + -> BasicBlock +{ + assert!(scopes.len() >= 1); + + // Build up the drops in **reverse** order. The end result will + // look like: + // + // [drops[n]] -...-> [drops[0]] -> [Free] -> [scopes[..n-1]] + // | | + // +------------------------------------+ + // code for scopes[n] + // + // The code in this function reads from right to left. At each + // point, we check for cached blocks representing the + // remainder. If everything is cached, we'll just walk right to + // left reading the cached results but never created anything. + + // To start, translate scopes[1..]. + let (scope, earlier_scopes) = scopes.split_last_mut().unwrap(); + let mut target = if let Some(cached_block) = scope.cached_block { + cached_block + } else if earlier_scopes.is_empty() { + // Diverging from the root scope creates a RESUME terminator. + // FIXME what span to use here? + let resumeblk = cfg.start_new_cleanup_block(); + cfg.terminate(resumeblk, scope.id, DUMMY_SP, TerminatorKind::Resume); + resumeblk + } else { + // Diverging from any other scope chains up to the previous scope. + build_diverge_scope(tcx, cfg, unit_temp, earlier_scopes) + }; + scope.cached_block = Some(target); - // Then, build the free branching into the prepared target. + // Next, build up any free. if let Some(ref mut free_data) = scope.free { target = if let Some(cached_block) = free_data.cached_block { cached_block @@ -665,29 +659,35 @@ fn build_diverge_scope<'tcx>(tcx: &TyCtxt<'tcx>, build_free(tcx, unit_temp, free_data, target)); free_data.cached_block = Some(into); into - } - }; + }; + } - if let Some((previous_block, previous_span, previous_value)) = previous { - // Finally, branch into that just-built `target` from the `previous_block`. - cfg.terminate(previous_block, - scope.id, - previous_span, - TerminatorKind::Drop { - value: previous_value, - target: target, - unwind: None - }); - last_drop_block.unwrap() - } else { - // If `previous.is_none()`, there were no drops in this scope – we return the - // target. - target + // Next, build up the drops. Here we iterate the vector in + // *forward* order, so that we generate drops[0] first (right to + // left in diagram above). + for drop_data in &mut scope.drops { + target = if let Some(cached_block) = drop_data.cached_block { + cached_block + } else { + let block = cfg.start_new_cleanup_block(); + cfg.terminate(block, + scope.id, + drop_data.span, + TerminatorKind::Drop { + value: drop_data.value.clone(), + target: target, + unwind: None + }); + drop_data.cached_block = Some(block); + block + }; } + + target } fn build_free<'tcx>(tcx: &TyCtxt<'tcx>, - unit_temp: Lvalue<'tcx>, + unit_temp: &Lvalue<'tcx>, data: &FreeData<'tcx>, target: BasicBlock) -> TerminatorKind<'tcx> { @@ -707,7 +707,7 @@ fn build_free<'tcx>(tcx: &TyCtxt<'tcx>, } }), args: vec![Operand::Consume(data.value.clone())], - destination: Some((unit_temp, target)), + destination: Some((unit_temp.clone(), target)), cleanup: None } }