Changeset 215533 in webkit

Timestamp:

Apr 19, 2017 3:05:51 PM (7 years ago)

Author:

jfbastien@apple.com

Message:

WebAssembly: fast memory cleanups
​https://bugs.webkit.org/show_bug.cgi?id=170909

Reviewed by Saam Barati.

• b3/B3LowerToAir.cpp: correct comment, and make wasm-independent

(JSC::B3::Air::LowerToAir::lower):

• b3/B3Procedure.h:
• b3/B3Validate.cpp:
• b3/B3Value.cpp:

(JSC::B3::Value::effects):

• b3/B3WasmBoundsCheckValue.cpp: have the creator pass in a

maximum, so we don't have to know so much about wasm here
(JSC::B3::WasmBoundsCheckValue::WasmBoundsCheckValue):
(JSC::B3::WasmBoundsCheckValue::cloneImpl):
(JSC::B3::WasmBoundsCheckValue::dumpMeta):

• b3/B3WasmBoundsCheckValue.h:

(JSC::B3::WasmBoundsCheckValue::boundsType):
(JSC::B3::WasmBoundsCheckValue::bounds):

• b3/air/AirCode.h:
• b3/air/AirCustom.h:

(JSC::B3::Air::WasmBoundsCheckCustom::generate):

• b3/testb3.cpp:

(JSC::B3::testWasmBoundsCheck):

• wasm/WasmB3IRGenerator.cpp:

(JSC::Wasm::B3IRGenerator::B3IRGenerator):
(JSC::Wasm::B3IRGenerator::emitCheckAndPreparePointer):
(JSC::Wasm::createJSToWasmWrapper): remove dead code

• wasm/WasmMemory.cpp: don't GC if no memory could possibly be free'd

