Clarify which kinds of MIR are allowed during which phases.

This enhances documentation with these details and extends the validator to check these requirements
more thoroughly. As a part of this, we add a new `Deaggregated` phase, and rename other phases so
that their names more naturally correspond to what they represent.
This commit is contained in:
Jakob Degen 2022-03-05 20:37:04 -05:00
parent 547369d3d8
commit fe40240e4d
8 changed files with 98 additions and 52 deletions

View File

@ -42,7 +42,7 @@ pub struct PromoteTemps<'tcx> {
impl<'tcx> MirPass<'tcx> for PromoteTemps<'tcx> {
fn phase_change(&self) -> Option<MirPhase> {
Some(MirPhase::ConstPromotion)
Some(MirPhase::ConstsPromoted)
}
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {

View File

@ -266,22 +266,15 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
);
}
}
// The deaggregator currently does not deaggreagate arrays.
// So for now, we ignore them here.
Rvalue::Aggregate(box AggregateKind::Array { .. }, _) => {}
// All other aggregates must be gone after some phases.
Rvalue::Aggregate(box kind, _) => {
if self.mir_phase > MirPhase::DropLowering
&& !matches!(kind, AggregateKind::Generator(..))
{
// Generators persist until the state machine transformation, but all
// other aggregates must have been lowered.
self.fail(
location,
format!("{:?} have been lowered to field assignments", rvalue),
)
} else if self.mir_phase > MirPhase::GeneratorLowering {
// No more aggregates after drop and generator lowering.
Rvalue::Aggregate(agg_kind, _) => {
let disallowed = match **agg_kind {
AggregateKind::Array(..) => false,
AggregateKind::Generator(..) => {
self.mir_phase >= MirPhase::GeneratorsLowered
}
_ => self.mir_phase >= MirPhase::Deaggregated,
};
if disallowed {
self.fail(
location,
format!("{:?} have been lowered to field assignments", rvalue),
@ -289,7 +282,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
}
}
Rvalue::Ref(_, BorrowKind::Shallow, _) => {
if self.mir_phase > MirPhase::DropLowering {
if self.mir_phase >= MirPhase::DropsLowered {
self.fail(
location,
"`Assign` statement with a `Shallow` borrow should have been removed after drop lowering phase",
@ -300,7 +293,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
}
}
StatementKind::AscribeUserType(..) => {
if self.mir_phase > MirPhase::DropLowering {
if self.mir_phase >= MirPhase::DropsLowered {
self.fail(
location,
"`AscribeUserType` should have been removed after drop lowering phase",
@ -308,7 +301,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
}
}
StatementKind::FakeRead(..) => {
if self.mir_phase > MirPhase::DropLowering {
if self.mir_phase >= MirPhase::DropsLowered {
self.fail(
location,
"`FakeRead` should have been removed after drop lowering phase",
@ -351,10 +344,18 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
self.fail(location, format!("bad arg ({:?} != usize)", op_cnt_ty))
}
}
StatementKind::SetDiscriminant { .. }
| StatementKind::StorageLive(..)
StatementKind::SetDiscriminant { .. } => {
if self.mir_phase < MirPhase::DropsLowered {
self.fail(location, "`SetDiscriminant` is not allowed until drop elaboration");
}
}
StatementKind::Retag(_, _) => {
// FIXME(JakobDegen) The validator should check that `self.mir_phase <
// DropsLowered`. However, this causes ICEs with generation of drop shims, which
// seem to fail to set their `MirPhase` correctly.
}
StatementKind::StorageLive(..)
| StatementKind::StorageDead(..)
| StatementKind::Retag(_, _)
| StatementKind::Coverage(_)
| StatementKind::Nop => {}
}
@ -424,10 +425,10 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
}
}
TerminatorKind::DropAndReplace { target, unwind, .. } => {
if self.mir_phase > MirPhase::DropLowering {
if self.mir_phase >= MirPhase::DropsLowered {
self.fail(
location,
"`DropAndReplace` is not permitted to exist after drop elaboration",
"`DropAndReplace` should have been removed during drop elaboration",
);
}
self.check_edge(location, *target, EdgeKind::Normal);
@ -494,7 +495,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
}
}
TerminatorKind::Yield { resume, drop, .. } => {
if self.mir_phase > MirPhase::GeneratorLowering {
if self.mir_phase >= MirPhase::GeneratorsLowered {
self.fail(location, "`Yield` should have been replaced by generator lowering");
}
self.check_edge(location, *resume, EdgeKind::Normal);
@ -503,10 +504,22 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
}
}
TerminatorKind::FalseEdge { real_target, imaginary_target } => {
if self.mir_phase >= MirPhase::DropsLowered {
self.fail(
location,
"`FalseEdge` should have been removed after drop elaboration",
);
}
self.check_edge(location, *real_target, EdgeKind::Normal);
self.check_edge(location, *imaginary_target, EdgeKind::Normal);
}
TerminatorKind::FalseUnwind { real_target, unwind } => {
if self.mir_phase >= MirPhase::DropsLowered {
self.fail(
location,
"`FalseUnwind` should have been removed after drop elaboration",
);
}
self.check_edge(location, *real_target, EdgeKind::Normal);
if let Some(unwind) = unwind {
self.check_edge(location, *unwind, EdgeKind::Unwind);
@ -520,12 +533,19 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
self.check_edge(location, *cleanup, EdgeKind::Unwind);
}
}
TerminatorKind::GeneratorDrop => {
if self.mir_phase >= MirPhase::GeneratorsLowered {
self.fail(
location,
"`GeneratorDrop` should have been replaced by generator lowering",
);
}
}
// Nothing to validate for these.
TerminatorKind::Resume
| TerminatorKind::Abort
| TerminatorKind::Return
| TerminatorKind::Unreachable
| TerminatorKind::GeneratorDrop => {}
| TerminatorKind::Unreachable => {}
}
self.super_terminator(terminator, location);

View File

@ -127,14 +127,11 @@ pub trait MirPass<'tcx> {
/// These phases all describe dialects of MIR. Since all MIR uses the same datastructures, the
/// dialects forbid certain variants or values in certain phases.
///
/// Note: Each phase's validation checks all invariants of the *previous* phases' dialects. A phase
/// that changes the dialect documents what invariants must be upheld *after* that phase finishes.
///
/// Warning: ordering of variants is significant.
#[derive(Copy, Clone, TyEncodable, TyDecodable, Debug, PartialEq, Eq, PartialOrd, Ord)]
#[derive(HashStable)]
pub enum MirPhase {
Build = 0,
Built = 0,
// FIXME(oli-obk): it's unclear whether we still need this phase (and its corresponding query).
// We used to have this for pre-miri MIR based const eval.
Const = 1,
@ -142,17 +139,32 @@ pub enum MirPhase {
/// by creating a new MIR body per promoted element. After this phase (and thus the termination
/// of the `mir_promoted` query), these promoted elements are available in the `promoted_mir`
/// query.
ConstPromotion = 2,
/// After this phase
/// * the only `AggregateKind`s allowed are `Array` and `Generator`,
/// * `DropAndReplace` is gone for good
/// * `Drop` now uses explicit drop flags visible in the MIR and reaching a `Drop` terminator
/// means that the auto-generated drop glue will be invoked.
DropLowering = 3,
/// After this phase, generators are explicit state machines (no more `Yield`).
/// `AggregateKind::Generator` is gone for good.
GeneratorLowering = 4,
Optimization = 5,
ConstsPromoted = 2,
/// Beginning with this phase, the following variants are disallowed:
/// * [`TerminatorKind::DropAndReplace`](terminator::TerminatorKind::DropAndReplace)
/// * [`TerminatorKind::FalseUnwind`](terminator::TerminatorKind::FalseUnwind)
/// * [`TerminatorKind::FalseEdge`](terminator::TerminatorKind::FalseEdge)
/// * [`StatementKind::FakeRead`]
/// * [`StatementKind::AscribeUserType`]
/// * [`Rvalue::Ref`] with `BorrowKind::Shallow`
///
/// And the following variant is allowed:
/// * [`StatementKind::Retag`]
///
/// Furthermore, `Drop` now uses explicit drop flags visible in the MIR and reaching a `Drop`
/// terminator means that the auto-generated drop glue will be invoked.
DropsLowered = 3,
/// Beginning with this phase, the following variant is disallowed:
/// * [`Rvalue::Aggregate`] for any `AggregateKind` except `Array`
///
/// And the following variant is allowed:
/// * [`StatementKind::SetDiscriminant`]
Deaggregated = 4,
/// Beginning with this phase, the following variants are disallowed:
/// * [`TerminatorKind::Yield`](terminator::TerminatorKind::Yield)
/// * [`TerminatorKind::GeneratorDrop](terminator::TerminatorKind::GeneratorDrop)
GeneratorsLowered = 5,
Optimized = 6,
}
impl MirPhase {
@ -311,7 +323,7 @@ impl<'tcx> Body<'tcx> {
);
let mut body = Body {
phase: MirPhase::Build,
phase: MirPhase::Built,
source,
basic_blocks,
source_scopes,
@ -346,7 +358,7 @@ impl<'tcx> Body<'tcx> {
/// crate.
pub fn new_cfg_only(basic_blocks: IndexVec<BasicBlock, BasicBlockData<'tcx>>) -> Self {
let mut body = Body {
phase: MirPhase::Build,
phase: MirPhase::Built,
source: MirSource::item(DefId::local(CRATE_DEF_INDEX)),
basic_blocks,
source_scopes: IndexVec::new(),
@ -1541,6 +1553,11 @@ impl Statement<'_> {
}
}
/// The various kinds of statements that can appear in MIR.
///
/// Not all of these are allowed at every [`MirPhase`]. Check the documentation there to see which
/// ones you do not have to worry about. The MIR validator will generally enforce such restrictions,
/// causing an ICE if they are violated.
#[derive(Clone, Debug, PartialEq, TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable)]
pub enum StatementKind<'tcx> {
/// Write the RHS Rvalue to the LHS Place.
@ -2223,6 +2240,11 @@ impl<'tcx> Operand<'tcx> {
/// Rvalues
#[derive(Clone, TyEncodable, TyDecodable, Hash, HashStable, PartialEq)]
/// The various kinds of rvalues that can appear in MIR.
///
/// Not all of these are allowed at every [`MirPhase`]. Check the documentation there to see which
/// ones you do not have to worry about. The MIR validator will generally enforce such restrictions,
/// causing an ICE if they are violated.
pub enum Rvalue<'tcx> {
/// x (either a move or copy, depending on type of x)
Use(Operand<'tcx>),

View File

@ -6,6 +6,10 @@ use rustc_middle::ty::TyCtxt;
pub struct Deaggregator;
impl<'tcx> MirPass<'tcx> for Deaggregator {
fn phase_change(&self) -> Option<MirPhase> {
Some(MirPhase::Deaggregated)
}
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
let (basic_blocks, local_decls) = body.basic_blocks_and_local_decls_mut();
let local_decls = &*local_decls;

View File

@ -20,7 +20,7 @@ pub struct ElaborateDrops;
impl<'tcx> MirPass<'tcx> for ElaborateDrops {
fn phase_change(&self) -> Option<MirPhase> {
Some(MirPhase::DropLowering)
Some(MirPhase::DropsLowered)
}
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {

View File

@ -1235,7 +1235,7 @@ fn create_cases<'tcx>(
impl<'tcx> MirPass<'tcx> for StateTransform {
fn phase_change(&self) -> Option<MirPhase> {
Some(MirPhase::GeneratorLowering)
Some(MirPhase::GeneratorsLowered)
}
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {

View File

@ -341,7 +341,7 @@ fn inner_mir_for_ctfe(tcx: TyCtxt<'_>, def: ty::WithOptConstParam<LocalDefId>) -
pm::run_passes(
tcx,
&mut body,
&[&const_prop::ConstProp, &marker::PhaseChange(MirPhase::Optimization)],
&[&const_prop::ConstProp, &marker::PhaseChange(MirPhase::Optimized)],
);
}
}
@ -398,7 +398,7 @@ fn mir_drops_elaborated_and_const_checked<'tcx>(
}
run_post_borrowck_cleanup_passes(tcx, &mut body);
assert!(body.phase == MirPhase::DropLowering);
assert!(body.phase == MirPhase::Deaggregated);
tcx.alloc_steal_mir(body)
}
@ -458,7 +458,7 @@ fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
],
);
assert!(body.phase == MirPhase::GeneratorLowering);
assert!(body.phase == MirPhase::GeneratorsLowered);
// The main optimizations that we do on MIR.
pm::run_passes(
@ -495,7 +495,7 @@ 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::Optimization),
&marker::PhaseChange(MirPhase::Optimized),
// Dump the end result for testing and debugging purposes.
&dump_mir::Marker("PreCodegen"),
],

View File

@ -114,7 +114,7 @@ pub fn run_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>, passes: &[&dyn
}
}
if validate || body.phase == MirPhase::Optimization {
if validate || body.phase == MirPhase::Optimized {
validate_body(tcx, body, format!("end of phase transition to {:?}", body.phase));
}
}