Disambiguate non-deterministic constants.

This commit is contained in:
Camille GILLOT 2023-10-14 12:12:54 +00:00
parent f08dc9be17
commit 5e78b9cdb3
7 changed files with 238 additions and 53 deletions

View File

@ -497,6 +497,40 @@ impl<'tcx> Const<'tcx> {
_ => Self::Ty(c),
}
}
/// Return true if any evaluation of this constant returns the same value.
pub fn is_deterministic(&self) -> bool {
// Some constants may contain pointers. We need to preserve the provenance of these
// pointers, but not all constants guarantee this:
// - valtrees purposefully do not;
// - ConstValue::Slice does not either.
match self {
Const::Ty(c) => match c.kind() {
ty::ConstKind::Value(valtree) => match valtree {
// This is just an integer, keep it.
ty::ValTree::Leaf(_) => true,
// This branch may be a reference. Valtree references correspond to a
// different allocation each time they are evaluated.
ty::ValTree::Branch(_) => false,
},
ty::ConstKind::Param(..) => true,
ty::ConstKind::Unevaluated(..) | ty::ConstKind::Expr(..) => false,
// Should not appear in runtime MIR.
ty::ConstKind::Infer(..)
| ty::ConstKind::Bound(..)
| ty::ConstKind::Placeholder(..)
| ty::ConstKind::Error(..) => bug!(),
},
Const::Unevaluated(..) => false,
// If the same slice appears twice in the MIR, we cannot guarantee that we will
// give the same `AllocId` to the data.
Const::Val(ConstValue::Slice { .. }, _) => false,
Const::Val(
ConstValue::ZeroSized | ConstValue::Scalar(_) | ConstValue::Indirect { .. },
_,
) => true,
}
}
}
/// An unevaluated (potentially generic) constant used in MIR.

View File

