1
Fork 0

Rollup merge of #94655 - JakobDegen:mir-phase-docs, r=oli-obk

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. Most of these conditions were already being checked.

There was also some disagreement between the `MirPhase` docs and validator as to what it meant for the `body.phase` field to have a certain value. This PR resolves those disagreements in favor of the `MirPhase` docs (which is what the pass manager implemented), adjusting the validator accordingly. The result is now that the `DropLowering` phase begins with the end of the elaborate drops pass, and lasts until the beginning of the generator lowring pass. This doesn't feel entirely natural to me, but as long as it's documented accurately it should be ok.

r? rust-lang/mir-opt
This commit is contained in:
Dylan DPC 2022-03-25 01:34:29 +01:00 committed by GitHub
commit c66e0c8726
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 113 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);