(JSC::Wasm::Memory::initializePreallocations): verbose-only code,
and copy-pasta bug

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• b3/B3LowerToAir.cpp (modified) (2 diffs)
• b3/B3Procedure.h (modified) (1 diff)
• b3/B3Validate.cpp (modified) (1 diff)
• b3/B3Value.cpp (modified) (1 diff)
• b3/B3WasmBoundsCheckValue.cpp (modified) (2 diffs)
• b3/B3WasmBoundsCheckValue.h (modified) (3 diffs)
• b3/air/AirCode.h (modified) (1 diff)
• b3/air/AirCustom.h (modified) (1 diff)
• b3/testb3.cpp (modified) (2 diffs)
• wasm/WasmB3IRGenerator.cpp (modified) (5 diffs)
• wasm/WasmMemory.cpp (modified) (6 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2017-04-19  JF Bastien  <jfbastien@apple.com>
+
+        WebAssembly: fast memory cleanups
+        https://bugs.webkit.org/show_bug.cgi?id=170909
+
+        Reviewed by Saam Barati.
+
+        * b3/B3LowerToAir.cpp: correct comment, and make wasm-independent
+        (JSC::B3::Air::LowerToAir::lower):
+        * b3/B3Procedure.h:
+        * b3/B3Validate.cpp:
+        * b3/B3Value.cpp:
+        (JSC::B3::Value::effects):
+        * b3/B3WasmBoundsCheckValue.cpp: have the creator pass in a
+        maximum, so we don't have to know so much about wasm here
+        (JSC::B3::WasmBoundsCheckValue::WasmBoundsCheckValue):
+        (JSC::B3::WasmBoundsCheckValue::cloneImpl):
+        (JSC::B3::WasmBoundsCheckValue::dumpMeta):
+        * b3/B3WasmBoundsCheckValue.h:
+        (JSC::B3::WasmBoundsCheckValue::boundsType):
+        (JSC::B3::WasmBoundsCheckValue::bounds):
+        * b3/air/AirCode.h:
+        * b3/air/AirCustom.h:
+        (JSC::B3::Air::WasmBoundsCheckCustom::generate):
+        * b3/testb3.cpp:
+        (JSC::B3::testWasmBoundsCheck):
+        * wasm/WasmB3IRGenerator.cpp:
+        (JSC::Wasm::B3IRGenerator::B3IRGenerator):
+        (JSC::Wasm::B3IRGenerator::emitCheckAndPreparePointer):
+        (JSC::Wasm::createJSToWasmWrapper): remove dead code
+        * wasm/WasmMemory.cpp: don't GC if no memory could possibly be free'd
+        (JSC::Wasm::Memory::initializePreallocations): verbose-only code,
+        and copy-pasta bug
+
 2017-04-19  Mark Lam  <mark.lam@apple.com>
 

trunk/Source/JavaScriptCore/b3/B3LowerToAir.cpp

@@ -3147 +3147 @@
 
         case B3::WasmBoundsCheck: {
-#if ENABLE(WEBASSEMBLY)
             WasmBoundsCheckValue* value = m_value->as<WasmBoundsCheckValue>();
 
@@ -3165 +3164 @@
 
             Arg limit;
-            if (value->pinnedGPR() != InvalidGPRReg)
-                limit = Arg(value->pinnedGPR());
-            else {
-                // Signaling memories don't pin a register because only the accesses whose reg+imm could ever overflow 4GiB+redzone need to be checked,
-                // and we don't think these will be frequent. All other accesses will trap due to PROT_NONE pages.
-                //
-                // If we got here it's because a memory access had a very large offset. We could check that it doesn't exceed 4GiB+redzone since that's
-                // technically the limit we need to avoid overflowing, but it's better if we use a smaller immediate which codegens more easily.
-                // We know that anything above the declared 'maximum' will trap, so we can compare against that number. If there was no declared
-                // 'maximum' then we still know that any access above 4GiB will trap, no need to add the redzone.
+            switch (value->boundsType()) {
+            case WasmBoundsCheckValue::Type::Pinned:
+                limit = Arg(value->bounds().pinned);
+                break;
+
+            case WasmBoundsCheckValue::Type::Maximum:
                 limit = m_code.newTmp(GP);
-                size_t limitValue = value->maximum() ? value->maximum().bytes() : std::numeric_limits<uint32_t>::max();
-                ASSERT(limitValue <= value->redzoneLimit());
-                if (imm(limitValue))
-                    append(Move, imm(limitValue), limit);
+                if (imm(value->bounds().maximum))
+                    append(Move, imm(value->bounds().maximum), limit);
                 else
-                    append(Move, Arg::bigImm(limitValue), limit);
-            }
+                    append(Move, Arg::bigImm(value->bounds().maximum), limit);
+                break;
+            }
+
             append(Inst(Air::WasmBoundsCheck, value, ptrPlusImm, limit));
-#else
-            append(Air::Oops);
-#endif // ENABLE(WEBASSEMBLY)
             return;
         }

trunk/Source/JavaScriptCore/b3/B3Procedure.h

@@ -59 +59 @@
 namespace Air { class Code; }
 
-typedef void WasmBoundsCheckGeneratorFunction(CCallHelpers&, GPRReg, unsigned);
+typedef void WasmBoundsCheckGeneratorFunction(CCallHelpers&, GPRReg);
 typedef SharedTask<WasmBoundsCheckGeneratorFunction> WasmBoundsCheckGenerator;
 

trunk/Source/JavaScriptCore/b3/B3Validate.cpp

@@ -471 +471 @@
                 VALIDATE(value->numChildren() == 1, ("At ", *value));
                 VALIDATE(value->child(0)->type() == Int32, ("At ", *value));
-                if (value->as<WasmBoundsCheckValue>()->pinnedGPR() != InvalidGPRReg)
-                    VALIDATE(m_procedure.code().isPinned(value->as<WasmBoundsCheckValue>()->pinnedGPR()), ("At ", *value));
+                switch (value->as<WasmBoundsCheckValue>()->boundsType()) {
+                case WasmBoundsCheckValue::Type::Pinned:
+                    VALIDATE(m_procedure.code().isPinned(value->as<WasmBoundsCheckValue>()->bounds().pinned), ("At ", *value));
+                    break;
+                case WasmBoundsCheckValue::Type::Maximum:
+                    break;
+                }
                 VALIDATE(m_procedure.code().wasmBoundsCheckGenerator(), ("At ", *value));
                 break;

trunk/Source/JavaScriptCore/b3/B3Value.cpp

@@ -666 +666 @@
         break;
     case WasmBoundsCheck:
-        if (as<WasmBoundsCheckValue>()->pinnedGPR() != InvalidGPRReg)
+        switch (as<WasmBoundsCheckValue>()->boundsType()) {
+        case WasmBoundsCheckValue::Type::Pinned:
             result.readsPinned = true;
+            break;
+        case WasmBoundsCheckValue::Type::Maximum:
+            break;
+        }
         result.exitsSideways = true;
         break;

trunk/Source/JavaScriptCore/b3/B3WasmBoundsCheckValue.cpp

@@ -36 +36 @@
 }
 
