Source/JavaScriptCore:
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 23 Sep 2016 18:09:44 +0000 (18:09 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 23 Sep 2016 18:09:44 +0000 (18:09 +0000)
Need a store-load fence between setting cell state and visiting the object in SlotVisitor
https://bugs.webkit.org/show_bug.cgi?id=162354

Reviewed by Mark Lam.

This was meant to be a small change, but then it became bigger as I found small
opportunities for improving this code. This adds a store-load fence and is performance-
neutral. That's probably partly due to other optimizations that I did to visitChildren().

Initially, I found that adding an mfence as a store-load fence was terribly expensive. So,
I thought that I needed to buffer up a bunch of objects, set their states, do one mfence,
and then visit all of them. This seemed like a win, so I went with it. Unfortunately, this
made no sense for two reasons:

- I shouldn't use mfence. I should use ortop (lock orl $0, (%rsp)) instead. Ortop is
  basically free, and it's what WTF now uses for storeLoadFence().

- My data saying that buffering up objects was not a slow-down was wrong. That was actually
  almost as expensive as the mfence.

But in order to implement that, I made some other improvements that I think we should stick
with:

- SlotVisitor::visitChildren() now uses a switch on type. This replaces what used to be
  some nasty ClassInfo look-ups.

- We no longer save the object's old CellState. We would do that so that we would know what
  state the object had been before we blackened it. But I believe that the more logical
  solution is to have two kinds of black - one for black-for-the-first-time objects and one
  for repeat offenders. This is a lot easier to reason about, since you can now just figure
  this out by looking at the cell directly.

The latter change meant rewiring a bunch of barriers. It didn't make them any more
expensive.

* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::emitStoreBarrier):
* heap/CellState.h:
(JSC::blacken):
* heap/Heap.cpp:
(JSC::Heap::addToRememberedSet):
* heap/Heap.h:
* heap/HeapInlines.h:
(JSC::Heap::writeBarrier):
(JSC::Heap::reportExtraMemoryVisited):
(JSC::Heap::reportExternalMemoryVisited):
* heap/MarkStack.cpp:
* heap/MarkStack.h:
* heap/SlotVisitor.cpp:
(JSC::SlotVisitor::visitChildren):
* heap/SlotVisitor.h:
* heap/SlotVisitorInlines.h:
(JSC::SlotVisitor::reportExtraMemoryVisited):
(JSC::SlotVisitor::reportExternalMemoryVisited):
* jit/AssemblyHelpers.h:
(JSC::AssemblyHelpers::jumpIfIsRememberedOrInEden):
* llint/LLIntData.cpp:
(JSC::LLInt::Data::performAssertions):
* llint/LowLevelInterpreter.asm:
* llint/LowLevelInterpreter32_64.asm:
* llint/LowLevelInterpreter64.asm:
* runtime/JSObject.h:
(JSC::isJSFinalObject):

Source/WTF:
REGRESSION(r194387): Crash on github.com in IntlDateTimeFormat::resolvedOptions in C locale
https://bugs.webkit.org/show_bug.cgi?id=162139

Patch by Carlos Garcia Campos <cgarcia@igalia.com> on 2016-09-23
Reviewed by Michael Catanzaro.

Handle the case of "C" or "POSIX" locale and use "en-US" as default. That matches what ICU and other ports do,
as well as what layout tests expect (some tests like js/intl-collator.html pass in the bots only because we use
en-US as system locale in those bots).

* wtf/PlatformUserPreferredLanguagesUnix.cpp:
(WTF::platformLanguage):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@206314 268f45cc-cd09-0410-ab3c-d52691b4dbfc

19 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
Source/JavaScriptCore/heap/CellState.h
Source/JavaScriptCore/heap/Heap.cpp
Source/JavaScriptCore/heap/Heap.h
Source/JavaScriptCore/heap/HeapInlines.h
Source/JavaScriptCore/heap/MarkStack.cpp
Source/JavaScriptCore/heap/MarkStack.h
Source/JavaScriptCore/heap/SlotVisitor.cpp
Source/JavaScriptCore/heap/SlotVisitor.h
Source/JavaScriptCore/heap/SlotVisitorInlines.h
Source/JavaScriptCore/jit/AssemblyHelpers.h
Source/JavaScriptCore/llint/LLIntData.cpp
Source/JavaScriptCore/llint/LowLevelInterpreter.asm
Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm
Source/JavaScriptCore/llint/LowLevelInterpreter64.asm
Source/JavaScriptCore/runtime/JSObject.h
Source/WTF/ChangeLog
Source/WTF/wtf/Atomics.h

