Skip to content

Commit

Permalink
Rollup merge of rust-lang#55244 - wesleywiser:issue-50411, r=nikomats…
Browse files Browse the repository at this point in the history
…akis

Don't rerun MIR passes when inlining

Fixes rust-lang#50411

r? @nikomatsakis

I updated your commit message with additional details. Let me know if any of that is incorrect. I also added the appropriate `compile-flags` directive to the test.

Thanks for you help on this!

cc @RalfJung related to your PR rust-lang#55086
  • Loading branch information
pietroalbini committed Oct 25, 2018
2 parents 298dcd5 + 4655866 commit cdc3d6d
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 72 deletions.
36 changes: 36 additions & 0 deletions src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,38 @@ impl<'tcx> HasLocalDecls<'tcx> for Mir<'tcx> {
}
}

/// The various "big phases" that MIR goes through.
///
/// Warning: ordering of variants is significant
#[derive(Copy, Clone, RustcEncodable, RustcDecodable, Debug, PartialEq, Eq, PartialOrd, Ord)]
pub enum MirPhase {
Build = 0,
Const = 1,
Validated = 2,
Optimized = 3,
}

impl MirPhase {
/// Gets the index of the current MirPhase within the set of all MirPhases.
pub fn phase_index(&self) -> usize {
*self as usize
}
}

/// Lowered representation of a single function.
#[derive(Clone, RustcEncodable, RustcDecodable, Debug)]
pub struct Mir<'tcx> {
/// List of basic blocks. References to basic block use a newtyped index type `BasicBlock`
/// that indexes into this vector.
basic_blocks: IndexVec<BasicBlock, BasicBlockData<'tcx>>,

/// Records how far through the "desugaring and optimization" process this particular
/// MIR has traversed. This is particularly useful when inlining, since in that context
/// we instantiate the promoted constants and add them to our promoted vector -- but those
/// promoted items have already been optimized, whereas ours have not. This field allows
/// us to see the difference and forego optimization on the inlined promoted items.
pub phase: MirPhase,

/// List of source scopes; these are referenced by statements
/// and used for debuginfo. Indexed by a `SourceScope`.
pub source_scopes: IndexVec<SourceScope, SourceScopeData>,
Expand Down Expand Up @@ -151,6 +176,7 @@ impl<'tcx> Mir<'tcx> {
);

