[ic] Use the Map for hashing in the secondary stub cache

Avoid repeated collisions when the name doesn't hold much entropy.
This is typically the case with minified sources where 1 or 2 letter
names are used very frequently.

Bug: v8:12316
Change-Id: I20df3a6b0c5daf7975668d25404eca94a1230fe0
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3222759Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77416}

src/ic/accessor-assembler.cc

@@ -2769,14 +2769,17 @@ TNode<IntPtrT> AccessorAssembler::StubCachePrimaryOffset(TNode<Name> name,
   return Signed(result);
 }
 
-TNode<IntPtrT> AccessorAssembler::StubCacheSecondaryOffset(
-    TNode<Name> name, TNode<IntPtrT> seed) {
+TNode<IntPtrT> AccessorAssembler::StubCacheSecondaryOffset(TNode<Name> name,
+                                                           TNode<Map> map) {
   // See v8::internal::StubCache::SecondaryOffset().
 
   // Use the seed from the primary cache in the secondary cache.
   TNode<Int32T> name32 = TruncateIntPtrToInt32(BitcastTaggedToWord(name));
-  TNode<Int32T> hash = Int32Sub(TruncateIntPtrToInt32(seed), name32);
-  hash = Int32Add(hash, Int32Constant(StubCache::kSecondaryMagic));
+  TNode<Int32T> map32 = TruncateIntPtrToInt32(BitcastTaggedToWord(map));
+  // Base the offset on a simple combination of name and map.
+  TNode<Word32T> hash_a = Int32Add(map32, name32);
+  TNode<Word32T> hash_b = Word32Shr(hash_a, StubCache::kSecondaryKeyShift);
+  TNode<Word32T> hash = Int32Add(hash_a, hash_b);
   int32_t mask = (StubCache::kSecondaryTableSize - 1)
                  << StubCache::kCacheIndexShift;
   TNode<UintPtrT> result =
@@ -2846,7 +2849,7 @@ void AccessorAssembler::TryProbeStubCache(StubCache* stub_cache,
   {
     // Probe the secondary table.
     TNode<IntPtrT> secondary_offset =
-        StubCacheSecondaryOffset(name, primary_offset);
+        StubCacheSecondaryOffset(name, lookup_start_object_map);
     TryProbeStubCacheTable(stub_cache, kSecondary, secondary_offset, name,
                            lookup_start_object_map, if_handler, var_handler,
                            &miss);

src/ic/accessor-assembler.h

@@ -83,8 +83,8 @@ class V8_EXPORT_PRIVATE AccessorAssembler : public CodeStubAssembler {
     return StubCachePrimaryOffset(name, map);
   }
   TNode<IntPtrT> StubCacheSecondaryOffsetForTesting(TNode<Name> name,
-                                                    TNode<IntPtrT> seed) {
-    return StubCacheSecondaryOffset(name, seed);
+                                                    TNode<Map> map) {
+    return StubCacheSecondaryOffset(name, map);
   }
 
   struct LoadICParameters {
@@ -512,8 +512,7 @@ class V8_EXPORT_PRIVATE AccessorAssembler : public CodeStubAssembler {
   enum StubCacheTable : int;
 
   TNode<IntPtrT> StubCachePrimaryOffset(TNode<Name> name, TNode<Map> map);
-  TNode<IntPtrT> StubCacheSecondaryOffset(TNode<Name> name,
-                                          TNode<IntPtrT> seed);
+  TNode<IntPtrT> StubCacheSecondaryOffset(TNode<Name> name, TNode<Map> map);
 
   void TryProbeStubCacheTable(StubCache* stub_cache, StubCacheTable table_id,
                               TNode<IntPtrT> entry_offset, TNode<Object> name,

src/ic/stub-cache.cc

@@ -44,12 +44,14 @@ int StubCache::PrimaryOffset(Name name, Map map) {
 }
 
 // Hash algorithm for the secondary table.  This algorithm is replicated in
-// assembler for every architecture.  Returns an index into the table that
-// is scaled by 1 << kCacheIndexShift.
-int StubCache::SecondaryOffset(Name name, int seed) {
-  // Use the seed from the primary cache in the secondary cache.
+// assembler. This hash should be sufficiently different from the primary one
+// in order to avoid collisions for minified code with short names.
+// Returns an index into the table that is scaled by 1 << kCacheIndexShift.
+int StubCache::SecondaryOffset(Name name, Map old_map) {
   uint32_t name_low32bits = static_cast<uint32_t>(name.ptr());
-  uint32_t key = (seed - name_low32bits) + kSecondaryMagic;
+  uint32_t map_low32bits = static_cast<uint32_t>(old_map.ptr());
+  uint32_t key = (map_low32bits + name_low32bits);
+  key = key + (key >> kSecondaryKeyShift);
   return key & ((kSecondaryTableSize - 1) << kCacheIndexShift);
 }
 
@@ -57,8 +59,8 @@ int StubCache::PrimaryOffsetForTesting(Name name, Map map) {
   return PrimaryOffset(name, map);
 }
 
-int StubCache::SecondaryOffsetForTesting(Name name, int seed) {
-  return SecondaryOffset(name, seed);
+int StubCache::SecondaryOffsetForTesting(Name name, Map map) {
+  return SecondaryOffset(name, map);
 }
 
 #ifdef DEBUG
@@ -93,11 +95,9 @@ void StubCache::Set(Name name, Map map, MaybeObject handler) {
       !primary->map.IsSmi()) {
     Map old_map =
         Map::cast(StrongTaggedValue::ToObject(isolate(), primary->map));
-    int seed = PrimaryOffset(
-        Name::cast(StrongTaggedValue::ToObject(isolate(), primary->key)),
-        old_map);
-    int secondary_offset = SecondaryOffset(
-        Name::cast(StrongTaggedValue::ToObject(isolate(), primary->key)), seed);
+    Name old_name =
+        Name::cast(StrongTaggedValue::ToObject(isolate(), primary->key));
+    int secondary_offset = SecondaryOffset(old_name, old_map);
     Entry* secondary = entry(secondary_, secondary_offset);
     *secondary = *primary;
   }
@@ -116,7 +116,7 @@ MaybeObject StubCache::Get(Name name, Map map) {
   if (primary->key == name && primary->map == map) {
     return TaggedValue::ToMaybeObject(isolate(), primary->value);
   }
-  int secondary_offset = SecondaryOffset(name, primary_offset);
+  int secondary_offset = SecondaryOffset(name, map);
   Entry* secondary = entry(secondary_, secondary_offset);
   if (secondary->key == name && secondary->map == map) {
     return TaggedValue::ToMaybeObject(isolate(), secondary->value);

src/ic/stub-cache.h

@@ -90,15 +90,13 @@ class V8_EXPORT_PRIVATE StubCache {
   static const int kSecondaryTableBits = 9;
   static const int kSecondaryTableSize = (1 << kSecondaryTableBits);
 
-  // We compute the hash code for a map as follows:
-  //   <code> = <address> ^ (<address> >> kMapKeyShift)
+  // Used to introduce more entropy from the higher bits of the Map address.
+  // This should fill in the masked out kCacheIndexShift-bits.
   static const int kMapKeyShift = kPrimaryTableBits + kCacheIndexShift;
-
-  // Some magic number used in the secondary hash computation.
-  static const int kSecondaryMagic = 0xb16ca6e5;
+  static const int kSecondaryKeyShift = kSecondaryTableBits + kCacheIndexShift;
 
   static int PrimaryOffsetForTesting(Name name, Map map);
-  static int SecondaryOffsetForTesting(Name name, int seed);
+  static int SecondaryOffsetForTesting(Name name, Map map);
 
   // The constructor is made public only for the purposes of testing.
   explicit StubCache(Isolate* isolate);
@@ -121,7 +119,7 @@ class V8_EXPORT_PRIVATE StubCache {
   // Hash algorithm for the secondary table.  This algorithm is replicated in
   // assembler for every architecture.  Returns an index into the table that
   // is scaled by 1 << kCacheIndexShift.
-  static int SecondaryOffset(Name name, int seed);
+  static int SecondaryOffset(Name name, Map map);
 
   // Compute the entry for a given offset in exactly the same way as
   // we do in generated code.  We generate an hash code that already

test/cctest/test-accessor-assembler.cc

@@ -37,7 +37,7 @@ void TestStubCacheOffsetCalculation(StubCache::Table table) {
       result = primary_offset;
     } else {
       CHECK_EQ(StubCache::kSecondary, table);
-      result = m.StubCacheSecondaryOffsetForTesting(name, primary_offset);
+      result = m.StubCacheSecondaryOffsetForTesting(name, map);
     }
     m.Return(m.SmiTag(result));
   }
@@ -83,8 +83,7 @@ void TestStubCacheOffsetCalculation(StubCache::Table table) {
         if (table == StubCache::kPrimary) {
           expected_result = primary_offset;
         } else {
-          expected_result =
-              StubCache::SecondaryOffsetForTesting(*name, primary_offset);
+          expected_result = StubCache::SecondaryOffsetForTesting(*name, *map);
         }
       }
       Handle<Object> result = ft.Call(name, map).ToHandleChecked();