Skip to content

Commit

Permalink
Auto merge of rust-lang#99102 - JakobDegen:reorder-generators, r=oli-obk
Browse files Browse the repository at this point in the history 
Rework definition of MIR phases to more closely reflect semantic concerns

Implements most of rust-lang/compiler-team#522 .

I tried my best to restrict this PR to the "core" parts of the MCP. In other words, this includes just enough changes to make the new definition of `MirPhase` make sense. That means there are a couple of FIXMEs lying around. Depending on what reviewers prefer, I can either fix them in this PR or send follow up PRs. There are also a couple other refactorings of the `rustc_mir_transform/src/lib.rs` file that I want to do in follow ups that I didn't leave explicit FIXMEs for.
  • Loading branch information
@bors
bors committed Aug 30, 2022
2 parents 02654a0 + d56751c commit f07d6e8
Show file tree
Hide file tree
Showing 17 changed files with 301 additions and 208 deletions.
5 changes: 3 additions & 2 deletions compiler/rustc_const_eval/src/transform/promote_consts.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ pub struct PromoteTemps<'tcx> {

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

fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
Expand Down Expand Up @@ -964,7 +964,7 @@ pub fn promote_candidates<'tcx>(
let mut scope = body.source_scopes[body.source_info(candidate.location).scope].clone();
scope.parent_scope = None;

