Introduce external forwarding index

Distinguish internalized forwarding indices from external forwarding
indices stored in a strings hash.

Bug: v8:12957
Change-Id: Ic01386a3291ac8d618cf4282aa7112e74e1b9169
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3829471Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Patrick Thier <pthier@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82513}

src/codegen/code-stub-assembler.cc

@@ -8078,9 +8078,11 @@ void CodeStubAssembler::TryToName(TNode<Object> key, Label* if_keyisindex,
                                THIN_ONE_BYTE_STRING_TYPE),
              &if_thinstring);
 
-      // Check if the hash field encodes a string forwarding index.
-      GotoIf(IsEqualInWord32<Name::HashFieldTypeBits>(
-                 raw_hash_field, Name::HashFieldType::kForwardingIndex),
+      // Check if the hash field encodes an internalized string forwarding
+      // index.
+      GotoIf(IsBothEqualInWord32<Name::HashFieldTypeBits,
+                                 Name::IsInternalizedForwardingIndexBit>(
+                 raw_hash_field, Name::HashFieldType::kForwardingIndex, true),
              &if_forwarding_index);
 
       // Finally, check if |key| is internalized.
@@ -8106,8 +8108,9 @@ void CodeStubAssembler::TryToName(TNode<Object> key, Label* if_keyisindex,
         TNode<Object> result = CAST(CallCFunction(
             function, MachineType::AnyTagged(),
             std::make_pair(MachineType::Pointer(), isolate_ptr),
-            std::make_pair(MachineType::Int32(),
-                           DecodeWord32<Name::HashBits>(raw_hash_field))));
+            std::make_pair(
+                MachineType::Int32(),
+                DecodeWord32<Name::ForwardingIndexValueBits>(raw_hash_field))));
 
         *var_unique = CAST(result);
         Goto(if_keyisunique);

src/codegen/code-stub-assembler.h

@@ -2957,6 +2957,19 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
     return Word32Equal(masked_word32, Int32Constant(BitField::encode(value)));
   }
 
+  // Checks if two values of non-overlapping bitfields are both set.
+  template <typename BitField1, typename BitField2>
+  TNode<BoolT> IsBothEqualInWord32(TNode<Word32T> word32,
+                                   typename BitField1::FieldType value1,
+                                   typename BitField2::FieldType value2) {
+    static_assert((BitField1::kMask & BitField2::kMask) == 0);
+    TNode<Word32T> combined_masked_word32 =
+        Word32And(word32, Int32Constant(BitField1::kMask | BitField2::kMask));
+    TNode<Int32T> combined_value =
+        Int32Constant(BitField1::encode(value1) | BitField2::encode(value2));
+    return Word32Equal(combined_masked_word32, combined_value);
+  }
+
   // Returns true if the bit field |BitField| in |word32| is not equal to a
   // given constant |value|. Avoids a shift compared to using DecodeWord32.
   template <typename BitField>

src/diagnostics/objects-debug.cc

@@ -878,7 +878,7 @@ void ConsString::ConsStringVerify(Isolate* isolate) {
 
 void ThinString::ThinStringVerify(Isolate* isolate) {
   TorqueGeneratedClassVerifiers::ThinStringVerify(*this, isolate);
-  CHECK(!HasForwardingIndex());
+  CHECK(!HasForwardingIndex(kAcquireLoad));
   CHECK(actual().IsInternalizedString());
   CHECK(actual().IsSeqString() || actual().IsExternalString());
 }

src/objects/name-inl.h

@@ -84,33 +84,75 @@ bool Name::Equals(Isolate* isolate, Handle<Name> one, Handle<Name> two) {
                             Handle<String>::cast(two));
 }
 
+// static
 bool Name::IsHashFieldComputed(uint32_t raw_hash_field) {
   return (raw_hash_field & kHashNotComputedMask) == 0;
 }
 
