From 3d0eda7af8699b3a9dc51dc339d7d5faae753e97 Mon Sep 17 00:00:00 2001 From: Camille Gillot Date: Fri, 22 Aug 2025 01:38:58 +0000 Subject: [PATCH 1/4] Introduce ValueSet. --- Cargo.lock | 1 + compiler/rustc_mir_transform/Cargo.toml | 1 + compiler/rustc_mir_transform/src/gvn.rs | 117 +++++++++++++++++++----- 3 files changed, 98 insertions(+), 21 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4677d34d2a630..328641de1e0c2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4235,6 +4235,7 @@ name = "rustc_mir_transform" version = "0.0.0" dependencies = [ "either", + "hashbrown", "itertools", "rustc_abi", "rustc_arena", diff --git a/compiler/rustc_mir_transform/Cargo.toml b/compiler/rustc_mir_transform/Cargo.toml index 08c43a4648c67..511c1960e40b4 100644 --- a/compiler/rustc_mir_transform/Cargo.toml +++ b/compiler/rustc_mir_transform/Cargo.toml @@ -6,6 +6,7 @@ edition = "2024" [dependencies] # tidy-alphabetical-start either = "1" +hashbrown = "0.15" itertools = "0.12" rustc_abi = { path = "../rustc_abi" } rustc_arena = { path = "../rustc_arena" } diff --git a/compiler/rustc_mir_transform/src/gvn.rs b/compiler/rustc_mir_transform/src/gvn.rs index bf6aa800d20f4..3765dc5c92aef 100644 --- a/compiler/rustc_mir_transform/src/gvn.rs +++ b/compiler/rustc_mir_transform/src/gvn.rs @@ -85,8 +85,10 @@ //! that contain `AllocId`s. use std::borrow::Cow; +use std::hash::{Hash, Hasher}; use either::Either; +use hashbrown::hash_table::{Entry, HashTable}; use itertools::Itertools as _; use rustc_abi::{self as abi, BackendRepr, FIRST_VARIANT, FieldIdx, Primitive, Size, VariantIdx}; use rustc_const_eval::const_eval::DummyMachine; @@ -94,7 +96,7 @@ use rustc_const_eval::interpret::{ ImmTy, Immediate, InterpCx, MemPlaceMeta, MemoryKind, OpTy, Projectable, Scalar, intern_const_alloc_for_constprop, }; -use rustc_data_structures::fx::{FxIndexSet, MutableValues}; +use rustc_data_structures::fx::FxHasher; use rustc_data_structures::graph::dominators::Dominators; use rustc_hir::def::DefKind; use rustc_index::bit_set::DenseBitSet; @@ -152,6 +154,7 @@ impl<'tcx> crate::MirPass<'tcx> for GVN { } newtype_index! { + #[debug_format = "_v{}"] struct VnIndex {} } @@ -213,6 +216,89 @@ enum Value<'tcx> { }, } +/// Stores and deduplicates pairs of `(Value, Ty)` into in `VnIndex` numbered values. +/// +/// This data structure is mostly a partial reimplementation of `FxIndexMap`. +/// We do not use a regular `FxIndexMap` to skip hashing values that are unique by construction, +/// like opaque values, address with provenance and non-deterministic constants. +struct ValueSet<'tcx> { + indices: HashTable, + hashes: IndexVec, + values: IndexVec>, + types: IndexVec>, + /// Counter to generate different values. + next_opaque: usize, +} + +impl<'tcx> ValueSet<'tcx> { + fn new(num_values: usize) -> ValueSet<'tcx> { + ValueSet { + indices: HashTable::with_capacity(num_values), + hashes: IndexVec::with_capacity(num_values), + values: IndexVec::with_capacity(num_values), + types: IndexVec::with_capacity(num_values), + next_opaque: 1, + } + } + + /// Insert a `(Value, Ty)` pair to be deduplicated. + /// Returns `true` as second tuple field if this value did not exist previously. + #[allow(rustc::pass_by_value)] // closures take `&VnIndex` + fn insert(&mut self, ty: Ty<'tcx>, value: Value<'tcx>) -> (VnIndex, bool) { + let hash: u64 = { + let mut h = FxHasher::default(); + value.hash(&mut h); + ty.hash(&mut h); + h.finish() + }; + + let eq = |index: &VnIndex| self.values[*index] == value && self.types[*index] == ty; + let hasher = |index: &VnIndex| self.hashes[*index]; + match self.indices.entry(hash, eq, hasher) { + Entry::Occupied(entry) => { + let index = *entry.get(); + (index, false) + } + Entry::Vacant(entry) => { + let index = self.hashes.push(hash); + entry.insert(index); + let _index = self.values.push(value); + debug_assert_eq!(index, _index); + let _index = self.types.push(ty); + debug_assert_eq!(index, _index); + (index, true) + } + } + } + + /// Increment the opaque index counter return a new unique value. + #[inline] + fn next_opaque(&mut self) -> usize { + let next_opaque = self.next_opaque; + self.next_opaque += 1; + next_opaque + } + + /// Return the `Value` associated with the given `VnIndex`. + #[inline] + fn value(&self, index: VnIndex) -> &Value<'tcx> { + &self.values[index] + } + + /// Return the type associated with the given `VnIndex`. + #[inline] + fn ty(&self, index: VnIndex) -> Ty<'tcx> { + self.types[index] + } + + /// Replace the value associated with `index` with an opaque value. + #[inline] + fn forget(&mut self, index: VnIndex) { + let opaque = self.next_opaque(); + self.values[index] = Value::Opaque(opaque); + } +} + struct VnState<'body, 'tcx> { tcx: TyCtxt<'tcx>, ecx: InterpCx<'tcx, DummyMachine>, @@ -223,11 +309,9 @@ struct VnState<'body, 'tcx> { /// Locals that are assigned that value. // This vector does not hold all the values of `VnIndex` that we create. rev_locals: IndexVec>, - values: FxIndexSet<(Value<'tcx>, Ty<'tcx>)>, + values: ValueSet<'tcx>, /// Values evaluated as constants if possible. evaluated: IndexVec>>, - /// Counter to generate different values. - next_opaque: usize, /// Cache the deref values. derefs: Vec, ssa: &'body SsaLocals, @@ -258,9 +342,8 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { is_coroutine: body.coroutine.is_some(), locals: IndexVec::from_elem(None, local_decls), rev_locals: IndexVec::with_capacity(num_values), - values: FxIndexSet::with_capacity_and_hasher(num_values, Default::default()), + values: ValueSet::new(num_values), evaluated: IndexVec::with_capacity(num_values), - next_opaque: 1, derefs: Vec::new(), ssa, dominators, @@ -274,8 +357,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { #[instrument(level = "trace", skip(self), ret)] fn insert(&mut self, ty: Ty<'tcx>, value: Value<'tcx>) -> VnIndex { - let (index, new) = self.values.insert_full((value, ty)); - let index = VnIndex::from_usize(index); + let (index, new) = self.values.insert(ty, value); if new { // Grow `evaluated` and `rev_locals` here to amortize the allocations. let evaluated = self.eval_to_const(index); @@ -287,17 +369,11 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { index } - fn next_opaque(&mut self) -> usize { - let next_opaque = self.next_opaque; - self.next_opaque += 1; - next_opaque - } - /// Create a new `Value` for which we have no information at all, except that it is distinct /// from all the others. #[instrument(level = "trace", skip(self), ret)] fn new_opaque(&mut self, ty: Ty<'tcx>) -> VnIndex { - let value = Value::Opaque(self.next_opaque()); + let value = Value::Opaque(self.values.next_opaque()); self.insert(ty, value) } @@ -311,18 +387,18 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { } AddressKind::Address(mutbl) => Ty::new_ptr(self.tcx, pty, mutbl.to_mutbl_lossy()), }; - let value = Value::Address { place, kind, provenance: self.next_opaque() }; + let value = Value::Address { place, kind, provenance: self.values.next_opaque() }; self.insert(ty, value) } #[inline] fn get(&self, index: VnIndex) -> &Value<'tcx> { - &self.values.get_index(index.as_usize()).unwrap().0 + self.values.value(index) } #[inline] fn ty(&self, index: VnIndex) -> Ty<'tcx> { - self.values.get_index(index.as_usize()).unwrap().1 + self.values.ty(index) } /// Record that `local` is assigned `value`. `local` must be SSA. @@ -340,7 +416,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { } 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 disambiguator = self.next_opaque(); + let disambiguator = self.values.next_opaque(); // `disambiguator: 0` means deterministic. debug_assert_ne!(disambiguator, 0); disambiguator @@ -374,8 +450,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { fn invalidate_derefs(&mut self) { for deref in std::mem::take(&mut self.derefs) { - let opaque = self.next_opaque(); - self.values.get_index_mut2(deref.index()).unwrap().0 = Value::Opaque(opaque); + self.values.forget(deref); } } From 7f34f6e25f143d5c8495aca9ddafb32a87b02303 Mon Sep 17 00:00:00 2001 From: Camille Gillot Date: Fri, 22 Aug 2025 01:50:21 +0000 Subject: [PATCH 2/4] Do not hash opaques in GVN. --- compiler/rustc_mir_transform/src/gvn.rs | 129 +++++++++++++++++------- 1 file changed, 90 insertions(+), 39 deletions(-) diff --git a/compiler/rustc_mir_transform/src/gvn.rs b/compiler/rustc_mir_transform/src/gvn.rs index 3765dc5c92aef..93b60aacbbaca 100644 --- a/compiler/rustc_mir_transform/src/gvn.rs +++ b/compiler/rustc_mir_transform/src/gvn.rs @@ -100,7 +100,7 @@ use rustc_data_structures::fx::FxHasher; use rustc_data_structures::graph::dominators::Dominators; use rustc_hir::def::DefKind; use rustc_index::bit_set::DenseBitSet; -use rustc_index::{IndexVec, newtype_index}; +use rustc_index::{Idx, IndexVec, newtype_index}; use rustc_middle::bug; use rustc_middle::mir::interpret::GlobalAlloc; use rustc_middle::mir::visit::*; @@ -158,6 +158,14 @@ newtype_index! { struct VnIndex {} } +newtype_index! { + /// Counter type to ensure that all unique values are created using `insert_unique`. + #[debug_format = "_o{}"] + struct VnOpaque { + const DETERMINISTIC = 0; + } +} + #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] enum AddressKind { Ref(BorrowKind), @@ -169,14 +177,14 @@ enum Value<'tcx> { // Root values. /// Used to represent values we know nothing about. /// The `usize` is a counter incremented by `new_opaque`. - Opaque(usize), + Opaque(VnOpaque), /// Evaluated or unevaluated constant value. 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` is 0 iff the constant is deterministic. - disambiguator: usize, + // `disambiguator` is `DETERMINISTIC` iff the constant is deterministic. + disambiguator: VnOpaque, }, /// An aggregate value, either tuple/closure/struct/enum. /// This does not contain unions, as we cannot reason with the value. @@ -195,7 +203,7 @@ enum Value<'tcx> { place: Place<'tcx>, kind: AddressKind, /// Give each borrow and pointer a different provenance, so we don't merge them. - provenance: usize, + provenance: VnOpaque, }, // Extractions. @@ -227,7 +235,7 @@ struct ValueSet<'tcx> { values: IndexVec>, types: IndexVec>, /// Counter to generate different values. - next_opaque: usize, + next_opaque: VnOpaque, } impl<'tcx> ValueSet<'tcx> { @@ -237,14 +245,45 @@ impl<'tcx> ValueSet<'tcx> { hashes: IndexVec::with_capacity(num_values), values: IndexVec::with_capacity(num_values), types: IndexVec::with_capacity(num_values), - next_opaque: 1, + // The first opaque is 1, as 0 means deterministic constant. + next_opaque: VnOpaque::from_u32(1), } } + /// Insert a `(Value, Ty)` pair without hashing or deduplication. + #[inline] + fn insert_unique( + &mut self, + ty: Ty<'tcx>, + value: impl FnOnce(VnOpaque) -> Value<'tcx>, + ) -> VnIndex { + let value = value(self.next_opaque); + self.next_opaque.increment_by(1); + + debug_assert!(match value { + Value::Opaque(_) | Value::Address { .. } => true, + Value::Constant { disambiguator, .. } => disambiguator != DETERMINISTIC, + _ => false, + }); + + let index = self.hashes.push(0); + let _index = self.types.push(ty); + debug_assert_eq!(index, _index); + let _index = self.values.push(value); + debug_assert_eq!(index, _index); + index + } + /// Insert a `(Value, Ty)` pair to be deduplicated. /// Returns `true` as second tuple field if this value did not exist previously. #[allow(rustc::pass_by_value)] // closures take `&VnIndex` fn insert(&mut self, ty: Ty<'tcx>, value: Value<'tcx>) -> (VnIndex, bool) { + debug_assert!(match value { + Value::Opaque(_) | Value::Address { .. } => false, + Value::Constant { disambiguator, .. } => disambiguator == DETERMINISTIC, + _ => true, + }); + let hash: u64 = { let mut h = FxHasher::default(); value.hash(&mut h); @@ -271,14 +310,6 @@ impl<'tcx> ValueSet<'tcx> { } } - /// Increment the opaque index counter return a new unique value. - #[inline] - fn next_opaque(&mut self) -> usize { - let next_opaque = self.next_opaque; - self.next_opaque += 1; - next_opaque - } - /// Return the `Value` associated with the given `VnIndex`. #[inline] fn value(&self, index: VnIndex) -> &Value<'tcx> { @@ -294,8 +325,8 @@ impl<'tcx> ValueSet<'tcx> { /// Replace the value associated with `index` with an opaque value. #[inline] fn forget(&mut self, index: VnIndex) { - let opaque = self.next_opaque(); - self.values[index] = Value::Opaque(opaque); + self.values[index] = Value::Opaque(self.next_opaque); + self.next_opaque.increment_by(1); } } @@ -373,8 +404,12 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { /// from all the others. #[instrument(level = "trace", skip(self), ret)] fn new_opaque(&mut self, ty: Ty<'tcx>) -> VnIndex { - let value = Value::Opaque(self.values.next_opaque()); - self.insert(ty, value) + let index = self.values.insert_unique(ty, Value::Opaque); + let _index = self.evaluated.push(None); + debug_assert_eq!(index, _index); + let _index = self.rev_locals.push(SmallVec::new()); + debug_assert_eq!(index, _index); + index } /// Create a new `Value::Address` distinct from all the others. @@ -387,8 +422,39 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { } AddressKind::Address(mutbl) => Ty::new_ptr(self.tcx, pty, mutbl.to_mutbl_lossy()), }; - let value = Value::Address { place, kind, provenance: self.values.next_opaque() }; - self.insert(ty, value) + let index = + self.values.insert_unique(ty, |provenance| Value::Address { place, kind, provenance }); + let evaluated = self.eval_to_const(index); + let _index = self.evaluated.push(evaluated); + debug_assert_eq!(index, _index); + let _index = self.rev_locals.push(SmallVec::new()); + debug_assert_eq!(index, _index); + index + } + + #[instrument(level = "trace", skip(self), ret)] + fn insert_constant(&mut self, value: Const<'tcx>) -> VnIndex { + let (index, new) = if value.is_deterministic() { + // The constant is deterministic, no need to disambiguate. + let constant = Value::Constant { value, disambiguator: DETERMINISTIC }; + self.values.insert(value.ty(), constant) + } 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 index = self.values.insert_unique(value.ty(), |disambiguator| { + debug_assert_ne!(disambiguator, DETERMINISTIC); + Value::Constant { value, disambiguator } + }); + (index, true) + }; + if new { + let evaluated = self.eval_to_const(index); + let _index = self.evaluated.push(evaluated); + debug_assert_eq!(index, _index); + let _index = self.rev_locals.push(SmallVec::new()); + debug_assert_eq!(index, _index); + } + index } #[inline] @@ -409,33 +475,18 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { self.rev_locals[value].push(local); } - fn insert_constant(&mut self, value: Const<'tcx>) -> 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 disambiguator = self.values.next_opaque(); - // `disambiguator: 0` means deterministic. - debug_assert_ne!(disambiguator, 0); - disambiguator - }; - self.insert(value.ty(), Value::Constant { value, disambiguator }) - } - fn insert_bool(&mut self, flag: bool) -> VnIndex { // Booleans are deterministic. let value = Const::from_bool(self.tcx, flag); debug_assert!(value.is_deterministic()); - self.insert(self.tcx.types.bool, Value::Constant { value, disambiguator: 0 }) + self.insert(self.tcx.types.bool, Value::Constant { value, disambiguator: DETERMINISTIC }) } fn insert_scalar(&mut self, ty: Ty<'tcx>, scalar: Scalar) -> VnIndex { // Scalars are deterministic. let value = Const::from_scalar(self.tcx, scalar, ty); debug_assert!(value.is_deterministic()); - self.insert(ty, Value::Constant { value, disambiguator: 0 }) + self.insert(ty, Value::Constant { value, disambiguator: DETERMINISTIC }) } fn insert_tuple(&mut self, ty: Ty<'tcx>, values: Vec) -> VnIndex { @@ -1680,7 +1731,7 @@ impl<'tcx> VnState<'_, '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: 0 } = *self.get(index) { + if let Value::Constant { value, disambiguator: DETERMINISTIC } = *self.get(index) { debug_assert!(value.is_deterministic()); return Some(ConstOperand { span: DUMMY_SP, user_ty: None, const_: value }); } From 0a911ec97f9ce39eb52dd3e5da6232e733cdee47 Mon Sep 17 00:00:00 2001 From: Camille Gillot Date: Tue, 9 Sep 2025 01:37:48 +0000 Subject: [PATCH 3/4] Stop counting opaques. --- compiler/rustc_mir_transform/src/gvn.rs | 49 ++++++++++++------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_mir_transform/src/gvn.rs b/compiler/rustc_mir_transform/src/gvn.rs index 93b60aacbbaca..f8b3e5c6f8794 100644 --- a/compiler/rustc_mir_transform/src/gvn.rs +++ b/compiler/rustc_mir_transform/src/gvn.rs @@ -100,7 +100,7 @@ use rustc_data_structures::fx::FxHasher; use rustc_data_structures::graph::dominators::Dominators; use rustc_hir::def::DefKind; use rustc_index::bit_set::DenseBitSet; -use rustc_index::{Idx, IndexVec, newtype_index}; +use rustc_index::{IndexVec, newtype_index}; use rustc_middle::bug; use rustc_middle::mir::interpret::GlobalAlloc; use rustc_middle::mir::visit::*; @@ -158,11 +158,16 @@ newtype_index! { struct VnIndex {} } -newtype_index! { - /// Counter type to ensure that all unique values are created using `insert_unique`. - #[debug_format = "_o{}"] - struct VnOpaque { - const DETERMINISTIC = 0; +#[derive(Copy, Clone, Debug, Eq)] +struct VnOpaque; +impl PartialEq for VnOpaque { + fn eq(&self, _: &VnOpaque) -> bool { + unreachable!() + } +} +impl Hash for VnOpaque { + fn hash(&self, _: &mut T) { + unreachable!() } } @@ -183,8 +188,8 @@ enum Value<'tcx> { 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` is `DETERMINISTIC` iff the constant is deterministic. - disambiguator: VnOpaque, + // `disambiguator` is `None` iff the constant is deterministic. + disambiguator: Option, }, /// An aggregate value, either tuple/closure/struct/enum. /// This does not contain unions, as we cannot reason with the value. @@ -234,8 +239,6 @@ struct ValueSet<'tcx> { hashes: IndexVec, values: IndexVec>, types: IndexVec>, - /// Counter to generate different values. - next_opaque: VnOpaque, } impl<'tcx> ValueSet<'tcx> { @@ -245,8 +248,6 @@ impl<'tcx> ValueSet<'tcx> { hashes: IndexVec::with_capacity(num_values), values: IndexVec::with_capacity(num_values), types: IndexVec::with_capacity(num_values), - // The first opaque is 1, as 0 means deterministic constant. - next_opaque: VnOpaque::from_u32(1), } } @@ -257,12 +258,11 @@ impl<'tcx> ValueSet<'tcx> { ty: Ty<'tcx>, value: impl FnOnce(VnOpaque) -> Value<'tcx>, ) -> VnIndex { - let value = value(self.next_opaque); - self.next_opaque.increment_by(1); + let value = value(VnOpaque); debug_assert!(match value { Value::Opaque(_) | Value::Address { .. } => true, - Value::Constant { disambiguator, .. } => disambiguator != DETERMINISTIC, + Value::Constant { disambiguator, .. } => disambiguator.is_some(), _ => false, }); @@ -280,7 +280,7 @@ impl<'tcx> ValueSet<'tcx> { fn insert(&mut self, ty: Ty<'tcx>, value: Value<'tcx>) -> (VnIndex, bool) { debug_assert!(match value { Value::Opaque(_) | Value::Address { .. } => false, - Value::Constant { disambiguator, .. } => disambiguator == DETERMINISTIC, + Value::Constant { disambiguator, .. } => disambiguator.is_none(), _ => true, }); @@ -325,8 +325,7 @@ impl<'tcx> ValueSet<'tcx> { /// Replace the value associated with `index` with an opaque value. #[inline] fn forget(&mut self, index: VnIndex) { - self.values[index] = Value::Opaque(self.next_opaque); - self.next_opaque.increment_by(1); + self.values[index] = Value::Opaque(VnOpaque); } } @@ -436,14 +435,14 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { fn insert_constant(&mut self, value: Const<'tcx>) -> VnIndex { let (index, new) = if value.is_deterministic() { // The constant is deterministic, no need to disambiguate. - let constant = Value::Constant { value, disambiguator: DETERMINISTIC }; + let constant = Value::Constant { value, disambiguator: None }; self.values.insert(value.ty(), constant) } 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 index = self.values.insert_unique(value.ty(), |disambiguator| { - debug_assert_ne!(disambiguator, DETERMINISTIC); - Value::Constant { value, disambiguator } + let index = self.values.insert_unique(value.ty(), |disambiguator| Value::Constant { + value, + disambiguator: Some(disambiguator), }); (index, true) }; @@ -479,14 +478,14 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { // Booleans are deterministic. let value = Const::from_bool(self.tcx, flag); debug_assert!(value.is_deterministic()); - self.insert(self.tcx.types.bool, Value::Constant { value, disambiguator: DETERMINISTIC }) + self.insert(self.tcx.types.bool, Value::Constant { value, disambiguator: None }) } fn insert_scalar(&mut self, ty: Ty<'tcx>, scalar: Scalar) -> VnIndex { // Scalars are deterministic. let value = Const::from_scalar(self.tcx, scalar, ty); debug_assert!(value.is_deterministic()); - self.insert(ty, Value::Constant { value, disambiguator: DETERMINISTIC }) + self.insert(ty, Value::Constant { value, disambiguator: None }) } fn insert_tuple(&mut self, ty: Ty<'tcx>, values: Vec) -> VnIndex { @@ -1731,7 +1730,7 @@ impl<'tcx> VnState<'_, '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: DETERMINISTIC } = *self.get(index) { + if let Value::Constant { value, disambiguator: None } = *self.get(index) { debug_assert!(value.is_deterministic()); return Some(ConstOperand { span: DUMMY_SP, user_ty: None, const_: value }); } From df04be8cf744b500b5972fdc76898a96067d5517 Mon Sep 17 00:00:00 2001 From: Camille Gillot Date: Sun, 14 Sep 2025 13:18:19 +0000 Subject: [PATCH 4/4] Comment. --- compiler/rustc_mir_transform/src/gvn.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/compiler/rustc_mir_transform/src/gvn.rs b/compiler/rustc_mir_transform/src/gvn.rs index f8b3e5c6f8794..fc0a03b1aab52 100644 --- a/compiler/rustc_mir_transform/src/gvn.rs +++ b/compiler/rustc_mir_transform/src/gvn.rs @@ -154,19 +154,25 @@ impl<'tcx> crate::MirPass<'tcx> for GVN { } newtype_index! { + /// This represents a `Value` in the symbolic execution. #[debug_format = "_v{}"] struct VnIndex {} } +/// Marker type to forbid hashing and comparing opaque values. +/// This struct should only be constructed by `ValueSet::insert_unique` to ensure we use that +/// method to create non-unifiable values. It will ICE if used in `ValueSet::insert`. #[derive(Copy, Clone, Debug, Eq)] struct VnOpaque; impl PartialEq for VnOpaque { fn eq(&self, _: &VnOpaque) -> bool { + // ICE if we try to compare unique values unreachable!() } } impl Hash for VnOpaque { fn hash(&self, _: &mut T) { + // ICE if we try to hash unique values unreachable!() } } @@ -191,6 +197,8 @@ enum Value<'tcx> { // `disambiguator` is `None` iff the constant is deterministic. disambiguator: Option, }, + + // Aggregates. /// An aggregate value, either tuple/closure/struct/enum. /// This does not contain unions, as we cannot reason with the value. Aggregate(VariantIdx, Vec), @@ -252,6 +260,7 @@ impl<'tcx> ValueSet<'tcx> { } /// Insert a `(Value, Ty)` pair without hashing or deduplication. + /// This always creates a new `VnIndex`. #[inline] fn insert_unique( &mut self,