Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions compiler/rustc_middle/src/mir/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,11 @@ impl<'tcx> Const<'tcx> {
/// Return true if any evaluation of this constant always returns the same value,
/// taking into account even pointer identity tests.
pub fn is_deterministic(&self) -> bool {
if self.ty().is_primitive() {
// Primitive types hold no provenance, so their result is deterministic.
return true;
}

// Some constants may generate fresh allocations for pointers they contain,
// so using the same constant twice can yield two different results.
// Notably, valtrees purposefully generate new allocations.
Expand Down
87 changes: 62 additions & 25 deletions compiler/rustc_mir_transform/src/gvn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,10 @@ impl<'tcx> crate::MirPass<'tcx> for GVN {
state.visit_basic_block_data(bb, data);
}

for var_debug_info in body.var_debug_info.iter_mut() {
state.visit_var_debug_info(var_debug_info);
}

// For each local that is reused (`y` above), we remove its storage statements do avoid any
// difficulty. Those locals are SSA, so should be easy to optimize by LLVM without storage
// statements.
Expand Down Expand Up @@ -865,10 +869,11 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {

/// Simplify the projection chain if we know better.
#[instrument(level = "trace", skip(self))]
fn simplify_place_projection(&mut self, place: &mut Place<'tcx>, location: Location) {
fn simplify_place_projection(&mut self, place: &mut Place<'tcx>, location: Option<Location>) {
// If the projection is indirect, we treat the local as a value, so can replace it with
// another local.
if place.is_indirect_first_projection()
if let Some(location) = location
&& place.is_indirect_first_projection()
&& let Some(base) = self.locals[place.local]
&& let Some(new_local) = self.try_as_local(base, location)
&& place.local != new_local
Expand All @@ -890,7 +895,8 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
{
projection.to_mut()[i] =
ProjectionElem::ConstantIndex { offset, min_length, from_end: false };
} else if let Some(new_idx_local) = self.try_as_local(idx, location)
} else if let Some(location) = location
&& let Some(new_idx_local) = self.try_as_local(idx, location)
&& idx_local != new_idx_local
{
projection.to_mut()[i] = ProjectionElem::Index(new_idx_local);
Expand All @@ -912,7 +918,7 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
fn compute_place_value(
&mut self,
place: Place<'tcx>,
location: Location,
location: Option<Location>,
) -> Result<VnIndex, PlaceRef<'tcx>> {
// Invariant: `place` and `place_ref` point to the same value, even if they point to
// different memory locations.
Expand All @@ -923,7 +929,9 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
// Invariant: `value` has type `place_ty`, with optional downcast variant if needed.
let mut place_ty = PlaceTy::from_ty(self.local_decls[place.local].ty);
for (index, proj) in place.projection.iter().enumerate() {
if let Some(local) = self.try_as_local(value, location) {
if let Some(location) = location
&& let Some(local) = self.try_as_local(value, location)
{
// Both `local` and `Place { local: place.local, projection: projection[..index] }`
// hold the same value. Therefore, following place holds the value in the original
// `place`.
Expand All @@ -948,13 +956,14 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
fn simplify_place_value(
&mut self,
place: &mut Place<'tcx>,
location: Location,
location: Option<Location>,
) -> Option<VnIndex> {
self.simplify_place_projection(place, location);

match self.compute_place_value(*place, location) {
Ok(value) => {
if let Some(new_place) = self.try_as_place(value, location, true)
if let Some(location) = location
&& let Some(new_place) = self.try_as_place(value, location, true)
&& (new_place.local != place.local
|| new_place.projection.len() < place.projection.len())
{
Expand Down Expand Up @@ -982,16 +991,16 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
operand: &mut Operand<'tcx>,
location: Location,
) -> Option<VnIndex> {
match *operand {
Operand::Constant(ref constant) => Some(self.insert_constant(constant.const_)),
let value = match *operand {
Operand::Constant(ref constant) => self.insert_constant(constant.const_),
Operand::Copy(ref mut place) | Operand::Move(ref mut place) => {
let value = self.simplify_place_value(place, location)?;
if let Some(const_) = self.try_as_constant(value) {
*operand = Operand::Constant(Box::new(const_));
}
Some(value)
self.simplify_place_value(place, Some(location))?
}
};
if let Some(const_) = self.try_as_constant(value) {
*operand = Operand::Constant(Box::new(const_));
}
Some(value)
}

#[instrument(level = "trace", skip(self), ret)]
Expand Down Expand Up @@ -1019,11 +1028,11 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
Rvalue::NullaryOp(op, ty) => Value::NullaryOp(op, ty),
Rvalue::Aggregate(..) => return self.simplify_aggregate(lhs, rvalue, location),
Rvalue::Ref(_, borrow_kind, ref mut place) => {
self.simplify_place_projection(place, location);
self.simplify_place_projection(place, Some(location));
return self.new_pointer(*place, AddressKind::Ref(borrow_kind));
}
Rvalue::RawPtr(mutbl, ref mut place) => {
self.simplify_place_projection(place, location);
self.simplify_place_projection(place, Some(location));
return self.new_pointer(*place, AddressKind::Address(mutbl));
}
Rvalue::WrapUnsafeBinder(ref mut op, _) => {
Expand All @@ -1042,7 +1051,7 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
return self.simplify_unary(op, arg_op, location);
}
Rvalue::Discriminant(ref mut place) => {
let place = self.simplify_place_value(place, location)?;
let place = self.simplify_place_value(place, Some(location))?;
if let Some(discr) = self.simplify_discriminant(place) {
return Some(discr);
}
Expand Down Expand Up @@ -1762,14 +1771,30 @@ impl<'tcx> VnState<'_, '_, 'tcx> {

/// If `index` is a `Value::Constant`, return the `Constant` to be put in the MIR.
fn try_as_constant(&mut self, index: VnIndex) -> Option<ConstOperand<'tcx>> {
// This was already constant in MIR, do not change it. If the constant is not
// deterministic, adding an additional mention of it in MIR will not give the same value as
// the former mention.
if let Value::Constant { value, disambiguator: None } = self.get(index) {
let value = self.get(index);

// This was already an *evaluated* constant in MIR, do not change it.
if let Value::Constant { value, disambiguator: None } = value
&& let Const::Val(..) = value
{
debug_assert!(value.is_deterministic());
return Some(ConstOperand { span: DUMMY_SP, user_ty: None, const_: value });
}

if let Some(value) = self.try_as_evaluated_constant(index) {
return Some(ConstOperand { span: DUMMY_SP, user_ty: None, const_: value });
}

// We failed to provide an evaluated form, fallback to using the unevaluated constant.
if let Value::Constant { value, disambiguator: None } = value {
debug_assert!(value.is_deterministic());
return Some(ConstOperand { span: DUMMY_SP, user_ty: None, const_: value });
}

None
}

fn try_as_evaluated_constant(&mut self, index: VnIndex) -> Option<Const<'tcx>> {
let op = self.evaluated[index].as_ref()?;
if op.layout.is_unsized() {
// Do not attempt to propagate unsized locals.
Expand All @@ -1783,8 +1808,7 @@ impl<'tcx> VnState<'_, '_, 'tcx> {
// FIXME: remove this hack once https://github.com/rust-lang/rust/issues/79738 is fixed.
assert!(!value.may_have_provenance(self.tcx, op.layout.size));

let const_ = Const::Val(value, op.layout.ty);
Some(ConstOperand { span: DUMMY_SP, user_ty: None, const_ })
Some(Const::Val(value, op.layout.ty))
}

/// Construct a place which holds the same value as `index` and for which all locals strictly
Expand Down Expand Up @@ -1837,8 +1861,21 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, '_, 'tcx> {
self.tcx
}

fn visit_var_debug_info(&mut self, var_debug_info: &mut VarDebugInfo<'tcx>) {
match &mut var_debug_info.value {
VarDebugInfoContents::Const(_) => {}
VarDebugInfoContents::Place(place) => {
if let Some(value) = self.simplify_place_value(place, None)
&& let Some(constant) = self.try_as_constant(value)
{
var_debug_info.value = VarDebugInfoContents::Const(constant);
}
}
}
}

fn visit_place(&mut self, place: &mut Place<'tcx>, context: PlaceContext, location: Location) {
self.simplify_place_projection(place, location);
self.simplify_place_projection(place, Some(location));
if context.is_mutating_use() && place.is_indirect() {
// Non-local mutation maybe invalidate deref.
self.invalidate_derefs();
Expand All @@ -1857,7 +1894,7 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, '_, 'tcx> {
rvalue: &mut Rvalue<'tcx>,
location: Location,
) {
self.simplify_place_projection(lhs, location);
self.simplify_place_projection(lhs, Some(location));

let value = self.simplify_rvalue(lhs, rvalue, location);
if let Some(value) = value {
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_mir_transform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,6 @@ declare_passes! {
Final
};
mod simplify_comparison_integral : SimplifyComparisonIntegral;
mod single_use_consts : SingleUseConsts;
mod sroa : ScalarReplacementOfAggregates;
mod strip_debuginfo : StripDebugInfo;
mod unreachable_enum_branching : UnreachableEnumBranching;
Expand Down Expand Up @@ -709,7 +708,6 @@ pub(crate) fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'
&simplify::SimplifyLocals::AfterGVN,
&match_branches::MatchBranchSimplification,
&dataflow_const_prop::DataflowConstProp,
&single_use_consts::SingleUseConsts,
&o1(simplify_branches::SimplifyConstCondition::AfterConstProp),
&jump_threading::JumpThreading,
&early_otherwise_branch::EarlyOtherwiseBranch,
Expand Down
Loading
Loading