-WasmBoundsCheckValue::WasmBoundsCheckValue(Origin origin, Value* ptr, GPRReg pinnedGPR, unsigned offset, PageCount maximum)
+WasmBoundsCheckValue::WasmBoundsCheckValue(Origin origin, Value* ptr, unsigned offset, GPRReg pinnedGPR)
     : Value(CheckedOpcode, WasmBoundsCheck, origin, ptr)
-    , m_pinnedGPR(pinnedGPR)
     , m_offset(offset)
-    , m_maximum(maximum)
+    , m_boundsType(Type::Pinned)
 {
+    m_bounds.pinned = pinnedGPR;
+}
+
+WasmBoundsCheckValue::WasmBoundsCheckValue(Origin origin, Value* ptr, unsigned offset, size_t maximum)
+    : Value(CheckedOpcode, WasmBoundsCheck, origin, ptr)
+    , m_offset(offset)
+    , m_boundsType(Type::Maximum)
+{
+#if ENABLE(WEBASSEMBLY)
+    size_t redzoneLimit = static_cast<uint64_t>(std::numeric_limits<uint32_t>::max()) + Wasm::Memory::fastMappedRedzoneBytes();
+    ASSERT_UNUSED(redzoneLimit, maximum <= redzoneLimit);
+#endif
+    m_bounds.maximum = maximum;
 }
 
@@ -49 +61 @@
 }
 
-size_t WasmBoundsCheckValue::redzoneLimit() const
-{
-    ASSERT(m_pinnedGPR == InvalidGPRReg);
-#if ENABLE(WEBASSEMBLY)
-    return static_cast<uint64_t>(std::numeric_limits<uint32_t>::max()) + Wasm::Memory::fastMappedRedzoneBytes();
-#else
-    RELEASE_ASSERT_NOT_REACHED();
-#endif
-}
-
 void WasmBoundsCheckValue::dumpMeta(CommaPrinter& comma, PrintStream& out) const
 {
-    if (m_pinnedGPR == InvalidGPRReg)
-        out.print(comma, "redzoneLimit = ", redzoneLimit(), ", offset = ", m_offset);
-    else
-        out.print(comma, "sizeRegister = ", m_pinnedGPR, ", offset = ", m_offset);
+    switch (m_boundsType) {
+    case Type::Pinned:
+        out.print(comma, "offset = ", m_offset, ", pinned = ", m_bounds.pinned);
+        break;
+    case Type::Maximum:
+        out.print(comma, "offset = ", m_offset, ", maximum = ", m_bounds.maximum);
+        break;
+    }
 }
 

trunk/Source/JavaScriptCore/b3/B3WasmBoundsCheckValue.h

@@ -30 +30 @@
 #include "B3Value.h"
 #include "CCallHelpers.h"
