Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split phase change from MirPass #102340

Merged
merged 1 commit into from
Oct 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions compiler/rustc_const_eval/src/transform/promote_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,6 @@ pub struct PromoteTemps<'tcx> {
}

impl<'tcx> MirPass<'tcx> for PromoteTemps<'tcx> {
fn phase_change(&self) -> Option<MirPhase> {
Some(MirPhase::Analysis(AnalysisPhase::Initial))
}

fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
// There's not really any point in promoting errorful MIR.
//
Expand Down
39 changes: 34 additions & 5 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,6 @@ pub trait MirPass<'tcx> {

fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>);

/// If this pass causes the MIR to enter a new phase, return that phase.
fn phase_change(&self) -> Option<MirPhase> {
None
}

fn is_mir_dump_enabled(&self) -> bool {
true
}
Expand All @@ -145,6 +140,35 @@ impl MirPhase {
}
}

impl Display for MirPhase {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
match self {
MirPhase::Built => write!(f, "built"),
MirPhase::Analysis(p) => write!(f, "analysis-{}", p),
MirPhase::Runtime(p) => write!(f, "runtime-{}", p),
}
}
}

impl Display for AnalysisPhase {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
match self {
AnalysisPhase::Initial => write!(f, "initial"),
AnalysisPhase::PostCleanup => write!(f, "post_cleanup"),
}
}
}

impl Display for RuntimePhase {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
match self {
RuntimePhase::Initial => write!(f, "initial"),
RuntimePhase::PostCleanup => write!(f, "post_cleanup"),
RuntimePhase::Optimized => write!(f, "optimized"),
}
}
}

/// Where a specific `mir::Body` comes from.
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
#[derive(HashStable, TyEncodable, TyDecodable, TypeFoldable, TypeVisitable)]
Expand Down Expand Up @@ -207,6 +231,9 @@ pub struct Body<'tcx> {
/// us to see the difference and forego optimization on the inlined promoted items.
pub phase: MirPhase,

/// How many passses we have executed since starting the current phase. Used for debug output.
pub pass_count: usize,

pub source: MirSource<'tcx>,

/// A list of source scopes; these are referenced by statements
Expand Down Expand Up @@ -292,6 +319,7 @@ impl<'tcx> Body<'tcx> {

let mut body = Body {
phase: MirPhase::Built,
pass_count: 1,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks like pretty fragile initialization. Is this always 1 initially?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is a counter that is always set/reset to 1 everywhere.

source,
basic_blocks: BasicBlocks::new(basic_blocks),
source_scopes,
Expand Down Expand Up @@ -325,6 +353,7 @@ impl<'tcx> Body<'tcx> {
pub fn new_cfg_only(basic_blocks: IndexVec<BasicBlock, BasicBlockData<'tcx>>) -> Self {
let mut body = Body {
phase: MirPhase::Built,
pass_count: 1,
source: MirSource::item(CRATE_DEF_ID.to_def_id()),
basic_blocks: BasicBlocks::new(basic_blocks),
source_scopes: IndexVec::new(),
Expand Down
21 changes: 9 additions & 12 deletions compiler/rustc_mir_transform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ mod inline;
mod instcombine;
mod lower_intrinsics;
mod lower_slice_len;
mod marker;
mod match_branches;
mod multiple_return_terminators;
mod normalize_array_len;
Expand Down Expand Up @@ -303,6 +302,7 @@ fn mir_const<'tcx>(
&simplify::SimplifyCfg::new("initial"),
&rustc_peek::SanityCheck, // Just a lint
],
None,
);
tcx.alloc_steal_mir(body)
}
Expand Down Expand Up @@ -342,6 +342,7 @@ fn mir_promoted<'tcx>(
&simplify::SimplifyCfg::new("promote-consts"),
&coverage::InstrumentCoverage,
],
Some(MirPhase::Analysis(AnalysisPhase::Initial)),
);

