Auto merge of #145513 - beepster4096:erasedereftemps, r=saethlin,cjgillot

Validate CopyForDeref and DerefTemps better and remove them from runtime MIR

(split from my WIP rust-lang/rust#145344)

This PR:
- Removes `Rvalue::CopyForDeref` and `LocalInfo::DerefTemp` from runtime MIR
    - Using a new mir pass `EraseDerefTemps`
    - `CopyForDeref(x)` is turned into `Use(Copy(x))`
    - `DerefTemp` is turned into `Boring`
        - Not sure if this part is actually necessary, it made more sense in rust-lang/rust#145344 with `DerefTemp` storing actual data that I wanted to keep from having to be kept in sync with the rest of the body in runtime MIR
- Checks in validation that `CopyForDeref` and `DerefTemp` are only used together
- Removes special handling for `CopyForDeref` from many places
- Removes `CopyForDeref` from `custom_mir` reverting rust-lang/rust#111587
    - In runtime MIR simple copies can be used instead
    - In post cleanup analysis MIR it was already wrong to use due to the lack of support for creating `DerefTemp` locals
    - Possibly this should be its own PR?
 - Adds an argument to `deref_finder` to avoid creating new `DerefTemp`s and `CopyForDeref` in runtime MIR.
     - Ideally we would just avoid making intermediate derefs instead of fixing it at the end of a pass / during shim building
 - Removes some usages of `deref_finder` that I found out don't actually do anything

r? oli-obk
This commit is contained in:
bors
2025-10-12 02:34:20 +00:00
55 changed files with 418 additions and 407 deletions

View File

@@ -74,9 +74,7 @@ fn fully_moved_locals(ssa: &SsaLocals, body: &Body<'_>) -> DenseBitSet<Local> {
let mut fully_moved = DenseBitSet::new_filled(body.local_decls.len());
for (_, rvalue, _) in ssa.assignments(body) {
let (Rvalue::Use(Operand::Copy(place) | Operand::Move(place))
| Rvalue::CopyForDeref(place)) = rvalue
else {
let Rvalue::Use(Operand::Copy(place) | Operand::Move(place)) = rvalue else {
continue;
};
@@ -85,7 +83,7 @@ fn fully_moved_locals(ssa: &SsaLocals, body: &Body<'_>) -> DenseBitSet<Local> {
continue;
}
if let Rvalue::Use(Operand::Copy(_)) | Rvalue::CopyForDeref(_) = rvalue {
if let Rvalue::Use(Operand::Copy(_)) = rvalue {
fully_moved.remove(rhs);
}
}
@@ -146,8 +144,7 @@ impl<'tcx> MutVisitor<'tcx> for Replacer<'_, 'tcx> {
// Do not leave tautological assignments around.
if let StatementKind::Assign(box (lhs, ref rhs)) = stmt.kind
&& let Rvalue::Use(Operand::Copy(rhs) | Operand::Move(rhs)) | Rvalue::CopyForDeref(rhs) =
*rhs
&& let Rvalue::Use(Operand::Copy(rhs) | Operand::Move(rhs)) = *rhs
&& lhs == rhs
{
stmt.make_nop(true);

View File

@@ -1625,19 +1625,19 @@ impl<'tcx> crate::MirPass<'tcx> for StateTransform {
let mut drop_shim =
create_coroutine_drop_shim_async(tcx, &transform, body, drop_clean, can_unwind);
// Run derefer to fix Derefs that are not in the first place
deref_finder(tcx, &mut drop_shim);
deref_finder(tcx, &mut drop_shim, false);
body.coroutine.as_mut().unwrap().coroutine_drop_async = Some(drop_shim);
} else {
// If coroutine has no async drops, generating sync drop shim
let mut drop_shim =
create_coroutine_drop_shim(tcx, &transform, coroutine_ty, body, drop_clean);
// Run derefer to fix Derefs that are not in the first place
deref_finder(tcx, &mut drop_shim);
deref_finder(tcx, &mut drop_shim, false);
body.coroutine.as_mut().unwrap().coroutine_drop = Some(drop_shim);
// For coroutine with sync drop, generating async proxy for `future_drop_poll` call
let mut proxy_shim = create_coroutine_drop_shim_proxy_async(tcx, body);
deref_finder(tcx, &mut proxy_shim);
deref_finder(tcx, &mut proxy_shim, false);
body.coroutine.as_mut().unwrap().coroutine_drop_proxy_async = Some(proxy_shim);
}
@@ -1645,7 +1645,7 @@ impl<'tcx> crate::MirPass<'tcx> for StateTransform {
create_coroutine_resume_function(tcx, transform, body, can_return, can_unwind);
// Run derefer to fix Derefs that are not in the first place
deref_finder(tcx, body);
deref_finder(tcx, body, false);
}
fn is_required(&self) -> bool {

View File

@@ -305,12 +305,7 @@ impl<'a, 'tcx> ConstAnalysis<'a, 'tcx> {
self.assign_operand(state, target, operand);
}
}
Rvalue::CopyForDeref(rhs) => {
state.flood(target.as_ref(), &self.map);
if let Some(target) = self.map.find(target.as_ref()) {
self.assign_operand(state, target, &Operand::Copy(*rhs));
}
}
Rvalue::CopyForDeref(_) => bug!("`CopyForDeref` in runtime MIR"),
Rvalue::Aggregate(kind, operands) => {
// If we assign `target = Enum::Variant#0(operand)`,
// we must make sure that all `target as Variant#i` are `Top`.
@@ -484,9 +479,7 @@ impl<'a, 'tcx> ConstAnalysis<'a, 'tcx> {
}
Rvalue::Discriminant(place) => state.get_discr(place.as_ref(), &self.map),
Rvalue::Use(operand) => return self.handle_operand(operand, state),
Rvalue::CopyForDeref(place) => {
return self.handle_operand(&Operand::Copy(*place), state);
}
Rvalue::CopyForDeref(_) => bug!("`CopyForDeref` in runtime MIR"),
Rvalue::Ref(..) | Rvalue::RawPtr(..) => {
// We don't track such places.
return ValueOrPlace::TOP;

View File

@@ -11,6 +11,7 @@ struct DerefChecker<'a, 'tcx> {
tcx: TyCtxt<'tcx>,
patcher: MirPatch<'tcx>,
local_decls: &'a LocalDecls<'tcx>,
add_deref_metadata: bool,
}
impl<'a, 'tcx> MutVisitor<'tcx> for DerefChecker<'a, 'tcx> {
@@ -39,7 +40,11 @@ impl<'a, 'tcx> MutVisitor<'tcx> for DerefChecker<'a, 'tcx> {
let temp = self.patcher.new_local_with_info(
ty,
self.local_decls[p_ref.local].source_info.span,
LocalInfo::DerefTemp,
if self.add_deref_metadata {
LocalInfo::DerefTemp
} else {
LocalInfo::Boring
},
);
// We are adding current p_ref's projections to our
@@ -50,7 +55,11 @@ impl<'a, 'tcx> MutVisitor<'tcx> for DerefChecker<'a, 'tcx> {
self.patcher.add_assign(
loc,
Place::from(temp),
Rvalue::CopyForDeref(deref_place),
if self.add_deref_metadata {
Rvalue::CopyForDeref(deref_place)
} else {
Rvalue::Use(Operand::Copy(deref_place))
},
);
place_local = temp;
last_len = p_ref.projection.len();
@@ -67,9 +76,14 @@ impl<'a, 'tcx> MutVisitor<'tcx> for DerefChecker<'a, 'tcx> {
}
}
pub(super) fn deref_finder<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
pub(super) fn deref_finder<'tcx>(
tcx: TyCtxt<'tcx>,
body: &mut Body<'tcx>,
add_deref_metadata: bool,
) {
let patch = MirPatch::new(body);
let mut checker = DerefChecker { tcx, patcher: patch, local_decls: &body.local_decls };
let mut checker =
DerefChecker { tcx, patcher: patch, local_decls: &body.local_decls, add_deref_metadata };
for (bb, data) in body.basic_blocks.as_mut_preserves_cfg().iter_enumerated_mut() {
checker.visit_basic_block_data(bb, data);
@@ -80,7 +94,7 @@ pub(super) fn deref_finder<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
impl<'tcx> crate::MirPass<'tcx> for Derefer {
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
deref_finder(tcx, body);
deref_finder(tcx, body, true);
}
fn is_required(&self) -> bool {

View File

@@ -284,8 +284,7 @@ impl<'tcx> MutVisitor<'tcx> for Merger<'tcx> {
match &statement.kind {
StatementKind::Assign(box (dest, rvalue)) => {
match rvalue {
Rvalue::CopyForDeref(place)
| Rvalue::Use(Operand::Copy(place) | Operand::Move(place)) => {
Rvalue::Use(Operand::Copy(place) | Operand::Move(place)) => {
// These might've been turned into self-assignments by the replacement
// (this includes the original statement we wanted to eliminate).
if dest == place {
@@ -399,7 +398,7 @@ impl<'tcx> Visitor<'tcx> for FindAssignments<'_, 'tcx> {
fn visit_statement(&mut self, statement: &Statement<'tcx>, _: Location) {
if let StatementKind::Assign(box (
lhs,
Rvalue::CopyForDeref(rhs) | Rvalue::Use(Operand::Copy(rhs) | Operand::Move(rhs)),
Rvalue::Use(Operand::Copy(rhs) | Operand::Move(rhs)),
)) = &statement.kind
&& let Some(src) = lhs.as_local()
&& let Some(dest) = rhs.as_local()

View File

@@ -14,7 +14,6 @@ use rustc_mir_dataflow::{
use rustc_span::Span;
use tracing::{debug, instrument};
use crate::deref_separator::deref_finder;
use crate::elaborate_drop::{DropElaborator, DropFlagMode, DropStyle, Unwind, elaborate_drop};
use crate::patch::MirPatch;
@@ -87,7 +86,6 @@ impl<'tcx> crate::MirPass<'tcx> for ElaborateDrops {
.elaborate()
};
elaborate_patch.apply(body);
deref_finder(tcx, body);
}
fn is_required(&self) -> bool {

View File

@@ -0,0 +1,41 @@
//! This pass converts all `DerefTemp` locals into normal temporaries
//! and turns their `CopyForDeref` rvalues into normal copies.
use rustc_middle::mir::visit::MutVisitor;
use rustc_middle::mir::*;
use rustc_middle::ty::TyCtxt;
struct EraseDerefTempsVisitor<'tcx> {
tcx: TyCtxt<'tcx>,
}
impl<'tcx> MutVisitor<'tcx> for EraseDerefTempsVisitor<'tcx> {
fn tcx(&self) -> TyCtxt<'tcx> {
self.tcx
}
fn visit_rvalue(&mut self, rvalue: &mut Rvalue<'tcx>, _: Location) {
if let &mut Rvalue::CopyForDeref(place) = rvalue {
*rvalue = Rvalue::Use(Operand::Copy(place))
}
}
fn visit_local_decl(&mut self, _: Local, local_decl: &mut LocalDecl<'tcx>) {
if local_decl.is_deref_temp() {
let info = local_decl.local_info.as_mut().unwrap_crate_local();
**info = LocalInfo::Boring;
}
}
}
pub(super) struct EraseDerefTemps;
impl<'tcx> crate::MirPass<'tcx> for EraseDerefTemps {
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
EraseDerefTempsVisitor { tcx }.visit_body_preserves_cfg(body);
}
fn is_required(&self) -> bool {
true
}
}

View File

@@ -1027,12 +1027,6 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
let value = match *rvalue {
// Forward values.
Rvalue::Use(ref mut operand) => return self.simplify_operand(operand, location),
Rvalue::CopyForDeref(place) => {
let mut operand = Operand::Copy(place);
let val = self.simplify_operand(&mut operand, location);
*rvalue = Rvalue::Use(operand);
return val;
}
// Roots.
Rvalue::Repeat(ref mut op, amount) => {
@@ -1074,6 +1068,7 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
// Unsupported values.
Rvalue::ThreadLocalRef(..) | Rvalue::ShallowInitBox(..) => return None,
Rvalue::CopyForDeref(_) => bug!("`CopyForDeref` in runtime MIR"),
};
let ty = rvalue.ty(self.local_decls, self.tcx);
Some(self.insert(ty, value))

View File

@@ -20,7 +20,6 @@ use rustc_span::source_map::Spanned;
use tracing::{debug, instrument, trace, trace_span};
use crate::cost_checker::{CostChecker, is_call_like};
use crate::deref_separator::deref_finder;
use crate::simplify::{UsedInStmtLocals, simplify_cfg};
use crate::validate::validate_types;
use crate::{check_inline, util};
@@ -64,7 +63,6 @@ impl<'tcx> crate::MirPass<'tcx> for Inline {
if inline::<NormalInliner<'tcx>>(tcx, body) {
debug!("running simplify cfg on {:?}", body.source);
simplify_cfg(tcx, body);
deref_finder(tcx, body);
}
}
@@ -100,7 +98,6 @@ impl<'tcx> crate::MirPass<'tcx> for ForceInline {
if inline::<ForceInliner<'tcx>>(tcx, body) {
debug!("running simplify cfg on {:?}", body.source);
simplify_cfg(tcx, body);
deref_finder(tcx, body);
}
}
}

View File

@@ -454,7 +454,6 @@ impl<'a, 'tcx> TOFinder<'a, 'tcx> {
match rhs {
Rvalue::Use(operand) => self.process_operand(bb, lhs, operand, state),
// Transfer the conditions on the copy rhs.
Rvalue::CopyForDeref(rhs) => self.process_operand(bb, lhs, &Operand::Copy(*rhs), state),
Rvalue::Discriminant(rhs) => {
let Some(rhs) = self.map.find_discr(rhs.as_ref()) else { return };
state.insert_place_idx(rhs, lhs, &self.map);

View File

@@ -139,6 +139,7 @@ declare_passes! {
mod dest_prop : DestinationPropagation;
pub mod dump_mir : Marker;
mod early_otherwise_branch : EarlyOtherwiseBranch;
mod erase_deref_temps : EraseDerefTemps;
mod elaborate_box_derefs : ElaborateBoxDerefs;
mod elaborate_drops : ElaborateDrops;
mod function_item_references : FunctionItemReferences;
@@ -619,6 +620,7 @@ fn run_runtime_lowering_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
// `AddRetag` needs to run after `ElaborateDrops` but before `ElaborateBoxDerefs`.
// Otherwise it should run fairly late, but before optimizations begin.
&add_retag::AddRetag,
&erase_deref_temps::EraseDerefTemps,
&elaborate_box_derefs::ElaborateBoxDerefs,
&coroutine::StateTransform,
&Lint(known_panics_lint::KnownPanicsLint),

View File

@@ -247,8 +247,7 @@ fn compute_replacement<'tcx>(
// This is a copy, just use the value we have in store for the previous one.
// As we are visiting in `assignment_order`, ie. reverse postorder, `rhs` should
// have been visited before.
Rvalue::Use(Operand::Copy(place) | Operand::Move(place))
| Rvalue::CopyForDeref(place) => {
Rvalue::Use(Operand::Copy(place) | Operand::Move(place)) => {
if let Some(rhs) = place.as_local()
&& ssa.is_ssa(rhs)
{

View File

@@ -17,12 +17,13 @@ use rustc_span::source_map::{Spanned, dummy_spanned};
use rustc_span::{DUMMY_SP, Span};
use tracing::{debug, instrument};
use crate::deref_separator::deref_finder;
use crate::elaborate_drop::{DropElaborator, DropFlagMode, DropStyle, Unwind, elaborate_drop};
use crate::patch::MirPatch;
use crate::{
abort_unwinding_calls, add_call_guards, add_moves_for_packed_drops, deref_separator, inline,
instsimplify, mentioned_items, pass_manager as pm, remove_noop_landing_pads,
run_optimization_passes, simplify,
abort_unwinding_calls, add_call_guards, add_moves_for_packed_drops, inline, instsimplify,
mentioned_items, pass_manager as pm, remove_noop_landing_pads, run_optimization_passes,
simplify,
};
mod async_destructor_ctor;
@@ -222,6 +223,8 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceKind<'tcx>) -> Body<
};
debug!("make_shim({:?}) = untransformed {:?}", instance, result);
deref_finder(tcx, &mut result, false);
// We don't validate MIR here because the shims may generate code that's
// only valid in a `PostAnalysis` param-env. However, since we do initial
// validation with the MirBuilt phase, which uses a user-facing param-env.
@@ -232,7 +235,6 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceKind<'tcx>) -> Body<
&[
&mentioned_items::MentionedItems,
&add_moves_for_packed_drops::AddMovesForPackedDrops,
&deref_separator::Derefer,
&remove_noop_landing_pads::RemoveNoopLandingPads,
&simplify::SimplifyCfg::MakeShim,
&instsimplify::InstSimplify::BeforeInline,

View File

@@ -240,7 +240,7 @@ fn build_adrop_for_coroutine_shim<'tcx>(
source_info,
StatementKind::Assign(Box::new((
Place::from(proxy_ref_local),
Rvalue::CopyForDeref(proxy_ref_place),
Rvalue::Use(Operand::Copy(proxy_ref_place)),
))),
),
);
@@ -261,7 +261,7 @@ fn build_adrop_for_coroutine_shim<'tcx>(
source_info,
StatementKind::Assign(Box::new((
Place::from(cor_ptr_local),
Rvalue::CopyForDeref(impl_ptr_place),
Rvalue::Use(Operand::Copy(impl_ptr_place)),
))),
),
);
@@ -334,7 +334,7 @@ fn build_adrop_for_adrop_shim<'tcx>(
source_info,
StatementKind::Assign(Box::new((
Place::from(proxy_ref_local),
Rvalue::CopyForDeref(proxy_ref_place),
Rvalue::Use(Operand::Copy(proxy_ref_place)),
))),
));
@@ -350,7 +350,7 @@ fn build_adrop_for_adrop_shim<'tcx>(
source_info,
StatementKind::Assign(Box::new((
Place::from(cor_ptr_local),
Rvalue::CopyForDeref(impl_ptr_place),
Rvalue::Use(Operand::Copy(impl_ptr_place)),
))),
));
}

View File

@@ -297,9 +297,7 @@ fn compute_copy_classes(ssa: &mut SsaLocals, body: &Body<'_>) {
let mut copies = IndexVec::from_fn_n(|l| l, body.local_decls.len());
for (local, rvalue, _) in ssa.assignments(body) {
let (Rvalue::Use(Operand::Copy(place) | Operand::Move(place))
| Rvalue::CopyForDeref(place)) = rvalue
else {
let Rvalue::Use(Operand::Copy(place) | Operand::Move(place)) = rvalue else {
continue;
};

View File

@@ -9,7 +9,7 @@ use rustc_index::bit_set::DenseBitSet;
use rustc_infer::infer::TyCtxtInferExt;
use rustc_infer::traits::{Obligation, ObligationCause};
use rustc_middle::mir::coverage::CoverageKind;
use rustc_middle::mir::visit::{NonUseContext, PlaceContext, Visitor};
use rustc_middle::mir::visit::{MutatingUseContext, NonUseContext, PlaceContext, Visitor};
use rustc_middle::mir::*;
use rustc_middle::ty::adjustment::PointerCoercion;
use rustc_middle::ty::print::with_no_trimmed_paths;
@@ -907,6 +907,20 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
}
}
if let ClearCrossCrate::Set(box LocalInfo::DerefTemp) =
self.body.local_decls[place.local].local_info
&& !place.is_indirect_first_projection()
{
if cntxt != PlaceContext::MutatingUse(MutatingUseContext::Store)
|| place.as_local().is_none()
{
self.fail(
location,
format!("`DerefTemp` locals must only be dereferenced or directly assigned to"),
);
}
}
self.super_place(place, cntxt, location);
}
@@ -919,7 +933,12 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
};
}
match rvalue {
Rvalue::Use(_) | Rvalue::CopyForDeref(_) => {}
Rvalue::Use(_) => {}
Rvalue::CopyForDeref(_) => {
if self.body.phase >= MirPhase::Runtime(RuntimePhase::Initial) {
self.fail(location, "`CopyForDeref` should have been removed in runtime MIR");
}
}
Rvalue::Aggregate(kind, fields) => match **kind {
AggregateKind::Tuple => {}
AggregateKind::Array(dest) => {
@@ -1417,13 +1436,13 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
),
);
}
if let Rvalue::CopyForDeref(place) = rvalue {
if place.ty(&self.body.local_decls, self.tcx).ty.builtin_deref(true).is_none() {
self.fail(
location,
"`CopyForDeref` should only be used for dereferenceable types",
)
}
if let Some(local) = dest.as_local()
&& let ClearCrossCrate::Set(box LocalInfo::DerefTemp) =
self.body.local_decls[local].local_info
&& !matches!(rvalue, Rvalue::CopyForDeref(_))
{
self.fail(location, "assignment to a `DerefTemp` must use `CopyForDeref`")
}
}
StatementKind::AscribeUserType(..) => {
@@ -1590,6 +1609,24 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
self.super_terminator(terminator, location);
}
fn visit_local_decl(&mut self, local: Local, local_decl: &LocalDecl<'tcx>) {
if let ClearCrossCrate::Set(box LocalInfo::DerefTemp) = local_decl.local_info {
if self.body.phase >= MirPhase::Runtime(RuntimePhase::Initial) {
self.fail(
START_BLOCK.start_location(),
"`DerefTemp` should have been removed in runtime MIR",
);
} else if local_decl.ty.builtin_deref(true).is_none() {
self.fail(
START_BLOCK.start_location(),
"`DerefTemp` should only be used for dereferenceable types",
)
}
}
self.super_local_decl(local, local_decl);
}
}
pub(super) fn validate_debuginfos<'tcx>(body: &Body<'tcx>) -> Vec<(Location, String)> {