-#include "WasmPageCount.h"
 
 namespace JSC { namespace B3 {
@@ -48 +47 @@
     ~WasmBoundsCheckValue();
 
-#if ENABLE(WEBASSEMBLY)
-    typedef Wasm::PageCount PageCount;
-#else
-    typedef char PageCount;
-#endif
+    enum class Type {
+        Pinned,
+        Maximum,
+    };
 
-    GPRReg pinnedGPR() const { return m_pinnedGPR; }
+    union Bounds {
+        GPRReg pinned;
+        size_t maximum;
+    };
+
     unsigned offset() const { return m_offset; }
-    size_t redzoneLimit() const;
-    PageCount maximum() const { return m_maximum; }
+    Type boundsType() const { return m_boundsType; }
+    Bounds bounds() const { return m_bounds; }
 
 protected:
@@ -67 +69 @@
     friend class Procedure;
 
-    JS_EXPORT_PRIVATE WasmBoundsCheckValue(Origin, Value* ptr, GPRReg pinnedGPR, unsigned offset, PageCount maximum);
+    JS_EXPORT_PRIVATE WasmBoundsCheckValue(Origin, Value* ptr, unsigned offset, GPRReg pinnedGPR);
+    JS_EXPORT_PRIVATE WasmBoundsCheckValue(Origin, Value* ptr, unsigned offset, size_t maximum);
 
-    GPRReg m_pinnedGPR;
     unsigned m_offset;
-    PageCount m_maximum;
+    Type m_boundsType;
+    Bounds m_bounds;
 };
 

trunk/Source/JavaScriptCore/b3/air/AirCode.h

@@ -56 +56 @@
 class Disassembler;
 
-typedef void WasmBoundsCheckGeneratorFunction(CCallHelpers&, GPRReg, unsigned);
+typedef void WasmBoundsCheckGeneratorFunction(CCallHelpers&, GPRReg);
 typedef SharedTask<WasmBoundsCheckGeneratorFunction> WasmBoundsCheckGenerator;
 

trunk/Source/JavaScriptCore/b3/air/AirCustom.h

@@ -315 +315 @@
             [outOfBounds, value] (CCallHelpers& jit, Air::GenerationContext& context) {
                 outOfBounds.link(&jit);
-                context.code->wasmBoundsCheckGenerator()->run(jit, value->pinnedGPR(), value->offset());
+                switch (value->boundsType()) {
+                case WasmBoundsCheckValue::Type::Pinned:
+                    context.code->wasmBoundsCheckGenerator()->run(jit, value->bounds().pinned);
+                    break;
+
+                case WasmBoundsCheckValue::Type::Maximum:
+                    context.code->wasmBoundsCheckGenerator()->run(jit, InvalidGPRReg);
+                    break;
+                }
             }));
 

trunk/Source/JavaScriptCore/b3/testb3.cpp

@@ -15171 +15171 @@
     proc.pinRegister(pinned);
 
