Revert of [heap] Reland "Make non-atomic markbit operations consistent with atomic ones." (patchset #2 id:20001 of https://codereview.chromium.org/2860323003/ )

Reason for revert:
box2d failures on buildbot

Original issue's description:
> [heap] Reland "Make non-atomic markbit operations consistent with atomic ones."
>
> Now non-atomic color transition operations return a boolean indicating
> whether the transition succeeded or not.
>
> This allows to replace color check and transition operations with a
> single transition operation. For example:
>
> if (IsWhite(object)) {
>   WhiteToBlack(object);
>   Foo();
> }
>
> becomes
>
> if (WhiteToBlack(object)) {
>   Foo();
> }
>
> BUG=chromium:694255
>
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_rel_ng
>
> Review-Url: https://codereview.chromium.org/2860323003
> Cr-Commit-Position: refs/heads/master@{#45123}
> Committed: c0a65cd295

TBR=mlippautz@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:694255

Review-Url: https://codereview.chromium.org/2863953002
Cr-Commit-Position: refs/heads/master@{#45127}

src/heap/heap.cc

@@ -4270,7 +4270,7 @@ void Heap::RegisterReservationsForBlackAllocation(Reservation* reservations) {
 void Heap::NotifyObjectLayoutChange(HeapObject* object,
                                     const DisallowHeapAllocation&) {
   if (FLAG_incremental_marking && incremental_marking()->IsMarking()) {
-    incremental_marking()->WhiteToGreyAndPush(object);
+    incremental_marking()->MarkGrey(object);
   }
 #ifdef VERIFY_HEAP
   DCHECK(pending_layout_change_object_ == nullptr);
@@ -4834,7 +4834,7 @@ class IterateAndScavengePromotedObjectsVisitor final : public ObjectVisitor {
     // promoted objects.
     if (heap_->incremental_marking()->black_allocation()) {
       Code* code = Code::cast(Code::GetObjectFromEntryAddress(code_entry_slot));
-      heap_->incremental_marking()->WhiteToGreyAndPush(code);
+      heap_->incremental_marking()->MarkGrey(code);
     }
   }
 
@@ -5628,7 +5628,7 @@ void Heap::RegisterExternallyReferencedObject(Object** object) {
   HeapObject* heap_object = HeapObject::cast(*object);
   DCHECK(Contains(heap_object));
   if (FLAG_incremental_marking_wrappers && incremental_marking()->IsMarking()) {
-    incremental_marking()->WhiteToGreyAndPush(heap_object);
+    incremental_marking()->MarkGrey(heap_object);
   } else {
     DCHECK(mark_compact_collector()->in_use());
     mark_compact_collector()->MarkObject(heap_object);

src/heap/incremental-marking.cc

@@ -57,7 +57,9 @@ bool IncrementalMarking::BaseRecordWrite(HeapObject* obj, Object* value) {
   DCHECK(!ObjectMarking::IsImpossible(obj, marking_state(obj)));
   const bool is_black = ObjectMarking::IsBlack(obj, marking_state(obj));
 
-  if (is_black && WhiteToGreyAndPush(value_heap_obj)) {
+  if (is_black &&
+      ObjectMarking::IsWhite(value_heap_obj, marking_state(value_heap_obj))) {
+    WhiteToGreyAndPush(value_heap_obj);
     RestartIfNotMarking();
   }
   return is_compacting_ && is_black;
@@ -128,12 +130,9 @@ void IncrementalMarking::RecordWriteIntoCodeSlow(Code* host, RelocInfo* rinfo,
   }
 }
 
-bool IncrementalMarking::WhiteToGreyAndPush(HeapObject* obj) {
-  if (ObjectMarking::WhiteToGrey(obj, marking_state(obj))) {
-    marking_deque()->Push(obj);
-    return true;
-  }
-  return false;
+void IncrementalMarking::WhiteToGreyAndPush(HeapObject* obj) {
+  ObjectMarking::WhiteToGrey(obj, marking_state(obj));
+  marking_deque()->Push(obj);
 }
 
 void IncrementalMarking::TransferMark(Heap* heap, HeapObject* from,
@@ -154,29 +153,9 @@ void IncrementalMarking::TransferMark(Heap* heap, HeapObject* from,
   MarkBit old_mark_bit = ObjectMarking::MarkBitFrom(from, marking_state(from));
 
   if (Marking::IsBlack(old_mark_bit)) {
-    if (from->address() + kPointerSize == to->address()) {
-      // The old and the new markbits overlap. The |to| object has the
-      // grey color. To make it black, we need to set second bit.
-      DCHECK(new_mark_bit.Get());
-      new_mark_bit.Next().Set();
-    } else {
-      bool success = Marking::WhiteToBlack(new_mark_bit);
-      DCHECK(success);
-      USE(success);
-    }
+    Marking::MarkBlack(new_mark_bit);
   } else if (Marking::IsGrey(old_mark_bit)) {
-    if (from->address() + kPointerSize == to->address()) {
-      // The old and the new markbits overlap. The |to| object has the
-      // white color. To make it black, we need to set both bits.
-      // Note that Marking::WhiteToGrey does not work here because
-      // old_mark_bit.Next() can be set by the concurrent marker at any time.
-      new_mark_bit.Set();
-      new_mark_bit.Next().Set();
-    } else {
-      bool success = Marking::WhiteToGrey(new_mark_bit);
-      DCHECK(success);
-      USE(success);
-    }
+    Marking::WhiteToGrey(new_mark_bit);
     marking_deque()->Push(to);
     RestartIfNotMarking();
   }
@@ -248,9 +227,12 @@ class IncrementalMarkingMarkingVisitor
         // Mark the object grey if it is white, do not enque it into the marking
         // deque.
         Heap* heap = map->GetHeap();
-        bool ignored = ObjectMarking::WhiteToGrey(
-            heap_obj, heap->incremental_marking()->marking_state(heap_obj));
-        USE(ignored);
+        if (ObjectMarking::IsWhite(
+                heap_obj,
+                heap->incremental_marking()->marking_state(heap_obj))) {
+          ObjectMarking::WhiteToGrey(
+              heap_obj, heap->incremental_marking()->marking_state(heap_obj));
+        }
       }
     }
     VisitNativeContext(map, context);
@@ -277,15 +259,21 @@ class IncrementalMarkingMarkingVisitor
 
   // Marks the object grey and pushes it on the marking stack.
   INLINE(static void MarkObject(Heap* heap, Object* obj)) {
-    heap->incremental_marking()->WhiteToGreyAndPush(HeapObject::cast(obj));
+    heap->incremental_marking()->MarkGrey(HeapObject::cast(obj));
   }
 
   // Marks the object black without pushing it on the marking stack.
   // Returns true if object needed marking and false otherwise.
   INLINE(static bool MarkObjectWithoutPush(Heap* heap, Object* obj)) {
     HeapObject* heap_object = HeapObject::cast(obj);
-    return ObjectMarking::WhiteToBlack(
-        heap_object, heap->incremental_marking()->marking_state(heap_object));
+    if (ObjectMarking::IsWhite(
+            heap_object,
+            heap->incremental_marking()->marking_state(heap_object))) {
+      ObjectMarking::WhiteToBlack(
+          heap_object, heap->incremental_marking()->marking_state(heap_object));
+      return true;
+    }
+    return false;
   }
 };
 
@@ -297,7 +285,7 @@ void IncrementalMarking::IterateBlackObject(HeapObject* object) {
       page->ResetProgressBar();
     }
     Map* map = object->map();
-    WhiteToGreyAndPush(map);
+    MarkGrey(map);
     IncrementalMarkingMarkingVisitor::IterateBody(map, object);
   }
 }
@@ -321,7 +309,7 @@ class IncrementalMarkingRootMarkingVisitor : public RootVisitor {
     Object* obj = *p;
     if (!obj->IsHeapObject()) return;
 
-    heap_->incremental_marking()->WhiteToGreyAndPush(HeapObject::cast(obj));
+    heap_->incremental_marking()->MarkGrey(HeapObject::cast(obj));
   }
 
   Heap* heap_;
@@ -712,7 +700,7 @@ void IncrementalMarking::RetainMaps() {
     if (i >= number_of_disposed_maps && !map_retaining_is_disabled &&
         ObjectMarking::IsWhite(map, marking_state(map))) {
       if (ShouldRetainMap(map, age)) {
-        WhiteToGreyAndPush(map);
+        MarkGrey(map);
       }
       Object* prototype = map->prototype();
       if (age > 0 && prototype->IsHeapObject() &&
@@ -832,7 +820,7 @@ void IncrementalMarking::UpdateMarkingDequeAfterScavenge() {
 
 
 void IncrementalMarking::VisitObject(Map* map, HeapObject* obj, int size) {
-  WhiteToGreyAndPush(map);
+  MarkGrey(map);
 
   IncrementalMarkingMarkingVisitor::IterateBody(map, obj);
 
@@ -843,6 +831,17 @@ void IncrementalMarking::VisitObject(Map* map, HeapObject* obj, int size) {
               (chunk->IsFlagSet(MemoryChunk::HAS_PROGRESS_BAR) &&
                Marking::IsBlack(mark_bit)));
 #endif
+  MarkBlack(obj, size);
+}
+
+void IncrementalMarking::MarkGrey(HeapObject* object) {
+  if (ObjectMarking::IsWhite(object, marking_state(object))) {
+    WhiteToGreyAndPush(object);
+  }
+}
+
+void IncrementalMarking::MarkBlack(HeapObject* obj, int size) {
+  if (ObjectMarking::IsBlack(obj, marking_state(obj))) return;
   ObjectMarking::GreyToBlack(obj, marking_state(obj));
 }
 
@@ -911,9 +910,9 @@ void IncrementalMarking::Hurry() {
     HeapObject* cache = HeapObject::cast(
         Context::cast(context)->get(Context::NORMALIZED_MAP_CACHE_INDEX));
     if (!cache->IsUndefined(heap_->isolate())) {
-      // Mark the cache black if it is grey.
-      bool ignored = ObjectMarking::GreyToBlack(cache, marking_state(cache));
-      USE(ignored);
+      if (ObjectMarking::IsGrey(cache, marking_state(cache))) {
+        ObjectMarking::GreyToBlack(cache, marking_state(cache));
+      }
     }
     context = Context::cast(context)->next_context_link();
   }

src/heap/incremental-marking.h

@@ -65,6 +65,9 @@ class V8_EXPORT_PRIVATE IncrementalMarking {
     return MarkingState::Internal(chunk);
   }
 
+  void MarkBlack(HeapObject* object, int size);
+  void MarkGrey(HeapObject* object);
+
   // Transfers mark bits without requiring proper object headers.
   void TransferMark(Heap* heap, HeapObject* from, HeapObject* to);
 
@@ -79,15 +82,9 @@ class V8_EXPORT_PRIVATE IncrementalMarking {
 
     DCHECK(ObjectMarking::IsWhite<access_mode>(to, marking_state(to)));
     if (ObjectMarking::IsGrey<access_mode>(from, marking_state(from))) {
-      bool success =
-          ObjectMarking::WhiteToGrey<access_mode>(to, marking_state(to));
-      DCHECK(success);
-      USE(success);
+      ObjectMarking::WhiteToGrey<access_mode>(to, marking_state(to));
     } else if (ObjectMarking::IsBlack<access_mode>(from, marking_state(from))) {
-      bool success =
-          ObjectMarking::WhiteToBlack<access_mode>(to, marking_state(to));
-      DCHECK(success);
-      USE(success);
+      ObjectMarking::WhiteToBlack<access_mode>(to, marking_state(to));
     }
   }
 
@@ -213,9 +210,7 @@ class V8_EXPORT_PRIVATE IncrementalMarking {
   void RecordCodeTargetPatch(Code* host, Address pc, HeapObject* value);
   void RecordCodeTargetPatch(Address pc, HeapObject* value);
 
-  // Returns true if the function succeeds in transitioning the object
-  // from white to grey.
-  bool WhiteToGreyAndPush(HeapObject* obj);
+  void WhiteToGreyAndPush(HeapObject* obj);
 
   inline void SetOldSpacePageFlags(MemoryChunk* chunk) {
     SetOldSpacePageFlags(chunk, IsMarking(), IsCompacting());

src/heap/mark-compact-inl.h

@@ -38,15 +38,19 @@ void MarkCompactCollector::UnshiftBlack(HeapObject* obj) {
 }
 
 void MarkCompactCollector::MarkObject(HeapObject* obj) {
-  if (ObjectMarking::WhiteToBlack<MarkBit::NON_ATOMIC>(
+  if (ObjectMarking::IsWhite<MarkBit::NON_ATOMIC>(
           obj, MarkingState::Internal(obj))) {
+    ObjectMarking::WhiteToBlack<MarkBit::NON_ATOMIC>(
+        obj, MarkingState::Internal(obj));
     PushBlack(obj);
   }
 }
 
 void MinorMarkCompactCollector::MarkObject(HeapObject* obj) {
-  if (ObjectMarking::WhiteToBlack<MarkBit::NON_ATOMIC>(
+  if (ObjectMarking::IsWhite<MarkBit::NON_ATOMIC>(
           obj, MarkingState::External(obj))) {
+    ObjectMarking::WhiteToBlack<MarkBit::NON_ATOMIC>(
+        obj, MarkingState::External(obj));
     PushBlack(obj);
   }
 }

src/heap/mark-compact.cc

@@ -1196,10 +1196,12 @@ class StaticYoungGenerationMarkingVisitor
     StackLimitCheck check(heap->isolate());
     if (check.HasOverflowed()) return false;
 
-    if (ObjectMarking::WhiteToBlack<MarkBit::NON_ATOMIC>(
-            object, MarkingState::External(object))) {
-      IterateBody(object->map(), object);
-    }
+    if (ObjectMarking::IsBlackOrGrey<MarkBit::NON_ATOMIC>(
+            object, MarkingState::External(object)))
+      return true;
+    ObjectMarking::WhiteToBlack<MarkBit::NON_ATOMIC>(
+        object, MarkingState::External(object));
+    IterateBody(object->map(), object);
     return true;
   }
 };
@@ -1235,7 +1237,11 @@ class MarkCompactMarkingVisitor
   // Marks the object black without pushing it on the marking stack.
   // Returns true if object needed marking and false otherwise.
   INLINE(static bool MarkObjectWithoutPush(Heap* heap, HeapObject* object)) {
-    return ObjectMarking::WhiteToBlack(object, MarkingState::Internal(object));
+    if (ObjectMarking::IsWhite(object, MarkingState::Internal(object))) {
+      ObjectMarking::WhiteToBlack(object, MarkingState::Internal(object));
+      return true;
+    }
+    return false;
   }
 
   // Mark object pointed to by p.
@@ -1253,15 +1259,14 @@ class MarkCompactMarkingVisitor
                                          HeapObject* obj)) {
 #ifdef DEBUG
     DCHECK(collector->heap()->Contains(obj));
+    DCHECK(ObjectMarking::IsWhite(obj, MarkingState::Internal(obj)));
 #endif
-    if (ObjectMarking::WhiteToBlack(obj, MarkingState::Internal(obj))) {
-      Map* map = obj->map();
-      Heap* heap = obj->GetHeap();
-      ObjectMarking::WhiteToBlack(obj, MarkingState::Internal(obj));
-      // Mark the map pointer and the body.
-      heap->mark_compact_collector()->MarkObject(map);
-      IterateBody(map, obj);
-    }
+    Map* map = obj->map();
+    Heap* heap = obj->GetHeap();
+    ObjectMarking::WhiteToBlack(obj, MarkingState::Internal(obj));
+    // Mark the map pointer and the body.
+    heap->mark_compact_collector()->MarkObject(map);
+    IterateBody(map, obj);
   }
 
   // Visit all unmarked objects pointed to by [start, end).
@@ -1279,6 +1284,8 @@ class MarkCompactMarkingVisitor
       if (!o->IsHeapObject()) continue;
       collector->RecordSlot(object, p, o);
       HeapObject* obj = HeapObject::cast(o);
+      if (ObjectMarking::IsBlackOrGrey(obj, MarkingState::Internal(obj)))
+        continue;
       VisitUnmarkedObject(collector, obj);
     }
     return true;
@@ -1475,12 +1482,16 @@ class MinorMarkCompactCollector::RootMarkingVisitor : public RootVisitor {
 
     if (!collector_->heap()->InNewSpace(object)) return;
 
-    if (ObjectMarking::WhiteToBlack<MarkBit::NON_ATOMIC>(
-            object, MarkingState::External(object))) {
-      Map* map = object->map();
-      StaticYoungGenerationMarkingVisitor::IterateBody(map, object);
-      collector_->EmptyMarkingDeque();
-    }
+    if (ObjectMarking::IsBlackOrGrey<MarkBit::NON_ATOMIC>(
+            object, MarkingState::External(object)))
+      return;
+
+    Map* map = object->map();
+    ObjectMarking::WhiteToBlack<MarkBit::NON_ATOMIC>(
+        object, MarkingState::External(object));
+    StaticYoungGenerationMarkingVisitor::IterateBody(map, object);
+
+    collector_->EmptyMarkingDeque();
   }
 
   MinorMarkCompactCollector* collector_;
@@ -1521,16 +1532,22 @@ class MarkCompactCollector::RootMarkingVisitor : public ObjectVisitor,
 
     HeapObject* object = HeapObject::cast(*p);
 
-    if (ObjectMarking::WhiteToBlack<MarkBit::NON_ATOMIC>(
-            object, MarkingState::Internal(object))) {
-      Map* map = object->map();
-      // Mark the map pointer and body, and push them on the marking stack.
-      collector_->MarkObject(map);
-      MarkCompactMarkingVisitor::IterateBody(map, object);
-      // Mark all the objects reachable from the map and body.  May leave
-      // overflowed objects in the heap.
-      collector_->EmptyMarkingDeque();
-    }
+    if (ObjectMarking::IsBlackOrGrey<MarkBit::NON_ATOMIC>(
+            object, MarkingState::Internal(object)))
+      return;
+
+    Map* map = object->map();
+    // Mark the object.
+    ObjectMarking::WhiteToBlack<MarkBit::NON_ATOMIC>(
+        object, MarkingState::Internal(object));
+
+    // Mark the map pointer and body, and push them on the marking stack.
+    collector_->MarkObject(map);
+    MarkCompactMarkingVisitor::IterateBody(map, object);
+
+    // Mark all the objects reachable from the map and body.  May leave
+    // overflowed objects in the heap.
+    collector_->EmptyMarkingDeque();
   }
 
   MarkCompactCollector* collector_;
@@ -1701,7 +1718,8 @@ void MarkCompactCollector::DiscoverGreyObjectsWithIterator(T* it) {
   Map* filler_map = heap()->one_pointer_filler_map();
   for (HeapObject* object = it->Next(); object != NULL; object = it->Next()) {
     if ((object->map() != filler_map) &&
-        ObjectMarking::GreyToBlack(object, MarkingState::Internal(object))) {
+        ObjectMarking::IsGrey(object, MarkingState::Internal(object))) {
+      ObjectMarking::GreyToBlack(object, MarkingState::Internal(object));
       PushBlack(object);
       if (marking_deque()->IsFull()) return;
     }
@@ -1713,10 +1731,8 @@ void MarkCompactCollector::DiscoverGreyObjectsOnPage(MemoryChunk* p) {
   LiveObjectIterator<kGreyObjects> it(p, MarkingState::Internal(p));
   HeapObject* object = NULL;
   while ((object = it.Next()) != NULL) {
-    bool success =
-        ObjectMarking::GreyToBlack(object, MarkingState::Internal(object));
-    DCHECK(success);
-    USE(success);
+    DCHECK(ObjectMarking::IsGrey(object, MarkingState::Internal(object)));
+    ObjectMarking::GreyToBlack(object, MarkingState::Internal(object));
     PushBlack(object);
     if (marking_deque()->IsFull()) return;
   }
@@ -2279,12 +2295,15 @@ bool MarkCompactCollector::IsUnmarkedHeapObject(Object** p) {
 void MarkCompactCollector::MarkStringTable(RootMarkingVisitor* visitor) {
   StringTable* string_table = heap()->string_table();
   // Mark the string table itself.
-  if (ObjectMarking::WhiteToBlack(string_table,
-                                  MarkingState::Internal(string_table))) {
-    // Explicitly mark the prefix.
-    string_table->IteratePrefix(visitor);
-    ProcessMarkingDeque();
+  if (ObjectMarking::IsWhite(string_table,
+                             MarkingState::Internal(string_table))) {
+    // String table could have already been marked by visiting the handles list.
+    ObjectMarking::WhiteToBlack(string_table,
+                                MarkingState::Internal(string_table));
   }
+  // Explicitly mark the prefix.
+  string_table->IteratePrefix(visitor);
+  ProcessMarkingDeque();
 }
 
 void MarkCompactCollector::MarkRoots(RootMarkingVisitor* visitor) {

src/heap/mark-compact.h

@@ -82,6 +82,8 @@ class ObjectMarking : public AllStatic {
   template <MarkBit::AccessMode access_mode = MarkBit::NON_ATOMIC>
   V8_INLINE static bool BlackToGrey(HeapObject* obj,
                                     const MarkingState& state) {
+    DCHECK(
+        (access_mode == MarkBit::ATOMIC || IsBlack<access_mode>(obj, state)));
     MarkBit markbit = MarkBitFrom(obj, state);
     if (!Marking::BlackToGrey<access_mode>(markbit)) return false;
     state.IncrementLiveBytes<access_mode>(-obj->Size());
@@ -91,19 +93,24 @@ class ObjectMarking : public AllStatic {
   template <MarkBit::AccessMode access_mode = MarkBit::NON_ATOMIC>
   V8_INLINE static bool WhiteToGrey(HeapObject* obj,
                                     const MarkingState& state) {
+    DCHECK(
+        (access_mode == MarkBit::ATOMIC || IsWhite<access_mode>(obj, state)));
     return Marking::WhiteToGrey<access_mode>(MarkBitFrom(obj, state));
   }
 
   template <MarkBit::AccessMode access_mode = MarkBit::NON_ATOMIC>
   V8_INLINE static bool WhiteToBlack(HeapObject* obj,
                                      const MarkingState& state) {
-    return ObjectMarking::WhiteToGrey<access_mode>(obj, state) &&
-           ObjectMarking::GreyToBlack<access_mode>(obj, state);
+    DCHECK(
+        (access_mode == MarkBit::ATOMIC || IsWhite<access_mode>(obj, state)));
+    if (!ObjectMarking::WhiteToGrey<access_mode>(obj, state)) return false;
+    return ObjectMarking::GreyToBlack<access_mode>(obj, state);
   }
 
   template <MarkBit::AccessMode access_mode = MarkBit::NON_ATOMIC>
   V8_INLINE static bool GreyToBlack(HeapObject* obj,
                                     const MarkingState& state) {
+    DCHECK((access_mode == MarkBit::ATOMIC || IsGrey<access_mode>(obj, state)));
     MarkBit markbit = MarkBitFrom(obj, state);
     if (!Marking::GreyToBlack<access_mode>(markbit)) return false;
     state.IncrementLiveBytes<access_mode>(obj->Size());

src/heap/marking.h

@@ -38,16 +38,12 @@ class MarkBit {
     }
   }
 
-  // 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();
 
@@ -61,9 +57,8 @@ class MarkBit {
 
 template <>
 inline bool MarkBit::Set<MarkBit::NON_ATOMIC>() {
-  base::Atomic32 old_value = *cell_;
-  *cell_ = old_value | mask_;
-  return (old_value & mask_) == 0;
+  *cell_ |= mask_;
+  return true;
 }
 
 template <>
@@ -91,9 +86,8 @@ inline bool MarkBit::Get<MarkBit::ATOMIC>() {
 
 template <>
 inline bool MarkBit::Clear<MarkBit::NON_ATOMIC>() {
-  base::Atomic32 old_value = *cell_;
-  *cell_ = old_value & ~mask_;
-  return (old_value & mask_) == mask_;
+  *cell_ &= ~mask_;
+  return true;
 }
 
 template <>
@@ -418,17 +412,24 @@ class Marking : public AllStatic {
 
   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 bool WhiteToBlack(MarkBit markbit)) {
-    return markbit.Set<mode>() && markbit.Next().Set<mode>();
+  INLINE(static void WhiteToBlack(MarkBit markbit)) {
+    DCHECK(mode == MarkBit::ATOMIC || IsWhite(markbit));
+    markbit.Set<mode>();
+    markbit.Next().Set<mode>();
   }
 
   template <MarkBit::AccessMode mode = MarkBit::NON_ATOMIC>
   INLINE(static bool GreyToBlack(MarkBit markbit)) {
-    return markbit.Get<mode>() && markbit.Next().Set<mode>();
+    DCHECK(mode == MarkBit::ATOMIC || IsGrey(markbit));
+    return markbit.Next().Set<mode>();
   }
 
   enum ObjectColor {