Mir {
phase: MirPhase::Build,
basic_blocks,
source_scopes,
source_scope_local_data,
Expand Down Expand Up @@ -368,6 +394,7 @@ pub enum Safety {
}

impl_stable_hash_for!(struct Mir<'tcx> {
phase,
basic_blocks,
source_scopes,
source_scope_local_data,
Expand Down Expand Up @@ -616,6 +643,13 @@ impl_stable_hash_for!(enum self::ImplicitSelfKind {
None
});

impl_stable_hash_for!(enum self::MirPhase {
Build,
Const,
Validated,
Optimized,
});

mod binding_form_impl {
use ich::StableHashingContext;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher, StableHasherResult};
Expand Down Expand Up @@ -2786,6 +2820,7 @@ pub enum ClosureOutlivesSubject<'tcx> {

CloneTypeFoldableAndLiftImpls! {
BlockTailInfo,
MirPhase,
Mutability,
SourceInfo,
UpvarDecl,
Expand All @@ -2798,6 +2833,7 @@ CloneTypeFoldableAndLiftImpls! {

BraceStructTypeFoldableImpl! {
impl<'tcx> TypeFoldable<'tcx> for Mir<'tcx> {
phase,
basic_blocks,
source_scopes,
source_scope_local_data,
Expand Down
1 change: 0 additions & 1 deletion src/librustc_mir/dataflow/impls/borrows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,6 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
}

crate fn borrows(&self) -> &IndexVec<BorrowIndex, BorrowData<'tcx>> { &self.borrow_set.borrows }
pub fn scope_tree(&self) -> &Lrc<region::ScopeTree> { &self.scope_tree }

pub fn location(&self, idx: BorrowIndex) -> &Location {
&self.borrow_set.borrows[idx].reserve_location
Expand Down
14 changes: 0 additions & 14 deletions src/librustc_mir/dataflow/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -724,20 +724,6 @@ impl<'a, 'tcx, D> DataflowAnalysis<'a, 'tcx, D> where D: BitDenotation
}
}
}

pub fn new_from_sets(mir: &'a Mir<'tcx>,
dead_unwinds: &'a BitSet<mir::BasicBlock>,
sets: AllSets<D::Idx>,
denotation: D) -> Self {
DataflowAnalysis {
mir,
dead_unwinds,
flow_state: DataflowState {
sets: sets,
operator: denotation,
}
}
}
}

impl<'a, 'tcx: 'a, D> DataflowAnalysis<'a, 'tcx, D> where D: BitDenotation
Expand Down
130 changes: 73 additions & 57 deletions src/librustc_mir/transform/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
use borrow_check::nll::type_check;
use build;
use rustc::hir::def_id::{CrateNum, DefId, LOCAL_CRATE};
use rustc::mir::{Mir, Promoted};
use rustc::mir::{Mir, MirPhase, Promoted};
use rustc::ty::TyCtxt;
use rustc::ty::query::Providers;
use rustc::ty::steal::Steal;
Expand Down Expand Up @@ -155,53 +155,69 @@ pub trait MirPass {
mir: &mut Mir<'tcx>);
}

pub macro run_passes($tcx:ident, $mir:ident, $def_id:ident, $suite_index:expr; $($pass:expr,)*) {{
let suite_index: usize = $suite_index;
let run_passes = |mir: &mut _, promoted| {
pub fn run_passes(
tcx: TyCtxt<'a, 'tcx, 'tcx>,
mir: &mut Mir<'tcx>,
def_id: DefId,
mir_phase: MirPhase,
passes: &[&dyn MirPass],
) {
let phase_index = mir_phase.phase_index();

let run_passes = |mir: &mut Mir<'tcx>, promoted| {
if mir.phase >= mir_phase {
return;
}

let source = MirSource {
def_id: $def_id,
promoted
def_id,
promoted,
};
let mut index = 0;
let mut run_pass = |pass: &dyn MirPass| {
let run_hooks = |mir: &_, index, is_after| {
dump_mir::on_mir_pass($tcx, &format_args!("{:03}-{:03}", suite_index, index),
dump_mir::on_mir_pass(tcx, &format_args!("{:03}-{:03}", phase_index, index),
&pass.name(), source, mir, is_after);
};
run_hooks(mir, index, false);
pass.run_pass($tcx, source, mir);
pass.run_pass(tcx, source, mir);
run_hooks(mir, index, true);

index += 1;
};
$(run_pass(&$pass);)*

for pass in passes {
run_pass(*pass);
}

mir.phase = mir_phase;
};

run_passes(&mut $mir, None);
run_passes(mir, None);

for (index, promoted_mir) in $mir.promoted.iter_enumerated_mut() {
for (index, promoted_mir) in mir.promoted.iter_enumerated_mut() {
run_passes(promoted_mir, Some(index));

// Let's make sure we don't miss any nested instances
assert!(promoted_mir.promoted.is_empty());
//Let's make sure we don't miss any nested instances
assert!(promoted_mir.promoted.is_empty())
}
}}
}

fn mir_const<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx Steal<Mir<'tcx>> {
// Unsafety check uses the raw mir, so make sure it is run
let _ = tcx.unsafety_check_result(def_id);

let mut mir = tcx.mir_built(def_id).steal();
run_passes![tcx, mir, def_id, 0;
run_passes(tcx, &mut mir, def_id, MirPhase::Const, &[
// Remove all `EndRegion` statements that are not involved in borrows.
cleanup_post_borrowck::CleanEndRegions,
&cleanup_post_borrowck::CleanEndRegions,

// What we need to do constant evaluation.
simplify::SimplifyCfg::new("initial"),
type_check::TypeckMir,
rustc_peek::SanityCheck,
uniform_array_move_out::UniformArrayMoveOut,
];
&simplify::SimplifyCfg::new("initial"),
&type_check::TypeckMir,
&rustc_peek::SanityCheck,
&uniform_array_move_out::UniformArrayMoveOut,
]);
tcx.alloc_steal_mir(mir)
}

Expand All @@ -214,11 +230,11 @@ fn mir_validated<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx
}

let mut mir = tcx.mir_const(def_id).steal();
run_passes![tcx, mir, def_id, 1;
run_passes(tcx, &mut mir, def_id, MirPhase::Validated, &[
// What we need to run borrowck etc.
qualify_consts::QualifyAndPromoteConstants,
simplify::SimplifyCfg::new("qualify-consts"),
];
&qualify_consts::QualifyAndPromoteConstants,
&simplify::SimplifyCfg::new("qualify-consts"),
]);
tcx.alloc_steal_mir(mir)
}

