[heap] Reland "Make non-atomic markbit operations consistent with atomic ones."

src/heap/marking.h

 // Copyright 2016 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.
 
 #ifndef V8_MARKING_H
 #define V8_MARKING_H
 
 #include "src/base/atomic-utils.h"
 #include "src/utils.h"
 
@@ skipping 20 matching lines @@
  private:
   inline MarkBit Next() {
     CellType new_mask = mask_ << 1;
     if (new_mask == 0) {
       return MarkBit(cell_ + 1, 1);
     } else {
       return MarkBit(cell_, new_mask);
     }
   }
 
+  // The function returns true if it succeeded to
+  // transition the bit from 0 to 1.
   template <AccessMode mode = NON_ATOMIC>
   inline bool Set();
 
   template <AccessMode mode = NON_ATOMIC>
   inline bool Get();
 
+  // The function returns true if it succeeded to
+  // transition the bit from 1 to 0.
   template <AccessMode mode = NON_ATOMIC>
   inline bool Clear();
 
   base::Atomic32* cell_;
   base::Atomic32 mask_;
 
   friend class IncrementalMarking;
   friend class ConcurrentMarkingMarkbits;
   friend class Marking;
 };
 
 template <>
 inline bool MarkBit::Set<MarkBit::NON_ATOMIC>() {
-  *cell_ |= mask_;
-  return true;
+  base::Atomic32 old_value = *cell_;
+  *cell_ = old_value | mask_;
+  return (old_value & mask_) == 0;
 }
 
 template <>
 inline bool MarkBit::Set<MarkBit::ATOMIC>() {
   base::Atomic32 old_value;
   base::Atomic32 new_value;
   do {
     old_value = base::NoBarrier_Load(cell_);
     if (old_value & mask_) return false;
     new_value = old_value | mask_;
   } while (base::Release_CompareAndSwap(cell_, old_value, new_value) !=
            old_value);
   return true;
 }
 
 template <>
 inline bool MarkBit::Get<MarkBit::NON_ATOMIC>() {
   return (base::NoBarrier_Load(cell_) & mask_) != 0;
 }
 
 template <>
 inline bool MarkBit::Get<MarkBit::ATOMIC>() {
   return (base::Acquire_Load(cell_) & mask_) != 0;
 }
 
 template <>
 inline bool MarkBit::Clear<MarkBit::NON_ATOMIC>() {
-  *cell_ &= ~mask_;
-  return true;
+  base::Atomic32 old_value = *cell_;
+  *cell_ = old_value & ~mask_;
+  return (old_value & mask_) == mask_;
 }
 
 template <>
 inline bool MarkBit::Clear<MarkBit::ATOMIC>() {
   base::Atomic32 old_value;
   base::Atomic32 new_value;
   do {
     old_value = base::NoBarrier_Load(cell_);
     if (!(old_value & mask_)) return false;
     new_value = old_value & ~mask_;
@@ skipping 304 matching lines @@
 
   template <MarkBit::AccessMode mode = MarkBit::NON_ATOMIC>
   INLINE(static bool BlackToGrey(MarkBit markbit)) {
     STATIC_ASSERT(mode == MarkBit::NON_ATOMIC);
     DCHECK(IsBlack(markbit));
     return markbit.Next().Clear<mode>();
   }
 
   template <MarkBit::AccessMode mode = MarkBit::NON_ATOMIC>
   INLINE(static bool WhiteToGrey(MarkBit markbit)) {
-    DCHECK(mode == MarkBit::ATOMIC || IsWhite(markbit));
     return markbit.Set<mode>();
   }
 
-  // Warning: this method is not safe in general in concurrent scenarios.
-  // If you know that nobody else will change the bits on the given location
-  // then you may use it.
   template <MarkBit::AccessMode mode = MarkBit::NON_ATOMIC>
-  INLINE(static void WhiteToBlack(MarkBit markbit)) {
-    DCHECK(mode == MarkBit::ATOMIC || IsWhite(markbit));
-    markbit.Set<mode>();
-    markbit.Next().Set<mode>();
+  INLINE(static bool WhiteToBlack(MarkBit markbit)) {
+    return markbit.Set<mode>() && markbit.Next().Set<mode>();
   }
 
   template <MarkBit::AccessMode mode = MarkBit::NON_ATOMIC>
   INLINE(static bool GreyToBlack(MarkBit markbit)) {
-    DCHECK(mode == MarkBit::ATOMIC || IsGrey(markbit));
-    return markbit.Next().Set<mode>();
+    return markbit.Get<mode>() && markbit.Next().Set<mode>();
   }
 
   enum ObjectColor {
     BLACK_OBJECT,
     WHITE_OBJECT,
     GREY_OBJECT,
     IMPOSSIBLE_COLOR
   };
 
   static const char* ColorName(ObjectColor color) {
@@ skipping 19 matching lines @@
   }
 
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(Marking);
 };
 
 }  // namespace internal
 }  // namespace v8
 
 #endif  // V8_MARKING_H_