[object] Remove synchronized_ from smi accessors

This ends up cleaning up the last of the macros in object-macros which
were using `synchronized_`. There are still a few methods which use
`synchronized_` but those were defined ad-hoc (i.e. w/o macros).

Bug: v8:7790
Change-Id: Ib2d35030fd032293e746c09e10156e526af8d032
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2897085Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74589}

src/compiler/heap-refs.cc

@@ -3118,7 +3118,7 @@ base::Optional<int> StringRef::length() const {
           "length for kNeverSerialized non-internalized string " << *this);
       return base::nullopt;
     } else {
-      return object()->synchronized_length();
+      return object()->length(kAcquireLoad);
     }
   }
   return data()->AsString()->length();

src/heap/concurrent-marking.cc

@@ -136,13 +136,13 @@ class ConcurrentMarkingVisitor final
   int VisitSeqOneByteString(Map map, SeqOneByteString object) {
     if (!ShouldVisit(object)) return 0;
     VisitMapPointer(object);
-    return SeqOneByteString::SizeFor(object.synchronized_length());
+    return SeqOneByteString::SizeFor(object.length(kAcquireLoad));
   }
 
   int VisitSeqTwoByteString(Map map, SeqTwoByteString object) {
     if (!ShouldVisit(object)) return 0;
     VisitMapPointer(object);
-    return SeqTwoByteString::SizeFor(object.synchronized_length());
+    return SeqTwoByteString::SizeFor(object.length(kAcquireLoad));
   }
 
   // Implements ephemeron semantics: Marks value if key is already reachable.