let promoted = Body::new(
let mut promoted = Body::new(
body.source, // `promoted` gets filled in below
IndexVec::new(),
IndexVec::from_elem_n(scope, 1),
Expand All @@ -976,6 +976,7 @@ pub fn promote_candidates<'tcx>(
body.generator_kind(),
body.tainted_by_errors,
);
promoted.phase = MirPhase::Analysis(AnalysisPhase::Initial);

let promoter = Promoter {
promoted,
Expand Down
38 changes: 20 additions & 18 deletions compiler/rustc_const_eval/src/transform/validate.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ use rustc_middle::mir::visit::NonUseContext::VarDebugInfo;
use rustc_middle::mir::visit::{PlaceContext, Visitor};
use rustc_middle::mir::{
traversal, AggregateKind, BasicBlock, BinOp, Body, BorrowKind, CastKind, Local, Location,
MirPass, MirPhase, Operand, Place, PlaceElem, PlaceRef, ProjectionElem, Rvalue, SourceScope,
Statement, StatementKind, Terminator, TerminatorKind, UnOp, START_BLOCK,
MirPass, MirPhase, Operand, Place, PlaceElem, PlaceRef, ProjectionElem, RuntimePhase, Rvalue,
SourceScope, Statement, StatementKind, Terminator, TerminatorKind, UnOp, START_BLOCK,
};
use rustc_middle::ty::fold::BottomUpFolder;
use rustc_middle::ty::subst::Subst;
Expand Down Expand Up @@ -221,7 +221,8 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {

fn visit_operand(&mut self, operand: &Operand<'tcx>, location: Location) {
// This check is somewhat expensive, so only run it when -Zvalidate-mir is passed.
if self.tcx.sess.opts.unstable_opts.validate_mir && self.mir_phase < MirPhase::DropsLowered
if self.tcx.sess.opts.unstable_opts.validate_mir
&& self.mir_phase < MirPhase::Runtime(RuntimePhase::Initial)
{
// `Operand::Copy` is only supposed to be used with `Copy` types.
if let Operand::Copy(place) = operand {
Expand Down Expand Up @@ -252,7 +253,9 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
self.fail(location, format!("bad index ({:?} != usize)", index_ty))
}
}
ProjectionElem::Deref if self.mir_phase >= MirPhase::GeneratorsLowered => {
ProjectionElem::Deref
if self.mir_phase >= MirPhase::Runtime(RuntimePhase::PostCleanup) =>
{
let base_ty = Place::ty_from(local, proj_base, &self.body.local_decls, self.tcx).ty;

if base_ty.is_box() {
Expand Down Expand Up @@ -360,7 +363,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
// Set off any `bug!`s in the type computation code
let _ = place.ty(&self.body.local_decls, self.tcx);

if self.mir_phase >= MirPhase::Derefered
if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial)
&& place.projection.len() > 1
&& cntxt != PlaceContext::NonUse(VarDebugInfo)
&& place.projection[1..].contains(&ProjectionElem::Deref)
Expand All @@ -384,8 +387,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
Rvalue::Aggregate(agg_kind, _) => {
let disallowed = match **agg_kind {
AggregateKind::Array(..) => false,
AggregateKind::Generator(..) => self.mir_phase >= MirPhase::GeneratorsLowered,
_ => self.mir_phase >= MirPhase::Deaggregated,
_ => self.mir_phase >= MirPhase::Runtime(RuntimePhase::PostCleanup),
};
if disallowed {
self.fail(
Expand All @@ -395,10 +397,10 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
}
}
Rvalue::Ref(_, BorrowKind::Shallow, _) => {
if self.mir_phase >= MirPhase::DropsLowered {
if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) {
self.fail(
location,
"`Assign` statement with a `Shallow` borrow should have been removed after drop lowering phase",
"`Assign` statement with a `Shallow` borrow should have been removed in runtime MIR",
);
}
}
Expand Down Expand Up @@ -612,15 +614,15 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
}
}
StatementKind::AscribeUserType(..) => {
if self.mir_phase >= MirPhase::DropsLowered {
if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) {
self.fail(
location,
"`AscribeUserType` should have been removed after drop lowering phase",
);
}
}
StatementKind::FakeRead(..) => {
if self.mir_phase >= MirPhase::DropsLowered {
if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) {
self.fail(
location,
"`FakeRead` should have been removed after drop lowering phase",
Expand Down Expand Up @@ -664,7 +666,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
}
}
StatementKind::SetDiscriminant { place, .. } => {
if self.mir_phase < MirPhase::Deaggregated {
if self.mir_phase < MirPhase::Runtime(RuntimePhase::Initial) {
self.fail(location, "`SetDiscriminant`is not allowed until deaggregation");
}
let pty = place.ty(&self.body.local_decls, self.tcx).ty.kind();
Expand All @@ -679,7 +681,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
}
}
StatementKind::Deinit(..) => {
if self.mir_phase < MirPhase::Deaggregated {
if self.mir_phase < MirPhase::Runtime(RuntimePhase::Initial) {
self.fail(location, "`Deinit`is not allowed until deaggregation");
}
}
Expand Down Expand Up @@ -759,7 +761,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
}
}
TerminatorKind::DropAndReplace { target, unwind, .. } => {
if self.mir_phase >= MirPhase::DropsLowered {
if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) {
self.fail(
location,
"`DropAndReplace` should have been removed during drop elaboration",
Expand Down Expand Up @@ -830,7 +832,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
if self.body.generator.is_none() {
self.fail(location, "`Yield` cannot appear outside generator bodies");
}
if self.mir_phase >= MirPhase::GeneratorsLowered {
if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) {
self.fail(location, "`Yield` should have been replaced by generator lowering");
}
self.check_edge(location, *resume, EdgeKind::Normal);
Expand All @@ -839,7 +841,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
}
}
TerminatorKind::FalseEdge { real_target, imaginary_target } => {
if self.mir_phase >= MirPhase::DropsLowered {
if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) {
self.fail(
location,
"`FalseEdge` should have been removed after drop elaboration",
Expand All @@ -849,7 +851,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
self.check_edge(location, *imaginary_target, EdgeKind::Normal);
}
TerminatorKind::FalseUnwind { real_target, unwind } => {
if self.mir_phase >= MirPhase::DropsLowered {
if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) {
self.fail(
location,
"`FalseUnwind` should have been removed after drop elaboration",
Expand All @@ -872,7 +874,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
if self.body.generator.is_none() {
self.fail(location, "`GeneratorDrop` cannot appear outside generator bodies");
}
if self.mir_phase >= MirPhase::GeneratorsLowered {
if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) {
self.fail(
location,
"`GeneratorDrop` should have been replaced by generator lowering",
Expand Down
14 changes: 13 additions & 1 deletion compiler/rustc_middle/src/mir/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,20 @@ pub trait MirPass<'tcx> {

impl MirPhase {
/// Gets the index of the current MirPhase within the set of all `MirPhase`s.
///
/// FIXME(JakobDegen): Return a `(usize, usize)` instead.
pub fn phase_index(&self) -> usize {
*self as usize
const BUILT_PHASE_COUNT: usize = 1;
const ANALYSIS_PHASE_COUNT: usize = 2;
match self {
MirPhase::Built => 1,
MirPhase::Analysis(analysis_phase) => {
1 + BUILT_PHASE_COUNT + (*analysis_phase as usize)
}
MirPhase::Runtime(runtime_phase) => {
1 + BUILT_PHASE_COUNT + ANALYSIS_PHASE_COUNT + (*runtime_phase as usize)
}
}
}
}

Expand Down
143 changes: 89 additions & 54 deletions compiler/rustc_middle/src/mir/syntax.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,75 +23,110 @@ use rustc_span::symbol::Symbol;
use rustc_span::Span;
use rustc_target::asm::InlineAsmRegOrRegClass;

/// The various "big phases" that MIR goes through.
/// Represents the "flavors" of MIR.
///
/// These phases all describe dialects of MIR. Since all MIR uses the same datastructures, the
/// dialects forbid certain variants or values in certain phases. The sections below summarize the
/// changes, but do not document them thoroughly. The full documentation is found in the appropriate
/// documentation for the thing the change is affecting.
/// All flavors of MIR use the same data structure, but there are some important differences. These
/// differences come in two forms: Dialects and phases.
///
/// Warning: ordering of variants is significant.
/// Dialects represent a stronger distinction than phases. This is because the transitions between
/// dialects are semantic changes, and therefore technically *lowerings* between distinct IRs. In
/// other words, the same [`Body`](crate::mir::Body) might be well-formed for multiple dialects, but
/// have different semantic meaning and different behavior at runtime.
///
/// Each dialect additionally has a number of phases. However, phase changes never involve semantic
/// changes. If some MIR is well-formed both before and after a phase change, it is also guaranteed
/// that it has the same semantic meaning. In this sense, phase changes can only add additional
/// restrictions on what MIR is well-formed.
///
/// When adding phases, remember to update [`MirPhase::phase_index`].
#[derive(Copy, Clone, TyEncodable, TyDecodable, Debug, PartialEq, Eq, PartialOrd, Ord)]
#[derive(HashStable)]
pub enum MirPhase {
/// The dialect of MIR used during all phases before `DropsLowered` is the same. This is also
/// the MIR that analysis such as borrowck uses.
///
/// One important thing to remember about the behavior of this section of MIR is that drop terminators
/// (including drop and replace) are *conditional*. The elaborate drops pass will then replace each
/// instance of a drop terminator with a nop, an unconditional drop, or a drop conditioned on a drop
/// flag. Of course, this means that it is important that the drop elaboration can accurately recognize
/// when things are initialized and when things are de-initialized. That means any code running on this
/// version of MIR must be sure to produce output that drop elaboration can reason about. See the
/// section on the drop terminatorss for more details.
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,
/// This phase checks the MIR for promotable elements and takes them out of the main MIR body
/// 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.
ConstsPromoted = 2,
/// After this projections may only contain deref projections as the first element.
Derefered = 3,
/// Beginning with this phase, the following variants are disallowed:
/// * [`TerminatorKind::DropAndReplace`]
/// The MIR that is generated by MIR building.
///
/// The only things that operate on this dialect are unsafeck, the various MIR lints, and const
/// qualifs.
///
/// This has no distinct phases.
Built,
/// The MIR used for most analysis.
///
/// The only semantic change between analysis and built MIR is constant promotion. In built MIR,
/// sequences of statements that would generally be subject to constant promotion are
/// semantically constants, while in analysis MIR all constants are explicit.
///
/// The result of const promotion is available from the `mir_promoted` and `promoted_mir` queries.
///
/// This is the version of MIR used by borrowck and friends.
Analysis(AnalysisPhase),
/// The MIR used for CTFE, optimizations, and codegen.
///
/// The semantic changes that occur in the lowering from analysis to runtime MIR are as follows:
///
/// - Drops: In analysis MIR, `Drop` terminators represent *conditional* drops; roughly speaking,
/// if dataflow analysis determines that the place being dropped is uninitialized, the drop will
/// not be executed. The exact semantics of this aren't written down anywhere, which means they
/// are essentially "what drop elaboration does." In runtime MIR, the drops are unconditional;
/// when a `Drop` terminator is reached, if the type has drop glue that drop glue is always
/// executed. This may be UB if the underlying place is not initialized.
/// - Packed drops: Places might in general be misaligned - in most cases this is UB, the exception
/// is fields of packed structs. In analysis MIR, `Drop(P)` for a `P` that might be misaligned
/// for this reason implicitly moves `P` to a temporary before dropping. Runtime MIR has no such
/// rules, and dropping a misaligned place is simply UB.
/// - Unwinding: in analysis MIR, unwinding from a function which may not unwind aborts. In runtime
/// MIR, this is UB.
/// - Retags: If `-Zmir-emit-retag` is enabled, analysis MIR has "implicit" retags in the same way
/// that Rust itself has them. Where exactly these are is generally subject to change, and so we
/// don't document this here. Runtime MIR has all retags explicit.
/// - Generator bodies: In analysis MIR, locals may actually be behind a pointer that user code has
/// access to. This occurs in generator bodies. Such locals do not behave like other locals,
/// because they eg may be aliased in surprising ways. Runtime MIR has no such special locals -
/// all generator bodies are lowered and so all places that look like locals really are locals.
/// - Const prop lints: The lint pass which reports eg `200_u8 + 200_u8` as an error is run as a
/// part of analysis to runtime MIR lowering. This means that transformations which may supress
/// such errors may not run on analysis MIR.
Runtime(RuntimePhase),
}

/// See [`MirPhase::Analysis`].
#[derive(Copy, Clone, TyEncodable, TyDecodable, Debug, PartialEq, Eq, PartialOrd, Ord)]
#[derive(HashStable)]
pub enum AnalysisPhase {
Initial = 0,
/// Beginning in this phase, the following variants are disallowed:
/// * [`TerminatorKind::FalseUnwind`]
/// * [`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. Also, `Copy` operands
/// are allowed for non-`Copy` types.
DropsLowered = 4,
/// Beginning with this phase, the following variant is disallowed:
/// Furthermore, `Deref` projections must be the first projection within any place (if they
/// appear at all)
PostCleanup = 1,
}

/// See [`MirPhase::Runtime`].
#[derive(Copy, Clone, TyEncodable, TyDecodable, Debug, PartialEq, Eq, PartialOrd, Ord)]
#[derive(HashStable)]
pub enum RuntimePhase {
/// In addition to the semantic changes, beginning with this phase, the following variants are
/// disallowed:
/// * [`TerminatorKind::DropAndReplace`]
/// * [`TerminatorKind::Yield`]
/// * [`TerminatorKind::GeneratorDrop`]
/// * [`Rvalue::Aggregate`] for any `AggregateKind` except `Array`
///
/// And the following variant is allowed:
/// And the following variants are allowed:
/// * [`StatementKind::Retag`]
/// * [`StatementKind::SetDiscriminant`]
Deaggregated = 5,
/// Before this phase, generators are in the "source code" form, featuring `yield` statements
/// and such. With this phase change, they are transformed into a proper state machine. Running
/// optimizations before this change can be potentially dangerous because the source code is to
/// some extent a "lie." In particular, `yield` terminators effectively make the value of all
/// locals visible to the caller. This means that dead store elimination before them, or code
/// motion across them, is not correct in general. This is also exasperated by type checking
/// having pre-computed a list of the types that it thinks are ok to be live across a yield
/// point - this is necessary to decide eg whether autotraits are implemented. Introducing new
/// types across a yield point will lead to ICEs becaues of this.
///
/// Beginning with this phase, the following variants are disallowed:
/// * [`TerminatorKind::Yield`]
/// * [`TerminatorKind::GeneratorDrop`]
/// * [`StatementKind::Deinit`]
///
/// Furthermore, `Copy` operands are allowed for non-`Copy` types.
Initial = 0,
/// Beginning with this phase, the following variant is disallowed:
/// * [`ProjectionElem::Deref`] of `Box`
GeneratorsLowered = 6,
Optimized = 7,
PostCleanup = 1,
Optimized = 2,
}

///////////////////////////////////////////////////////////////////////////
Expand Down
4 changes: 0 additions & 4 deletions compiler/rustc_mir_transform/src/deaggregator.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@ 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 = body.basic_blocks.as_mut_preserves_cfg();
for bb in basic_blocks {
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_mir_transform/src/deref_separator.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,5 @@ pub fn deref_finder<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
impl<'tcx> MirPass<'tcx> for Derefer {
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
deref_finder(tcx, body);
body.phase = MirPhase::Derefered;
}
}
Loading

0 comments on commit f07d6e8

Please sign in to comment.