Skip to content

Commit

Permalink
avoid marking as immutable what is already immutable
Browse files Browse the repository at this point in the history
this has been demonstrated to help performance
  • Loading branch information
RalfJung committed Dec 7, 2023
1 parent 29c95e9 commit 8188bd4
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 6 deletions.
5 changes: 4 additions & 1 deletion compiler/rustc_const_eval/src/const_eval/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -714,11 +714,14 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
_kind: mir::RetagKind,
val: &ImmTy<'tcx, CtfeProvenance>,
) -> InterpResult<'tcx, ImmTy<'tcx, CtfeProvenance>> {
// If it's a frozen shared reference that's not already immutable, make it immutable.
// (Do nothing on `None` provenance, that cannot store immutability anyway.)
if let ty::Ref(_, ty, mutbl) = val.layout.ty.kind()
&& *mutbl == Mutability::Not
&& val.to_scalar_and_meta().0.to_pointer(ecx)?.provenance.is_some_and(|p| !p.immutable())
// That next check is expensive, that's why we have all the guards above.
&& ty.is_freeze(*ecx.tcx, ecx.param_env)
{
// This is a frozen shared reference, mark it immutable.
let place = ecx.ref_to_mplace(val)?;
let new_place = place.map_provenance(|p| p.map(CtfeProvenance::as_immutable));
Ok(ImmTy::from_immediate(new_place.to_ref(ecx), val.layout))
Expand Down
11 changes: 11 additions & 0 deletions compiler/rustc_const_eval/src/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,17 @@ impl<Prov: Provenance> Immediate<Prov> {
Immediate::Uninit => bug!("Got uninit where a scalar pair was expected"),
}
}

/// Returns the scalar from the first component and optionally the 2nd component as metadata.
#[inline]
#[cfg_attr(debug_assertions, track_caller)] // only in debug builds due to perf (see #98980)
pub fn to_scalar_and_meta(self) -> (Scalar<Prov>, MemPlaceMeta<Prov>) {
match self {
Immediate::ScalarPair(val1, val2) => (val1, MemPlaceMeta::Meta(val2)),
Immediate::Scalar(val) => (val, MemPlaceMeta::None),
Immediate::Uninit => bug!("Got uninit where a scalar or scalar pair was expected"),
}
}
}

// ScalarPair needs a type to interpret, so we often have an immediate and a type together
Expand Down
6 changes: 1 addition & 5 deletions compiler/rustc_const_eval/src/interpret/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,11 +406,7 @@ where
let pointee_type =
val.layout.ty.builtin_deref(true).expect("`ref_to_mplace` called on non-ptr type").ty;
let layout = self.layout_of(pointee_type)?;
let (ptr, meta) = match **val {
Immediate::Scalar(ptr) => (ptr, MemPlaceMeta::None),
Immediate::ScalarPair(ptr, meta) => (ptr, MemPlaceMeta::Meta(meta)),
Immediate::Uninit => throw_ub!(InvalidUninitBytes(None)),
};
let (ptr, meta) = val.to_scalar_and_meta();

// `ref_to_mplace` is called on raw pointers even if they don't actually get dereferenced;
// we hence can't call `size_and_align_of` since that asserts more validity than we want.
Expand Down

0 comments on commit 8188bd4

Please sign in to comment.