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.

Sign up for GitHub

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

mir validator: don't store mir phase #133749

Merged
merged 1 commit into from
Dec 2, 2024
Merged

mir validator: don't store mir phase #133749

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 1 addition & 9 deletions compiler/rustc_mir_transform/src/inline.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,15 +220,7 @@ impl<'tcx> Inliner<'tcx> {

// Normally, this shouldn't be required, but trait normalization failure can create a
// validation ICE.
if !validate_types(
self.tcx,
MirPhase::Runtime(RuntimePhase::Optimized),
self.typing_env,
&callee_body,
&caller_body,
)
.is_empty()
{
if !validate_types(self.tcx, self.typing_env, &callee_body, &caller_body).is_empty() {
return Err("failed to validate callee body");
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/pass_manager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ fn run_passes_inner<'tcx>(
}

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

fn dump_mir_for_pass<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, pass_name: &str, is_after: bool) {
Expand Down
79 changes: 32 additions & 47 deletions compiler/rustc_mir_transform/src/validate.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,6 @@ enum EdgeKind {
pub(super) struct Validator {
/// Describes at which point in the pipeline this validation is happening.
pub when: String,
/// The phase for which we are upholding the dialect. If the given phase forbids a specific
/// element, this validator will now emit errors if that specific element is encountered.
/// Note that phases that change the dialect cause all *following* phases to check the
/// invariants of the new dialect. A phase that changes dialects never checks the new invariants
/// itself.
pub mir_phase: MirPhase,
}

impl<'tcx> crate::MirPass<'tcx> for Validator {
Expand All @@ -46,11 +40,9 @@ impl<'tcx> crate::MirPass<'tcx> for Validator {
if matches!(body.source.instance, InstanceKind::Intrinsic(..) | InstanceKind::Virtual(..)) {
return;
}
debug_assert_eq!(self.mir_phase, body.phase);
let def_id = body.source.def_id();
let mir_phase = body.phase;
let typing_env = body.typing_env(tcx);
let can_unwind = if mir_phase <= MirPhase::Runtime(RuntimePhase::Initial) {
let can_unwind = if body.phase <= MirPhase::Runtime(RuntimePhase::Initial) {
// In this case `AbortUnwindingCalls` haven't yet been executed.
true
} else if !tcx.def_kind(def_id).is_fn_like() {
Expand All @@ -64,9 +56,7 @@ impl<'tcx> crate::MirPass<'tcx> for Validator {
ty::Coroutine(..) => ExternAbi::Rust,
// No need to do MIR validation on error bodies
ty::Error(_) => return,
_ => {
span_bug!(body.span, "unexpected body ty: {:?} phase {:?}", body_ty, mir_phase)
}
_ => span_bug!(body.span, "unexpected body ty: {body_ty:?}"),
};

ty::layout::fn_can_unwind(tcx, Some(def_id), body_abi)
Expand All @@ -76,7 +66,6 @@ impl<'tcx> crate::MirPass<'tcx> for Validator {
when: &self.when,
body,
tcx,
mir_phase,
unwind_edge_count: 0,
reachable_blocks: traversal::reachable_as_bitset(body),
value_cache: FxHashSet::default(),
Expand All @@ -86,7 +75,7 @@ impl<'tcx> crate::MirPass<'tcx> for Validator {
cfg_checker.check_cleanup_control_flow();

// Also run the TypeChecker.
for (location, msg) in validate_types(tcx, self.mir_phase, typing_env, body, body) {
for (location, msg) in validate_types(tcx, typing_env, body, body) {
cfg_checker.fail(location, msg);
}

Expand All @@ -107,7 +96,6 @@ struct CfgChecker<'a, 'tcx> {
when: &'a str,
body: &'a Body<'tcx>,
tcx: TyCtxt<'tcx>,
mir_phase: MirPhase,
unwind_edge_count: usize,
reachable_blocks: BitSet<BasicBlock>,
value_cache: FxHashSet<u128>,
Expand Down Expand Up @@ -294,41 +282,41 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> {
fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) {
match &statement.kind {
StatementKind::AscribeUserType(..) => {
if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) {
if self.body.phase >= MirPhase::Runtime(RuntimePhase::Initial) {
self.fail(
location,
"`AscribeUserType` should have been removed after drop lowering phase",
);
}
}
StatementKind::FakeRead(..) => {
if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) {
if self.body.phase >= MirPhase::Runtime(RuntimePhase::Initial) {
self.fail(
location,
"`FakeRead` should have been removed after drop lowering phase",
);
}
}
StatementKind::SetDiscriminant { .. } => {
if self.mir_phase < MirPhase::Runtime(RuntimePhase::Initial) {
if self.body.phase < MirPhase::Runtime(RuntimePhase::Initial) {
self.fail(location, "`SetDiscriminant`is not allowed until deaggregation");
}
}
StatementKind::Deinit(..) => {
if self.mir_phase < MirPhase::Runtime(RuntimePhase::Initial) {
if self.body.phase < MirPhase::Runtime(RuntimePhase::Initial) {
self.fail(location, "`Deinit`is not allowed until deaggregation");
}
}
StatementKind::Retag(kind, _) => {
// FIXME(JakobDegen) The validator should check that `self.mir_phase <
// FIXME(JakobDegen) The validator should check that `self.body.phase <
// DropsLowered`. However, this causes ICEs with generation of drop shims, which
// seem to fail to set their `MirPhase` correctly.
if matches!(kind, RetagKind::TwoPhase) {
self.fail(location, format!("explicit `{kind:?}` is forbidden"));
}
}
StatementKind::Coverage(kind) => {
if self.mir_phase >= MirPhase::Analysis(AnalysisPhase::PostCleanup)
if self.body.phase >= MirPhase::Analysis(AnalysisPhase::PostCleanup)
&& let CoverageKind::BlockMarker { .. } | CoverageKind::SpanMarker { .. } = kind
{
self.fail(
Expand Down Expand Up @@ -391,7 +379,7 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> {
// the return edge from the call. FIXME(tmiasko): Since this is a strictly code
// generation concern, the code generation should be responsible for handling
// it.
if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Optimized)
if self.body.phase >= MirPhase::Runtime(RuntimePhase::Optimized)
&& self.is_critical_call_edge(target, unwind)
{
self.fail(
Expand Down Expand Up @@ -440,7 +428,7 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> {
if self.body.coroutine.is_none() {
self.fail(location, "`Yield` cannot appear outside coroutine bodies");
}
if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) {
if self.body.phase >= MirPhase::Runtime(RuntimePhase::Initial) {
self.fail(location, "`Yield` should have been replaced by coroutine lowering");
}
self.check_edge(location, *resume, EdgeKind::Normal);
Expand All @@ -449,7 +437,7 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> {
}
}
TerminatorKind::FalseEdge { real_target, imaginary_target } => {
if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) {
if self.body.phase >= MirPhase::Runtime(RuntimePhase::Initial) {
self.fail(
location,
"`FalseEdge` should have been removed after drop elaboration",
Expand All @@ -459,7 +447,7 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> {
self.check_edge(location, *imaginary_target, EdgeKind::Normal);
}
TerminatorKind::FalseUnwind { real_target, unwind } => {
if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) {
if self.body.phase >= MirPhase::Runtime(RuntimePhase::Initial) {
self.fail(
location,
"`FalseUnwind` should have been removed after drop elaboration",
Expand All @@ -478,7 +466,7 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> {
if self.body.coroutine.is_none() {
self.fail(location, "`CoroutineDrop` cannot appear outside coroutine bodies");
}
if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) {
if self.body.phase >= MirPhase::Runtime(RuntimePhase::Initial) {
self.fail(
location,
"`CoroutineDrop` should have been replaced by coroutine lowering",
Expand Down Expand Up @@ -532,13 +520,11 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> {
/// `optimized_mir` is available.
pub(super) fn validate_types<'tcx>(
tcx: TyCtxt<'tcx>,
mir_phase: MirPhase,
typing_env: ty::TypingEnv<'tcx>,
body: &Body<'tcx>,
caller_body: &Body<'tcx>,
) -> Vec<(Location, String)> {
let mut type_checker =
TypeChecker { body, caller_body, tcx, typing_env, mir_phase, failures: Vec::new() };
let mut type_checker = TypeChecker { body, caller_body, tcx, typing_env, failures: Vec::new() };
type_checker.visit_body(body);
type_checker.failures
}
Expand All @@ -548,7 +534,6 @@ struct TypeChecker<'a, 'tcx> {
caller_body: &'a Body<'tcx>,
tcx: TyCtxt<'tcx>,
typing_env: ty::TypingEnv<'tcx>,
mir_phase: MirPhase,
failures: Vec<(Location, String)>,
}

Expand Down Expand Up @@ -577,7 +562,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {

// After borrowck subtyping should be fully explicit via
// `Subtype` projections.
let variance = if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) {
let variance = if self.body.phase >= MirPhase::Runtime(RuntimePhase::Initial) {
Variance::Invariant
} else {
Variance::Covariant
Expand Down Expand Up @@ -618,7 +603,7 @@ 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::Runtime(RuntimePhase::Initial)
&& self.body.phase < MirPhase::Runtime(RuntimePhase::Initial)
{
// `Operand::Copy` is only supposed to be used with `Copy` types.
if let Operand::Copy(place) = operand {
Expand All @@ -642,7 +627,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
) {
match elem {
ProjectionElem::OpaqueCast(ty)
if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) =>
if self.body.phase >= MirPhase::Runtime(RuntimePhase::Initial) =>
{
self.fail(
location,
Expand All @@ -656,7 +641,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
}
}
ProjectionElem::Deref
if self.mir_phase >= MirPhase::Runtime(RuntimePhase::PostCleanup) =>
if self.body.phase >= MirPhase::Runtime(RuntimePhase::PostCleanup) =>
{
let base_ty = place_ref.ty(&self.body.local_decls, self.tcx).ty;

Expand Down Expand Up @@ -856,7 +841,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::Runtime(RuntimePhase::Initial)
if self.body.phase >= MirPhase::Runtime(RuntimePhase::Initial)
&& place.projection.len() > 1
&& cntxt != PlaceContext::NonUse(NonUseContext::VarDebugInfo)
&& place.projection[1..].contains(&ProjectionElem::Deref)
Expand Down Expand Up @@ -974,7 +959,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
}
}
AggregateKind::RawPtr(pointee_ty, mutability) => {
if !matches!(self.mir_phase, MirPhase::Runtime(_)) {
if !matches!(self.body.phase, MirPhase::Runtime(_)) {
// It would probably be fine to support this in earlier phases, but at the
// time of writing it's only ever introduced from intrinsic lowering, so
// earlier things just `bug!` on it.
Expand Down Expand Up @@ -1016,7 +1001,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
}
},
Rvalue::Ref(_, BorrowKind::Fake(_), _) => {
if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) {
if self.body.phase >= MirPhase::Runtime(RuntimePhase::Initial) {
self.fail(
location,
"`Assign` statement with a `Fake` borrow should have been removed in runtime MIR",
Expand Down Expand Up @@ -1130,7 +1115,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
);
}
UnOp::PtrMetadata => {
if !matches!(self.mir_phase, MirPhase::Runtime(_)) {
if !matches!(self.body.phase, MirPhase::Runtime(_)) {
// It would probably be fine to support this in earlier phases, but at
// the time of writing it's only ever introduced from intrinsic
// lowering or other runtime-phase optimization passes, so earlier
Expand Down Expand Up @@ -1206,7 +1191,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
"CastKind::{kind:?} output must be a raw const pointer, not {:?}",
ty::RawPtr(_, Mutability::Not)
);
if self.mir_phase >= MirPhase::Analysis(AnalysisPhase::PostCleanup) {
if self.body.phase >= MirPhase::Analysis(AnalysisPhase::PostCleanup) {
self.fail(location, format!("After borrowck, MIR disallows {kind:?}"));
}
}
Expand All @@ -1222,7 +1207,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
"CastKind::{kind:?} output must be a raw pointer, not {:?}",
ty::RawPtr(..)
);
if self.mir_phase >= MirPhase::Analysis(AnalysisPhase::PostCleanup) {
if self.body.phase >= MirPhase::Analysis(AnalysisPhase::PostCleanup) {
self.fail(location, format!("After borrowck, MIR disallows {kind:?}"));
}
}
Expand Down Expand Up @@ -1288,7 +1273,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
}
}
CastKind::Transmute => {
if let MirPhase::Runtime(..) = self.mir_phase {
if let MirPhase::Runtime(..) = self.body.phase {
// Unlike `mem::transmute`, a MIR `Transmute` is well-formed
// for any two `Sized` types, just potentially UB to run.

Expand Down Expand Up @@ -1317,7 +1302,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
location,
format!(
"Transmute is not supported in non-runtime phase {:?}.",
self.mir_phase
self.body.phase
),
);
}
Expand Down Expand Up @@ -1404,15 +1389,15 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
}
}
StatementKind::AscribeUserType(..) => {
if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) {
if self.body.phase >= MirPhase::Runtime(RuntimePhase::Initial) {
self.fail(
location,
"`AscribeUserType` should have been removed after drop lowering phase",
);
}
}
StatementKind::FakeRead(..) => {
if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) {
if self.body.phase >= MirPhase::Runtime(RuntimePhase::Initial) {
self.fail(
location,
"`FakeRead` should have been removed after drop lowering phase",
Expand Down Expand Up @@ -1463,7 +1448,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
}
}
StatementKind::SetDiscriminant { place, .. } => {
if self.mir_phase < MirPhase::Runtime(RuntimePhase::Initial) {
if self.body.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 @@ -1477,12 +1462,12 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
}
}
StatementKind::Deinit(..) => {
if self.mir_phase < MirPhase::Runtime(RuntimePhase::Initial) {
if self.body.phase < MirPhase::Runtime(RuntimePhase::Initial) {
self.fail(location, "`Deinit`is not allowed until deaggregation");
}
}
StatementKind::Retag(kind, _) => {
// FIXME(JakobDegen) The validator should check that `self.mir_phase <
// FIXME(JakobDegen) The validator should check that `self.body.phase <
// DropsLowered`. However, this causes ICEs with generation of drop shims, which
// seem to fail to set their `MirPhase` correctly.
if matches!(kind, RetagKind::TwoPhase) {
Expand Down
Loading