Revert of [heap] Make non-atomic markbit operations consistent with atomic ones.

Revert of [heap] Make non-atomic markbit operations consistent with atomic ones. (patchset #11 id:200001 of https://codereview.chromium.org/2857713002/ )

Reason for revert:
Seems to block the roll:
https://codereview.chromium.org/2857423002/

Original issue's description:
> [heap] 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
> 
> Review-Url: https://codereview.chromium.org/2857713002
> Cr-Commit-Position: refs/heads/master@{#45085}
> Committed: https://chromium.googlesource.com/v8/v8/+/dd37366fb56a57be5ef36ed0c2b3c7893cd6c6d8

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

[Michael Achenbach, 2017-05-05 06:37:08]

The CQ bit was checked by machenbach@chromium.org

[Michael Achenbach, 2017-05-05 06:37:09]

Created Revert of [heap] Make non-atomic markbit operations consistent with
atomic ones.

[commit-bot: I haz the power, 2017-05-05 06:37:12]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-05 06:37:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-05 06:37:25]

Failed to apply patch for src/heap/incremental-marking.cc:
While running git apply --index -3 -p1;
  error: patch failed: src/heap/incremental-marking.cc:48
  error: repository lacks the necessary blob to fall back on 3-way merge.
  error: src/heap/incremental-marking.cc: patch does not apply

Patch:       src/heap/incremental-marking.cc
Index: src/heap/incremental-marking.cc
diff --git a/src/heap/incremental-marking.cc b/src/heap/incremental-marking.cc
index
db17d3844f2b0c01be85800e47eb503fcdf5d93f..b5d023512f2c62609862ea02c2c84af4753fe9b8
100644
--- a/src/heap/incremental-marking.cc
+++ b/src/heap/incremental-marking.cc
@@ -48,7 +48,9 @@
   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;
@@ -119,12 +121,9 @@
   }
 }
 
-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,
@@ -145,31 +144,11 @@
   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_deque()->Push(to);
-      RestartIfNotMarking();
-    }
+    Marking::WhiteToGrey(new_mark_bit);
+    marking_deque()->Push(to);
+    RestartIfNotMarking();
   }
 }
 
@@ -239,9 +218,12 @@
         // 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);
@@ -268,15 +250,21 @@
 
   // 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;
   }
 };
 
@@ -288,7 +276,7 @@
       page->ResetProgressBar();
     }
     Map* map = object->map();
-    WhiteToGreyAndPush(map);
+    MarkGrey(map);
     IncrementalMarkingMarkingVisitor::IterateBody(map, object);
   }
 }
@@ -312,7 +300,7 @@
     Object* obj = *p;
     if (!obj->IsHeapObject()) return;
 
-    heap_->incremental_marking()->WhiteToGreyAndPush(HeapObject::cast(obj));
+    heap_->incremental_marking()->MarkGrey(HeapObject::cast(obj));
   }
 
   Heap* heap_;
@@ -703,7 +691,7 @@
     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() &&
@@ -823,7 +811,7 @@
 
 
 void IncrementalMarking::VisitObject(Map* map, HeapObject* obj, int size) {
-  WhiteToGreyAndPush(map);
+  MarkGrey(map);
 
   IncrementalMarkingMarkingVisitor::IterateBody(map, obj);
 
@@ -834,6 +822,17 @@
               (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));
 }
 
@@ -902,9 +901,9 @@
     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();
   }

[Michael Achenbach, 2017-05-05 06:51:26]

Revert with dependencies:
https://chromium-review.googlesource.com/c/497387/