@ -52,6 +52,35 @@
//! _a = *_b // _b is &Freeze
//! _c = *_b // replaced by _c = _a
//! ```
//!
//! # Determinism of constant propagation
//!
//! When registering a new `Value`, we attempt to opportunistically evaluate it as a constant.
//! The evaluated form is inserted in `evaluated` as an `OpTy` or `None` if evaluation failed.
//!
//! The difficulty is non-deterministic evaluation of MIR constants. Some `Const` can have
//! different runtime values each time they are evaluated. This is the case with
//! `Const::Slice` which have a new pointer each time they are evaluated, and constants that
//! contain a fn pointer (`AllocId` pointing to a `GlobalAlloc::Function`) pointing to a different
//! symbol in each codegen unit.
//!
//! Meanwhile, we want to be able to read indirect constants. For instance:
//! ```
//! static A: &'static &'static u8 = &&63;
//! fn foo() -> u8 {
//! **A // We want to replace by 63.
//! }
//! fn bar() -> u8 {
//! b"abc"[1] // We want to replace by 'b'.
//! }
//! ```
//!
//! The `Value::Constant` variant stores a possibly unevaluated constant. Evaluating that constant
//! may be non-deterministic. When that happens, we assign a disambiguator to ensure that we do not
//! merge the constants. See `duplicate_slice` test in `gvn.rs`.
//!
//! Second, when writing constants in MIR, we do not write `Const::Slice` or `Const`
//! that contain `AllocId`s.
use rustc_const_eval::interpret::{intern_const_alloc_for_constprop, MemoryKind};
use rustc_const_eval::interpret::{ImmTy, InterpCx, OpTy, Projectable, Scalar};
@ -161,7 +190,12 @@ enum Value<'tcx> {
/// The `usize` is a counter incremented by `new_opaque`.
Opaque(usize),
/// Evaluated or unevaluated constant value.
Constant(Const<'tcx>),
Constant {
value: Const<'tcx>,
/// Some constants do not have a deterministic value. To avoid merging two instances of the
/// same `Const`, we assign them an additional integer index.
disambiguator: usize,
},
/// An aggregate value, either tuple/closure/struct/enum.
/// This does not contain unions, as we cannot reason with the value.
Aggregate(AggregateTy<'tcx>, VariantIdx, Vec<VnIndex>),
@ -288,8 +322,24 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
}
}
fn insert_constant(&mut self, value: Const<'tcx>) -> Option<VnIndex> {
let disambiguator = if value.is_deterministic() {
// The constant is deterministic, no need to disambiguate.
0
} else {
// Multiple mentions of this constant will yield different values,
// so assign a different `disambiguator` to ensure they do not get the same `VnIndex`.
let next_opaque = self.next_opaque.as_mut()?;
let disambiguator = *next_opaque;
*next_opaque += 1;
disambiguator
};
Some(self.insert(Value::Constant { value, disambiguator }))
}
fn insert_scalar(&mut self, scalar: Scalar, ty: Ty<'tcx>) -> VnIndex {
self.insert(Value::Constant(Const::from_scalar(self.tcx, scalar, ty)))
self.insert_constant(Const::from_scalar(self.tcx, scalar, ty))
.expect("scalars are deterministic")
}
#[instrument(level = "trace", skip(self), ret)]
@ -300,7 +350,9 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
// Do not bother evaluating repeat expressions. This would uselessly consume memory.
Repeat(..) => return None,
Constant(ref constant) => self.ecx.eval_mir_constant(constant, None, None).ok()?,
Constant { ref value, disambiguator: _ } => {
self.ecx.eval_mir_constant(value, None, None).ok()?
}
Aggregate(kind, variant, ref fields) => {
let fields = fields
.iter()
@ -657,7 +709,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
match *operand {
Operand::Constant(ref mut constant) => {
let const_ = constant.const_.normalize(self.tcx, self.param_env);
Some(self.insert(Value::Constant(const_)))
self.insert_constant(const_)
}
Operand::Copy(ref mut place) | Operand::Move(ref mut place) => {
let value = self.simplify_place_value(place, location)?;
@ -691,7 +743,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
Value::Repeat(op, amount)
}
Rvalue::NullaryOp(op, ty) => Value::NullaryOp(op, ty),
Rvalue::Aggregate(..) => self.simplify_aggregate(rvalue, location)?,
Rvalue::Aggregate(..) => return self.simplify_aggregate(rvalue, location),
Rvalue::Ref(_, borrow_kind, ref mut place) => {
self.simplify_place_projection(place, location);
return self.new_pointer(*place, AddressKind::Ref(borrow_kind));
@ -757,7 +809,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
&mut self,
rvalue: &mut Rvalue<'tcx>,
location: Location,
) -> Option<Value<'tcx>> {
) -> Option<VnIndex> {
let Rvalue::Aggregate(box ref kind, ref mut fields) = *rvalue else { bug!() };
let tcx = self.tcx;
@ -774,8 +826,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
if is_zst {
let ty = rvalue.ty(self.local_decls, tcx);
let value = Value::Constant(Const::zero_sized(ty));
return Some(value);
return self.insert_constant(Const::zero_sized(ty));
}
}
@ -814,12 +865,11 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
*rvalue = Rvalue::Repeat(Operand::Copy(local.into()), len);
self.reused_locals.insert(local);
}
return Some(Value::Repeat(first, len));
return Some(self.insert(Value::Repeat(first, len)));
}
}
let value = Value::Aggregate(ty, variant_index, fields);
Some(value)
Some(self.insert(Value::Aggregate(ty, variant_index, fields)))
}
}
@ -882,39 +932,12 @@ 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 let Value::Constant(const_) = *self.get(index) {
// Some constants may contain pointers. We need to preserve the provenance of these
// pointers, but not all constants guarantee this:
// - valtrees purposefully do not;
// - ConstValue::Slice does not either.
let const_ok = match const_ {
Const::Ty(c) => match c.kind() {
ty::ConstKind::Value(valtree) => match valtree {
// This is just an integer, keep it.
ty::ValTree::Leaf(_) => true,
ty::ValTree::Branch(_) => false,
},
ty::ConstKind::Param(..)
| ty::ConstKind::Unevaluated(..)
| ty::ConstKind::Expr(..) => true,
// Should not appear in runtime MIR.
ty::ConstKind::Infer(..)
| ty::ConstKind::Bound(..)
| ty::ConstKind::Placeholder(..)
| ty::ConstKind::Error(..) => bug!(),
},
Const::Unevaluated(..) => true,
// If the same slice appears twice in the MIR, we cannot guarantee that we will
// give the same `AllocId` to the data.
Const::Val(ConstValue::Slice { .. }, _) => false,
Const::Val(
ConstValue::ZeroSized | ConstValue::Scalar(_) | ConstValue::Indirect { .. },
_,
) => true,
};
if const_ok {
return Some(ConstOperand { span: rustc_span::DUMMY_SP, user_ty: None, const_ });
}
if let Value::Constant { value, disambiguator: _ } = *self.get(index)
// If the constant is not deterministic, adding an additional mention of it in MIR will
// not give the same value as the former mention.
&& value.is_deterministic()
{
return Some(ConstOperand { span: rustc_span::DUMMY_SP, user_ty: None, const_: value });
}
let op = self.evaluated[index].as_ref()?;

View File

@ -0,0 +1,38 @@
- // MIR for `duplicate_slice` before GVN
+ // MIR for `duplicate_slice` after GVN
fn duplicate_slice() -> (bool, bool) {
let mut _0: (bool, bool);
let mut _1: u128;
let mut _2: u128;
let mut _3: u128;
let mut _4: u128;
let mut _5: &str;
let mut _6: &str;
let mut _7: (&str,);
let mut _8: &str;
let mut _9: bool;
let mut _10: bool;
bb0: {
_7 = (const "a",);
_1 = (_7.0: &str) as u128 (Transmute);
_5 = identity::<&str>((_7.0: &str)) -> [return: bb1, unwind unreachable];
}
bb1: {
_3 = _5 as u128 (Transmute);
_8 = const "a";
_2 = _8 as u128 (Transmute);
_6 = identity::<&str>(_8) -> [return: bb2, unwind unreachable];
}
bb2: {
_4 = _6 as u128 (Transmute);
_9 = Eq(_1, _2);
_10 = Eq(_3, _4);
_0 = (_9, _10);
return;
}
}

View File

@ -0,0 +1,38 @@
- // MIR for `duplicate_slice` before GVN
+ // MIR for `duplicate_slice` after GVN
fn duplicate_slice() -> (bool, bool) {
let mut _0: (bool, bool);
let mut _1: u128;
let mut _2: u128;
let mut _3: u128;
let mut _4: u128;
let mut _5: &str;
let mut _6: &str;
let mut _7: (&str,);
let mut _8: &str;
let mut _9: bool;
let mut _10: bool;
bb0: {
_7 = (const "a",);
_1 = (_7.0: &str) as u128 (Transmute);
_5 = identity::<&str>((_7.0: &str)) -> [return: bb1, unwind continue];
}
bb1: {
_3 = _5 as u128 (Transmute);
_8 = const "a";
_2 = _8 as u128 (Transmute);
_6 = identity::<&str>(_8) -> [return: bb2, unwind continue];
}
bb2: {
_4 = _6 as u128 (Transmute);
_9 = Eq(_1, _2);
_10 = Eq(_3, _4);
_0 = (_9, _10);
return;
}
}

View File

@ -1,11 +1,17 @@
// skip-filecheck
// unit-test: GVN
// EMIT_MIR_FOR_EACH_PANIC_STRATEGY
// only-64bit
#![feature(raw_ref_op)]
#![feature(rustc_attrs)]
#![feature(custom_mir)]
#![feature(core_intrinsics)]
#![allow(unconditional_panic)]
use std::intrinsics::mir::*;
use std::mem::transmute;
struct S<T>(T);
fn subexpression_elimination(x: u64, y: u64, mut z: u64) {
@ -225,11 +231,49 @@ fn slices() {
let t = s; // This should be the same pointer, so cannot be a `Const::Slice`.
opaque(t);
assert_eq!(s.as_ptr(), t.as_ptr());
let u = unsafe { std::mem::transmute::<&str, &[u8]>(s) };
let u = unsafe { transmute::<&str, &[u8]>(s) };
opaque(u);
assert_eq!(s.as_ptr(), u.as_ptr());
}
#[custom_mir(dialect = "analysis")]
fn duplicate_slice() -> (bool, bool) {
mir!(
let au: u128;
let bu: u128;
let cu: u128;
let du: u128;
let c: &str;
let d: &str;
{
let a = ("a",);
Call(au = transmute::<_, u128>(a.0), bb1)
}
bb1 = {
Call(c = identity(a.0), bb2)
}
bb2 = {
Call(cu = transmute::<_, u128>(c), bb3)
}
bb3 = {
let b = "a"; // This slice is different from `a.0`.
Call(bu = transmute::<_, u128>(b), bb4) // Hence `bu` is not `au`.
}
bb4 = {
Call(d = identity(b), bb5) // This returns a copy of `b`, which is not `a`.
}
bb5 = {
Call(du = transmute::<_, u128>(d), bb6)
}
bb6 = {
let direct = au == bu; // Must not fold to `true`...
let indirect = cu == du; // ...as this will not.
RET = (direct, indirect);
Return()
}
)
}
fn aggregates() {
let a_array: S<[u8; 0]> = S([]);
let b_array: S<[u16; 0]> = S([]); // This must not be merged with `a_array`.
@ -253,12 +297,19 @@ fn main() {
references(5);
dereferences(&mut 5, &6, &S(7));
slices();
let (direct, indirect) = duplicate_slice();
assert_eq!(direct, indirect);
aggregates();
}
#[inline(never)]
fn opaque(_: impl Sized) {}
#[inline(never)]
fn identity<T>(x: T) -> T {
x
}
// EMIT_MIR gvn.subexpression_elimination.GVN.diff
// EMIT_MIR gvn.wrap_unwrap.GVN.diff
// EMIT_MIR gvn.repeated_index.GVN.diff
@ -270,4 +321,5 @@ fn opaque(_: impl Sized) {}
// EMIT_MIR gvn.references.GVN.diff
// EMIT_MIR gvn.dereferences.GVN.diff
// EMIT_MIR gvn.slices.GVN.diff
// EMIT_MIR gvn.duplicate_slice.GVN.diff
// EMIT_MIR gvn.aggregates.GVN.diff

View File

@ -207,8 +207,8 @@
_26 = &(*_27);
StorageLive(_28);
_28 = Option::<Arguments<'_>>::None;
- _22 = core::panicking::assert_failed::<*const u8, *const u8>(move _23, move _24, move _26, move _28) -> unwind unreachable;
+ _22 = core::panicking::assert_failed::<*const u8, *const u8>(const core::panicking::AssertKind::Eq, move _24, move _26, move _28) -> unwind unreachable;
- _22 = assert_failed::<*const u8, *const u8>(move _23, move _24, move _26, move _28) -> unwind unreachable;
+ _22 = assert_failed::<*const u8, *const u8>(const core::panicking::AssertKind::Eq, move _24, move _26, move _28) -> unwind unreachable;
}
bb7: {
@ -306,8 +306,8 @@
_52 = &(*_53);
StorageLive(_54);
_54 = Option::<Arguments<'_>>::None;
- _48 = core::panicking::assert_failed::<*const u8, *const u8>(move _49, move _50, move _52, move _54) -> unwind unreachable;
+ _48 = core::panicking::assert_failed::<*const u8, *const u8>(const core::panicking::AssertKind::Eq, move _50, move _52, move _54) -> unwind unreachable;
- _48 = assert_failed::<*const u8, *const u8>(move _49, move _50, move _52, move _54) -> unwind unreachable;
+ _48 = assert_failed::<*const u8, *const u8>(const core::panicking::AssertKind::Eq, move _50, move _52, move _54) -> unwind unreachable;
}
}

View File

@ -207,8 +207,8 @@
_26 = &(*_27);
StorageLive(_28);
_28 = Option::<Arguments<'_>>::None;
- _22 = core::panicking::assert_failed::<*const u8, *const u8>(move _23, move _24, move _26, move _28) -> unwind continue;
+ _22 = core::panicking::assert_failed::<*const u8, *const u8>(const core::panicking::AssertKind::Eq, move _24, move _26, move _28) -> unwind continue;
- _22 = assert_failed::<*const u8, *const u8>(move _23, move _24, move _26, move _28) -> unwind continue;
+ _22 = assert_failed::<*const u8, *const u8>(const core::panicking::AssertKind::Eq, move _24, move _26, move _28) -> unwind continue;
}
bb7: {
@ -306,8 +306,8 @@
_52 = &(*_53);
StorageLive(_54);
_54 = Option::<Arguments<'_>>::None;
- _48 = core::panicking::assert_failed::<*const u8, *const u8>(move _49, move _50, move _52, move _54) -> unwind continue;
+ _48 = core::panicking::assert_failed::<*const u8, *const u8>(const core::panicking::AssertKind::Eq, move _50, move _52, move _54) -> unwind continue;
- _48 = assert_failed::<*const u8, *const u8>(move _49, move _50, move _52, move _54) -> unwind continue;
+ _48 = assert_failed::<*const u8, *const u8>(const core::panicking::AssertKind::Eq, move _50, move _52, move _54) -> unwind continue;
}
}