Expand All @@ -232,59 +248,59 @@ fn optimized_mir<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx
}

let mut mir = tcx.mir_validated(def_id).steal();
run_passes![tcx, mir, def_id, 2;
run_passes(tcx, &mut mir, def_id, MirPhase::Optimized, &[
// Remove all things not needed by analysis
no_landing_pads::NoLandingPads,
simplify_branches::SimplifyBranches::new("initial"),
remove_noop_landing_pads::RemoveNoopLandingPads,
&no_landing_pads::NoLandingPads,
&simplify_branches::SimplifyBranches::new("initial"),
&remove_noop_landing_pads::RemoveNoopLandingPads,
// Remove all `AscribeUserType` statements.
cleanup_post_borrowck::CleanAscribeUserType,
&cleanup_post_borrowck::CleanAscribeUserType,
// Remove all `FakeRead` statements and the borrows that are only
// used for checking matches
cleanup_post_borrowck::CleanFakeReadsAndBorrows,
simplify::SimplifyCfg::new("early-opt"),
&cleanup_post_borrowck::CleanFakeReadsAndBorrows,
&simplify::SimplifyCfg::new("early-opt"),

// These next passes must be executed together
add_call_guards::CriticalCallEdges,
elaborate_drops::ElaborateDrops,
no_landing_pads::NoLandingPads,
&add_call_guards::CriticalCallEdges,
&elaborate_drops::ElaborateDrops,
&no_landing_pads::NoLandingPads,
// AddValidation needs to run after ElaborateDrops and before EraseRegions, and it needs
// an AllCallEdges pass right before it.
add_call_guards::AllCallEdges,
add_validation::AddValidation,
&add_call_guards::AllCallEdges,
&add_validation::AddValidation,
// AddMovesForPackedDrops needs to run after drop
// elaboration.
add_moves_for_packed_drops::AddMovesForPackedDrops,
&add_moves_for_packed_drops::AddMovesForPackedDrops,

simplify::SimplifyCfg::new("elaborate-drops"),
&simplify::SimplifyCfg::new("elaborate-drops"),

// No lifetime analysis based on borrowing can be done from here on out.

// From here on out, regions are gone.
erase_regions::EraseRegions,
&erase_regions::EraseRegions,

lower_128bit::Lower128Bit,
&lower_128bit::Lower128Bit,


// Optimizations begin.
uniform_array_move_out::RestoreSubsliceArrayMoveOut,
inline::Inline,
&uniform_array_move_out::RestoreSubsliceArrayMoveOut,
&inline::Inline,

// Lowering generator control-flow and variables
// has to happen before we do anything else to them.
generator::StateTransform,

instcombine::InstCombine,
const_prop::ConstProp,
simplify_branches::SimplifyBranches::new("after-const-prop"),
deaggregator::Deaggregator,
copy_prop::CopyPropagation,
remove_noop_landing_pads::RemoveNoopLandingPads,
simplify::SimplifyCfg::new("final"),
simplify::SimplifyLocals,

add_call_guards::CriticalCallEdges,
dump_mir::Marker("PreCodegen"),
];
&generator::StateTransform,

&instcombine::InstCombine,
&const_prop::ConstProp,
&simplify_branches::SimplifyBranches::new("after-const-prop"),
&deaggregator::Deaggregator,
&copy_prop::CopyPropagation,
&remove_noop_landing_pads::RemoveNoopLandingPads,
&simplify::SimplifyCfg::new("final"),
&simplify::SimplifyLocals,

&add_call_guards::CriticalCallEdges,
&dump_mir::Marker("PreCodegen"),
]);
tcx.alloc_mir(mir)
}
11 changes: 11 additions & 0 deletions src/test/ui/issues/issue-50411.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Regression test for #50411: the MIR inliner was causing problems
// here because it would inline promoted code (which had already had
// elaborate-drops invoked on it) and then try to elaboate drops a
// second time. Uncool.

// compile-flags:-Zmir-opt-level=3
// compile-pass

fn main() {
let _ = (0 .. 1).filter(|_| [1].iter().all(|_| true)).count();
}

0 comments on commit cdc3d6d

Please sign in to comment.