From c0e4230bf5596d83d61c7a4ca5b06f0490ac4dba Mon Sep 17 00:00:00 2001 From: ouz-a Date: Thu, 12 May 2022 00:27:06 +0300 Subject: [PATCH 1/4] simplify some code that depend on Deref --- compiler/rustc_codegen_ssa/src/mir/place.rs | 11 ++----- compiler/rustc_middle/src/mir/mod.rs | 20 +++++++++++++ compiler/rustc_mir_transform/src/add_retag.rs | 29 ++++++------------- 3 files changed, 32 insertions(+), 28 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/place.rs b/compiler/rustc_codegen_ssa/src/mir/place.rs index 58cee0c8bb0db..38e09f539ded1 100644 --- a/compiler/rustc_codegen_ssa/src/mir/place.rs +++ b/compiler/rustc_codegen_ssa/src/mir/place.rs @@ -435,16 +435,11 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { LocalRef::Place(place) => place, LocalRef::UnsizedPlace(place) => bx.load_operand(place).deref(cx), LocalRef::Operand(..) => { - if let Some(elem) = place_ref - .projection - .iter() - .enumerate() - .find(|elem| matches!(elem.1, mir::ProjectionElem::Deref)) - { - base = elem.0 + 1; + if place_ref.ret_deref().is_some() { + base = 1; let cg_base = self.codegen_consume( bx, - mir::PlaceRef { projection: &place_ref.projection[..elem.0], ..place_ref }, + mir::PlaceRef { projection: &place_ref.projection[..0], ..place_ref }, ); cg_base.deref(bx.cx()) diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 702cc48ff7bb3..2a31441ec7b6d 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1461,6 +1461,16 @@ impl<'tcx> Place<'tcx> { self.projection.iter().any(|elem| elem.is_indirect()) } + /// If MirPhase >= Derefered and if projection contains Deref, + /// It's guaranteed to be in the first place + pub fn ret_deref(&self) -> Option> { + if !self.projection.is_empty() && self.projection[0] == PlaceElem::Deref { + return Some(self.projection[0]); + } else { + None + } + } + /// Finds the innermost `Local` from this `Place`, *if* it is either a local itself or /// a single deref of a local. #[inline(always)] @@ -1533,6 +1543,16 @@ impl<'tcx> PlaceRef<'tcx> { } } + /// If MirPhase >= Derefered and if projection contains Deref, + /// It's guaranteed to be in the first place + pub fn ret_deref(&self) -> Option> { + if !self.projection.is_empty() && self.projection[0] == PlaceElem::Deref { + return Some(self.projection[0]); + } else { + None + } + } + /// If this place represents a local variable like `_X` with no /// projections, return `Some(_X)`. #[inline] diff --git a/compiler/rustc_mir_transform/src/add_retag.rs b/compiler/rustc_mir_transform/src/add_retag.rs index b91ae083cf594..b0cbcff600c59 100644 --- a/compiler/rustc_mir_transform/src/add_retag.rs +++ b/compiler/rustc_mir_transform/src/add_retag.rs @@ -15,22 +15,13 @@ pub struct AddRetag; /// (Concurrent accesses by other threads are no problem as these are anyway non-atomic /// copies. Data races are UB.) fn is_stable(place: PlaceRef<'_>) -> bool { - place.projection.iter().all(|elem| { - match elem { - // Which place this evaluates to can change with any memory write, - // so cannot assume this to be stable. - ProjectionElem::Deref => false, - // Array indices are interesting, but MIR building generates a *fresh* - // temporary for every array access, so the index cannot be changed as - // a side-effect. - ProjectionElem::Index { .. } | - // The rest is completely boring, they just offset by a constant. - ProjectionElem::Field { .. } | - ProjectionElem::ConstantIndex { .. } | - ProjectionElem::Subslice { .. } | - ProjectionElem::Downcast { .. } => true, - } - }) + if place.ret_deref().is_some() { + // Which place this evaluates to can change with any memory write, + // so cannot assume deref to be stable. + return false; + } else { + return true; + } } /// Determine whether this type may contain a reference (or box), and thus needs retagging. @@ -91,10 +82,8 @@ impl<'tcx> MirPass<'tcx> for AddRetag { }; let place_base_raw = |place: &Place<'tcx>| { // If this is a `Deref`, get the type of what we are deref'ing. - let deref_base = - place.projection.iter().rposition(|p| matches!(p, ProjectionElem::Deref)); - if let Some(deref_base) = deref_base { - let base_proj = &place.projection[..deref_base]; + if place.ret_deref().is_some() { + let base_proj = &place.projection[..0]; let ty = Place::ty_from(place.local, base_proj, &*local_decls, tcx).ty; ty.is_unsafe_ptr() } else { From c3e1e7a947e65f16b35cbd4af9607fc670474542 Mon Sep 17 00:00:00 2001 From: ouz-a Date: Mon, 16 May 2022 16:23:42 +0300 Subject: [PATCH 2/4] simplify more, ret_deref -> has_deref --- compiler/rustc_codegen_ssa/src/mir/place.rs | 3 +-- compiler/rustc_middle/src/mir/mod.rs | 12 ++++++------ compiler/rustc_mir_transform/src/add_retag.rs | 5 ++--- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/place.rs b/compiler/rustc_codegen_ssa/src/mir/place.rs index 38e09f539ded1..268c4d7650305 100644 --- a/compiler/rustc_codegen_ssa/src/mir/place.rs +++ b/compiler/rustc_codegen_ssa/src/mir/place.rs @@ -435,13 +435,12 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { LocalRef::Place(place) => place, LocalRef::UnsizedPlace(place) => bx.load_operand(place).deref(cx), LocalRef::Operand(..) => { - if place_ref.ret_deref().is_some() { + if place_ref.has_deref() { base = 1; let cg_base = self.codegen_consume( bx, mir::PlaceRef { projection: &place_ref.projection[..0], ..place_ref }, ); - cg_base.deref(bx.cx()) } else { bug!("using operand local {:?} as place", place_ref); diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 2a31441ec7b6d..e2084a12cbe6b 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1463,11 +1463,11 @@ impl<'tcx> Place<'tcx> { /// If MirPhase >= Derefered and if projection contains Deref, /// It's guaranteed to be in the first place - pub fn ret_deref(&self) -> Option> { + pub fn has_deref(&self) -> bool { if !self.projection.is_empty() && self.projection[0] == PlaceElem::Deref { - return Some(self.projection[0]); + true } else { - None + false } } @@ -1545,11 +1545,11 @@ impl<'tcx> PlaceRef<'tcx> { /// If MirPhase >= Derefered and if projection contains Deref, /// It's guaranteed to be in the first place - pub fn ret_deref(&self) -> Option> { + pub fn has_deref(&self) -> bool { if !self.projection.is_empty() && self.projection[0] == PlaceElem::Deref { - return Some(self.projection[0]); + true } else { - None + false } } diff --git a/compiler/rustc_mir_transform/src/add_retag.rs b/compiler/rustc_mir_transform/src/add_retag.rs index b0cbcff600c59..c91b3044c4fdf 100644 --- a/compiler/rustc_mir_transform/src/add_retag.rs +++ b/compiler/rustc_mir_transform/src/add_retag.rs @@ -15,7 +15,7 @@ pub struct AddRetag; /// (Concurrent accesses by other threads are no problem as these are anyway non-atomic /// copies. Data races are UB.) fn is_stable(place: PlaceRef<'_>) -> bool { - if place.ret_deref().is_some() { + if place.has_deref() { // Which place this evaluates to can change with any memory write, // so cannot assume deref to be stable. return false; @@ -83,8 +83,7 @@ impl<'tcx> MirPass<'tcx> for AddRetag { let place_base_raw = |place: &Place<'tcx>| { // If this is a `Deref`, get the type of what we are deref'ing. if place.ret_deref().is_some() { - let base_proj = &place.projection[..0]; - let ty = Place::ty_from(place.local, base_proj, &*local_decls, tcx).ty; + let ty = place.ty(local_decls, tcx).ty; ty.is_unsafe_ptr() } else { // Not a deref, and thus not raw. From 447aaceed717a516bb9f4b295d582f5e2594b83b Mon Sep 17 00:00:00 2001 From: ouz-a Date: Mon, 16 May 2022 16:36:33 +0300 Subject: [PATCH 3/4] has_deref: simpler comparison, ty fix --- compiler/rustc_middle/src/mir/mod.rs | 12 ++---------- compiler/rustc_mir_transform/src/add_retag.rs | 14 +++++--------- 2 files changed, 7 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index e2084a12cbe6b..a8408cfe57019 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1464,11 +1464,7 @@ impl<'tcx> Place<'tcx> { /// If MirPhase >= Derefered and if projection contains Deref, /// It's guaranteed to be in the first place pub fn has_deref(&self) -> bool { - if !self.projection.is_empty() && self.projection[0] == PlaceElem::Deref { - true - } else { - false - } + self.projection.first() == Some(&PlaceElem::Deref) } /// Finds the innermost `Local` from this `Place`, *if* it is either a local itself or @@ -1546,11 +1542,7 @@ impl<'tcx> PlaceRef<'tcx> { /// If MirPhase >= Derefered and if projection contains Deref, /// It's guaranteed to be in the first place pub fn has_deref(&self) -> bool { - if !self.projection.is_empty() && self.projection[0] == PlaceElem::Deref { - true - } else { - false - } + self.projection.first() == Some(&PlaceElem::Deref) } /// If this place represents a local variable like `_X` with no diff --git a/compiler/rustc_mir_transform/src/add_retag.rs b/compiler/rustc_mir_transform/src/add_retag.rs index c91b3044c4fdf..9c5896c4e4aed 100644 --- a/compiler/rustc_mir_transform/src/add_retag.rs +++ b/compiler/rustc_mir_transform/src/add_retag.rs @@ -15,13 +15,9 @@ pub struct AddRetag; /// (Concurrent accesses by other threads are no problem as these are anyway non-atomic /// copies. Data races are UB.) fn is_stable(place: PlaceRef<'_>) -> bool { - if place.has_deref() { - // Which place this evaluates to can change with any memory write, - // so cannot assume deref to be stable. - return false; - } else { - return true; - } + // Which place this evaluates to can change with any memory write, + // so cannot assume deref to be stable. + !place.has_deref() } /// Determine whether this type may contain a reference (or box), and thus needs retagging. @@ -82,8 +78,8 @@ impl<'tcx> MirPass<'tcx> for AddRetag { }; let place_base_raw = |place: &Place<'tcx>| { // If this is a `Deref`, get the type of what we are deref'ing. - if place.ret_deref().is_some() { - let ty = place.ty(local_decls, tcx).ty; + if place.has_deref() { + let ty = &local_decls[place.local].ty; ty.is_unsafe_ptr() } else { // Not a deref, and thus not raw. From 9f00d836af0a02110a0a47f4d4c7c7182f04574f Mon Sep 17 00:00:00 2001 From: ouz-a Date: Sun, 24 Jul 2022 13:26:20 +0300 Subject: [PATCH 4/4] make sure has_deref is correct --- compiler/rustc_middle/src/mir/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index a8408cfe57019..f7311ebdabfd9 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1464,6 +1464,8 @@ impl<'tcx> Place<'tcx> { /// If MirPhase >= Derefered and if projection contains Deref, /// It's guaranteed to be in the first place pub fn has_deref(&self) -> bool { + // To make sure this is not accidently used in wrong mir phase + debug_assert!(!self.projection[1..].contains(&PlaceElem::Deref)); self.projection.first() == Some(&PlaceElem::Deref) }