Auto merge of #112307 - lcnr:operand-ref, r=compiler-errors
mir opt + codegen: handle subtyping fixes #107205 the same issue was caused in multiple places: - mir opts: both copy and destination propagation - codegen: assigning operands to locals (which also propagates values) I changed codegen to always update the type in the operands used for locals which should guard against any new occurrences of this bug going forward. I don't know how to make mir optimizations more resilient here. Hopefully the added tests will be enough to detect any trivially wrong optimizations going forward.
This commit is contained in:
commit
bb95b7dcd6
12 changed files with 207 additions and 39 deletions
|
@ -37,6 +37,10 @@
|
|||
//! if they do not consistently refer to the same place in memory. This is satisfied if they do
|
||||
//! not contain any indirection through a pointer or any indexing projections.
|
||||
//!
|
||||
//! * `p` and `q` must have the **same type**. If we replace a local with a subtype or supertype,
|
||||
//! we may end up with a differnet vtable for that local. See the `subtyping-impacts-selection`
|
||||
//! tests for an example where that causes issues.
|
||||
//!
|
||||
//! * We need to make sure that the goal of "merging the memory" is actually structurally possible
|
||||
//! in MIR. For example, even if all the other conditions are satisfied, there is no way to
|
||||
//! "merge" `_5.foo` and `_6.bar`. For now, we ensure this by requiring that both `p` and `q` are
|
||||
|
@ -134,6 +138,7 @@ use crate::MirPass;
|
|||
use rustc_data_structures::fx::FxHashMap;
|
||||
use rustc_index::bit_set::BitSet;
|
||||
use rustc_middle::mir::visit::{MutVisitor, PlaceContext, Visitor};
|
||||
use rustc_middle::mir::HasLocalDecls;
|
||||
use rustc_middle::mir::{dump_mir, PassWhere};
|
||||
use rustc_middle::mir::{
|
||||
traversal, Body, InlineAsmOperand, Local, LocalKind, Location, Operand, Place, Rvalue,
|
||||
|
@ -763,12 +768,22 @@ impl<'tcx> Visitor<'tcx> for FindAssignments<'_, '_, 'tcx> {
|
|||
return;
|
||||
};
|
||||
|
||||
// As described at the top of the file, we do not go near things that have their address
|
||||
// taken.
|
||||
// As described at the top of the file, we do not go near things that have
|
||||
// their address taken.
|
||||
if self.borrowed.contains(src) || self.borrowed.contains(dest) {
|
||||
return;
|
||||
}
|
||||
|
||||
// As described at the top of this file, we do not touch locals which have
|
||||
// different types.
|
||||
let src_ty = self.body.local_decls()[src].ty;
|
||||
let dest_ty = self.body.local_decls()[dest].ty;
|
||||
if src_ty != dest_ty {
|
||||
// FIXME(#112651): This can be removed afterwards. Also update the module description.
|
||||
trace!("skipped `{src:?} = {dest:?}` due to subtyping: {src_ty} != {dest_ty}");
|
||||
return;
|
||||
}
|
||||
|
||||
// Also, we need to make sure that MIR actually allows the `src` to be removed
|
||||
if is_local_required(src, self.body) {
|
||||
return;
|
||||
|
|
|
@ -32,13 +32,15 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> Body<'
|
|||
let mut result = match instance {
|
||||
ty::InstanceDef::Item(..) => bug!("item {:?} passed to make_shim", instance),
|
||||
ty::InstanceDef::VTableShim(def_id) => {
|
||||
build_call_shim(tcx, instance, Some(Adjustment::Deref), CallKind::Direct(def_id))
|
||||
let adjustment = Adjustment::Deref { source: DerefSource::MutPtr };
|
||||
build_call_shim(tcx, instance, Some(adjustment), CallKind::Direct(def_id))
|
||||
}
|
||||
ty::InstanceDef::FnPtrShim(def_id, ty) => {
|
||||
let trait_ = tcx.trait_of_item(def_id).unwrap();
|
||||
let adjustment = match tcx.fn_trait_kind_from_def_id(trait_) {
|
||||
Some(ty::ClosureKind::FnOnce) => Adjustment::Identity,
|
||||
Some(ty::ClosureKind::FnMut | ty::ClosureKind::Fn) => Adjustment::Deref,
|
||||
Some(ty::ClosureKind::Fn) => Adjustment::Deref { source: DerefSource::ImmRef },
|
||||
Some(ty::ClosureKind::FnMut) => Adjustment::Deref { source: DerefSource::MutRef },
|
||||
None => bug!("fn pointer {:?} is not an fn", ty),
|
||||
};
|
||||
|
||||
|
@ -107,16 +109,26 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> Body<'
|
|||
result
|
||||
}
|
||||
|
||||
#[derive(Copy, Clone, Debug, PartialEq)]
|
||||
enum DerefSource {
|
||||
/// `fn shim(&self) { inner(*self )}`.
|
||||
ImmRef,
|
||||
/// `fn shim(&mut self) { inner(*self )}`.
|
||||
MutRef,
|
||||
/// `fn shim(*mut self) { inner(*self )}`.
|
||||
MutPtr,
|
||||
}
|
||||
|
||||
#[derive(Copy, Clone, Debug, PartialEq)]
|
||||
enum Adjustment {
|
||||
/// Pass the receiver as-is.
|
||||
Identity,
|
||||
|
||||
/// We get passed `&[mut] self` and call the target with `*self`.
|
||||
/// We get passed a reference or a raw pointer to `self` and call the target with `*self`.
|
||||
///
|
||||
/// This either copies `self` (if `Self: Copy`, eg. for function items), or moves out of it
|
||||
/// (for `VTableShim`, which effectively is passed `&own Self`).
|
||||
Deref,
|
||||
Deref { source: DerefSource },
|
||||
|
||||
/// We get passed `self: Self` and call the target with `&mut self`.
|
||||
///
|
||||
|
@ -667,8 +679,12 @@ fn build_call_shim<'tcx>(
|
|||
let self_arg = &mut inputs_and_output[0];
|
||||
*self_arg = match rcvr_adjustment.unwrap() {
|
||||
Adjustment::Identity => fnty,
|
||||
Adjustment::Deref => tcx.mk_imm_ptr(fnty),
|
||||
Adjustment::RefMut => tcx.mk_mut_ptr(fnty),
|
||||
Adjustment::Deref { source } => match source {
|
||||
DerefSource::ImmRef => tcx.mk_imm_ref(tcx.lifetimes.re_erased, fnty),
|
||||
DerefSource::MutRef => tcx.mk_mut_ref(tcx.lifetimes.re_erased, fnty),
|
||||
DerefSource::MutPtr => tcx.mk_mut_ptr(fnty),
|
||||
},
|
||||
Adjustment::RefMut => bug!("`RefMut` is never used with indirect calls: {instance:?}"),
|
||||
};
|
||||
sig.inputs_and_output = tcx.mk_type_list(&inputs_and_output);
|
||||
}
|
||||
|
@ -699,7 +715,7 @@ fn build_call_shim<'tcx>(
|
|||
|
||||
let rcvr = rcvr_adjustment.map(|rcvr_adjustment| match rcvr_adjustment {
|
||||
Adjustment::Identity => Operand::Move(rcvr_place()),
|
||||
Adjustment::Deref => Operand::Move(tcx.mk_place_deref(rcvr_place())),
|
||||
Adjustment::Deref { source: _ } => Operand::Move(tcx.mk_place_deref(rcvr_place())),
|
||||
Adjustment::RefMut => {
|
||||
// let rcvr = &mut rcvr;
|
||||
let ref_rcvr = local_decls.push(
|
||||
|
|
|
@ -271,6 +271,14 @@ fn compute_copy_classes(ssa: &mut SsaLocals, body: &Body<'_>) {
|
|||
else { continue };
|
||||
|
||||
let Some(rhs) = place.as_local() else { continue };
|
||||
let local_ty = body.local_decls()[local].ty;
|
||||
let rhs_ty = body.local_decls()[rhs].ty;
|
||||
if local_ty != rhs_ty {
|
||||
// FIXME(#112651): This can be removed afterwards.
|
||||
trace!("skipped `{local:?} = {rhs:?}` due to subtyping: {local_ty} != {rhs_ty}");
|
||||
continue;
|
||||
}
|
||||
|
||||
if !ssa.is_ssa(rhs) {
|
||||
continue;
|
||||
}
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue