Auto merge of #145737 - cjgillot:gvn-valueset, r=<try>

GVN: stop hashing opaque values

Author: rust-bors[bot]

Cargo.lock

@@ -4293,6 +4293,7 @@ name = "rustc_mir_transform"
 version = "0.0.0"
 dependencies = [
  "either",
+ "hashbrown",
  "itertools",
  "rustc_abi",
  "rustc_arena",

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" }

compiler/rustc_mir_transform/src/gvn.rs

@@ -85,15 +85,17 @@
 //! that contain `AllocId`s.
 
 use std::borrow::Cow;
+use std::hash::{Hash, Hasher};
 
 use either::Either;
+use hashbrown::hash_table::{Entry, HashTable};
 use rustc_abi::{self as abi, BackendRepr, FIRST_VARIANT, FieldIdx, Primitive, Size, VariantIdx};
 use rustc_const_eval::const_eval::DummyMachine;
 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;
@@ -151,9 +153,15 @@ impl<'tcx> crate::MirPass<'tcx> for GVN {
 }
 
 newtype_index! {
+    #[debug_format = "_v{}"]
     struct VnIndex {}
 }
 
+newtype_index! {
+    #[debug_format = "_o{}"]
+    struct VnOpaque {}
+}
+
 #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
 enum AddressKind {
     Ref(BorrowKind),
@@ -165,7 +173,7 @@ 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>,
@@ -212,6 +220,85 @@ enum Value<'tcx> {
     },
 }
 
+struct ValueSet<'tcx> {
+    indices: HashTable<VnIndex>,
+    hashes: IndexVec<VnIndex, u64>,
+    values: IndexVec<VnIndex, Value<'tcx>>,
+    types: IndexVec<VnIndex, Ty<'tcx>>,
+    opaques: IndexVec<VnOpaque, VnIndex>,
+}
+
+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),
+            opaques: IndexVec::with_capacity(num_values),
+        }
+    }
+
+    #[allow(rustc::pass_by_value)]
+    fn insert(&mut self, value: Value<'tcx>, ty: Ty<'tcx>) -> (VnIndex, bool) {
+        if let Value::Opaque(opaque) = value {
+            return (self.opaques[opaque], false);
+        }
+
+        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)
+            }
+        }
+    }
+
+    #[inline]
+    fn new_opaque(&mut self, ty: Ty<'tcx>) -> VnIndex {
+        let index = self.hashes.push(0);
+        let _index = self.types.push(ty);
+        debug_assert_eq!(index, _index);
+        let opaque = self.opaques.push(index);
+        let _index = self.values.push(Value::Opaque(opaque));
+        debug_assert_eq!(index, _index);
+        index
+    }
+
+    #[inline]
+    fn value(&self, index: VnIndex) -> &Value<'tcx> {
+        &self.values[index]
+    }
+
+    #[inline]
+    fn ty(&self, index: VnIndex) -> Ty<'tcx> {
+        self.types[index]
+    }
+
+    #[inline]
+    fn forget(&mut self, index: VnIndex) {
+        let opaque = self.opaques.push(index);
+        self.values[index] = Value::Opaque(opaque);
+    }
+}
+
 struct VnState<'body, 'tcx> {
     tcx: TyCtxt<'tcx>,
     ecx: InterpCx<'tcx, DummyMachine>,
@@ -222,11 +309,11 @@ 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<VnIndex, SmallVec<[Local; 1]>>,
-    values: FxIndexSet<(Value<'tcx>, Ty<'tcx>)>,
+    values: ValueSet<'tcx>,
     /// Values evaluated as constants if possible.
     evaluated: IndexVec<VnIndex, Option<OpTy<'tcx>>>,
     /// Counter to generate different values.
-    next_opaque: usize,
+    next_disambiguator: usize,
     /// Cache the deref values.
     derefs: Vec<VnIndex>,
     ssa: &'body SsaLocals,
@@ -257,9 +344,9 @@ 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,
+            next_disambiguator: 1,
             derefs: Vec::new(),
             ssa,
             dominators,
@@ -273,8 +360,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(value, ty);
         if new {
             // Grow `evaluated` and `rev_locals` here to amortize the allocations.
             let evaluated = self.eval_to_const(index);
@@ -286,18 +372,23 @@ 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());
-        self.insert(ty, value)
+        let index = self.values.new_opaque(ty);
+        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
+    }
+
+    #[inline]
+    fn next_disambiguator(&mut self) -> usize {
+        let next_disambiguator = self.next_disambiguator;
+        self.next_disambiguator += 1;
+        next_disambiguator
     }
 
     /// Create a new `Value::Address` distinct from all the others.
@@ -310,18 +401,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.next_disambiguator() };
         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.
@@ -339,7 +430,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.next_disambiguator();
             // `disambiguator: 0` means deterministic.
             debug_assert_ne!(disambiguator, 0);
             disambiguator
@@ -373,8 +464,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);
         }
     }