+// static
 bool Name::IsHash(uint32_t raw_hash_field) {
   return HashFieldTypeBits::decode(raw_hash_field) == HashFieldType::kHash;
 }
 
+// static
 bool Name::IsIntegerIndex(uint32_t raw_hash_field) {
   return HashFieldTypeBits::decode(raw_hash_field) ==
          HashFieldType::kIntegerIndex;
 }
 
+// static
 bool Name::IsForwardingIndex(uint32_t raw_hash_field) {
   return HashFieldTypeBits::decode(raw_hash_field) ==
          HashFieldType::kForwardingIndex;
 }
 
+// static
+bool Name::IsInternalizedForwardingIndex(uint32_t raw_hash_field) {
+  return HashFieldTypeBits::decode(raw_hash_field) ==
+             HashFieldType::kForwardingIndex &&
+         IsInternalizedForwardingIndexBit::decode(raw_hash_field);
+}
+
+// static
+bool Name::IsExternalForwardingIndex(uint32_t raw_hash_field) {
+  return HashFieldTypeBits::decode(raw_hash_field) ==
+             HashFieldType::kForwardingIndex &&
+         IsExternalForwardingIndexBit::decode(raw_hash_field);
+}
+
+// static
 uint32_t Name::CreateHashFieldValue(uint32_t hash, HashFieldType type) {
+  DCHECK_NE(type, HashFieldType::kForwardingIndex);
   return HashBits::encode(hash & HashBits::kMax) |
          HashFieldTypeBits::encode(type);
 }
 
+// static
+uint32_t Name::CreateInternalizedForwardingIndex(uint32_t index) {
+  return ForwardingIndexValueBits::encode(index) |
+         IsExternalForwardingIndexBit::encode(false) |
+         IsInternalizedForwardingIndexBit::encode(true) |
+         HashFieldTypeBits::encode(HashFieldType::kForwardingIndex);
+}
+
+// static
+uint32_t Name::CreateExternalForwardingIndex(uint32_t index) {
+  return ForwardingIndexValueBits::encode(index) |
+         IsExternalForwardingIndexBit::encode(true) |
+         IsInternalizedForwardingIndexBit::encode(false) |
+         HashFieldTypeBits::encode(HashFieldType::kForwardingIndex);
+}
+
 bool Name::HasHashCode() const { return IsHashFieldComputed(raw_hash_field()); }