index e2aaba0..6131514 100644 (file)
@@ -1,3 +1,69 @@
+2016-09-22  Filip Pizlo  <fpizlo@apple.com>
+
+        Need a store-load fence between setting cell state and visiting the object in SlotVisitor
+        https://bugs.webkit.org/show_bug.cgi?id=162354
+
+        Reviewed by Mark Lam.
+        
+        This was meant to be a small change, but then it became bigger as I found small
+        opportunities for improving this code. This adds a store-load fence and is performance-
+        neutral. That's probably partly due to other optimizations that I did to visitChildren().
+        
+        Initially, I found that adding an mfence as a store-load fence was terribly expensive. So,
+        I thought that I needed to buffer up a bunch of objects, set their states, do one mfence,
+        and then visit all of them. This seemed like a win, so I went with it. Unfortunately, this
+        made no sense for two reasons:
+        
+        - I shouldn't use mfence. I should use ortop (lock orl $0, (%rsp)) instead. Ortop is
+          basically free, and it's what WTF now uses for storeLoadFence().
+        
+        - My data saying that buffering up objects was not a slow-down was wrong. That was actually
+          almost as expensive as the mfence.
+        
+        But in order to implement that, I made some other improvements that I think we should stick
+        with:
+        
+        - SlotVisitor::visitChildren() now uses a switch on type. This replaces what used to be
+          some nasty ClassInfo look-ups.
+        
+        - We no longer save the object's old CellState. We would do that so that we would know what
+          state the object had been before we blackened it. But I believe that the more logical
+          solution is to have two kinds of black - one for black-for-the-first-time objects and one
+          for repeat offenders. This is a lot easier to reason about, since you can now just figure
+          this out by looking at the cell directly.
+        
+        The latter change meant rewiring a bunch of barriers. It didn't make them any more
+        expensive.
+
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::emitStoreBarrier):
+        * heap/CellState.h:
+        (JSC::blacken):
+        * heap/Heap.cpp:
+        (JSC::Heap::addToRememberedSet):
+        * heap/Heap.h:
+        * heap/HeapInlines.h:
+        (JSC::Heap::writeBarrier):
+        (JSC::Heap::reportExtraMemoryVisited):
+        (JSC::Heap::reportExternalMemoryVisited):
+        * heap/MarkStack.cpp:
+        * heap/MarkStack.h:
+        * heap/SlotVisitor.cpp:
+        (JSC::SlotVisitor::visitChildren):
+        * heap/SlotVisitor.h:
+        * heap/SlotVisitorInlines.h:
+        (JSC::SlotVisitor::reportExtraMemoryVisited):
+        (JSC::SlotVisitor::reportExternalMemoryVisited):
+        * jit/AssemblyHelpers.h:
+        (JSC::AssemblyHelpers::jumpIfIsRememberedOrInEden):
+        * llint/LLIntData.cpp:
+        (JSC::LLInt::Data::performAssertions):
+        * llint/LowLevelInterpreter.asm:
+        * llint/LowLevelInterpreter32_64.asm:
+        * llint/LowLevelInterpreter64.asm:
+        * runtime/JSObject.h:
+        (JSC::isJSFinalObject):
+
 2016-09-23  Csaba Osztrogon√°c  <ossy@webkit.org>
 
         ARM EABI buildfix after r206289
index af5cc39..a085556 100644 (file)
@@ -11435,7 +11435,8 @@ private:
         LBasicBlock continuation = m_out.newBlock();
 
         m_out.branch(
-            m_out.notZero32(loadCellState(base)), usually(continuation), rarely(slowPath));
+            m_out.above(loadCellState(base), m_out.constInt32(blackThreshold)),
+            usually(continuation), rarely(slowPath));
 
         LBasicBlock lastNext = m_out.appendTo(slowPath, continuation);
 
index edc716c..2cb9d4b 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2015-2016 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
 #ifndef CellState_h
 #define CellState_h
 
