Reland "[strings] Fix raw hash lookup for forwarded strings"

This is a reland of commit 0a1f0e33

Changes since revert:
- Deferred label for loading from forwarding table.
- Check if hash is computed instead of checking if it is a forwarding index.
- Retreive hash from forwarding table only if hash is assumed to be computed.

Original change's description:
> [strings] Fix raw hash lookup for forwarded strings
>
> Raw hashes may need to be looked up via the forwarding table when
> internalized strings are forwarded to external resources. Notably, the
> megamorphic ICs were not correctly fetching the raw hash.
>
> Bug: v8:12007
> Change-Id: Ibbc75de57e707788f544fbd1a0f8f0041350e29d
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3885379
> Reviewed-by: Leszek Swirski <leszeks@chromium.org>
> Commit-Queue: Shu-yu Guo <syg@chromium.org>
> Reviewed-by: Patrick Thier <pthier@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#83115}

Bug: v8:12007
Change-Id: Ia88ed51a49c62170bc960b8f69673bb1e59a6009
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3888057
Commit-Queue: Patrick Thier <pthier@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#83246}

src/codegen/code-stub-assembler.cc

@@ -1996,7 +1996,7 @@ TNode<IntPtrT> CodeStubAssembler::LoadJSReceiverIdentityHash(
 }
 
 TNode<Uint32T> CodeStubAssembler::LoadNameHashAssumeComputed(TNode<Name> name) {
-  TNode<Uint32T> hash_field = LoadNameRawHashField(name);
+  TNode<Uint32T> hash_field = LoadNameRawHash(name);
   CSA_DCHECK(this, IsClearWord32(hash_field, Name::kHashNotComputedMask));
   return DecodeWord32<Name::HashBits>(hash_field);
 }
@@ -2011,6 +2011,42 @@ TNode<Uint32T> CodeStubAssembler::LoadNameHash(TNode<Name> name,
   return DecodeWord32<Name::HashBits>(raw_hash_field);
 }
 
+TNode<Uint32T> CodeStubAssembler::LoadNameRawHash(TNode<Name> name) {
+  TVARIABLE(Uint32T, var_raw_hash);
+
+  Label if_forwarding_index(this, Label::kDeferred), done(this);
+
+  TNode<Uint32T> raw_hash_field = LoadNameRawHashField(name);
+  GotoIf(IsSetWord32(raw_hash_field, Name::kHashNotComputedMask),
+         &if_forwarding_index);
+
+  var_raw_hash = raw_hash_field;
+  Goto(&done);
+
+  BIND(&if_forwarding_index);
+  {
+    CSA_DCHECK(this,
+               IsEqualInWord32<Name::HashFieldTypeBits>(
+                   raw_hash_field, Name::HashFieldType::kForwardingIndex));
+    TNode<ExternalReference> function =
+        ExternalConstant(ExternalReference::raw_hash_from_forward_table());
+    const TNode<ExternalReference> isolate_ptr =
+        ExternalConstant(ExternalReference::isolate_address(isolate()));
+    TNode<Uint32T> result = UncheckedCast<Uint32T>(CallCFunction(
+        function, MachineType::Uint32(),
+        std::make_pair(MachineType::Pointer(), isolate_ptr),
+        std::make_pair(
+            MachineType::Int32(),
+            DecodeWord32<Name::ForwardingIndexValueBits>(raw_hash_field))));
+
+    var_raw_hash = result;
+    Goto(&done);
+  }
+
+  BIND(&done);
+  return var_raw_hash.value();
+}
+
 TNode<Smi> CodeStubAssembler::LoadStringLengthAsSmi(TNode<String> string) {
   return SmiFromIntPtr(LoadStringLengthAsWord(string));
 }
