Attachment 302731 Details for Bug 162349 – a bit more

[patch] a bit more

blah.patch (text/plain), 36.71 KB, created by Filip Pizlo on 2017-02-24 20:53:49 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Filip Pizlo

Created: 2017-02-24 20:53:49 PST

Size: 36.71 KB

patch

obsolete

>Index: Source/JavaScriptCore/b3/B3AtomicValue.cpp
>===================================================================
>--- Source/JavaScriptCore/b3/B3AtomicValue.cpp	(nonexistent)
>+++ Source/JavaScriptCore/b3/B3AtomicValue.cpp	(working copy)
>@@ -0,0 +1,89 @@
>+/*
>+ * Copyright (C) 2017 Apple Inc. All rights reserved.
>+ *
>+ * Redistribution and use in source and binary forms, with or without
>+ * modification, are permitted provided that the following conditions
>+ * are met:
>+ * 1. Redistributions of source code must retain the above copyright
>+ *    notice, this list of conditions and the following disclaimer.
>+ * 2. Redistributions in binary form must reproduce the above copyright
>+ *    notice, this list of conditions and the following disclaimer in the
>+ *    documentation and/or other materials provided with the distribution.
>+ *
>+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
>+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE INC. OR
>+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
>+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
>+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
>+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
>+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>+ */
>+
>+#include "config.h"
>+#include "B3AtomicValue.h"
>+
>+#if ENABLE(B3_JIT)
>+
>+namespace JSC { namespace B3 {
>+
>+AtomicValue::~AtomicValue()
>+{
>+}
>+
>+Width AtomicValue::accessWidth() const
>+{
>+    return m_width;
>+}
>+
>+void AtomicValue::dumpMeta(CommaPrinter& comma, PrintStream& out) const
>+{
>+    out.print(comma, "width = ", m_width);
>+    
>+    MemoryValue::dumpMeta(comma, out);
>+}
>+
>+Value* AtomicValue::cloneImpl() const
>+{
>+    return new AtomicValue(*this);
>+}
>+
>+AtomicValue::AtomicValue(Kind kind, Origin origin, Value* operand, Value* pointer, int32_t offset = 0)
>+    : MemoryValue(CheckedOpcode, kind, operand->type(), origin, offset, operand, pointer)
>+{
>+    setFenceRange(HeapRange::top());
>+    
>+    switch (kind.opcode()) {
>+    case AtomicXchgAdd:
>+    case AtomicXchgAnd:
>+    case AtomicXchgOr:
>+    case AtomicXchgSub:
>+    case AtomicXchgXor:
>+    case AtomicXchg:
>+        break;
>+    default:
>+        ASSERT_NOT_REACHED();
>+    }
>+}
>+
>+AtomicValue::AtomicValue(Kind kind, Origin origin, Value* expectedValue, Value* newValue, Value* pointer, int32_t offset = 0)
>+    : MemoryValue(CheckedOpcode, kind, kind.opcode() == AtomicWeakCAS ? Int32 : expectedValue->type(), origin, offset, expectedValue, newValue, pointer)
>+{
>+    setFenceRange(HeapRange::top());
>+    
>+    switch (kind.opcode()) {
>+    case AtomicWeakCAS:
>+    case AtomicStrongCAS:
>+        break;
>+    default:
>+        ASSERT_NOT_REACHED();
>+    }
>+}
>+
>+} } // namespace JSC::B3
>+
>+#endif // ENABLE(B3_JIT)
>+
>Index: Source/JavaScriptCore/b3/B3AtomicValue.h
>===================================================================
>--- Source/JavaScriptCore/b3/B3AtomicValue.h	(nonexistent)
>+++ Source/JavaScriptCore/b3/B3AtomicValue.h	(working copy)
>@@ -0,0 +1,64 @@
>+/*
>+ * Copyright (C) 2017 Apple Inc. All rights reserved.
>+ *
>+ * Redistribution and use in source and binary forms, with or without
>+ * modification, are permitted provided that the following conditions
>+ * are met:
>+ * 1. Redistributions of source code must retain the above copyright
>+ *    notice, this list of conditions and the following disclaimer.
>+ * 2. Redistributions in binary form must reproduce the above copyright
>+ *    notice, this list of conditions and the following disclaimer in the
>+ *    documentation and/or other materials provided with the distribution.
>+ *
>+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
>+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE INC. OR
>+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
>+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
>+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
>+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
>+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>+ */
>+
>+#pragma once
>+
>+#if ENABLE(B3_JIT)
>+
>+#include "B3MemoryValue.h"
>+#include "B3Width.h"
>+
>+namespace JSC { namespace B3 {
>+
>+class JS_EXPORT_PRIVATE AtomicValue : public MemoryValue {
>+public:
>+    static bool accepts(Kind kind)
>+    {
>+        return isAtomic(kind.opcode());
>+    }
>+    
>+    ~AtomicValue();
>+    
>+    Width accessWidth() const override;
>+    
>+protected:
>+    void dumpMeta(CommaPrinter&, PrintStream&) const override;
>+    
>+    Value* cloneImpl() const override;
>+    
>+private:
>+    friend class Procedure;
>+    
>+    AtomicValue(Kind, Origin, Value* operand, Value* pointer, int32_t offset = 0);
>+    
>+    AtomicValue(Kind, Origin, Value* expectedValue, Value* newValue, Value* pointer, int32_t offset = 0);
>+    
>+    Width m_width;
>+};
>+
>+} } // namespace JSC::B3
>+
>+#endif // ENABLE(B3_JIT)
>+
>Index: Source/JavaScriptCore/b3/B3Effects.cpp
>===================================================================
>--- Source/JavaScriptCore/b3/B3Effects.cpp	(revision 212990)
>+++ Source/JavaScriptCore/b3/B3Effects.cpp	(working copy)
>@@ -82,7 +82,8 @@ bool Effects::interferes(const Effects& 
>         || interferesWithWritesPinned(other, *this)
>         || writes.overlaps(other.writes)
>         || writes.overlaps(other.reads)
>-        || reads.overlaps(other.writes);
>+        || reads.overlaps(other.writes)
>+        || (fence && other.fence);
> }
> 
> bool Effects::operator==(const Effects& other) const
>@@ -95,7 +96,8 @@ bool Effects::operator==(const Effects& 
>         && writesPinned == other.writesPinned
>         && readsPinned == other.readsPinned
>         && writes == other.writes
>-        && reads == other.reads;
>+        && reads == other.reads
>+        && fence == other.fence;
> }
> 
> bool Effects::operator!=(const Effects& other) const
>@@ -120,6 +122,8 @@ void Effects::dump(PrintStream& out) con
>         out.print(comma, "WritesPinned");
>     if (readsPinned)
>         out.print(comma, "ReadsPinned");
>+    if (fence)
>+        out.print(comma, "Fence");
>     if (writes)
>         out.print(comma, "Writes:", writes);
>     if (reads)
>Index: Source/JavaScriptCore/b3/B3Effects.h
>===================================================================
>--- Source/JavaScriptCore/b3/B3Effects.h	(revision 212990)
>+++ Source/JavaScriptCore/b3/B3Effects.h	(working copy)
>@@ -71,6 +71,10 @@ struct Effects {
>     // https://bugs.webkit.org/show_bug.cgi?id=163173
>     bool readsPinned { false };
>     bool writesPinned { false };
>+    
>+    // Memory fences cannot be reordered around each other regardless of their effects. This is flagged
>+    // if the operation is a memory fence.
>+    bool fence { false };
> 
>     HeapRange writes;
>     HeapRange reads;
>@@ -89,6 +93,7 @@ struct Effects {
>         result.reads = HeapRange::top();
>         result.readsPinned = true;
>         result.writesPinned = true;
>+        result.fence = true;
>         return result;
>     }
> 
>@@ -103,7 +108,7 @@ struct Effects {
> 
>     bool mustExecute() const
>     {
>-        return terminal || exitsSideways || writesLocalState || writes || writesPinned;
>+        return terminal || exitsSideways || writesLocalState || writes || writesPinned || fence;
>     }
> 
>     // Returns true if reordering instructions with these respective effects would change program
>Index: Source/JavaScriptCore/b3/B3EliminateCommonSubexpressions.cpp
>===================================================================
>--- Source/JavaScriptCore/b3/B3EliminateCommonSubexpressions.cpp	(revision 212990)
>+++ Source/JavaScriptCore/b3/B3EliminateCommonSubexpressions.cpp	(working copy)
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2016 Apple Inc. All rights reserved.
>+ * Copyright (C) 2016-2017 Apple Inc. All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -142,6 +142,7 @@ struct ImpureBlockData {
> 
>     RangeSet<HeapRange> reads; // This only gets used for forward store elimination.
>     RangeSet<HeapRange> writes; // This gets used for both load and store elimination.
>+    bool fence;
> 
>     MemoryValueMap storesAtHead;
>     MemoryValueMap memoryValuesAtTail;
>@@ -174,12 +175,14 @@ public:
>                 
>                 if (memory && memory->isStore()
>                     && !data.reads.overlaps(memory->range())
>-                    && !data.writes.overlaps(memory->range()))
>+                    && !data.writes.overlaps(memory->range())
>+                    && (!data.fence || !memory->hasFence()))
>                     data.storesAtHead.add(memory);
>                 data.reads.add(effects.reads);
> 
>                 if (HeapRange writes = effects.writes)
>                     clobber(data, writes);
>+                data.fence |= effects.fence;
> 
>                 if (memory)
>                     data.memoryValuesAtTail.add(memory);
>@@ -478,6 +481,9 @@ private:
>     template<typename Filter>
>     bool findStoreAfterClobber(Value* ptr, HeapRange range, const Filter& filter)
>     {
>+        if (m_value->as<MemoryValue>()->hasFence())
>+            return false;
>+        
>         // We can eliminate a store if every forward path hits a store to the same location before
>         // hitting any operation that observes the store. This search seems like it should be
>         // expensive, but in the overwhelming majority of cases it will almost immediately hit an 
>@@ -614,6 +620,12 @@ private:
>         if (verbose)
>             dataLog(*m_value, ": looking backward for ", *ptr, "...\n");
>         
>+        if (m_value->as<MemoryValue>()->hasFence()) {
>+            if (verbose)
>+                dataLog("    Giving up because fences.\n");
>+            return false;
>+        }
>+        
>         if (MemoryValue* match = m_data.memoryValuesAtTail.find(ptr, filter)) {
>             if (verbose)
>                 dataLog("    Found ", *match, " locally.\n");
>Index: Source/JavaScriptCore/b3/B3HeapRange.h
>===================================================================
>--- Source/JavaScriptCore/b3/B3HeapRange.h	(revision 212990)
>+++ Source/JavaScriptCore/b3/B3HeapRange.h	(working copy)
>@@ -88,6 +88,13 @@ public:
>         return !(*this == other);
>     }
>     
>+    HeapRange operator|(const HeapRange& other) const
>+    {
>+        return HeapRange(
>+            std::min(m_begin, other.m_begin),
>+            std::max(m_end, other.m_end));
>+    }
>+    
>     explicit operator bool() const { return m_begin != m_end; }
> 
>     unsigned begin() const { return m_begin; }
>Index: Source/JavaScriptCore/b3/B3LowerMacros.cpp
>===================================================================
>--- Source/JavaScriptCore/b3/B3LowerMacros.cpp	(revision 212990)
>+++ Source/JavaScriptCore/b3/B3LowerMacros.cpp	(working copy)
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2015-2016 Apple Inc. All rights reserved.
>+ * Copyright (C) 2015-2017 Apple Inc. All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -185,6 +185,22 @@ private:
>                 m_changed = true;
>                 break;
>             }
>+                
>+            case Depend: {
>+                if (isX86()) {
>+                    // Create a load-load fence. This codegens to nothing on X86. We use it to tell the
>+                    // compiler not to block load motion.
>+                    FenceValue* fence = m_insertionSet.insert<FenceValue>(m_index, m_origin);
>+                    fence->read = HeapRange();
>+                    fence->write = HeapRange::top();
>+                    
>+                    // Kill the Depend, which should unlock a bunch of code simplification.
>+                    m_node->replaceWithBottom(m_insertionSet, m_index);
>+                    
>+                    m_changed = true;
>+                }
>+                break;
>+            }
> 
>             default:
>                 break;
>Index: Source/JavaScriptCore/b3/B3LowerToAir.cpp
>===================================================================
>--- Source/JavaScriptCore/b3/B3LowerToAir.cpp	(revision 212990)
>+++ Source/JavaScriptCore/b3/B3LowerToAir.cpp	(working copy)
>@@ -562,11 +562,16 @@ private:
>     
>     ArgPromise loadPromiseAnyOpcode(Value* loadValue)
>     {
>+        RELEASE_ASSERT(loadValue->as<MemoryValue>());
>         if (!canBeInternal(loadValue))
>             return Arg();
>         if (crossesInterference(loadValue))
>             return Arg();
>-        ArgPromise result(addr(loadValue), loadValue);
>+        if (loadValue->as<MemoryValue>()->hasFence())
>+            return Arg();
>+        Arg loadAddr = addr(loadValue);
>+        RELEASE_ASSERT(loadAddr);
>+        ArgPromise result(loadAddr, loadValue);
>         if (loadValue->traps())
>             result.setTraps(true);
>         return result;
>@@ -931,10 +936,15 @@ private:
>         Air::Opcode opcode32, Air::Opcode opcode64, Commutativity commutativity = NotCommutative>
>     bool tryAppendStoreBinOp(Value* left, Value* right)
>     {
>+        RELEASE_ASSERT(m_value->as<MemoryValue>());
>+        
>         Air::Opcode opcode = tryOpcodeForType(opcode32, opcode64, left->type());
>         if (opcode == Air::Oops)
>             return false;
>         
>+        if (m_value->as<MemoryValue>()->hasFence())
>+            return false;
>+        
>         Arg storeAddr = addr(m_value);
>         ASSERT(storeAddr);
> 
>@@ -2166,6 +2176,12 @@ private:
>                 m_value->child(0), m_value->child(1));
>             return;
>         }
>+            
>+        case Depend: {
>+            appendBinOp<Xor32, Xor64, Air::Oops, Air::Oops, Commutative>(
>+                m_value->child(0), m_value->child(0));
>+            return;
>+        }
> 
>         case Shl: {
>             if (m_value->child(1)->isInt32(1)) {
>Index: Source/JavaScriptCore/b3/B3MemoryValue.cpp
>===================================================================
>--- Source/JavaScriptCore/b3/B3MemoryValue.cpp	(revision 212990)
>+++ Source/JavaScriptCore/b3/B3MemoryValue.cpp	(working copy)
>@@ -34,32 +34,40 @@ MemoryValue::~MemoryValue()
> {
> }
> 
>+Type MemoryValue::accessType() const
>+{
>+    if (isLoad(opcode()))
>+        return type();
>+    // This happens to work for atomics, too. That's why AtomicValue does not need to override this.
>+    return child(0)->type();
>+}
>+
> size_t MemoryValue::accessByteSize() const
> {
>+    return bytes(accessWidth());
>+}
>+
>+Width MemoryValue::accessWidth() const
>+{
>     switch (opcode()) {
>     case Load8Z:
>     case Load8S:
>     case Store8:
>-        return 1;
>+        return Width8;
>     case Load16Z:
>     case Load16S:
>     case Store16:
>-        return 2;
>+        return Width16;
>     case Load:
>-        return sizeofType(type());
>+        return widthForType(type());
>     case Store:
>-        return sizeofType(child(0)->type());
>+        return widthForType(child(0)->type());
>     default:
>         RELEASE_ASSERT_NOT_REACHED();
>-        return 0;
>+        return Width8;
>     }
> }
> 
>-Width MemoryValue::accessWidth() const
>-{
>-    return widthForBytes(accessByteSize());
>-}
>-
> void MemoryValue::dumpMeta(CommaPrinter& comma, PrintStream& out) const
> {
>     if (m_offset)
>@@ -67,6 +75,8 @@ void MemoryValue::dumpMeta(CommaPrinter&
>     if ((isLoad() && effects().reads != range())
>         || (isStore() && effects().writes != range()))
>         out.print(comma, "range = ", range());
>+    if (phantomRange())
>+        out.print(comma, "phantomRange = ", phantomRange());
> }
> 
> Value* MemoryValue::cloneImpl() const
>@@ -74,6 +84,58 @@ Value* MemoryValue::cloneImpl() const
>     return new MemoryValue(*this);
> }
> 
>+// Use this form for Load (but not Load8Z, Load8S, or any of the Loads that have a suffix that
>+// describes the returned type).
>+MemoryValue::MemoryValue(Kind kind, Type type, Origin origin, Value* pointer, int32_t offset = 0)
>+    : Value(CheckedOpcode, kind, type, origin, pointer)
>+    , m_offset(offset)
>+{
>+    if (!ASSERT_DISABLED) {
>+        switch (kind.opcode()) {
>+        case Load:
>+            break;
>+        case Load8Z:
>+        case Load8S:
>+        case Load16Z:
>+        case Load16S:
>+            ASSERT(type == Int32);
>+            break;
>+        case Store8:
>+        case Store16:
>+        case Store:
>+            ASSERT(type == Void);
>+            break;
>+        default:
>+            ASSERT_NOT_REACHED();
>+        }
>+    }
>+}
>+
>+// Use this form for loads where the return type is implied.
>+MemoryValue::MemoryValue(Kind kind, Origin origin, Value* pointer, int32_t offset = 0)
>+    : MemoryValue(kind, Int32, origin, pointer, offset)
>+{
>+    if (!ASSERT_DISABLED) {
>+        switch (kind.opcode()) {
>+        case Load8Z:
>+        case Load8S:
>+        case Load16Z:
>+        case Load16S:
>+            break;
>+        default:
>+            ASSERT_NOT_REACHED();
>+        }
>+    }
>+}
>+
>+// Use this form for stores.
>+MemoryValue::MemoryValue(Kind kind, Origin origin, Value* value, Value* pointer, int32_t offset = 0)
>+    : Value(CheckedOpcode, kind, Void, origin, value, pointer)
>+    , m_offset(offset)
>+{
>+    ASSERT(isStore(kind.opcode()));
>+}
>+
> } } // namespace JSC::B3
> 
> #endif // ENABLE(B3_JIT)
>Index: Source/JavaScriptCore/b3/B3MemoryValue.h
>===================================================================
>--- Source/JavaScriptCore/b3/B3MemoryValue.h	(revision 212990)
>+++ Source/JavaScriptCore/b3/B3MemoryValue.h	(working copy)
>@@ -32,43 +32,11 @@
> 
> namespace JSC { namespace B3 {
> 
>-// FIXME: We want to allow fenced memory accesses on ARM.
>-// https://bugs.webkit.org/show_bug.cgi?id=162349
>-
> class JS_EXPORT_PRIVATE MemoryValue : public Value {
> public:
>     static bool accepts(Kind kind)
>     {
>-        switch (kind.opcode()) {
>-        case Load8Z:
>-        case Load8S:
>-        case Load16Z:
>-        case Load16S:
>-        case Load:
>-        case Store8:
>-        case Store16:
>-        case Store:
>-            return true;
>-        default:
>-            return false;
>-        }
>-    }
>-
>-    static bool isStore(Kind kind)
>-    {
>-        switch (kind.opcode()) {
>-        case Store8:
>-        case Store16:
>-        case Store:
>-            return true;
>-        default:
>-            return false;
>-        }
>-    }
>-
>-    static bool isLoad(Kind kind)
>-    {
>-        return accepts(kind) && !isStore(kind);
>+        return isMemory(kind.opcode());
>     }
> 
>     ~MemoryValue();
>@@ -78,76 +46,52 @@ public:
> 
>     const HeapRange& range() const { return m_range; }
>     void setRange(const HeapRange& range) { m_range = range; }
>+    
>+    // This is an alias for range.
>+    const HeapRange& accessRange() const { return range(); }
>+    void setAccessRange(const HeapRange& range) { setRange(range); }
>+    
>+    const HeapRange& fenceRange() const { return m_fenceRange; }
>+    void setFenceRange(const HeapRange& range) { m_fenceRange = range; }
>+
>+    bool isStore() const { return isStore(opcode()); }
>+    bool isLoad() const { return isLoad(opcode()); }
> 
>-    bool isStore() const { return type() == Void; }
>-    bool isLoad() const { return type() != Void; }
>+    bool hasFence() const { return !!fenceRange(); }
> 
>+    Type accessType() const;
>     size_t accessByteSize() const;
>-    Width accessWidth() const;
>+
>+    virtual Width accessWidth() const;
> 
> protected:
>-    void dumpMeta(CommaPrinter& comma, PrintStream&) const override;
>+    void dumpMeta(CommaPrinter&, PrintStream&) const override;
> 
>     Value* cloneImpl() const override;
> 
> private:
>     friend class Procedure;
>-
>-    // Use this form for Load (but not Load8Z, Load8S, or any of the Loads that have a suffix that
>-    // describes the returned type).
>-    MemoryValue(Kind kind, Type type, Origin origin, Value* pointer, int32_t offset = 0)
>-        : Value(CheckedOpcode, kind, type, origin, pointer)
>+    
>+    template<typename... Arguments>
>+    MemoryValue(CheckedOpcodeTag, Kind kind, Type type, Origin origin, int32_t offset, Arguments... arguments)
>+        : Value(CheckedOpcode, kind, type, origin, arguments...)
>         , m_offset(offset)
>-        , m_range(HeapRange::top())
>     {
>-        if (!ASSERT_DISABLED) {
>-            switch (kind.opcode()) {
>-            case Load:
>-                break;
>-            case Load8Z:
>-            case Load8S:
>-            case Load16Z:
>-            case Load16S:
>-                ASSERT(type == Int32);
>-                break;
>-            case Store8:
>-            case Store16:
>-            case Store:
>-                ASSERT(type == Void);
>-                break;
>-            default:
>-                ASSERT_NOT_REACHED();
>-            }
>-        }
>     }
>+    
>+    // Use this form for Load (but not Load8Z, Load8S, or any of the Loads that have a suffix that
>+    // describes the returned type).
>+    MemoryValue(Kind, Type, Origin, Value* pointer, int32_t offset = 0);
> 
>     // Use this form for loads where the return type is implied.
>-    MemoryValue(Kind kind, Origin origin, Value* pointer, int32_t offset = 0)
>-        : MemoryValue(kind, Int32, origin, pointer, offset)
>-    {
>-    }
>+    MemoryValue(Kind, Origin, Value* pointer, int32_t offset = 0);
> 
>     // Use this form for stores.
>-    MemoryValue(Kind kind, Origin origin, Value* value, Value* pointer, int32_t offset = 0)
>-        : Value(CheckedOpcode, kind, Void, origin, value, pointer)
>-        , m_offset(offset)
>-        , m_range(HeapRange::top())
>-    {
>-        if (!ASSERT_DISABLED) {
>-            switch (kind.opcode()) {
>-            case Store8:
>-            case Store16:
>-            case Store:
>-                break;
>-            default:
>-                ASSERT_NOT_REACHED();
>-                break;
>-            }
>-        }
>-    }
>-
>+    MemoryValue(Kind, Origin, Value* value, Value* pointer, int32_t offset = 0);
>+    
>     int32_t m_offset { 0 };
>-    HeapRange m_range;
>+    HeapRange m_range { HeapRange::top() };
>+    HeapRange m_fenceRange { HeapRange() };
> };
> 
> } } // namespace JSC::B3
>Index: Source/JavaScriptCore/b3/B3Opcode.h
>===================================================================
>--- Source/JavaScriptCore/b3/B3Opcode.h	(revision 212990)
>+++ Source/JavaScriptCore/b3/B3Opcode.h	(working copy)
>@@ -82,7 +82,6 @@ enum Opcode : int16_t {
>     Mod, // All bets are off as to what will happen when you execute this for -2^31%-1 and x%0.
>     UMod,
> 
>-
>     // Polymorphic negation. Note that we only need this for floating point, since integer negation
>     // is exactly like Sub(0, x). But that's not true for floating point. Sub(0, 0) is 0, while
>     // Neg(0) is -0. Also, we canonicalize Sub(0, x) into Neg(x) in case of integers.
>@@ -162,6 +161,99 @@ enum Opcode : int16_t {
>     Store16,
>     // This is a polymorphic store for Int32, Int64, Float, and Double.
>     Store,
>+    
>+    // Atomic compare and swap that returns a boolean. May choose to do nothing and return false. You can
>+    // usually assume that this is faster and results in less code than AtomicStrongCAS, though that's
>+    // not necessarily true on Intel, if instruction selection does its job. Imagine that this opcode is
>+    // as if you did this atomically:
>+    //
>+    // template<typename T>
>+    // bool AtomicWeakCAS(T expectedValue, T newValue, T* ptr)
>+    // {
>+    //     if (!rand())
>+    //         return false; // Real world example of this: context switch on ARM while doing CAS.
>+    //     if (*ptr != expectedValue)
>+    //         return false;
>+    //     *ptr = newValue;
>+    //     return true;
>+    // }
>+    //
>+    // Note that all atomics put the pointer last to be consistent with how loads and stores work. This
>+    // is a goofy tradition, but it's harmless, and better than being inconsistent.
>+    //
>+    // Note that weak CAS has no fencing guarantees when it fails. This means that the following
>+    // transformation is always valid:
>+    //
>+    // Before:
>+    //
>+    //         Branch(AtomicWeakCAS(expected, new, ptr))
>+    //       Successors: Then:#success, Else:#fail
>+    //
>+    // After:
>+    //
>+    //         Branch(Equal(Load(ptr), expected))
>+    //       Successors: Then:#attempt, Else:#fail
>+    //     BB#attempt:
>+    //         Branch(AtomicWeakCAS(expected, new, ptr))
>+    //       Successors: Then:#success, Else:#fail
>+    AtomicWeakCAS,
>+    
>+    // Atomic compare and swap that returns the old value. Does not have the nondeterminism of WeakCAS.
>+    // This is a bit more code and a bit slower in some cases, though not by a lot. Imagine that this
>+    // opcode is as if you did this atomically:
>+    //
>+    // template<typename T>
>+    // T AtomicStrongCAS(T expectedValue, T newValue, T* ptr)
>+    // {
>+    //     T oldValue = *ptr;
>+    //     if (oldValue == expectedValue)
>+    //         *ptr = newValue;
>+    //     return oldValue
>+    // }
>+    //
>+    // Note that AtomicWeakCAS and AtomicStrongCAS sort of have this kind of equivalence:
>+    //
>+    // AtomicWeakCAS(@exp, @new, @ptr) == Equal(AtomicStrongCAS(@exp, @new, @ptr), @exp)
>+    //
>+    // Assuming that the WeakCAS does not spuriously fail, of course.
>+    AtomicStrongCAS,
>+    
>+    // Atomically ___ a memory location and return the old value. Syntax:
>+    //
>+    // @oldValue = AtomicXchg___(@operand, @ptr)
>+    AtomicXchgAdd,
>+    AtomicXchgAnd,
>+    AtomicXchgOr,
>+    AtomicXchgSub,
>+    AtomicXchgXor,
>+    
>+    // Atomically exchange a value with a memory location. Syntax:
>+    //
>+    // @oldValue = AtomicXchg(@newValue, @ptr)
>+    AtomicXchg,
>+    
>+    // Introduce an invisible dependency for blocking motion of loads with respect to each other. Syntax:
>+    //
>+    // @result = Depend(@phantom)
>+    //
>+    // This is eventually codegenerated to have local semantics as if we did:
>+    //
>+    // @result = $0
>+    //
>+    // But it ensures that the users of @result cannot execute until @phantom is computed.
>+    //
>+    // The compiler is not allowed to reason about the fact that Depend codegenerates this way. Any kind
>+    // of transformation or analysis that relies on the insight that Depend is really zero is unsound,
>+    // because it unlocks reordering of users of @result and @phantom.
>+    //
>+    // On X86, this is lowered to a load-load fence and @result uses @phantom directly.
>+    //
>+    // On ARM, this is lowered as if like:
>+    //
>+    // @result = BitXor(@phantom, @phantom)
>+    //
>+    // Except that the compiler never gets an opportunity to simplify out the BitXor.
>+    Depend,
> 
>     // This is used to compute the actual address of a Wasm memory operation. It takes an IntPtr
>     // and a pinned register then computes the appropriate IntPtr address. For the use-case of
>@@ -301,6 +393,71 @@ inline bool isDefinitelyTerminal(Opcode 
>     }
> }
> 
>+inline bool isLoad(Opcode opcode)
>+{
>+    switch (opcode) {
>+    case Load8Z:
>+    case Load8S:
>+    case Load16Z:
>+    case Load16S:
>+    case Load:
>+        return true;
>+    default:
>+        return false;
>+    }
>+}
>+
>+inline bool isStore(Opcode opcode)
>+{
>+    switch (opcode) {
>+    case Store8:
>+    case Store16:
>+    case Store:
>+        return true;
>+    default:
>+        return false;
>+    }
>+}
>+
>+inline bool isLoadStore(Opcode opcode)
>+{
>+    switch (opcode) {
>+    case Load8Z:
>+    case Load8S:
>+    case Load16Z:
>+    case Load16S:
>+    case Load:
>+    case Store8:
>+    case Store16:
>+    case Store:
>+        return true;
>+    default:
>+        return false;
>+    }
>+}
>+
>+inline bool isAtomic(Opcode opcode)
>+{
>+    switch (opcode) {
>+    case AtomicWeakCAS:
>+    case AtomicStrongCAS:
>+    case AtomicXchgAdd:
>+    case AtomicXchgAnd:
>+    case AtomicXchgOr:
>+    case AtomicXchgSub:
>+    case AtomicXchgXor:
>+    case AtomicXchg:
>+        return true;
>+    default:
>+        return false;
>+    }
>+}
>+
>+inline bool isMemoryAccess(Opcode opcode)
>+{
>+    return isAtomic(opcode) || isLoadStore(opcode);
>+}
>+
> } } // namespace JSC::B3
> 
> namespace WTF {
>Index: Source/JavaScriptCore/b3/B3Procedure.h
>===================================================================
>--- Source/JavaScriptCore/b3/B3Procedure.h	(revision 212990)
>+++ Source/JavaScriptCore/b3/B3Procedure.h	(working copy)
>@@ -117,6 +117,7 @@ public:
>     Value* addIntConstant(Origin, Type, int64_t value);
>     Value* addIntConstant(Value*, int64_t value);
> 
>+    // You're guaranteed that bottom is zero.
>     Value* addBottom(Origin, Type);
>     Value* addBottom(Value*);
> 
>Index: Source/JavaScriptCore/b3/B3Validate.cpp
>===================================================================
>--- Source/JavaScriptCore/b3/B3Validate.cpp	(revision 212990)
>+++ Source/JavaScriptCore/b3/B3Validate.cpp	(working copy)
>@@ -371,6 +371,43 @@ public:
>                 VALIDATE(value->type() == Void, ("At ", *value));
>                 validateStackAccess(value);
>                 break;
>+            case AtomicWeakCAS:
>+                VALIDATE(!value->kind().isChill(), ("At ", *value));
>+                VALIDATE(value->numChildren() == 3, ("At ", *value));
>+                VALIDATE(value->type() == Void, ("At ", *value));
>+                VALIDATE(value->child(0)->type() == value->child(1)->type(), ("At ", *value));
>+                VALIDATE(isInt(value->child(0)->type()), ("At ", *value));
>+                VALIDATE(value->child(2)->type() == pointerType(), ("At ", *value));
>+                validateStackAccess(value);
>+                break;
>+            case AtomicStrongCAS:
>+                VALIDATE(!value->kind().isChill(), ("At ", *value));
>+                VALIDATE(value->numChildren() == 3, ("At ", *value));
>+                VALIDATE(value->type() == value->child(0)->type(), ("At ", *value));
>+                VALIDATE(value->type() == value->child(1)->type(), ("At ", *value));
>+                VALIDATE(isInt(value->type()), ("At ", *value));
>+                VALIDATE(value->child(2)->type() == pointerType(), ("At ", *value));
>+                validateStackAccess(value);
>+                break;
>+            case AtomicXchgAdd:
>+            case AtomicXchgAnd:
>+            case AtomicXchgOr:
>+            case AtomicXchgSub:
>+            case AtomicXchgXor:
>+            case AtomicXchg:
>+                VALIDATE(!value->kind().isChill(), ("At ", *value));
>+                VALIDATE(value->numChildren() == 2, ("At ", *value));
>+                VALIDATE(value->type() == value->child(0)->type(), ("At ", *value));
>+                VALIDATE(isInt(value->type()), ("At ", *value));
>+                VALIDATE(value->child(1)->type() == pointerType(), ("At ", *value));
>+                validateStackAccess(value);
>+                break;
>+            case Depend:
>+                VALIDATE(!value->kind().hasExtraBits(), ("At ", *value));
>+                VALIDATE(value->numChildren() == 1, ("At ", *value));
>+                VALIDATE(value->type() == value->child(0)->type(), ("At ", *value));
>+                VALIDATE(isInt(value->type()), ("At ", *value));
>+                break;
>             case WasmAddress:
>                 VALIDATE(!value->kind().hasExtraBits(), ("At ", *value));
>                 VALIDATE(value->numChildren() == 1, ("At ", *value));
>Index: Source/JavaScriptCore/b3/B3Value.cpp
>===================================================================
>--- Source/JavaScriptCore/b3/B3Value.cpp	(revision 212990)
>+++ Source/JavaScriptCore/b3/B3Value.cpp	(working copy)
>@@ -589,6 +589,7 @@ Effects Value::effects() const
>     case BelowEqual:
>     case EqualOrUnordered:
>     case Select:
>+    case Depend:
>         break;
>     case Div:
>     case UDiv:
>@@ -600,16 +601,44 @@ Effects Value::effects() const
>     case Load8S:
>     case Load16Z:
>     case Load16S:
>-    case Load:
>-        result.reads = as<MemoryValue>()->range();
>+    case Load: {
>+        MemoryValue* memory = as<MemoryValue>();
>+        result.reads = memory->range();
>+        if (memory->phantomRange()) {
>+            result.writes = memory->phantomRange();
>+            result.fence = true;
>+        }
>         result.controlDependent = true;
>         break;
>+    }
>     case Store8:
>     case Store16:
>-    case Store:
>-        result.writes = as<MemoryValue>()->range();
>+    case Store: {
>+        MemoryValue* memory = as<MemoryValue>();
>+        result.writes = memory->range();
>+        if (memory->phantomRange()) {
>+            result.reads = memory->phantomRange();
>+            result.fence = true;
>+        }
>+        result.controlDependent = true;
>+        break;
>+    }
>+    case AtomicWeakCAS:
>+    case AtomicStrongCAS:
>+    case AtomicXchgAdd:
>+    case AtomicXchgAnd:
>+    case AtomicXchgOr:
>+    case AtomicXchgSub:
>+    case AtomicXchgXor:
>+    case AtomicXchg: {
>+        MemoryValue* memory = as<MemoryValue>();
>+        result.reads = memory->range() | memory->phantomRange();
>+        result.writes = memory->range() | memorty->phantomRange();
>+        if (memory->phantomRange())
>+            result.fence = true;
>         result.controlDependent = true;
>         break;
>+    }
>     case WasmAddress:
>         result.readsPinned = true;
>         break;
>@@ -617,18 +646,7 @@ Effects Value::effects() const
>         const FenceValue* fence = as<FenceValue>();
>         result.reads = fence->read;
>         result.writes = fence->write;
>-        
>-        // Prevent killing of fences that claim not to write anything. It's a bit weird that we use
>-        // local state as the way to do this, but it happens to work: we must assume that we cannot
>-        // kill writesLocalState unless we understands exactly what the instruction is doing (like
>-        // the way that fixSSA understands Set/Get and the way that reduceStrength and others
>-        // understand Upsilon). This would only become a problem if we had some analysis that was
>-        // looking to use the writesLocalState bit to invalidate a CSE over local state operations.
>-        // Then a Fence would look block, say, the elimination of a redundant Get. But it like
>-        // that's not at all how our optimizations for Set/Get/Upsilon/Phi work - they grok their
>-        // operations deeply enough that they have no need to check this bit - so this cheat is
>-        // fine.
>-        result.writesLocalState = true;
>+        result.fence = true;
>         break;
>     }
>     case CCall:
>@@ -694,6 +712,7 @@ ValueKey Value::key() const
>     case Check:
>     case BitwiseCast:
>     case Neg:
>+    case Depend:
>         return ValueKey(kind(), type(), child(0));
>     case Add:
>     case Sub:
>Index: Source/JavaScriptCore/b3/B3Value.h
>===================================================================
>--- Source/JavaScriptCore/b3/B3Value.h	(revision 212990)
>+++ Source/JavaScriptCore/b3/B3Value.h	(working copy)
>@@ -107,6 +107,8 @@ public:
>     // values with Identities on bottom constants. For this reason, this takes a callback that is
>     // responsible for creating bottoms. There's a utility for this, see B3BottomProvider.h. You
>     // can also access that utility using replaceWithBottom(InsertionSet&, size_t).
>+    //
>+    // You're guaranteed that bottom is zero.
>     template<typename BottomProvider>
>     void replaceWithBottom(const BottomProvider&);
>     
>Index: Source/JavaScriptCore/b3/air/AirOpcode.opcodes
>===================================================================
>--- Source/JavaScriptCore/b3/air/AirOpcode.opcodes	(revision 212990)
>+++ Source/JavaScriptCore/b3/air/AirOpcode.opcodes	(working copy)
>@@ -1,4 +1,4 @@
>-# Copyright (C) 2015-2016 Apple Inc. All rights reserved.
>+# Copyright (C) 2015-2017 Apple Inc. All rights reserved.
> #
> # Redistribution and use in source and binary forms, with or without
> # modification, are permitted provided that the following conditions
>@@ -668,28 +668,76 @@ Load8 U:G:8, ZD:G:32
>     Addr, Tmp
>     Index, Tmp
> 
>+LoadAcq8 U:G:8, ZD:G:32
>+    Addr, Tmp
>+    Index, Tmp
>+
> Store8 U:G:8, D:G:8
>     Tmp, Index
>     Tmp, Addr
>     x86: Imm, Index
>     x86: Imm, Addr
> 
>+StoreRel8 U:G:8, D:G:8
>+    Tmp, Index
>+    Tmp, Addr
>+    x86: Imm, Index
>+    x86: Imm, Addr
>+
> Load8SignedExtendTo32 U:G:8, ZD:G:32
>     Addr, Tmp
>     Index, Tmp
> 
>+LoadAcq8SignedExtendTo32 U:G:8, ZD:G:32
>+    Addr, Tmp
>+    Index, Tmp
>+
> Load16 U:G:16, ZD:G:32
>     Addr, Tmp
>     Index, Tmp
> 
>+LoadAcq16 U:G:16, ZD:G:32
>+    Addr, Tmp
>+    Index, Tmp
>+
> Load16SignedExtendTo32 U:G:16, ZD:G:32
>     Addr, Tmp
>     Index, Tmp
> 
>+LoadAcq16SignedExtendTo32 U:G:16, ZD:G:32
>+    Addr, Tmp
>+    Index, Tmp
>+
> Store16 U:G:16, D:G:16
>     Tmp, Index
>     Tmp, Addr
> 
>+StoreAcq16 U:G:16, D:G:16
>+    Tmp, Index
>+    Tmp, Addr
>+
>+LoadAcq32 U:G:32 ZD:G:32
>+    Tmp, Tmp
>+    Addr, Tmp
>+    Index, Tmp
>+
>+StoreRel32 U:G:32 ZD:G:32
>+    Tmp, Addr
>+    Tmp, Index
>+    x86: Imm, Addr
>+    x86: Imm, Index
>+
>+64: LoadAcq64 U:G:64 ZD:G:64
>+    Tmp, Tmp
>+    Addr, Tmp
>+    Index, Tmp
>+
>+64: StoreRel64 U:G:64 ZD:G:64
>+    Tmp, Addr
>+    Tmp, Index
>+    x86: Imm, Addr
>+    x86: Imm, Index
>+
> Compare32 U:G:32, U:G:32, U:G:32, ZD:G:32
>     RelCond, Tmp, Tmp, Tmp
>     RelCond, Tmp, Imm, Tmp

Actions: View | Formatted Diff | Diff

Attachments on bug 162349: 302367 | 302571 | 302599 | 302731 | 302760 | 302761 | 302763 | 302786 | 302787 | 302795 | 302845 | 302892 | 302917 | 302922 | 302941 | 302949 | 302960 | 302963 | 302968 | 302981 | 303015 | 303078 | 303109 | 303150 | 303170 | 303237 | 303277 | 303328 | 303361 | 303369 | 303396 | 303397 | 303405 | 303435 | 303437 | 303609 | 303928