-bool Name::HasForwardingIndex() const {
+bool Name::HasForwardingIndex(AcquireLoadTag) const {
   return IsForwardingIndex(raw_hash_field(kAcquireLoad));
 }
+bool Name::HasInternalizedForwardingIndex(AcquireLoadTag) const {
+  return IsInternalizedForwardingIndex(raw_hash_field(kAcquireLoad));
+}
+bool Name::HasExternalForwardingIndex(AcquireLoadTag) const {
+  return IsExternalForwardingIndex(raw_hash_field(kAcquireLoad));
+}
 
 uint32_t Name::EnsureRawHash() {
   // Fast case: has hash code already been computed?

src/objects/name.h

@@ -30,7 +30,9 @@ class Name : public TorqueGeneratedName<Name, PrimitiveHeapObject> {
   inline bool HasHashCode() const;
   // Tells whether the name contains a forwarding index pointing to a row
   // in the string forwarding table.
-  inline bool HasForwardingIndex() const;
+  inline bool HasForwardingIndex(AcquireLoadTag) const;
+  inline bool HasInternalizedForwardingIndex(AcquireLoadTag) const;
+  inline bool HasExternalForwardingIndex(AcquireLoadTag) const;
 
   inline uint32_t raw_hash_field() const {
     return RELAXED_READ_UINT32_FIELD(*this, kRawHashFieldOffset);
@@ -126,6 +128,14 @@ class Name : public TorqueGeneratedName<Name, PrimitiveHeapObject> {
   static_assert((HashFieldTypeBits::encode(HashFieldType::kForwardingIndex) &
                  kHashNotComputedMask) != 0);
 
+  using IsInternalizedForwardingIndexBit = HashFieldTypeBits::Next<bool, 1>;
+  using IsExternalForwardingIndexBit =
+      IsInternalizedForwardingIndexBit::Next<bool, 1>;
+  using ForwardingIndexValueBits = IsExternalForwardingIndexBit::Next<
+      unsigned int, kBitsPerInt - HashFieldTypeBits::kSize -
+                        IsInternalizedForwardingIndexBit::kSize -
+                        IsExternalForwardingIndexBit::kSize>;
+
   // Array index strings this short can keep their index in the hash field.
   static const int kMaxCachedArrayIndexLength = 7;
 
@@ -185,9 +195,13 @@ class Name : public TorqueGeneratedName<Name, PrimitiveHeapObject> {
   static inline bool IsHash(uint32_t raw_hash_field);
   static inline bool IsIntegerIndex(uint32_t raw_hash_field);
   static inline bool IsForwardingIndex(uint32_t raw_hash_field);
+  static inline bool IsInternalizedForwardingIndex(uint32_t raw_hash_field);
+  static inline bool IsExternalForwardingIndex(uint32_t raw_hash_field);
 
   static inline uint32_t CreateHashFieldValue(uint32_t hash,
                                               HashFieldType type);
+  static inline uint32_t CreateInternalizedForwardingIndex(uint32_t index);
+  static inline uint32_t CreateExternalForwardingIndex(uint32_t index);
 
   TQ_OBJECT_CONSTRUCTORS(Name)
 };

src/objects/string-table.cc

@@ -435,7 +435,7 @@ void SetInternalizedReference(Isolate* isolate, String string,
   // TODO(v8:12007): Support external strings.
   DCHECK(!string.IsThinString());
   DCHECK(internalized.IsInternalizedString());
-  DCHECK(!internalized.HasForwardingIndex());
+  DCHECK(!internalized.HasForwardingIndex(kAcquireLoad));
   if ((string.IsShared() || FLAG_always_use_string_forwarding_table) &&
       !string.IsExternalString()) {
     uint32_t field = string.raw_hash_field();
@@ -443,15 +443,14 @@ void SetInternalizedReference(Isolate* isolate, String string,
     // Using the hash field for the integer index is more beneficial than
     // using it to store the forwarding index to the internalized string.
     if (Name::IsIntegerIndex(field)) return;
-    // Check one last time if we already have a forwarding index to prevent
-    // too many copies of the string in the forwarding table.
-    if (Name::IsForwardingIndex(field)) return;
+    // Check one last time if we already have an internalized forwarding index
+    // to prevent too many copies of the string in the forwarding table.
+    if (Name::IsInternalizedForwardingIndex(field)) return;
 
     const int forwarding_index =
         isolate->string_forwarding_table()->Add(isolate, string, internalized);
     string.set_raw_hash_field(
-        String::CreateHashFieldValue(forwarding_index,
-                                     String::HashFieldType::kForwardingIndex),
+        String::CreateInternalizedForwardingIndex(forwarding_index),
         kReleaseStore);
   } else {
     if (V8_UNLIKELY(FLAG_always_use_string_forwarding_table)) {
@@ -461,7 +460,7 @@ void SetInternalizedReference(Isolate* isolate, String string,
       DCHECK(string.IsExternalString());
       string.set_raw_hash_field(internalized.raw_hash_field());
     }
-    DCHECK(!string.HasForwardingIndex());
+    DCHECK(!string.HasForwardingIndex(kAcquireLoad));
     string.MakeThin(isolate, internalized);
   }
 }
@@ -502,8 +501,9 @@ Handle<String> StringTable::LookupString(Isolate* isolate,
   if (!result->IsInternalizedString()) {
     uint32_t raw_hash_field = result->raw_hash_field(kAcquireLoad);
 
-    if (String::IsForwardingIndex(raw_hash_field)) {
-      const int index = String::HashBits::decode(raw_hash_field);
+    if (String::IsInternalizedForwardingIndex(raw_hash_field)) {
+      const int index =
+          String::ForwardingIndexValueBits::decode(raw_hash_field);
       result = handle(
           isolate->string_forwarding_table()->GetForwardString(isolate, index),
           isolate);
@@ -691,8 +691,9 @@ Address StringTable::Data::TryStringToIndexOrLookupExisting(Isolate* isolate,
 
   // First check if the string constains a forwarding index.
   uint32_t raw_hash_field = source.raw_hash_field(kAcquireLoad);
-  if (Name::IsForwardingIndex(raw_hash_field) && is_source_hash_usable) {
-    const int index = Name::HashBits::decode(raw_hash_field);
+  if (Name::IsInternalizedForwardingIndex(raw_hash_field) &&
+      is_source_hash_usable) {
+    const int index = Name::ForwardingIndexValueBits::decode(raw_hash_field);
     String internalized =
         isolate->string_forwarding_table()->GetForwardString(isolate, index);
     return internalized.ptr();

src/objects/string.cc

@@ -198,7 +198,7 @@ void String::MakeThin(IsolateT* isolate, String internalized) {
   // transition is OK.
   DCHECK_IMPLIES(
       initial_shape.IsShared() && !isolate->has_active_deserializer(),
-      HasForwardingIndex());
+      HasForwardingIndex(kAcquireLoad));
 
   bool has_pointers = initial_shape.IsIndirect();
   int old_size = SizeFromMap(initial_map);
@@ -1512,10 +1512,10 @@ uint32_t String::ComputeAndSetRawHash(
   DCHECK_IMPLIES(!FLAG_shared_string_table, !HasHashCode());
 
   uint32_t field = raw_hash_field(kAcquireLoad);
-  if (Name::IsForwardingIndex(field)) {
+  if (Name::IsInternalizedForwardingIndex(field)) {
     // Get the real hash from the forwarded string.
     Isolate* isolate = GetIsolateFromWritableObject(*this);
-    const int forward_index = Name::HashBits::decode(field);
+    const int forward_index = Name::ForwardingIndexValueBits::decode(field);
     String internalized = isolate->string_forwarding_table()->GetForwardString(
         isolate, forward_index);
     uint32_t hash = internalized.raw_hash_field();
@@ -1558,7 +1558,7 @@ uint32_t String::ComputeAndSetRawHash(
   set_raw_hash_field_if_empty(raw_hash_field);
   // Check the hash code is there (or a forwarding index if the string was
   // internalized in parallel).
-  DCHECK(HasHashCode() || HasForwardingIndex());
+  DCHECK(HasHashCode() || HasForwardingIndex(kAcquireLoad));
   // Ensure that the hash value of 0 is never computed.
   DCHECK_NE(HashBits::decode(raw_hash_field), 0);
   return raw_hash_field;

test/cctest/test-shared-strings.cc

@@ -342,7 +342,7 @@ class ConcurrentInternalizationThread final
     if (hit_or_miss_ == kTestMiss) {
       CHECK_EQ(*input_string, *interned);
     } else {
-      CHECK(input_string->HasForwardingIndex());
+      CHECK(input_string->HasForwardingIndex(kAcquireLoad));
       CHECK(String::Equals(i_isolate, input_string, interned));
     }
   }
@@ -785,7 +785,7 @@ UNINITIALIZED_TEST(SharedStringsTransitionDuringGC) {
       Handle<String> interned = factory->InternalizeString(input_string);
       CHECK(input_string->IsShared());
       CHECK(!input_string->IsThinString());
-      CHECK(input_string->HasForwardingIndex());
+      CHECK(input_string->HasForwardingIndex(kAcquireLoad));
       CHECK(String::Equals(i_isolate, input_string, interned));
     }