let promoted = promote_pass.promoted_fragments.into_inner();
Expand Down Expand Up @@ -409,10 +410,8 @@ fn inner_mir_for_ctfe(tcx: TyCtxt<'_>, def: ty::WithOptConstParam<LocalDefId>) -
pm::run_passes(
tcx,
&mut body,
&[
&const_prop::ConstProp,
&marker::PhaseChange(MirPhase::Runtime(RuntimePhase::Optimized)),
],
&[&const_prop::ConstProp],
Some(MirPhase::Runtime(RuntimePhase::Optimized)),
);
}
}
Expand Down Expand Up @@ -474,6 +473,7 @@ fn run_analysis_to_runtime_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>
&remove_uninit_drops::RemoveUninitDrops,
&simplify::SimplifyCfg::new("remove-false-edges"),
],
None,
);
check_consts::post_drop_elaboration::check_live_drops(tcx, &body); // FIXME: make this a MIR lint
}
Expand All @@ -498,10 +498,9 @@ fn run_analysis_cleanup_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
&cleanup_post_borrowck::CleanupNonCodegenStatements,
&simplify::SimplifyCfg::new("early-opt"),
&deref_separator::Derefer,
&marker::PhaseChange(MirPhase::Analysis(AnalysisPhase::PostCleanup)),
];

pm::run_passes(tcx, body, passes);
pm::run_passes(tcx, body, passes, Some(MirPhase::Analysis(AnalysisPhase::PostCleanup)));
}

/// Returns the sequence of passes that lowers analysis to runtime MIR.
Expand All @@ -526,9 +525,8 @@ fn run_runtime_lowering_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
// CTFE support for aggregates.
&deaggregator::Deaggregator,
&Lint(const_prop_lint::ConstProp),
&marker::PhaseChange(MirPhase::Runtime(RuntimePhase::Initial)),
];
pm::run_passes_no_validate(tcx, body, passes);
pm::run_passes_no_validate(tcx, body, passes, Some(MirPhase::Runtime(RuntimePhase::Initial)));
}

/// Returns the sequence of passes that do the initial cleanup of runtime MIR.
Expand All @@ -537,10 +535,9 @@ fn run_runtime_cleanup_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
&elaborate_box_derefs::ElaborateBoxDerefs,
&lower_intrinsics::LowerIntrinsics,
&simplify::SimplifyCfg::new("elaborate-drops"),
&marker::PhaseChange(MirPhase::Runtime(RuntimePhase::PostCleanup)),
];

pm::run_passes(tcx, body, passes);
pm::run_passes(tcx, body, passes, Some(MirPhase::Runtime(RuntimePhase::PostCleanup)));
}

fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
Expand Down Expand Up @@ -591,10 +588,10 @@ fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
&deduplicate_blocks::DeduplicateBlocks,
// Some cleanup necessary at least for LLVM and potentially other codegen backends.
&add_call_guards::CriticalCallEdges,
&marker::PhaseChange(MirPhase::Runtime(RuntimePhase::Optimized)),
// Dump the end result for testing and debugging purposes.
&dump_mir::Marker("PreCodegen"),
],
Some(MirPhase::Runtime(RuntimePhase::Optimized)),
);
}

Expand Down
20 changes: 0 additions & 20 deletions compiler/rustc_mir_transform/src/marker.rs

This file was deleted.

89 changes: 56 additions & 33 deletions compiler/rustc_mir_transform/src/pass_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,6 @@ where
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
self.1.run_pass(tcx, body)
}

fn phase_change(&self) -> Option<MirPhase> {
self.1.phase_change()
}
}

/// Run the sequence of passes without validating the MIR after each pass. The MIR is still
Expand All @@ -78,31 +74,35 @@ pub fn run_passes_no_validate<'tcx>(
tcx: TyCtxt<'tcx>,
body: &mut Body<'tcx>,
passes: &[&dyn MirPass<'tcx>],
phase_change: Option<MirPhase>,
) {
run_passes_inner(tcx, body, passes, false);
run_passes_inner(tcx, body, passes, phase_change, false);
}