+#include <wtf/Assertions.h>
+
 namespace JSC {
 
 enum class CellState : uint8_t {
-    // The object is black as far as this GC is concerned. When not in GC, this just means that it's an
-    // old gen object. Note that we deliberately arrange OldBlack to be zero, so that the store barrier on
-    // a target object "from" is just:
-    //
-    // if (!from->cellState())
-    //     slowPath(from);
-    //
-    // There is a bunch of code in the LLInt and JITs that rely on this being the case. You'd have to
-    // change a lot of code if you ever wanted the store barrier to be anything but a non-zero check on
-    // cellState.
-    OldBlack = 0,
+    // The object is black for the first time during this GC.
+    NewBlack = 0,
+    
+    // The object is black for the Nth time during this full GC cycle (N > 1). An object may get to
+    // this state if it transitions from black back to grey during a concurrent GC, or because it
+    // wound up in the remembered set because of a generational barrier.
+    OldBlack = 1,
     
     // The object is in eden. During GC, this means that the object has not been marked yet.
-    NewWhite = 1,
+    NewWhite = 2,
+
+    // The object is grey - i.e. it will be scanned - and this is the first time in this GC that we are
+    // going to scan it. If this is an eden GC, this also means that the object is in eden.
+    NewGrey = 3,
 
     // The object is grey - i.e. it will be scanned - but it either belongs to old gen (if this is eden
     // GC) or it is grey a second time in this current GC (because a concurrent store barrier requested
     // re-greying).
-    OldGrey = 2,
-
-    // The object is grey - i.e. it will be scanned - and this is the first time in this GC that we are
-    // going to scan it. If this is an eden GC, this also means that the object is in eden.
-    NewGrey = 3
+    OldGrey = 4
 };
 
+static const unsigned blackThreshold = 1; // x <= blackThreshold means x is black.
+
+inline bool isBlack(CellState cellState)
+{
+    return static_cast<unsigned>(cellState) <= blackThreshold;
+}
+
+inline CellState blacken(CellState cellState)
+{
+    if (cellState == CellState::NewGrey)
+        return CellState::NewBlack;
+    ASSERT(cellState == CellState::NewBlack || cellState == CellState::OldBlack || cellState == CellState::OldGrey);
+    return CellState::OldBlack;
+}
+
 } // namespace JSC
 
 #endif // CellState_h
index 667fdf8..81da6bd 100644 (file)
@@ -28,6 +28,7 @@
 #include "FullGCActivityCallback.h"
 #include "GCActivityCallback.h"
 #include "GCIncomingRefCountedSetInlines.h"
+#include "GCSegmentedArrayInlines.h"
 #include "GCTypeMap.h"
 #include "HasOwnPropertyCache.h"
 #include "HeapHelperPool.h"
@@ -914,7 +915,7 @@ void Heap::addToRememberedSet(const JSCell* cell)
 {
     ASSERT(cell);
     ASSERT(!Options::useConcurrentJIT() || !isCompilationThread());
-    ASSERT(cell->cellState() == CellState::OldBlack);
+    ASSERT(isBlack(cell->cellState()));
     // Indicate that this object is grey and that it's one of the following:
     // - A re-greyed object during a concurrent collection.
     // - An old remembered object.
index bdb23ed..65c8a36 100644 (file)
@@ -175,11 +175,11 @@ public:
     // call both of these functions: Calling only one may trigger catastropic
     // memory growth.
     void reportExtraMemoryAllocated(size_t);
-    void reportExtraMemoryVisited(CellState cellStateBeforeVisiting, size_t);
+    void reportExtraMemoryVisited(JSCell*, size_t);
 
 #if ENABLE(RESOURCE_USAGE)
     // Use this API to report the subset of extra memory that lives outside this process.
-    void reportExternalMemoryVisited(CellState cellStateBeforeVisiting, size_t);
+    void reportExternalMemoryVisited(JSCell*, size_t);
     size_t externalMemorySize() { return m_externalMemorySize; }
 #endif
 
index 0168f93..89af914 100644 (file)
@@ -125,7 +125,7 @@ inline void Heap::writeBarrier(const JSCell* from, JSCell* to)
 #if ENABLE(WRITE_BARRIER_PROFILING)
     WriteBarrierCounters::countWriteBarrier();
 #endif
-    if (!from || from->cellState() != CellState::OldBlack)
+    if (!from || !isBlack(from->cellState()))
         return;
     if (!to || to->cellState() != CellState::NewWhite)
         return;
@@ -135,7 +135,7 @@ inline void Heap::writeBarrier(const JSCell* from, JSCell* to)
 inline void Heap::writeBarrier(const JSCell* from)
 {
     ASSERT_GC_OBJECT_LOOKS_VALID(const_cast<JSCell*>(from));
-    if (!from || from->cellState() != CellState::OldBlack)
+    if (!from || !isBlack(from->cellState()))
         return;
     addToRememberedSet(from);
 }
@@ -146,10 +146,10 @@ inline void Heap::reportExtraMemoryAllocated(size_t size)
         reportExtraMemoryAllocatedSlowCase(size);
 }
 