-    proc.setWasmBoundsCheckGenerator([=] (CCallHelpers& jit, GPRReg pinnedGPR, unsigned actualOffset) {
+    proc.setWasmBoundsCheckGenerator([=] (CCallHelpers& jit, GPRReg pinnedGPR) {
         CHECK_EQ(pinnedGPR, pinned);
-        CHECK_EQ(actualOffset, offset);
 
         // This should always work because a function this simple should never have callee
@@ -15186 +15185 @@
     if (pointerType() != Int32)
         left = root->appendNew<Value>(proc, Trunc, Origin(), left);
-    Wasm::PageCount maximum;
-    root->appendNew<WasmBoundsCheckValue>(proc, Origin(), left, pinned, offset, maximum);
+    root->appendNew<WasmBoundsCheckValue>(proc, Origin(), left, pinned, offset);
     Value* result = root->appendNew<Const32Value>(proc, Origin(), 0x42);
     root->appendNewControlValue(proc, Return, Origin(), result);

trunk/Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp

@@ -60 +60 @@
 #include "WasmOpcodeOrigin.h"
 #include "WasmThunks.h"
+#include <limits>
 #include <wtf/Optional.h>
 #include <wtf/StdLibExtras.h>
@@ -353 +354 @@
 
     if (info.memory) {
-        m_proc.setWasmBoundsCheckGenerator([=] (CCallHelpers& jit, GPRReg pinnedGPR, unsigned) {
+        m_proc.setWasmBoundsCheckGenerator([=] (CCallHelpers& jit, GPRReg pinnedGPR) {
             AllowMacroScratchRegisterUsage allowScratch(jit);
             switch (m_mode) {
@@ -550 +551 @@
 {
     ASSERT(m_memoryBaseGPR);
+
     switch (m_mode) {
     case MemoryMode::BoundsChecking:
@@ -555 +557 @@
         ASSERT(m_memorySizeGPR);
         ASSERT(sizeOfOperation + offset > offset);
-        m_currentBlock->appendNew<WasmBoundsCheckValue>(m_proc, origin(), pointer, m_memorySizeGPR, sizeOfOperation + offset - 1, m_info.memory.maximum());
+        m_currentBlock->appendNew<WasmBoundsCheckValue>(m_proc, origin(), pointer, sizeOfOperation + offset - 1, m_memorySizeGPR);
         break;
+
     case MemoryMode::Signaling:
-        // We've virtually mapped 4GiB+redzone for this memory. Only the user-allocated pages are addressable, contiguously in range [0, current], and everything above is mapped PROT_NONE. We don't need to perform any explicit bounds check in the 4GiB range because WebAssembly register memory accesses are 32-bit. However WebAssembly register+immediate accesses perform the addition in 64-bit which can push an access above the 32-bit limit. The redzone will catch most small immediates, and we'll explicitly bounds check any register + large immediate access.
-        if (offset >= Memory::fastMappedRedzoneBytes())
-            m_currentBlock->appendNew<WasmBoundsCheckValue>(m_proc, origin(), pointer, InvalidGPRReg, sizeOfOperation + offset - 1, m_info.memory.maximum());
+        // We've virtually mapped 4GiB+redzone for this memory. Only the user-allocated pages are addressable, contiguously in range [0, current],
+        // and everything above is mapped PROT_NONE. We don't need to perform any explicit bounds check in the 4GiB range because WebAssembly register
+        // memory accesses are 32-bit. However WebAssembly register + offset accesses perform the addition in 64-bit which can push an access above
+        // the 32-bit limit (the offset is unsigned 32-bit). The redzone will catch most small offsets, and we'll explicitly bounds check any
+        // register + large offset access. We don't think this will be generated frequently.
+        //
+        // We could check that register + large offset doesn't exceed 4GiB+redzone since that's technically the limit we need to avoid overflowing the
+        // PROT_NONE region, but it's better if we use a smaller immediate because it can codegens better. We know that anything equal to or greater
+        // than the declared 'maximum' will trap, so we can compare against that number. If there was no declared 'maximum' then we still know that
+        // any access equal to or greater than 4GiB will trap, no need to add the redzone.
+        if (offset >= Memory::fastMappedRedzoneBytes()) {
+            size_t maximum = m_info.memory.maximum() ? m_info.memory.maximum().bytes() : std::numeric_limits<uint32_t>::max();
+            m_currentBlock->appendNew<WasmBoundsCheckValue>(m_proc, origin(), pointer, sizeOfOperation + offset - 1, maximum);
+        }
         break;
+
     case MemoryMode::NumberOfMemoryModes:
         RELEASE_ASSERT_NOT_REACHED();
@@ -1301 +1316 @@
     compilationContext.jsEntrypointToWasmEntrypointCall = jit.call();
 
-    if (!!info.memory) {
-        // Resetting the register prevents the GC from mistakenly thinking that the context is still live.
-        GPRReg baseMemory = pinnedRegs.baseMemoryPointer;
-        jit.move(CCallHelpers::TrustedImm32(0), baseMemory);
-    }
-
     for (const RegisterAtOffset& regAtOffset : registersToSpill) {
         GPRReg reg = regAtOffset.reg().gpr();

trunk/Source/JavaScriptCore/wasm/WasmMemory.cpp

@@ -49 +49 @@
 
 NEVER_INLINE NO_RETURN_DUE_TO_CRASH void webAssemblyCouldntGetFastMemory() { CRASH(); }
-NEVER_INLINE NO_RETURN_DUE_TO_CRASH void webAssemblyCouldntUnmapMemoryBytes() { CRASH(); }
+NEVER_INLINE NO_RETURN_DUE_TO_CRASH void webAssemblyCouldntUnmapMemory() { CRASH(); }
 NEVER_INLINE NO_RETURN_DUE_TO_CRASH void webAssemblyCouldntUnprotectMemory() { CRASH(); }
 
@@ -67 +67 @@
 {
     if (UNLIKELY(munmap(memory, size)))
-        webAssemblyCouldntUnmapMemoryBytes();
+        webAssemblyCouldntUnmapMemory();
 }
 
@@ -150 +150 @@
         if (memory)
             dataLogLnIf(verbose, "tryGetFastMemory re-using ", RawPointer(memory));
-        else {
-            // No memory was available in the cache. Maybe waiting on GC will find a free one.
+        else if (currentlyAllocatedFastMemories.load(std::memory_order_acquire) >= 1) {
+            // No memory was available in the cache, but we know there's at least one currently live. Maybe GC will find a free one.
             // FIXME collectSync(Full) and custom eager destruction of wasm memories could be better. For now use collectAllGarbage. Also, nothing tells us the current VM is holding onto fast memories. https://bugs.webkit.org/show_bug.cgi?id=170748
             dataLogLnIf(verbose, "tryGetFastMemory waiting on GC and retrying");
@@ -299 +299 @@
     // Races cannot occur in this function: it is only called at program initialization, before WebAssembly can be invoked.
 
-    const auto startTime = MonotonicTime::now();
+    MonotonicTime startTime;
+    if (verbose)
+        startTime = MonotonicTime::now();
+
     const size_t desiredFastMemories = std::min<size_t>(Options::webAssemblyFastMemoryPreallocateCount(), fastMemoryCacheHardLimit);
 
@@ -306 +309 @@
         if (void *memory = mmapBytes(Memory::fastMappedBytes() * numContiguous)) {
             for (size_t subMemory = 0; subMemory < numContiguous; ++subMemory) {
-                void* startAddress = reinterpret_cast<char*>(memory) + Memory::fastMappedBytes() * subMemory + subMemory;
+                void* startAddress = reinterpret_cast<char*>(memory) + Memory::fastMappedBytes() * subMemory;
                 bool inserted = false;
                 for (size_t cacheEntry = 0; cacheEntry < fastMemoryCacheHardLimit; ++cacheEntry) {
@@ -338 +341 @@
     observedMaximumFastMemory.store(fastMemoryPreallocateCount, std::memory_order_relaxed);
 
-    const auto endTime = MonotonicTime::now();
-
-    for (size_t cacheEntry = 0; cacheEntry < fastMemoryPreallocateCount; ++cacheEntry) {
-        void* startAddress = fastMemoryCache[cacheEntry].load(std::memory_order_relaxed);
-        ASSERT(startAddress);
-        dataLogLnIf(verbose, "Pre-allocation of WebAssembly fast memory at ", RawPointer(startAddress));
-    }
-    dataLogLnIf(verbose, "Pre-allocated ", fastMemoryPreallocateCount, " WebAssembly fast memories in ", fastMemoryPreallocateCount == 0 ? 0 : fragments, fragments == 1 ? " fragment, took " : " fragments, took ", endTime - startTime);
+    if (verbose) {
+        MonotonicTime endTime = MonotonicTime::now();
+
+        for (size_t cacheEntry = 0; cacheEntry < fastMemoryPreallocateCount; ++cacheEntry) {
+            void* startAddress = fastMemoryCache[cacheEntry].load(std::memory_order_relaxed);
+            ASSERT(startAddress);
+            dataLogLn("Pre-allocation of WebAssembly fast memory at ", RawPointer(startAddress));
+        }
+
+        dataLogLn("Pre-allocated ", fastMemoryPreallocateCount, " WebAssembly fast memories in ", fastMemoryPreallocateCount == 0 ? 0 : fragments, fragments == 1 ? " fragment, took " : " fragments, took ", endTime - startTime);
+    }
 }