pub fn run_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>, passes: &[&dyn MirPass<'tcx>]) {
run_passes_inner(tcx, body, passes, true);
/// The optional `phase_change` is applied after executing all the passes, if present
pub fn run_passes<'tcx>(
tcx: TyCtxt<'tcx>,
body: &mut Body<'tcx>,
passes: &[&dyn MirPass<'tcx>],
phase_change: Option<MirPhase>,
) {
run_passes_inner(tcx, body, passes, phase_change, true);
}

fn run_passes_inner<'tcx>(
tcx: TyCtxt<'tcx>,
body: &mut Body<'tcx>,
passes: &[&dyn MirPass<'tcx>],
phase_change: Option<MirPhase>,
validate_each: bool,
) {
let start_phase = body.phase;
let mut cnt = 0;

let validate = validate_each & tcx.sess.opts.unstable_opts.validate_mir;
let overridden_passes = &tcx.sess.opts.unstable_opts.mir_enable_passes;
trace!(?overridden_passes);

for pass in passes {
let name = pass.name();

// Gather information about what we should be doing for this pass
let overridden =
overridden_passes.iter().rev().find(|(s, _)| s == &*name).map(|(_name, polarity)| {
trace!(
Expand All @@ -112,55 +112,78 @@ fn run_passes_inner<'tcx>(
);
*polarity
});
let is_enabled = overridden.unwrap_or_else(|| pass.is_enabled(&tcx.sess));
let new_phase = pass.phase_change();
let dump_enabled = (is_enabled && pass.is_mir_dump_enabled()) || new_phase.is_some();
let validate = (validate && is_enabled)
|| new_phase == Some(MirPhase::Runtime(RuntimePhase::Optimized));
if !overridden.unwrap_or_else(|| pass.is_enabled(&tcx.sess)) {
continue;
}

let dump_enabled = pass.is_mir_dump_enabled();

if dump_enabled {
dump_mir(tcx, body, start_phase, &name, cnt, false);
}
if is_enabled {
pass.run_pass(tcx, body);
dump_mir_for_pass(tcx, body, &name, false);
}
if dump_enabled {
dump_mir(tcx, body, start_phase, &name, cnt, true);
cnt += 1;
if validate {
validate_body(tcx, body, format!("before pass {}", name));
}
if let Some(new_phase) = pass.phase_change() {
if body.phase >= new_phase {
panic!("Invalid MIR phase transition from {:?} to {:?}", body.phase, new_phase);
}

body.phase = new_phase;
pass.run_pass(tcx, body);

if dump_enabled {
dump_mir_for_pass(tcx, body, &name, true);
}
if validate {
validate_body(tcx, body, format!("after pass {}", name));
}

body.pass_count += 1;
}

if let Some(new_phase) = phase_change {
if body.phase >= new_phase {
panic!("Invalid MIR phase transition from {:?} to {:?}", body.phase, new_phase);
}

body.phase = new_phase;

dump_mir_for_phase_change(tcx, body);
if validate || new_phase == MirPhase::Runtime(RuntimePhase::Optimized) {
validate_body(tcx, body, format!("after phase change to {}", new_phase));
}

body.pass_count = 1;
}
}

pub fn validate_body<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>, when: String) {
validate::Validator { when, mir_phase: body.phase }.run_pass(tcx, body);
}

pub fn dump_mir<'tcx>(
pub fn dump_mir_for_pass<'tcx>(
tcx: TyCtxt<'tcx>,
body: &Body<'tcx>,
phase: MirPhase,
pass_name: &str,
cnt: usize,
is_after: bool,
) {
let phase_index = phase.phase_index();
let phase_index = body.phase.phase_index();

mir::dump_mir(
tcx,
Some(&format_args!("{:03}-{:03}", phase_index, cnt)),
Some(&format_args!("{:03}-{:03}", phase_index, body.pass_count)),
pass_name,
if is_after { &"after" } else { &"before" },
body,
|_, _| Ok(()),
);
}

pub fn dump_mir_for_phase_change<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) {
let phase_index = body.phase.phase_index();

mir::dump_mir(
tcx,
Some(&format_args!("{:03}-000", phase_index)),
&format!("{}", body.phase),
&"after",
body,
|_, _| Ok(()),
)
}
4 changes: 2 additions & 2 deletions compiler/rustc_mir_transform/src/shim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use std::iter;

use crate::util::expand_aggregate;
use crate::{
abort_unwinding_calls, add_call_guards, add_moves_for_packed_drops, deref_separator, marker,
abort_unwinding_calls, add_call_guards, add_moves_for_packed_drops, deref_separator,
pass_manager as pm, remove_noop_landing_pads, simplify,
};
use rustc_middle::mir::patch::MirPatch;
Expand Down Expand Up @@ -97,8 +97,8 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> Body<'
&simplify::SimplifyCfg::new("make_shim"),
&add_call_guards::CriticalCallEdges,
&abort_unwinding_calls::AbortUnwindingCalls,
&marker::PhaseChange(MirPhase::Runtime(RuntimePhase::Optimized)),
],
Some(MirPhase::Runtime(RuntimePhase::Optimized)),
);

debug!("make_shim({:?}) = {:?}", instance, result);
Expand Down