-inline void Heap::reportExtraMemoryVisited(CellState dataBeforeVisiting, size_t size)
+inline void Heap::reportExtraMemoryVisited(JSCell* cell, size_t size)
 {
     // We don't want to double-count the extra memory that was reported in previous collections.
-    if (operationInProgress() == EdenCollection && dataBeforeVisiting == CellState::OldGrey)
+    if (operationInProgress() == EdenCollection && cell->cellState() == CellState::OldBlack)
         return;
 
     size_t* counter = &m_extraMemorySize;
@@ -162,10 +162,10 @@ inline void Heap::reportExtraMemoryVisited(CellState dataBeforeVisiting, size_t
 }
 
 #if ENABLE(RESOURCE_USAGE)
-inline void Heap::reportExternalMemoryVisited(CellState dataBeforeVisiting, size_t size)
+inline void Heap::reportExternalMemoryVisited(JSCell* cell, size_t size)
 {
     // We don't want to double-count the external memory that was reported in previous collections.
-    if (operationInProgress() == EdenCollection && dataBeforeVisiting == CellState::OldGrey)
+    if (operationInProgress() == EdenCollection && cell->cellState() == CellState::OldBlack)
         return;
 
     size_t* counter = &m_externalMemorySize;
index 318183a..c61079d 100644 (file)
@@ -26,6 +26,7 @@
 #include "config.h"
 #include "MarkStack.h"
 
+#include "GCSegmentedArrayInlines.h"
 #include "JSCInlines.h"
 
 namespace JSC {
index 04f19c6..1d04292 100644 (file)
@@ -26,7 +26,7 @@
 #ifndef MarkStack_h
 #define MarkStack_h
 
-#include "GCSegmentedArrayInlines.h"
+#include "GCSegmentedArray.h"
 
 namespace JSC {
 
index 8371721..8d5eeba 100644 (file)
@@ -25,9 +25,9 @@
 
 #include "config.h"
 #include "SlotVisitor.h"
-#include "SlotVisitorInlines.h"
 
 #include "ConservativeRoots.h"
+#include "GCSegmentedArrayInlines.h"
 #include "HeapCellInlines.h"
 #include "HeapProfiler.h"
 #include "HeapSnapshotBuilder.h"
@@ -36,6 +36,7 @@
 #include "JSObject.h"
 #include "JSString.h"
 #include "JSCInlines.h"
+#include "SlotVisitorInlines.h"
 #include "SuperSampler.h"
 #include "VM.h"
 #include <wtf/Lock.h>
@@ -296,25 +297,32 @@ ALWAYS_INLINE void SlotVisitor::visitChildren(const JSCell* cell)
     
     SetCurrentCellScope currentCellScope(*this, cell);
     
-    m_currentObjectCellStateBeforeVisiting = cell->cellState();
-    cell->setCellState(CellState::OldBlack);
+    cell->setCellState(blacken(cell->cellState()));
+    
+    // FIXME: Make this work on ARM also.
+    // https://bugs.webkit.org/show_bug.cgi?id=162461
+    if (isX86())
+        WTF::storeLoadFence();
     
-    if (isJSString(cell)) {
+    switch (cell->type()) {
+    case StringType:
         JSString::visitChildren(const_cast<JSCell*>(cell), *this);
-        return;
-    }
-
-    if (isJSFinalObject(cell)) {
+        break;
+        
+    case FinalObjectType:
         JSFinalObject::visitChildren(const_cast<JSCell*>(cell), *this);
-        return;
-    }
+        break;
 
-    if (isJSArray(cell)) {
+    case ArrayType:
         JSArray::visitChildren(const_cast<JSCell*>(cell), *this);
-        return;
+        break;
+        
+    default:
+        // FIXME: This could be so much better.
+        // https://bugs.webkit.org/show_bug.cgi?id=162462
+        cell->methodTable()->visitChildren(const_cast<JSCell*>(cell), *this);
+        break;
     }
-
-    cell->methodTable()->visitChildren(const_cast<JSCell*>(cell), *this);
 }
 
 void SlotVisitor::donateKnownParallel()
index b1e4b8b..f66251b 100644 (file)
@@ -168,8 +168,6 @@ private:
     HeapSnapshotBuilder* m_heapSnapshotBuilder { nullptr };
     JSCell* m_currentCell { nullptr };
 
-    CellState m_currentObjectCellStateBeforeVisiting { CellState::NewWhite };
-
 public:
 #if !ASSERT_DISABLED
     bool m_isCheckingForDefaultMarkViolation;
index 8ac49fa..bd5e719 100644 (file)
@@ -106,13 +106,13 @@ inline void SlotVisitor::addUnconditionalFinalizer(UnconditionalFinalizer* uncon
 
 inline void SlotVisitor::reportExtraMemoryVisited(size_t size)
 {
-    heap()->reportExtraMemoryVisited(m_currentObjectCellStateBeforeVisiting, size);
+    heap()->reportExtraMemoryVisited(m_currentCell, size);
 }
 
 #if ENABLE(RESOURCE_USAGE)
 inline void SlotVisitor::reportExternalMemoryVisited(size_t size)
 {
-    heap()->reportExternalMemoryVisited(m_currentObjectCellStateBeforeVisiting, size);
+    heap()->reportExternalMemoryVisited(m_currentCell, size);
 }
 #endif
 
index 639da49..8668a6b 100644 (file)
@@ -1308,13 +1308,13 @@ public:
 
     Jump jumpIfIsRememberedOrInEden(GPRReg cell)
     {
-        return branchTest8(MacroAssembler::NonZero, MacroAssembler::Address(cell, JSCell::cellStateOffset()));
+        return branch8(Above, Address(cell, JSCell::cellStateOffset()), TrustedImm32(blackThreshold));
     }
 
     Jump jumpIfIsRememberedOrInEden(JSCell* cell)
     {
         uint8_t* address = reinterpret_cast<uint8_t*>(cell) + JSCell::cellStateOffset();
-        return branchTest8(MacroAssembler::NonZero, MacroAssembler::AbsoluteAddress(address));
+        return branch8(Above, AbsoluteAddress(address), TrustedImm32(blackThreshold));
     }
     
     // Emits the branch structure for typeof. The code emitted by this doesn't fall through. The
index 2ffbf09..dfa9aa9 100644 (file)
@@ -214,6 +214,7 @@ void Data::performAssertions(VM& vm)
     STATIC_ASSERT(GetPutInfo::initializationBits == 0xffc00);
 
     STATIC_ASSERT(MarkedBlock::blockSize == 16 * 1024);
+    STATIC_ASSERT(blackThreshold == 1);
 
     ASSERT(bitwise_cast<uintptr_t>(ShadowChicken::Packet::tailMarker()) == static_cast<uintptr_t>(0x7a11));
 
index 997ef35..10037af 100644 (file)
@@ -409,6 +409,8 @@ const NotInitialization = 2
 const MarkedBlockSize = 16 * 1024
 const MarkedBlockMask = ~(MarkedBlockSize - 1)
 
+const BlackThreshold = 1
+
 # Allocation constants
 if JSVALUE64
     const JSFinalObjectSizeClassIndex = 1
@@ -888,9 +890,10 @@ macro arrayProfile(cellAndIndexingType, profile, scratch)
     loadb JSCell::m_indexingType[cell], indexingType
 end
 
-macro skipIfIsRememberedOrInEden(cell, scratch1, scratch2, continuation)
-    loadb JSCell::m_cellState[cell], scratch1
-    continuation(scratch1)
+macro skipIfIsRememberedOrInEden(cell, slowPath)
+    bba JSCell::m_cellState[cell], BlackThreshold, .done
+    slowPath()
+.done:
 end
 
 macro notifyWrite(set, slow)
index c3ebc62..90d2c36 100644 (file)
@@ -500,9 +500,9 @@ end
 macro writeBarrierOnOperand(cellOperand)
     loadisFromInstruction(cellOperand, t1)
     loadConstantOrVariablePayload(t1, CellTag, t2, .writeBarrierDone)
-    skipIfIsRememberedOrInEden(t2, t1, t3, 
-        macro(cellState)
-            btbnz cellState, .writeBarrierDone
+    skipIfIsRememberedOrInEden(
+        t2, 
+        macro()
             push cfr, PC
             # We make two extra slots because cCall2 will poke.
             subp 8, sp
@@ -511,8 +511,7 @@ macro writeBarrierOnOperand(cellOperand)
             cCall2Void(_llint_write_barrier_slow)
             addp 8, sp
             pop PC, cfr
-        end
-    )
+        end)
 .writeBarrierDone:
 end
 
@@ -532,9 +531,9 @@ macro writeBarrierOnGlobal(valueOperand, loadHelper)
 
     loadHelper(t3)
 
-    skipIfIsRememberedOrInEden(t3, t1, t2,
-        macro(gcData)
-            btbnz gcData, .writeBarrierDone
+    skipIfIsRememberedOrInEden(
+        t3,
+        macro()
             push cfr, PC
             # We make two extra slots because cCall2 will poke.
             subp 8, sp
@@ -543,8 +542,7 @@ macro writeBarrierOnGlobal(valueOperand, loadHelper)
             cCall2Void(_llint_write_barrier_slow)
             addp 8, sp
             pop PC, cfr
-        end
-    )
+        end)
 .writeBarrierDone:
 end
 
index d34a439..e345652 100644 (file)
@@ -404,16 +404,15 @@ end
 macro writeBarrierOnOperand(cellOperand)
     loadisFromInstruction(cellOperand, t1)
     loadConstantOrVariableCell(t1, t2, .writeBarrierDone)
-    skipIfIsRememberedOrInEden(t2, t1, t3, 
-        macro(cellState)
-            btbnz cellState, .writeBarrierDone
+    skipIfIsRememberedOrInEden(
+        t2,
+        macro()
             push PB, PC
             move t2, a1 # t2 can be a0 (not on 64 bits, but better safe than sorry)
             move cfr, a0
             cCall2Void(_llint_write_barrier_slow)
             pop PC, PB
-        end
-    )
+        end)
 .writeBarrierDone:
 end
 
@@ -432,9 +431,9 @@ macro writeBarrierOnGlobal(valueOperand, loadHelper)
     btpz t0, .writeBarrierDone
 
     loadHelper(t3)
-    skipIfIsRememberedOrInEden(t3, t1, t2,
-        macro(gcData)
-            btbnz gcData, .writeBarrierDone
+    skipIfIsRememberedOrInEden(
+        t3,
+        macro()
             push PB, PC
             move cfr, a0
             move t3, a1
index a928760..ed15ae1 100644 (file)
@@ -1093,7 +1093,7 @@ inline JSFinalObject* JSFinalObject::create(VM& vm, Structure* structure)
 
 inline bool isJSFinalObject(JSCell* cell)
 {
-    return cell->classInfo() == JSFinalObject::info();
+    return cell->type() == FinalObjectType;
 }
 
 inline bool isJSFinalObject(JSValue value)
index a36ef54..6a08f2c 100644 (file)
 
 2016-09-22  Filip Pizlo  <fpizlo@apple.com>
 
+        Need a store-load fence between setting cell state and visiting the object in SlotVisitor
+        https://bugs.webkit.org/show_bug.cgi?id=162354
+
+        Reviewed by Mark Lam.
+        
+        Fix this on x86-32.
+
+        * wtf/Atomics.h:
+        (WTF::x86_ortop):
+
+2016-09-22  Filip Pizlo  <fpizlo@apple.com>
+
         Fences on x86 should be a lot cheaper
         https://bugs.webkit.org/show_bug.cgi?id=162417
 
index c48879f..e46a4ba 100644 (file)
@@ -175,10 +175,12 @@ inline void x86_ortop()
     // know that it is equivalent for our purposes, but it would be good to
     // investigate if that is actually better.
     MemoryBarrier();
-#else
+#elif CPU(X86_64)
     // This has acqrel semantics and is much cheaper than mfence. For exampe, in the JSC GC, using
     // mfence as a store-load fence was a 9% slow-down on Octane/splay while using this was neutral.
     asm volatile("lock; orl $0, (%%rsp)" ::: "memory");
+#else
+    asm volatile("lock; orl $0, (%%esp)" ::: "memory");
 #endif
 }