Attachment 261712 Details for Bug 149438 – the patch

[patch] the patch

blah.patch (text/plain), 13.21 KB, created by Filip Pizlo on 2015-09-21 18:29:51 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Filip Pizlo

Created: 2015-09-21 18:29:51 PDT

Size: 13.21 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 190095)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,14 @@
>+2015-09-21  Filip Pizlo  <fpizlo@apple.com>
>+
>+        Always use the compiler's CAS implementation and get rid of ENABLE(COMPARE_AND_SWAP)
>+        https://bugs.webkit.org/show_bug.cgi?id=149438
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * heap/HeapInlines.h:
>+        (JSC::Heap::reportExtraMemoryVisited):
>+        (JSC::Heap::deprecatedReportExtraMemory):
>+
> 2015-09-21  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, rolling out r190086.
>Index: Source/JavaScriptCore/API/tests/CompareAndSwapTest.cpp
>===================================================================
>--- Source/JavaScriptCore/API/tests/CompareAndSwapTest.cpp	(revision 190077)
>+++ Source/JavaScriptCore/API/tests/CompareAndSwapTest.cpp	(working copy)
>@@ -63,7 +63,7 @@ inline bool Bitmap::concurrentTestAndSet
>         oldValue = *wordPtr;
>         if (oldValue & mask)
>             return true;
>-    } while (!WTF::weakCompareAndSwap(wordPtr, oldValue, oldValue | mask));
>+    } while (!WTF::weakCompareAndSwap(wordPtr, oldValue, static_cast<uint8_t>(oldValue | mask)));
>     return false;
> }
> 
>Index: Source/JavaScriptCore/heap/HeapInlines.h
>===================================================================
>--- Source/JavaScriptCore/heap/HeapInlines.h	(revision 190077)
>+++ Source/JavaScriptCore/heap/HeapInlines.h	(working copy)
>@@ -171,15 +171,11 @@ inline void Heap::reportExtraMemoryVisit
> 
>     size_t* counter = &m_extraMemorySize;
>     
>-#if ENABLE(COMPARE_AND_SWAP)
>     for (;;) {
>         size_t oldSize = *counter;
>-        if (WTF::weakCompareAndSwapSize(counter, oldSize, oldSize + size))
>+        if (WTF::weakCompareAndSwap(counter, oldSize, oldSize + size))
>             return;
>     }
>-#else
>-    (*counter) += size;
>-#endif
> }
> 
> inline void Heap::deprecatedReportExtraMemory(size_t size)
>Index: Source/WTF/ChangeLog
>===================================================================
>--- Source/WTF/ChangeLog	(revision 190095)
>+++ Source/WTF/ChangeLog	(working copy)
>@@ -1,3 +1,18 @@
>+2015-09-21  Filip Pizlo  <fpizlo@apple.com>
>+
>+        Always use the compiler's CAS implementation and get rid of ENABLE(COMPARE_AND_SWAP)
>+        https://bugs.webkit.org/show_bug.cgi?id=149438
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * wtf/Atomics.h: Make weakCompareAndSwap() just forward to the system CAS via WTF::Atomic.
>+        (WTF::weakCompareAndSwap):
>+        (WTF::weakCompareAndSwapUIntPtr): Deleted.
>+        (WTF::weakCompareAndSwapSize): Deleted.
>+        * wtf/Bitmap.h: Small changes to use the new API.
>+        (WTF::WordType>::concurrentTestAndSet):
>+        * wtf/Platform.h: Remove ENABLE(COMPARE_AND_SWAP)
>+
> 2015-09-21  Csaba OsztrogonÃ¡c  <ossy@webkit.org>
> 
>         Remove old GCC cruft from wtf/StdLibExtras.h
>Index: Source/WTF/wtf/Atomics.h
>===================================================================
>--- Source/WTF/wtf/Atomics.h	(revision 190077)
>+++ Source/WTF/wtf/Atomics.h	(working copy)
>@@ -113,146 +113,11 @@ struct Atomic {
>     std::atomic<T> value;
> };
> 
>-#if OS(WINDOWS)
>-inline bool weakCompareAndSwap(volatile unsigned* location, unsigned expected, unsigned newValue)
>-{
>-    return InterlockedCompareExchange(reinterpret_cast<LONG volatile*>(location), static_cast<LONG>(newValue), static_cast<LONG>(expected)) == static_cast<LONG>(expected);
>-}
>-
>-inline bool weakCompareAndSwap(void*volatile* location, void* expected, void* newValue)
>-{
>-    return InterlockedCompareExchangePointer(location, newValue, expected) == expected;
>-}
>-#else // OS(WINDOWS) --> not windows
>-inline bool weakCompareAndSwap(volatile unsigned* location, unsigned expected, unsigned newValue) 
>-{
>-#if ENABLE(COMPARE_AND_SWAP)
>-#if CPU(X86) || CPU(X86_64)
>-    unsigned char result;
>-    asm volatile(
>-        "lock; cmpxchgl %3, %2\n\t"
>-        "sete %1"
>-        : "+a"(expected), "=q"(result), "+m"(*location)
>-        : "r"(newValue)
>-        : "memory"
>-        );
>-#elif CPU(ARM_THUMB2)
>-    unsigned tmp;
>-    unsigned result;
>-    asm volatile(
>-        "movw %1, #1\n\t"
>-        "ldrex %2, %0\n\t"
>-        "cmp %3, %2\n\t"
>-        "bne.n 0f\n\t"
>-        "strex %1, %4, %0\n\t"
>-        "0:"
>-        : "+Q"(*location), "=&r"(result), "=&r"(tmp)
>-        : "r"(expected), "r"(newValue)
>-        : "memory");
>-    result = !result;
>-#elif CPU(ARM64) && COMPILER(GCC_OR_CLANG)
>-    unsigned tmp;
>-    unsigned result;
>-    asm volatile(
>-        "mov %w1, #1\n\t"
>-        "ldxr %w2, [%0]\n\t"
>-        "cmp %w3, %w2\n\t"
>-        "b.ne 0f\n\t"
>-        "stxr %w1, %w4, [%0]\n\t"
>-        "0:"
>-        : "+r"(location), "=&r"(result), "=&r"(tmp)
>-        : "r"(expected), "r"(newValue)
>-        : "memory");
>-    result = !result;
>-#elif CPU(ARM64)
>-    unsigned tmp;
>-    unsigned result;
>-    asm volatile(
>-        "mov %w1, #1\n\t"
>-        "ldxr %w2, %0\n\t"
>-        "cmp %w3, %w2\n\t"
>-        "b.ne 0f\n\t"
>-        "stxr %w1, %w4, %0\n\t"
>-        "0:"
>-        : "+m"(*location), "=&r"(result), "=&r"(tmp)
>-        : "r"(expected), "r"(newValue)
>-        : "memory");
>-    result = !result;
>-#else
>-#error "Bad architecture for compare and swap."
>-#endif
>-    return result;
>-#else
>-    UNUSED_PARAM(location);
>-    UNUSED_PARAM(expected);
>-    UNUSED_PARAM(newValue);
>-    CRASH();
>-    return false;
>-#endif
>-}
>-
>-inline bool weakCompareAndSwap(void*volatile* location, void* expected, void* newValue)
>-{
>-#if ENABLE(COMPARE_AND_SWAP)
>-#if CPU(X86_64)
>-    bool result;
>-    asm volatile(
>-        "lock; cmpxchgq %3, %2\n\t"
>-        "sete %1"
>-        : "+a"(expected), "=q"(result), "+m"(*location)
>-        : "r"(newValue)
>-        : "memory"
>-        );
>-    return result;
>-#elif CPU(ARM64) && COMPILER(GCC_OR_CLANG)
>-    bool result;
>-    void* tmp;
>-    asm volatile(
>-        "mov %w1, #1\n\t"
>-        "ldxr %x2, [%0]\n\t"
>-        "cmp %x3, %x2\n\t"
>-        "b.ne 0f\n\t"
>-        "stxr %w1, %x4, [%0]\n\t"
>-        "0:"
>-        : "+r"(location), "=&r"(result), "=&r"(tmp)
>-        : "r"(expected), "r"(newValue)
>-        : "memory");
>-    return !result;
>-#elif CPU(ARM64)
>-    bool result;
>-    void* tmp;
>-    asm volatile(
>-        "mov %w1, #1\n\t"
>-        "ldxr %x2, %0\n\t"
>-        "cmp %x3, %x2\n\t"
>-        "b.ne 0f\n\t"
>-        "stxr %w1, %x4, %0\n\t"
>-        "0:"
>-        : "+m"(*location), "=&r"(result), "=&r"(tmp)
>-        : "r"(expected), "r"(newValue)
>-        : "memory");
>-    return !result;
>-#else
>-    return weakCompareAndSwap(bitwise_cast<unsigned*>(location), bitwise_cast<unsigned>(expected), bitwise_cast<unsigned>(newValue));
>-#endif
>-#else // ENABLE(COMPARE_AND_SWAP)
>-    UNUSED_PARAM(location);
>-    UNUSED_PARAM(expected);
>-    UNUSED_PARAM(newValue);
>-    CRASH();
>-    return 0;
>-#endif // ENABLE(COMPARE_AND_SWAP)
>-}
>-#endif // OS(WINDOWS) (end of the not-windows case)
>-
>-inline bool weakCompareAndSwapUIntPtr(volatile uintptr_t* location, uintptr_t expected, uintptr_t newValue)
>-{
>-    return weakCompareAndSwap(reinterpret_cast<void*volatile*>(location), reinterpret_cast<void*>(expected), reinterpret_cast<void*>(newValue));
>-}
>-
>-inline bool weakCompareAndSwapSize(volatile size_t* location, size_t expected, size_t newValue)
>+// This is a weak CAS function that takes a direct pointer and has no portable fencing guarantees.
>+template<typename T>
>+inline bool weakCompareAndSwap(volatile T* location, T expected, T newValue)
> {
>-    return weakCompareAndSwap(reinterpret_cast<void*volatile*>(location), reinterpret_cast<void*>(expected), reinterpret_cast<void*>(newValue));
>+    return bitwise_cast<Atomic<T>*>(location)->compareExchangeWeak(expected, newValue, std::memory_order_relaxed);
> }
> 
> // Just a compiler fence. Has no effect on the hardware, but tells the compiler
>@@ -322,69 +187,6 @@ inline void memoryBarrierBeforeUnlock() 
> 
> #endif
> 
>-inline bool weakCompareAndSwap(uint8_t* location, uint8_t expected, uint8_t newValue)
>-{
>-#if ENABLE(COMPARE_AND_SWAP)
>-#if !OS(WINDOWS) && (CPU(X86) || CPU(X86_64))
>-    unsigned char result;
>-    asm volatile(
>-        "lock; cmpxchgb %3, %2\n\t"
>-        "sete %1"
>-        : "+a"(expected), "=q"(result), "+m"(*location)
>-        : "q"(newValue)
>-        : "memory"
>-        );
>-    return result;
>-#elif OS(WINDOWS) && CPU(X86)
>-    // FIXME: We need a 64-bit ASM implementation, but this cannot be inline due to
>-    // Microsoft's decision to exclude it from the compiler.
>-    bool result = false;
>-
>-    __asm {
>-        mov al, expected
>-        mov edx, location
>-        mov cl, newValue
>-        lock cmpxchg byte ptr[edx], cl
>-        setz result
>-    }
>-
>-    return result;
>-#else
>-    uintptr_t locationValue = bitwise_cast<uintptr_t>(location);
>-    uintptr_t alignedLocationValue = locationValue & ~(sizeof(unsigned) - 1);
>-    uintptr_t locationOffset = locationValue - alignedLocationValue;
>-    ASSERT(locationOffset < sizeof(unsigned));
>-    unsigned* alignedLocation = bitwise_cast<unsigned*>(alignedLocationValue);
>-    // Make sure that this load is always issued and never optimized away.
>-    unsigned oldAlignedValue = *const_cast<volatile unsigned*>(alignedLocation);
>-
>-    struct Splicer {
>-        static unsigned splice(unsigned value, uint8_t byte, uintptr_t byteIndex)
>-        {
>-            union {
>-                unsigned word;
>-                uint8_t bytes[sizeof(unsigned)];
>-            } u;
>-            u.word = value;
>-            u.bytes[byteIndex] = byte;
>-            return u.word;
>-        }
>-    };
>-    
>-    unsigned expectedAlignedValue = Splicer::splice(oldAlignedValue, expected, locationOffset);
>-    unsigned newAlignedValue = Splicer::splice(oldAlignedValue, newValue, locationOffset);
>-
>-    return weakCompareAndSwap(alignedLocation, expectedAlignedValue, newAlignedValue);
>-#endif
>-#else
>-    UNUSED_PARAM(location);
>-    UNUSED_PARAM(expected);
>-    UNUSED_PARAM(newValue);
>-    CRASH();
>-    return false;
>-#endif
>-}
>-
> } // namespace WTF
> 
> using WTF::Atomic;
>Index: Source/WTF/wtf/Bitmap.h
>===================================================================
>--- Source/WTF/wtf/Bitmap.h	(revision 190077)
>+++ Source/WTF/wtf/Bitmap.h	(working copy)
>@@ -124,7 +124,7 @@ inline bool Bitmap<size, atomicMode, Wor
>         oldValue = *wordPtr;
>         if (oldValue & mask)
>             return true;
>-    } while (!weakCompareAndSwap(wordPtr, oldValue, oldValue | mask));
>+    } while (!weakCompareAndSwap(wordPtr, oldValue, static_cast<WordType>(oldValue | mask)));
>     return false;
> }
> 
>@@ -144,7 +144,7 @@ inline bool Bitmap<size, atomicMode, Wor
>         oldValue = *wordPtr;
>         if (!(oldValue & mask))
>             return false;
>-    } while (!weakCompareAndSwap(wordPtr, oldValue, oldValue & ~mask));
>+    } while (!weakCompareAndSwap(wordPtr, oldValue, static_cast<WordType>(oldValue & ~mask)));
>     return true;
> }
> 
>Index: Source/WTF/wtf/Platform.h
>===================================================================
>--- Source/WTF/wtf/Platform.h	(revision 190077)
>+++ Source/WTF/wtf/Platform.h	(working copy)
>@@ -975,11 +975,7 @@
> #define USE_IMLANG_FONT_LINK2 1
> #endif
> 
>-#if !defined(ENABLE_COMPARE_AND_SWAP) && (OS(WINDOWS) || (COMPILER(GCC_OR_CLANG) && (CPU(X86) || CPU(X86_64) || CPU(ARM_THUMB2) || CPU(ARM64))))
>-#define ENABLE_COMPARE_AND_SWAP 1
>-#endif
>-
>-#if !defined(ENABLE_PARALLEL_GC) && (OS(DARWIN) || PLATFORM(EFL) || PLATFORM(GTK)) && ENABLE(COMPARE_AND_SWAP)
>+#if !defined(ENABLE_PARALLEL_GC) && (OS(DARWIN) || PLATFORM(EFL) || PLATFORM(GTK))
> #define ENABLE_PARALLEL_GC 1
> #endif
> 
>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 190095)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,16 @@
>+2015-09-21  Filip Pizlo  <fpizlo@apple.com>
>+
>+        Always use the compiler's CAS implementation and get rid of ENABLE(COMPARE_AND_SWAP)
>+        https://bugs.webkit.org/show_bug.cgi?id=149438
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        No new tests because no new behavior.
>+
>+        * platform/text/TextBreakIterator.cpp:
>+        (WebCore::compareAndSwapNonSharedCharacterBreakIterator):
>+        (WebCore::NonSharedCharacterBreakIterator::NonSharedCharacterBreakIterator):
>+
> 2015-09-21  Gwang Yoon Hwang  <yoon@igalia.com>
> 
>         [Threaded Compositor] Modified to use reference of GraphicsContext instead of pointer
>Index: Source/WebCore/platform/text/TextBreakIterator.cpp
>===================================================================
>--- Source/WebCore/platform/text/TextBreakIterator.cpp	(revision 190077)
>+++ Source/WebCore/platform/text/TextBreakIterator.cpp	(working copy)
>@@ -812,16 +812,7 @@ static TextBreakIterator* nonSharedChara
> 
> static inline bool compareAndSwapNonSharedCharacterBreakIterator(TextBreakIterator* expected, TextBreakIterator* newValue)
> {
>-#if ENABLE(COMPARE_AND_SWAP)
>     return WTF::weakCompareAndSwap(reinterpret_cast<void**>(&nonSharedCharacterBreakIterator), expected, newValue);
>-#else
>-    static StaticLock nonSharedCharacterBreakIteratorMutex;
>-    std::lock_guard<StaticLock> locker(nonSharedCharacterBreakIteratorMutex);
>-    if (nonSharedCharacterBreakIterator != expected)
>-        return false;
>-    nonSharedCharacterBreakIterator = newValue;
>-    return true;
>-#endif
> }
> 
> NonSharedCharacterBreakIterator::NonSharedCharacterBreakIterator(StringView string)

Flags:

mark.lam: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 149438: 261710 | 261712 | 261719