[vm] Move UntaggedObject::Tags -> AtomicBitFieldContainer.

Now that this class is used for more than wrapping Object tags, move it
to be alongside the BitField class and change its name to something more
representative of its role.

Also remove the need to change BitField declarations if the source type
of the bitfield is decltype(field) and the declaration of field is
changed from type T to type AtomicBitFieldContainer<T>.

TEST=Refactoring, so existing tests.

Cq-Include-Trybots: luci.dart.try:vm-kernel-tsan-linux-release-x64-try,vm-kernel-precomp-tsan-linux-release-x64-try,vm-kernel-precomp-linux-release-simarm_x64-try
Change-Id: If2f48ed4e209d71461c1b1ec81769e1485fbd6b0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/205782
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>

runtime/vm/bitfield.h

@@ -7,10 +7,121 @@
 
 #include <type_traits>
 
+#include "platform/atomic.h"
 #include "platform/globals.h"
+#include "platform/thread_sanitizer.h"
 
 namespace dart {
 
+class AtomicBitFieldContainerBase {
+ private:
+  AtomicBitFieldContainerBase() = delete;  // Only used for std::is_base_of.
+};
+
+template <typename T>
+class AtomicBitFieldContainer : AtomicBitFieldContainerBase {
+  static_assert(sizeof(std::atomic<T>) == sizeof(T),
+                "Size of type changes when made atomic");
+
+ public:
+  using ContainedType = T;
+
+  AtomicBitFieldContainer() : field_(0) {}
+
+  operator T() const { return field_.load(std::memory_order_relaxed); }
+  T operator=(T tags) {
+    field_.store(tags, std::memory_order_relaxed);
+    return tags;
+  }
+
+  T load(std::memory_order order) const { return field_.load(order); }
+  void store(T value, std::memory_order order) { field_.store(value, order); }
+
+  bool compare_exchange_weak(T old_tags, T new_tags, std::memory_order order) {
+    return field_.compare_exchange_weak(old_tags, new_tags, order);
+  }
+
+  template <class TargetBitField,
+            std::memory_order order = std::memory_order_relaxed>
+  typename TargetBitField::Type Read() const {
+    return TargetBitField::decode(field_.load(order));
+  }
+
+  template <class TargetBitField>
+  NO_SANITIZE_THREAD typename TargetBitField::Type ReadIgnoreRace() const {
+    return TargetBitField::decode(*reinterpret_cast<const T*>(&field_));
+  }
+
+  template <class TargetBitField,
+            std::memory_order order = std::memory_order_relaxed>
+  void UpdateBool(bool value) {
+    if (value) {
+      field_.fetch_or(TargetBitField::encode(true), order);
+    } else {
+      field_.fetch_and(~TargetBitField::encode(true), order);
+    }
+  }
+
+  template <class TargetBitField>
+  void FetchOr(typename TargetBitField::Type value) {
+    field_.fetch_or(TargetBitField::encode(value), std::memory_order_relaxed);
+  }
+
+  template <class TargetBitField>
+  void Update(typename TargetBitField::Type value) {
+    T old_field = field_.load(std::memory_order_relaxed);
+    T new_field;
+    do {
+      new_field = TargetBitField::update(value, old_field);
+    } while (!field_.compare_exchange_weak(old_field, new_field,
+                                           std::memory_order_relaxed));
+  }
+
+  template <class TargetBitField>
+  void UpdateUnsynchronized(typename TargetBitField::Type value) {
+    field_.store(
+        TargetBitField::update(value, field_.load(std::memory_order_relaxed)),
+        std::memory_order_relaxed);
+  }
+
+  template <class TargetBitField>
+  typename TargetBitField::Type UpdateConditional(
+      typename TargetBitField::Type value_to_be_set,
+      typename TargetBitField::Type conditional_old_value) {
+    T old_field = field_.load(std::memory_order_relaxed);
+    while (true) {
+      // This operation is only performed if the condition is met.
+      auto old_value = TargetBitField::decode(old_field);
+      if (old_value != conditional_old_value) {
+        return old_value;
+      }
+      T new_tags = TargetBitField::update(value_to_be_set, old_field);
+      if (field_.compare_exchange_weak(old_field, new_tags,
+                                       std::memory_order_relaxed)) {
+        return value_to_be_set;
+      }
+      // [old_tags] was updated to it's current value.
+    }
+  }
+
+  template <class TargetBitField>
+  bool TryAcquire() {
+    T mask = TargetBitField::encode(true);
+    T old_field = field_.fetch_or(mask, std::memory_order_relaxed);
+    return !TargetBitField::decode(old_field);
+  }
+
+  template <class TargetBitField>
+  bool TryClear() {
+    T mask = ~TargetBitField::encode(true);
+    T old_field = field_.fetch_and(mask, std::memory_order_relaxed);
+    return TargetBitField::decode(old_field);
+  }
+
+ private:
+  std::atomic<T> field_;
+};
+
 static const uword kUwordOne = 1U;
 
 // BitField is a template for encoding and decoding a value of type T
@@ -19,7 +130,8 @@ template <typename S,
           typename T,
           int position,
           int size = (sizeof(S) * kBitsPerByte) - position,
-          bool sign_extend = false>
+          bool sign_extend = false,
+          typename Enable = void>
 class BitField {
  public:
   typedef T Type;
@@ -86,6 +198,50 @@ class BitField {
   }
 };
 
+// Partial instantiations to avoid having to change BitField declarations if
+// S is decltype(field_) and the type of field_ is changed to be wrapped in an
+// AtomicBitFieldContainer.
+template <typename S, typename T, int position, int size, bool sign_extend>
+class BitField<S,
+               T,
+               position,
+               size,
+               sign_extend,
+               typename std::enable_if<
+                   std::is_base_of<AtomicBitFieldContainerBase, S>::value,
+                   void>::type> : public BitField<typename S::ContainedType,
+                                                  T,
+                                                  position,
+                                                  size,
+                                                  sign_extend> {};
+
+template <typename S, typename T, int position, int size>
+class BitField<S,
+               T,
+               position,
+               size,
+               false,
+               typename std::enable_if<
+                   std::is_base_of<AtomicBitFieldContainerBase, S>::value,
+                   void>::type>
+    : public BitField<typename S::ContainedType, T, position, size, false> {};
+
+template <typename S, typename T, int position>
+class BitField<S,
+               T,
+               position,
+               (sizeof(S) * kBitsPerByte) - position,
+               false,
+               typename std::enable_if<
+                   std::is_base_of<AtomicBitFieldContainerBase, S>::value,
+                   void>::type>
+    : public BitField<typename S::ContainedType,
+                      T,
+                      position,
+                      (sizeof(typename S::ContainedType) * kBitsPerByte) -
+                          position,
+                      false> {};
+
 }  // namespace dart
 
 #endif  // RUNTIME_VM_BITFIELD_H_

