From e63b9920302e860b4f50968eb332f534d62b8055 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Thu, 15 Aug 2019 06:39:31 -0400 Subject: [PATCH] Resolve PR comments --- src/librustc_metadata/decoder.rs | 7 +- src/librustc_metadata/encoder.rs | 6 +- src/librustc_mir/borrow_check/nll/renumber.rs | 13 +- src/librustc_mir/shim.rs | 2 +- src/librustc_mir/transform/mod.rs | 153 +++++++----------- src/librustc_mir/transform/promote_consts.rs | 31 ++-- 6 files changed, 81 insertions(+), 131 deletions(-) diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs index 2add8bf918398..5b9cb966af235 100644 --- a/src/librustc_metadata/decoder.rs +++ b/src/librustc_metadata/decoder.rs @@ -924,8 +924,11 @@ impl<'a, 'tcx> CrateMetadata { } } - pub fn maybe_get_promoted_mir(&self, tcx: TyCtxt<'tcx>, id: DefIndex) -> - Option>> { + pub fn maybe_get_promoted_mir( + &self, + tcx: TyCtxt<'tcx>, + id: DefIndex, + ) -> Option>> { match self.is_proc_macro(id) { true => None, false => self.entry(id).promoted_mir.map(|promoted| promoted.decode((self, tcx)),) diff --git a/src/librustc_metadata/encoder.rs b/src/librustc_metadata/encoder.rs index 0b8d2438752fa..1797d77461567 100644 --- a/src/librustc_metadata/encoder.rs +++ b/src/librustc_metadata/encoder.rs @@ -1060,8 +1060,10 @@ impl EncodeContext<'tcx> { } } - fn encode_promoted_mir(&mut self, def_id: DefId) -> - Option>>> { + fn encode_promoted_mir( + &mut self, + def_id: DefId, + ) -> Option>>> { debug!("EncodeContext::encode_promoted_mir({:?})", def_id); if self.tcx.mir_keys(LOCAL_CRATE).contains(&def_id) { let promoted = self.tcx.promoted_mir(def_id); diff --git a/src/librustc_mir/borrow_check/nll/renumber.rs b/src/librustc_mir/borrow_check/nll/renumber.rs index c6a5c6fdfd0f2..c479c38f30c7e 100644 --- a/src/librustc_mir/borrow_check/nll/renumber.rs +++ b/src/librustc_mir/borrow_check/nll/renumber.rs @@ -16,7 +16,11 @@ pub fn renumber_mir<'tcx>( debug!("renumber_mir: body.arg_count={:?}", body.arg_count); let mut visitor = NLLVisitor { infcx }; - visitor.visit_promoted(promoted); + + for body in promoted.iter_mut() { + visitor.visit_body(body); + } + visitor.visit_body(body); } @@ -47,13 +51,6 @@ impl<'a, 'tcx> NLLVisitor<'a, 'tcx> { { renumber_regions(self.infcx, value) } - - fn visit_promoted(&mut self, promoted: &mut IndexVec>) { - debug!("visiting promoted mir"); - for body in promoted.iter_mut() { - self.visit_body(body); - } - } } impl<'a, 'tcx> MutVisitor<'tcx> for NLLVisitor<'a, 'tcx> { diff --git a/src/librustc_mir/shim.rs b/src/librustc_mir/shim.rs index 9d31015f84558..aa83255bf62f4 100644 --- a/src/librustc_mir/shim.rs +++ b/src/librustc_mir/shim.rs @@ -112,7 +112,7 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> &'tcx }; debug!("make_shim({:?}) = untransformed {:?}", instance, result); - run_passes(tcx, &mut result, instance, MirPhase::Const, &[ + run_passes(tcx, &mut result, instance, None, MirPhase::Const, &[ &add_moves_for_packed_drops::AddMovesForPackedDrops, &no_landing_pads::NoLandingPads, &remove_noop_landing_pads::RemoveNoopLandingPads, diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs index c9bcdbe1bef50..ac291c2996d06 100644 --- a/src/librustc_mir/transform/mod.rs +++ b/src/librustc_mir/transform/mod.rs @@ -149,41 +149,38 @@ pub fn run_passes( tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>, instance: InstanceDef<'tcx>, + promoted: Option, mir_phase: MirPhase, passes: &[&dyn MirPass<'tcx>], ) { let phase_index = mir_phase.phase_index(); - let run_passes = |body: &mut Body<'tcx>, promoted| { - if body.phase >= mir_phase { - return; - } + if body.phase >= mir_phase { + return; + } - let source = MirSource { - instance, - promoted, - }; - let mut index = 0; - let mut run_pass = |pass: &dyn MirPass<'tcx>| { - let run_hooks = |body: &_, index, is_after| { - dump_mir::on_mir_pass(tcx, &format_args!("{:03}-{:03}", phase_index, index), - &pass.name(), source, body, is_after); - }; - run_hooks(body, index, false); - pass.run_pass(tcx, source, body); - run_hooks(body, index, true); - - index += 1; + let source = MirSource { + instance, + promoted, + }; + let mut index = 0; + let mut run_pass = |pass: &dyn MirPass<'tcx>| { + let run_hooks = |body: &_, index, is_after| { + dump_mir::on_mir_pass(tcx, &format_args!("{:03}-{:03}", phase_index, index), + &pass.name(), source, body, is_after); }; + run_hooks(body, index, false); + pass.run_pass(tcx, source, body); + run_hooks(body, index, true); - for pass in passes { - run_pass(*pass); - } - - body.phase = mir_phase; + index += 1; }; - run_passes(body, None); + for pass in passes { + run_pass(*pass); + } + + body.phase = mir_phase; } fn mir_const(tcx: TyCtxt<'_>, def_id: DefId) -> &Steal> { @@ -191,7 +188,7 @@ fn mir_const(tcx: TyCtxt<'_>, def_id: DefId) -> &Steal> { let _ = tcx.unsafety_check_result(def_id); let mut body = tcx.mir_built(def_id).steal(); - run_passes(tcx, &mut body, InstanceDef::Item(def_id), MirPhase::Const, &[ + run_passes(tcx, &mut body, InstanceDef::Item(def_id), None, MirPhase::Const, &[ // What we need to do constant evaluation. &simplify::SimplifyCfg::new("initial"), &rustc_peek::SanityCheck, @@ -213,7 +210,7 @@ fn mir_validated( let mut body = tcx.mir_const(def_id).steal(); let qualify_and_promote_pass = qualify_consts::QualifyAndPromoteConstants::default(); - run_passes(tcx, &mut body, InstanceDef::Item(def_id), MirPhase::Validated, &[ + run_passes(tcx, &mut body, InstanceDef::Item(def_id), None, MirPhase::Validated, &[ // What we need to run borrowck etc. &qualify_and_promote_pass, &simplify::SimplifyCfg::new("qualify-consts"), @@ -222,26 +219,13 @@ fn mir_validated( (tcx.alloc_steal_mir(body), tcx.alloc_steal_promoted(promoted)) } -fn optimized_mir(tcx: TyCtxt<'_>, def_id: DefId) -> &Body<'_> { - if tcx.is_constructor(def_id) { - // There's no reason to run all of the MIR passes on constructors when - // we can just output the MIR we want directly. This also saves const - // qualification and borrow checking the trouble of special casing - // constructors. - return shim::build_adt_ctor(tcx, def_id); - } - - // (Mir-)Borrowck uses `mir_validated`, so we have to force it to - // execute before we can steal. - tcx.ensure().mir_borrowck(def_id); - - if tcx.use_ast_borrowck() { - tcx.ensure().borrowck(def_id); - } - - let (body, _) = tcx.mir_validated(def_id); - let mut body = body.steal(); - run_passes(tcx, &mut body, InstanceDef::Item(def_id), MirPhase::Optimized, &[ +fn run_optimization_passes<'tcx>( + tcx: TyCtxt<'tcx>, + body: &mut Body<'tcx>, + def_id: DefId, + promoted: Option, +) { + run_passes(tcx, body, InstanceDef::Item(def_id), promoted, MirPhase::Optimized, &[ // Remove all things only needed by analysis &no_landing_pads::NoLandingPads, &simplify_branches::SimplifyBranches::new("initial"), @@ -292,6 +276,28 @@ fn optimized_mir(tcx: TyCtxt<'_>, def_id: DefId) -> &Body<'_> { &add_call_guards::CriticalCallEdges, &dump_mir::Marker("PreCodegen"), ]); +} + +fn optimized_mir(tcx: TyCtxt<'_>, def_id: DefId) -> &Body<'_> { + if tcx.is_constructor(def_id) { + // There's no reason to run all of the MIR passes on constructors when + // we can just output the MIR we want directly. This also saves const + // qualification and borrow checking the trouble of special casing + // constructors. + return shim::build_adt_ctor(tcx, def_id); + } + + // (Mir-)Borrowck uses `mir_validated`, so we have to force it to + // execute before we can steal. + tcx.ensure().mir_borrowck(def_id); + + if tcx.use_ast_borrowck() { + tcx.ensure().borrowck(def_id); + } + + let (body, _) = tcx.mir_validated(def_id); + let mut body = body.steal(); + run_optimization_passes(tcx, &mut body, def_id, None); tcx.arena.alloc(body) } @@ -304,57 +310,8 @@ fn promoted_mir<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> &'tcx IndexVec Promoter<'a, 'tcx> { let mut operand = { let promoted = &mut self.promoted; let promoted_id = Promoted::new(next_promoted_id); - let mut promoted_place = |ty, substs, span| { + let tcx = self.tcx; + let mut promoted_place = |ty, span| { promoted.span = span; promoted.local_decls[RETURN_PLACE] = LocalDecl::new_return_place(ty, span); Place { base: PlaceBase::Static(box Static { - kind: StaticKind::Promoted(promoted_id, substs), + kind: + StaticKind::Promoted( + promoted_id, + InternalSubsts::identity_for_item(tcx, def_id), + ), ty, def_id, }), @@ -329,11 +334,7 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> { Operand::Move(Place { base: mem::replace( &mut place.base, - promoted_place( - ty, - InternalSubsts::identity_for_item(self.tcx, def_id), - span, - ).base + promoted_place(ty, span).base ), projection: None, }) @@ -349,13 +350,7 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> { let span = statement.source_info.span; mem::replace( operand, - Operand::Copy( - promoted_place( - ty, - InternalSubsts::identity_for_item(self.tcx, def_id), - span, - ) - ) + Operand::Copy(promoted_place(ty, span)) ) } _ => bug!() @@ -367,12 +362,7 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> { TerminatorKind::Call { ref mut args, .. } => { let ty = args[index].ty(local_decls, self.tcx); let span = terminator.source_info.span; - let operand = - Operand::Copy( - promoted_place( - ty, - InternalSubsts::identity_for_item(self.tcx, def_id), - span)); + let operand = Operand::Copy(promoted_place(ty, span)); mem::replace(&mut args[index], operand) } // We expected a `TerminatorKind::Call` for which we'd like to promote an @@ -472,6 +462,7 @@ pub fn promote_candidates<'tcx>( keep_original: false }; + //FIXME(oli-obk): having a `maybe_push()` method on `IndexVec` might be nice if let Some(promoted) = promoter.promote_candidate(def_id, candidate, promotions.len()) { promotions.push(promoted); }