@@ -6760,6 +6796,9 @@ TNode<BoolT> CodeStubAssembler::IsUniqueName(TNode<HeapObject> object) {
 // characters, or is outside MAX_SAFE_INTEGER/size_t range). Note that for
 // non-TypedArray receivers, there are additional strings that must be treated
 // as named property keys, namely the range [0xFFFFFFFF, MAX_SAFE_INTEGER].
+// TODO(pthier): Handle forwarding indices correctly. The forwarded hash could
+// be an integer index. Consider using 1 bit of the forward index to indicate
+// the type of the forwarded hash.
 TNode<BoolT> CodeStubAssembler::IsUniqueNameNoIndex(TNode<HeapObject> object) {
   TNode<Uint16T> instance_type = LoadInstanceType(object);
   return Select<BoolT>(
@@ -6784,7 +6823,7 @@ TNode<BoolT> CodeStubAssembler::IsUniqueNameNoCachedIndex(
   return Select<BoolT>(
       IsInternalizedStringInstanceType(instance_type),
       [=] {
-        return IsSetWord32(LoadNameRawHashField(CAST(object)),
+        return IsSetWord32(LoadNameRawHash(CAST(object)),
                            Name::kDoesNotContainCachedArrayIndexMask);
       },
       [=] { return IsSymbolInstanceType(instance_type); });

src/codegen/code-stub-assembler.h

@@ -1451,6 +1451,10 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
                               Label* if_hash_not_computed = nullptr);
   TNode<Uint32T> LoadNameHashAssumeComputed(TNode<Name> name);
 
+  // Load the Name::RawHash() value of a name as an uint32 value. Follows
+  // through the forwarding table.
+  TNode<Uint32T> LoadNameRawHash(TNode<Name> name);
+
   // Load length field of a String object as Smi value.
   TNode<Smi> LoadStringLengthAsSmi(TNode<String> string);
   // Load length field of a String object as intptr_t value.

src/codegen/external-reference.cc

@@ -1022,6 +1022,8 @@ FUNCTION_REFERENCE(try_string_to_index_or_lookup_existing,
                    StringTable::TryStringToIndexOrLookupExisting)
 FUNCTION_REFERENCE(string_from_forward_table,
                    StringForwardingTable::GetForwardStringAddress)
+FUNCTION_REFERENCE(raw_hash_from_forward_table,
+                   StringForwardingTable::GetRawHashStatic)
 FUNCTION_REFERENCE(string_to_array_index_function, String::ToArrayIndex)
 FUNCTION_REFERENCE(array_indexof_includes_smi_or_object,
                    ArrayIndexOfIncludesSmiOrObject)

src/codegen/external-reference.h

@@ -208,6 +208,7 @@ class StatsCounter;
   V(try_string_to_index_or_lookup_existing,                                    \
     "try_string_to_index_or_lookup_existing")                                  \
   V(string_from_forward_table, "string_from_forward_table")                    \
+  V(raw_hash_from_forward_table, "raw_hash_from_forward_table")                \
   IF_WASM(V, wasm_call_trap_callback_for_testing,                              \
           "wasm::call_trap_callback_for_testing")                              \
   IF_WASM(V, wasm_f32_ceil, "wasm::f32_ceil_wrapper")                          \

src/ic/accessor-assembler.cc

@@ -2887,7 +2887,7 @@ enum AccessorAssembler::StubCacheTable : int {
 TNode<IntPtrT> AccessorAssembler::StubCachePrimaryOffset(TNode<Name> name,
                                                          TNode<Map> map) {
   // Compute the hash of the name (use entire hash field).
-  TNode<Uint32T> raw_hash_field = LoadNameRawHashField(name);
+  TNode<Uint32T> raw_hash_field = LoadNameRawHash(name);
   CSA_DCHECK(this,
              Word32Equal(Word32And(raw_hash_field,
                                    Int32Constant(Name::kHashNotComputedMask)),

src/ic/stub-cache.cc

@@ -50,7 +50,7 @@ void StubCache::Initialize() {
 // is scaled by 1 << kCacheIndexShift.
 int StubCache::PrimaryOffset(Name name, Map map) {
   // Compute the hash of the name (use entire hash field).
-  uint32_t field = name.raw_hash_field();
+  uint32_t field = name.RawHash();
   DCHECK(Name::IsHashFieldComputed(field));
   // Using only the low bits in 64-bit mode is unlikely to increase the
   // risk of collision even if the heap is spread over an area larger than

src/objects/name-inl.h

@@ -190,6 +190,14 @@ uint32_t Name::EnsureRawHash(
   return String::cast(*this).ComputeAndSetRawHash(access_guard);
 }
 
+uint32_t Name::RawHash() {
+  uint32_t field = raw_hash_field(kAcquireLoad);
+  if (V8_UNLIKELY(IsForwardingIndex(field))) {
+    return GetRawHashFromForwardingTable(field);
+  }
+  return field;
+}
+
 uint32_t Name::EnsureHash() { return HashBits::decode(EnsureRawHash()); }
 
 uint32_t Name::EnsureHash(const SharedStringAccessGuardIfNeeded& access_guard) {

src/objects/name.h

@@ -190,6 +190,7 @@ class Name : public TorqueGeneratedName<Name, PrimitiveHeapObject> {
   // a forwarding index.
   inline uint32_t EnsureRawHash();
   inline uint32_t EnsureRawHash(const SharedStringAccessGuardIfNeeded&);
+  inline uint32_t RawHash();
 
   static inline bool IsHashFieldComputed(uint32_t raw_hash_field);
   static inline bool IsHash(uint32_t raw_hash_field);

src/objects/string-forwarding-table.cc

@@ -261,6 +261,11 @@ uint32_t StringForwardingTable::GetRawHash(PtrComprCageBase cage_base,
   return block->record(index_in_block)->raw_hash(cage_base);
 }
 
+// static
+uint32_t StringForwardingTable::GetRawHashStatic(Isolate* isolate, int index) {
+  return isolate->string_forwarding_table()->GetRawHash(isolate, index);
+}
+
 v8::String::ExternalStringResourceBase*
 StringForwardingTable::GetExternalResource(int index, bool* is_one_byte) const {
   CHECK_LT(index, size());

src/objects/string-forwarding-table.h

@@ -56,6 +56,7 @@ class StringForwardingTable {
   static Address GetForwardStringAddress(Isolate* isolate, int index);
   V8_EXPORT_PRIVATE uint32_t GetRawHash(PtrComprCageBase cage_base,
                                         int index) const;
+  static uint32_t GetRawHashStatic(Isolate* isolate, int index);
   v8::String::ExternalStringResourceBase* GetExternalResource(
       int index, bool* is_one_byte) const;
 

src/objects/string.cc

@@ -1675,7 +1675,7 @@ uint32_t String::ComputeAndSetRawHash(
     string = ThinString::cast(string).actual(cage_base);
     shape = StringShape(string, cage_base);
     if (length() == string.length()) {
-      uint32_t raw_hash = string.raw_hash_field();
+      uint32_t raw_hash = string.RawHash();
       DCHECK(IsHashFieldComputed(raw_hash));
       set_raw_hash_field(raw_hash);
       return raw_hash;

test/mjsunit/shared-memory/shared-external-string-megamorphic-ic.js

+// Copyright 2022 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// Flags: --expose-externalize-string --shared-string-table
+// Flags: --allow-natives-syntax
+
+function set(o, ext_key) {
+  o[ext_key] = "bar";
+}
+function get(o, ext_key) {
+  o[ext_key];
+}
+
+%PrepareFunctionForOptimization(set);
+%OptimizeFunctionOnNextCall(set);
+%PrepareFunctionForOptimization(get);
+%OptimizeFunctionOnNextCall(get);
+
+(function test() {
+  let ext_key = "AAAAAAAAAAAAAAAAAAAAAA";
+  externalizeString(ext_key);
+
+  set({a:1}, ext_key);
+  set({b:2}, ext_key);
+  set({c:3}, ext_key);
+  set({d:4}, ext_key);
+  set({e:5}, ext_key);
+  set({f:6}, ext_key);
+
+  get({a:1}, ext_key);
+  get({b:2}, ext_key);
+  get({c:3}, ext_key);
+  get({d:4}, ext_key);
+  get({e:5}, ext_key);
+  get({f:6}, ext_key);
+})();