runtime/vm/raw_object.h

@@ -10,8 +10,6 @@
 #endif
 
 #include "platform/assert.h"
-#include "platform/atomic.h"
-#include "platform/thread_sanitizer.h"
 #include "vm/class_id.h"
 #include "vm/compiler/method_recognizer.h"
 #include "vm/compiler/runtime_api.h"
@@ -260,110 +258,6 @@ class UntaggedObject {
   class ReservedBits
       : public BitField<uword, intptr_t, kReservedTagPos, kReservedTagSize> {};
 
-  template <typename T>
-  class Tags {
-   public:
-    Tags() : tags_(0) {}
-
-    using ContentType = T;
-
-    operator T() const { return tags_.load(std::memory_order_relaxed); }
-    T operator=(T tags) {
-      tags_.store(tags, std::memory_order_relaxed);
-      return tags;
-    }
-
-    T load(std::memory_order order) const { return tags_.load(order); }
-    void store(T value, std::memory_order order) { tags_.store(value, order); }
-
-    bool compare_exchange_weak(T old_tags,
-                               T new_tags,
-                               std::memory_order order) {
-      return tags_.compare_exchange_weak(old_tags, new_tags, order);
-    }
-
-    template <class TagBitField,
-              std::memory_order order = std::memory_order_relaxed>
-    typename TagBitField::Type Read() const {
-      return TagBitField::decode(tags_.load(order));
-    }
-
-    template <class TagBitField>
-    NO_SANITIZE_THREAD typename TagBitField::Type ReadIgnoreRace() const {
-      return TagBitField::decode(*reinterpret_cast<const T*>(&tags_));
-    }
-
-    template <class TagBitField,
-              std::memory_order order = std::memory_order_relaxed>
-    void UpdateBool(bool value) {
-      if (value) {
-        tags_.fetch_or(TagBitField::encode(true), order);
-      } else {
-        tags_.fetch_and(~TagBitField::encode(true), order);
-      }
-    }
-
-    template <class TagBitField>
-    void FetchOr(typename TagBitField::Type value) {
-      tags_.fetch_or(TagBitField::encode(value), std::memory_order_relaxed);
-    }
-
-    template <class TagBitField>
-    void Update(typename TagBitField::Type value) {
-      T old_tags = tags_.load(std::memory_order_relaxed);
-      T new_tags;
-      do {
-        new_tags = TagBitField::update(value, old_tags);
-      } while (!tags_.compare_exchange_weak(old_tags, new_tags,
-                                            std::memory_order_relaxed));
-    }
-
-    template <class TagBitField>
-    void UpdateUnsynchronized(typename TagBitField::Type value) {
-      tags_.store(
-          TagBitField::update(value, tags_.load(std::memory_order_relaxed)),
-          std::memory_order_relaxed);
-    }
-
-    template <class TagBitField>
-    typename TagBitField::Type UpdateConditional(
-        typename TagBitField::Type value_to_be_set,
-        typename TagBitField::Type conditional_old_value) {
-      T old_tags = tags_.load(std::memory_order_relaxed);
-      while (true) {
-        // This operation is only performed if the condition is met.
-        auto old_value = TagBitField::decode(old_tags);
-        if (old_value != conditional_old_value) {
-          return old_value;
-        }
-        T new_tags = TagBitField::update(value_to_be_set, old_tags);
-        if (tags_.compare_exchange_weak(old_tags, new_tags,
-                                        std::memory_order_relaxed)) {
-          return value_to_be_set;
-        }
-        // [old_tags] was updated to it's current value.
-      }
-    }
-
-    template <class TagBitField>
-    bool TryAcquire() {
-      T mask = TagBitField::encode(true);
-      T old_tags = tags_.fetch_or(mask, std::memory_order_relaxed);
-      return !TagBitField::decode(old_tags);
-    }
-
-    template <class TagBitField>
-    bool TryClear() {
-      T mask = ~TagBitField::encode(true);
-      T old_tags = tags_.fetch_and(mask, std::memory_order_relaxed);
-      return TagBitField::decode(old_tags);
-    }
-
-   private:
-    std::atomic<T> tags_;
-    COMPILE_ASSERT(sizeof(std::atomic<T>) == sizeof(T));
-  };
-
   // Assumes this is a heap object.
   bool IsNewObject() const {
     uword addr = reinterpret_cast<uword>(this);
@@ -604,7 +498,7 @@ class UntaggedObject {
   }
 
  private:
-  Tags<uword> tags_;  // Various object tags (bits).
+  AtomicBitFieldContainer<uword> tags_;  // Various object tags (bits).
 
   intptr_t VisitPointersPredefined(ObjectPointerVisitor* visitor,
                                    intptr_t class_id);
@@ -1399,7 +1293,7 @@ class UntaggedFunction : public UntaggedObject {
   TokenPosition end_token_pos_;
 #endif
 
-  Tags<uint32_t> kind_tag_;  // See Function::KindTagBits.
+  AtomicBitFieldContainer<uint32_t> kind_tag_;  // See Function::KindTagBits.
 
 #define JIT_FUNCTION_COUNTERS(F)                                               \
   F(intptr_t, int32_t, usage_counter)                                          \
@@ -1418,14 +1312,12 @@ class UntaggedFunction : public UntaggedObject {
 
 #endif  // !defined(DART_PRECOMPILED_RUNTIME)
 
-  Tags<uint8_t> packed_fields_;
+  AtomicBitFieldContainer<uint8_t> packed_fields_;
 
   static constexpr intptr_t kMaxOptimizableBits = 1;
 
-  using PackedOptimizable = BitField<decltype(packed_fields_)::ContentType,
-                                     bool,
-                                     0,
-                                     kMaxOptimizableBits>;
+  using PackedOptimizable =
+      BitField<decltype(packed_fields_), bool, 0, kMaxOptimizableBits>;
 };
 
 class UntaggedClosureData : public UntaggedObject {
@@ -2446,7 +2338,7 @@ class UntaggedICData : public UntaggedCallSiteData {
   }
   NOT_IN_PRECOMPILED(int32_t deopt_id_);
   // Number of arguments tested in IC, deopt reasons.
-  Tags<uint32_t> state_bits_;
+  AtomicBitFieldContainer<uint32_t> state_bits_;
 };
 
 class UntaggedMegamorphicCache : public UntaggedCallSiteData {
@@ -2662,8 +2554,8 @@ class UntaggedFunctionType : public UntaggedAbstractType {
   COMPRESSED_POINTER_FIELD(ArrayPtr, named_parameter_names);
   COMPRESSED_POINTER_FIELD(SmiPtr, hash)
   VISIT_TO(hash)
-  Tags<uint32_t> packed_parameter_counts_;
-  Tags<uint16_t> packed_type_parameter_counts_;
+  AtomicBitFieldContainer<uint32_t> packed_parameter_counts_;
+  AtomicBitFieldContainer<uint16_t> packed_type_parameter_counts_;
   uint8_t type_state_;
   uint8_t nullability_;
 
@@ -2671,31 +2563,28 @@ class UntaggedFunctionType : public UntaggedAbstractType {
  public:
   // For packed_type_parameter_counts_.
   using PackedNumParentTypeArguments =
-      BitField<decltype(packed_type_parameter_counts_)::ContentType,
-               uint8_t,
-               0,
-               8>;
+      BitField<decltype(packed_type_parameter_counts_), uint8_t, 0, 8>;
   using PackedNumTypeParameters =
-      BitField<decltype(packed_type_parameter_counts_)::ContentType,
+      BitField<decltype(packed_type_parameter_counts_),
                uint8_t,
                PackedNumParentTypeArguments::kNextBit,
                8>;
 
   // For packed_parameter_counts_.
   using PackedNumImplicitParameters =
-      BitField<decltype(packed_parameter_counts_)::ContentType, uint8_t, 0, 1>;
+      BitField<decltype(packed_parameter_counts_), uint8_t, 0, 1>;
   using PackedHasNamedOptionalParameters =
-      BitField<decltype(packed_parameter_counts_)::ContentType,
+      BitField<decltype(packed_parameter_counts_),
                bool,
                PackedNumImplicitParameters::kNextBit,
                1>;
   using PackedNumFixedParameters =
-      BitField<decltype(packed_parameter_counts_)::ContentType,
+      BitField<decltype(packed_parameter_counts_),
                uint16_t,
                PackedHasNamedOptionalParameters::kNextBit,
                14>;
   using PackedNumOptionalParameters =
-      BitField<decltype(packed_parameter_counts_)::ContentType,
+      BitField<decltype(packed_parameter_counts_),
                uint16_t,
                PackedNumFixedParameters::kNextBit,
                14>;