@@ -232,9 +232,9 @@ class ConcurrentMarkingVisitor final
 
   template <typename T>
   int VisitLeftTrimmableArray(Map map, T object) {
-    // The synchronized_length() function checks that the length is a Smi.
+    // The length() function checks that the length is a Smi.
     // This is not necessarily the case if the array is being left-trimmed.
-    Object length = object.unchecked_synchronized_length();
+    Object length = object.unchecked_length(kAcquireLoad);
     if (!ShouldVisit(object)) return 0;
     // The cached length must be the actual length as the array is not black.
     // Left trimming marks the array black before over-writing the length.

src/heap/heap.cc

@@ -3434,7 +3434,7 @@ void Heap::CreateFillerForArray(T object, int elements_to_trim,
   // Initialize header of the trimmed array. We are storing the new length
   // using release store after creating a filler for the left-over space to
   // avoid races with the sweeper thread.
-  object.synchronized_set_length(object.length() - elements_to_trim);
+  object.set_length(object.length() - elements_to_trim, kReleaseStore);
 
   // Notify the heap object allocation tracker of change in object layout. The
   // array may not be moved during GC, and size has to be adjusted nevertheless.

src/heap/object-stats.cc

@@ -74,7 +74,7 @@ class FieldStatsCollector : public ObjectVisitor {
       raw_fields_count_in_object -= kDoubleSize / kTaggedSize;
       *boxed_double_fields_count_ += 1;
     } else if (host.IsSeqString()) {
-      int string_data = SeqString::cast(host).synchronized_length() *
+      int string_data = SeqString::cast(host).length(kAcquireLoad) *
                         (String::cast(host).IsOneByteRepresentation() ? 1 : 2) /
                         kTaggedSize;
       DCHECK_LE(string_data, raw_fields_count_in_object);

src/objects/bigint.cc

@@ -206,7 +206,7 @@ class MutableBigInt : public FreshlyAllocatedBigInt {
     bitfield = SignBits::update(bitfield, new_sign);
     RELAXED_WRITE_INT32_FIELD(*this, kBitfieldOffset, bitfield);
   }
-  inline void synchronized_set_length(int new_length) {
+  inline void set_length(int new_length, ReleaseStoreTag) {
     int32_t bitfield = RELAXED_READ_INT32_FIELD(*this, kBitfieldOffset);
     bitfield = LengthBits::update(bitfield, new_length);
     RELEASE_WRITE_INT32_FIELD(*this, kBitfieldOffset, bitfield);
@@ -418,7 +418,7 @@ void MutableBigInt::Canonicalize(MutableBigInt result) {
       // of the object changed significantly.
       heap->CreateFillerObjectAt(new_end, size_delta, ClearRecordedSlots::kNo);
     }
-    result.synchronized_set_length(new_length);
+    result.set_length(new_length, kReleaseStore);
 
     // Canonicalize -0n.
     if (new_length == 0) {
@@ -2204,7 +2204,7 @@ MaybeHandle<String> MutableBigInt::ToStringGeneric(Isolate* isolate,
   if (sign) chars[pos++] = '-';
   // Trim any over-allocation (which can happen due to conservative estimates).
   if (pos < static_cast<int>(chars_required)) {
-    result->synchronized_set_length(pos);
+    result->set_length(pos, kReleaseStore);
     int string_size =
         SeqOneByteString::SizeFor(static_cast<int>(chars_required));
     int needed_size = SeqOneByteString::SizeFor(pos);

src/objects/bigint.h

@@ -38,7 +38,7 @@ class BigIntBase : public PrimitiveHeapObject {
   }
 
   // For use by the GC.
-  inline int synchronized_length() const {
+  inline int length(AcquireLoadTag) const {
     int32_t bitfield = ACQUIRE_READ_INT32_FIELD(*this, kBitfieldOffset);
     return LengthBits::decode(static_cast<uint32_t>(bitfield));
   }

src/objects/fixed-array-inl.h

@@ -44,11 +44,11 @@ TQ_OBJECT_CONSTRUCTORS_IMPL(WeakArrayList)
 
 NEVER_READ_ONLY_SPACE_IMPL(WeakArrayList)
 
-SYNCHRONIZED_SMI_ACCESSORS(FixedArrayBase, length, kLengthOffset)
+RELEASE_ACQUIRE_SMI_ACCESSORS(FixedArrayBase, length, kLengthOffset)
 
-SYNCHRONIZED_SMI_ACCESSORS(WeakFixedArray, length, kLengthOffset)
+RELEASE_ACQUIRE_SMI_ACCESSORS(WeakFixedArray, length, kLengthOffset)
 
-Object FixedArrayBase::unchecked_synchronized_length() const {
+Object FixedArrayBase::unchecked_length(AcquireLoadTag) const {
   return ACQUIRE_READ_FIELD(*this, kLengthOffset);
 }
 
@@ -240,11 +240,9 @@ void FixedArray::CopyElements(Isolate* isolate, int dst_index, FixedArray src,
 // Due to left- and right-trimming, concurrent visitors need to read the length
 // with acquire semantics.
 // TODO(ulan): Acquire should not be needed anymore.
-inline int FixedArray::AllocatedSize() {
-  return SizeFor(synchronized_length());
-}
+inline int FixedArray::AllocatedSize() { return SizeFor(length(kAcquireLoad)); }
 inline int WeakFixedArray::AllocatedSize() {
-  return SizeFor(synchronized_length());
+  return SizeFor(length(kAcquireLoad));
 }
 inline int WeakArrayList::AllocatedSize() { return SizeFor(capacity()); }
 

src/objects/fixed-array.h

@@ -71,10 +71,12 @@ enum FixedArraySubInstanceType {
 class FixedArrayBase
     : public TorqueGeneratedFixedArrayBase<FixedArrayBase, HeapObject> {
  public:
-  // Get and set the length using acquire loads and release stores.
-  DECL_SYNCHRONIZED_INT_ACCESSORS(length)
+  // Forward declare the non-atomic (set_)length defined in torque.
+  using TorqueGeneratedFixedArrayBase::length;
+  using TorqueGeneratedFixedArrayBase::set_length;
+  DECL_RELEASE_ACQUIRE_INT_ACCESSORS(length)
 
-  inline Object unchecked_synchronized_length() const;
+  inline Object unchecked_length(AcquireLoadTag) const;
 
   static int GetMaxLengthForNewSpaceAllocation(ElementsKind kind);
 
@@ -283,8 +285,10 @@ class WeakFixedArray
       int index, MaybeObject value,
       WriteBarrierMode mode = WriteBarrierMode::UPDATE_WRITE_BARRIER);
 
-  // Get and set the length using acquire loads and release stores.
-  DECL_SYNCHRONIZED_INT_ACCESSORS(length)
+  // Forward declare the non-atomic (set_)length defined in torque.
+  using TorqueGeneratedWeakFixedArray::length;
+  using TorqueGeneratedWeakFixedArray::set_length;
+  DECL_RELEASE_ACQUIRE_INT_ACCESSORS(length)
 
   // Gives access to raw memory which stores the array's data.
   inline MaybeObjectSlot data_start();

src/objects/object-macros-undef.h

@@ -14,10 +14,8 @@
 #undef DECL_PRIMITIVE_GETTER
 #undef DECL_PRIMITIVE_SETTER
 #undef DECL_PRIMITIVE_ACCESSORS
-#undef DECL_SYNCHRONIZED_PRIMITIVE_ACCESSORS
 #undef DECL_BOOLEAN_ACCESSORS
 #undef DECL_INT_ACCESSORS
-#undef DECL_SYNCHRONIZED_INT_ACCESSORS
 #undef DECL_INT32_ACCESSORS
 #undef DECL_UINT16_ACCESSORS
 #undef DECL_INT16_ACCESSORS
@@ -61,7 +59,8 @@
 #undef RELEASE_ACQUIRE_WEAK_ACCESSORS
 #undef SMI_ACCESSORS_CHECKED
 #undef SMI_ACCESSORS
-#undef SYNCHRONIZED_SMI_ACCESSORS
+#undef DECL_RELEASE_ACQUIRE_INT_ACCESSORS
+#undef RELEASE_ACQUIRE_SMI_ACCESSORS
 #undef DECL_RELAXED_SMI_ACCESSORS
 #undef RELAXED_SMI_ACCESSORS
 #undef BOOL_GETTER

src/objects/object-macros.h

@@ -56,17 +56,10 @@
   DECL_PRIMITIVE_GETTER(name, type)          \
   DECL_PRIMITIVE_SETTER(name, type)
 
-#define DECL_SYNCHRONIZED_PRIMITIVE_ACCESSORS(name, type) \
-  inline type synchronized_##name() const;                \
-  inline void synchronized_set_##name(type value);
-
 #define DECL_BOOLEAN_ACCESSORS(name) DECL_PRIMITIVE_ACCESSORS(name, bool)
 
 #define DECL_INT_ACCESSORS(name) DECL_PRIMITIVE_ACCESSORS(name, int)
 
-#define DECL_SYNCHRONIZED_INT_ACCESSORS(name) \
-  DECL_SYNCHRONIZED_PRIMITIVE_ACCESSORS(name, int)
-
 #define DECL_INT32_ACCESSORS(name) DECL_PRIMITIVE_ACCESSORS(name, int32_t)
 
 #define DECL_UINT16_ACCESSORS(name) \
@@ -324,12 +317,16 @@
 #define SMI_ACCESSORS(holder, name, offset) \
   SMI_ACCESSORS_CHECKED(holder, name, offset, true)
 
-#define SYNCHRONIZED_SMI_ACCESSORS(holder, name, offset)                 \
-  int holder::synchronized_##name() const {                              \
+#define DECL_RELEASE_ACQUIRE_INT_ACCESSORS(name) \
+  inline int name(AcquireLoadTag) const;         \
+  inline void set_##name(int value, ReleaseStoreTag);
+
+#define RELEASE_ACQUIRE_SMI_ACCESSORS(holder, name, offset)              \
+  int holder::name(AcquireLoadTag) const {                               \
     Smi value = TaggedField<Smi, offset>::Acquire_Load(*this);           \
     return value.value();                                                \
   }                                                                      \
-  void holder::synchronized_set_##name(int value) {                      \
+  void holder::set_##name(int value, ReleaseStoreTag) {                  \
     TaggedField<Smi, offset>::Release_Store(*this, Smi::FromInt(value)); \
   }
 

src/objects/objects-body-descriptors-inl.h

@@ -430,7 +430,7 @@ class ByteArray::BodyDescriptor final : public BodyDescriptorBase {
                                  ObjectVisitor* v) {}
 
   static inline int SizeOf(Map map, HeapObject obj) {
-    return ByteArray::SizeFor(ByteArray::cast(obj).synchronized_length());
+    return ByteArray::SizeFor(ByteArray::cast(obj).length(kAcquireLoad));
   }
 };
 
@@ -451,7 +451,7 @@ class BytecodeArray::BodyDescriptor final : public BodyDescriptorBase {
 
   static inline int SizeOf(Map map, HeapObject obj) {
     return BytecodeArray::SizeFor(
-        BytecodeArray::cast(obj).synchronized_length());
+        BytecodeArray::cast(obj).length(kAcquireLoad));
   }
 };
 
@@ -464,7 +464,7 @@ class BigInt::BodyDescriptor final : public BodyDescriptorBase {
                                  ObjectVisitor* v) {}
 
   static inline int SizeOf(Map map, HeapObject obj) {
-    return BigInt::SizeFor(BigInt::cast(obj).synchronized_length());
+    return BigInt::SizeFor(BigInt::cast(obj).length(kAcquireLoad));
   }
 };
 
@@ -478,7 +478,7 @@ class FixedDoubleArray::BodyDescriptor final : public BodyDescriptorBase {
 
   static inline int SizeOf(Map map, HeapObject obj) {
     return FixedDoubleArray::SizeFor(
-        FixedDoubleArray::cast(obj).synchronized_length());
+        FixedDoubleArray::cast(obj).length(kAcquireLoad));
   }
 };
 

src/objects/objects.cc

@@ -2262,7 +2262,7 @@ int HeapObject::SizeFromMap(Map map) const {
   if (base::IsInRange(instance_type, FIRST_FIXED_ARRAY_TYPE,
                       LAST_FIXED_ARRAY_TYPE)) {
     return FixedArray::SizeFor(
-        FixedArray::unchecked_cast(*this).synchronized_length());
+        FixedArray::unchecked_cast(*this).length(kAcquireLoad));
   }
   if (base::IsInRange(instance_type, FIRST_CONTEXT_TYPE, LAST_CONTEXT_TYPE)) {
     if (instance_type == NATIVE_CONTEXT_TYPE) return NativeContext::kSize;
@@ -2273,15 +2273,15 @@ int HeapObject::SizeFromMap(Map map) const {
     // Strings may get concurrently truncated, hence we have to access its
     // length synchronized.
     return SeqOneByteString::SizeFor(
-        SeqOneByteString::unchecked_cast(*this).synchronized_length());
+        SeqOneByteString::unchecked_cast(*this).length(kAcquireLoad));
   }
   if (instance_type == BYTE_ARRAY_TYPE) {
     return ByteArray::SizeFor(
-        ByteArray::unchecked_cast(*this).synchronized_length());
+        ByteArray::unchecked_cast(*this).length(kAcquireLoad));
   }
   if (instance_type == BYTECODE_ARRAY_TYPE) {
     return BytecodeArray::SizeFor(
-        BytecodeArray::unchecked_cast(*this).synchronized_length());
+        BytecodeArray::unchecked_cast(*this).length(kAcquireLoad));
   }
   if (instance_type == FREE_SPACE_TYPE) {
     return FreeSpace::unchecked_cast(*this).size(kRelaxedLoad);
@@ -2291,11 +2291,11 @@ int HeapObject::SizeFromMap(Map map) const {
     // Strings may get concurrently truncated, hence we have to access its
     // length synchronized.
     return SeqTwoByteString::SizeFor(
-        SeqTwoByteString::unchecked_cast(*this).synchronized_length());
+        SeqTwoByteString::unchecked_cast(*this).length(kAcquireLoad));
   }
   if (instance_type == FIXED_DOUBLE_ARRAY_TYPE) {
     return FixedDoubleArray::SizeFor(
-        FixedDoubleArray::unchecked_cast(*this).synchronized_length());
+        FixedDoubleArray::unchecked_cast(*this).length(kAcquireLoad));
   }
   if (instance_type == FEEDBACK_METADATA_TYPE) {
     return FeedbackMetadata::SizeFor(
@@ -2309,7 +2309,7 @@ int HeapObject::SizeFromMap(Map map) const {
   if (base::IsInRange(instance_type, FIRST_WEAK_FIXED_ARRAY_TYPE,
                       LAST_WEAK_FIXED_ARRAY_TYPE)) {
     return WeakFixedArray::SizeFor(
-        WeakFixedArray::unchecked_cast(*this).synchronized_length());
+        WeakFixedArray::unchecked_cast(*this).length(kAcquireLoad));
   }
   if (instance_type == WEAK_ARRAY_LIST_TYPE) {
     return WeakArrayList::SizeForCapacity(
@@ -2333,7 +2333,7 @@ int HeapObject::SizeFromMap(Map map) const {
   }
   if (instance_type == PROPERTY_ARRAY_TYPE) {
     return PropertyArray::SizeFor(
-        PropertyArray::cast(*this).synchronized_length());
+        PropertyArray::cast(*this).length(kAcquireLoad));
   }
   if (instance_type == FEEDBACK_VECTOR_TYPE) {
     return FeedbackVector::SizeFor(

src/objects/property-array-inl.h

@@ -22,7 +22,8 @@ OBJECT_CONSTRUCTORS_IMPL(PropertyArray, HeapObject)
 CAST_ACCESSOR(PropertyArray)
 
 SMI_ACCESSORS(PropertyArray, length_and_hash, kLengthAndHashOffset)
-SYNCHRONIZED_SMI_ACCESSORS(PropertyArray, length_and_hash, kLengthAndHashOffset)
+RELEASE_ACQUIRE_SMI_ACCESSORS(PropertyArray, length_and_hash,
+                              kLengthAndHashOffset)
 
 Object PropertyArray::get(int index) const {
   PtrComprCageBase cage_base = GetPtrComprCageBase(*this);
@@ -64,8 +65,8 @@ void PropertyArray::initialize_length(int len) {
   set_length_and_hash(len);
 }
 
-int PropertyArray::synchronized_length() const {
-  return LengthField::decode(synchronized_length_and_hash());
+int PropertyArray::length(AcquireLoadTag) const {
+  return LengthField::decode(length_and_hash(kAcquireLoad));
 }
 
 int PropertyArray::Hash() const { return HashField::decode(length_and_hash()); }

src/objects/property-array.h

@@ -18,9 +18,7 @@ class PropertyArray : public HeapObject {
  public:
   // [length]: length of the array.
   inline int length() const;
-
-  // Get the length using acquire loads.
-  inline int synchronized_length() const;
+  inline int length(AcquireLoadTag) const;
 
   // This is only used on a newly allocated PropertyArray which
   // doesn't have an existing hash.
@@ -70,7 +68,7 @@ class PropertyArray : public HeapObject {
  private:
   DECL_INT_ACCESSORS(length_and_hash)
 
-  DECL_SYNCHRONIZED_INT_ACCESSORS(length_and_hash)
+  DECL_RELEASE_ACQUIRE_INT_ACCESSORS(length_and_hash)
 
   OBJECT_CONSTRUCTORS(PropertyArray, HeapObject);
 };

src/objects/string-inl.h

@@ -92,12 +92,12 @@ class V8_NODISCARD SharedStringAccessGuardIfNeeded {
   base::Optional<base::SharedMutexGuard<base::kShared>> mutex_guard;
 };
 
-int String::synchronized_length() const {
+int String::length(AcquireLoadTag) const {
   return base::AsAtomic32::Acquire_Load(
       reinterpret_cast<const int32_t*>(field_address(kLengthOffset)));
 }
 
-void String::synchronized_set_length(int value) {
+void String::set_length(int value, ReleaseStoreTag) {
   base::AsAtomic32::Release_Store(
       reinterpret_cast<int32_t*>(field_address(kLengthOffset)), value);
 }
@@ -827,10 +827,10 @@ void SeqTwoByteString::SeqTwoByteStringSet(int index, uint16_t value) {
 // Due to ThinString rewriting, concurrent visitors need to read the length with
 // acquire semantics.
 inline int SeqOneByteString::AllocatedSize() {
-  return SizeFor(synchronized_length());
+  return SizeFor(length(kAcquireLoad));
 }
 inline int SeqTwoByteString::AllocatedSize() {
-  return SizeFor(synchronized_length());
+  return SizeFor(length(kAcquireLoad));
 }
 
 void SlicedString::set_parent(String parent, WriteBarrierMode mode) {

src/objects/string.cc

@@ -1425,7 +1425,7 @@ Handle<String> SeqString::Truncate(Handle<SeqString> string, int new_length) {
                              ClearRecordedSlots::kNo);
   // We are storing the new length using release store after creating a filler
   // for the left-over space to avoid races with the sweeper thread.
-  string->synchronized_set_length(new_length);
+  string->set_length(new_length, kReleaseStore);
 
   return string;
 }

src/objects/string.h

@@ -194,9 +194,10 @@ class String : public TorqueGeneratedString<String, Name> {
   const byte* AddressOfCharacterAt(int start_index,
                                    const DisallowGarbageCollection& no_gc);
 
-  // Get and set the length of the string using acquire loads and release
-  // stores.
-  DECL_SYNCHRONIZED_INT_ACCESSORS(length)
+  // Forward declare the non-atomic (set_)length defined in torque.
+  using TorqueGeneratedString::length;
+  using TorqueGeneratedString::set_length;
+  DECL_RELEASE_ACQUIRE_INT_ACCESSORS(length)
 
   // Returns whether this string has only one-byte chars, i.e. all of them can
   // be one-byte encoded.  This might be the case even if the string is

test/cctest/test-concurrent-string.cc

@@ -82,7 +82,7 @@ class ConcurrentStringThread final : public v8::base::Thread {
     sema_started_->Signal();
     // Check the three operations we do from the StringRef concurrently: get the
     // string, the nth character, and convert into a double.
-    CHECK_EQ(str_->synchronized_length(), length_);
+    CHECK_EQ(str_->length(kAcquireLoad), length_);
     for (unsigned int i = 0; i < length_; ++i) {
       CHECK_EQ(str_->Get(i, &local_isolate), chars_[i]);
     }