Title: [214602] trunk/Source/_javascript_Core
Revision
214602
Author
sbar...@apple.com
Date
2017-03-30 00:29:02 -0700 (Thu, 30 Mar 2017)

Log Message

WebAssembly: pass Wasm::Context* to vmEntryToWasm when not using fast TLS
https://bugs.webkit.org/show_bug.cgi?id=170182

Reviewed by Mark Lam.

This is one more step in the direction of PIC-ified Wasm.
I'm removing assumptions that a wasm callee is a cell. We used to use
the callee to get the WasmContext off the callee's VM. Instead,
this patch makes it so that we pass in the context as a parameter
to the JS entrypoint.

* heap/MarkedBlock.h:
(JSC::MarkedBlock::offsetOfVM): Deleted.
* jit/AssemblyHelpers.cpp:
(JSC::AssemblyHelpers::loadWasmContext):
(JSC::AssemblyHelpers::storeWasmContext):
(JSC::AssemblyHelpers::loadWasmContextNeedsMacroScratchRegister):
(JSC::AssemblyHelpers::storeWasmContextNeedsMacroScratchRegister):
* jsc.cpp:
(functionTestWasmModuleFunctions):
* runtime/VM.h:
(JSC::VM::wasmContextOffset): Deleted.
* wasm/WasmB3IRGenerator.cpp:
(JSC::Wasm::B3IRGenerator::materializeWasmContext):
(JSC::Wasm::B3IRGenerator::restoreWasmContext):
(JSC::Wasm::B3IRGenerator::B3IRGenerator):
(JSC::Wasm::createJSToWasmWrapper):
* wasm/WasmContext.cpp:
(JSC::Wasm::loadContext):
(JSC::Wasm::storeContext):
(JSC::loadWasmContext): Deleted.
(JSC::storeWasmContext): Deleted.
* wasm/WasmContext.h:
(JSC::Wasm::useFastTLS):
(JSC::Wasm::useFastTLSForContext):
* wasm/WasmMemoryInformation.cpp:
(JSC::Wasm::PinnedRegisterInfo::get):
* wasm/WasmMemoryInformation.h:
(JSC::Wasm::useFastTLS): Deleted.
(JSC::Wasm::useFastTLSForWasmContext): Deleted.
* wasm/js/WebAssemblyFunction.cpp:
(JSC::callWebAssemblyFunction):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (214601 => 214602)


--- trunk/Source/_javascript_Core/ChangeLog	2017-03-30 07:16:27 UTC (rev 214601)
+++ trunk/Source/_javascript_Core/ChangeLog	2017-03-30 07:29:02 UTC (rev 214602)
@@ -1,3 +1,48 @@
+2017-03-30  Saam Barati  <sbar...@apple.com>
+
+        WebAssembly: pass Wasm::Context* to vmEntryToWasm when not using fast TLS
+        https://bugs.webkit.org/show_bug.cgi?id=170182
+
+        Reviewed by Mark Lam.
+
+        This is one more step in the direction of PIC-ified Wasm.
+        I'm removing assumptions that a wasm callee is a cell. We used to use
+        the callee to get the WasmContext off the callee's VM. Instead,
+        this patch makes it so that we pass in the context as a parameter
+        to the JS entrypoint.
+
+        * heap/MarkedBlock.h:
+        (JSC::MarkedBlock::offsetOfVM): Deleted.
+        * jit/AssemblyHelpers.cpp:
+        (JSC::AssemblyHelpers::loadWasmContext):
+        (JSC::AssemblyHelpers::storeWasmContext):
+        (JSC::AssemblyHelpers::loadWasmContextNeedsMacroScratchRegister):
+        (JSC::AssemblyHelpers::storeWasmContextNeedsMacroScratchRegister):
+        * jsc.cpp:
+        (functionTestWasmModuleFunctions):
+        * runtime/VM.h:
+        (JSC::VM::wasmContextOffset): Deleted.
+        * wasm/WasmB3IRGenerator.cpp:
+        (JSC::Wasm::B3IRGenerator::materializeWasmContext):
+        (JSC::Wasm::B3IRGenerator::restoreWasmContext):
+        (JSC::Wasm::B3IRGenerator::B3IRGenerator):
+        (JSC::Wasm::createJSToWasmWrapper):
+        * wasm/WasmContext.cpp:
+        (JSC::Wasm::loadContext):
+        (JSC::Wasm::storeContext):
+        (JSC::loadWasmContext): Deleted.
+        (JSC::storeWasmContext): Deleted.
+        * wasm/WasmContext.h:
+        (JSC::Wasm::useFastTLS):
+        (JSC::Wasm::useFastTLSForContext):
+        * wasm/WasmMemoryInformation.cpp:
+        (JSC::Wasm::PinnedRegisterInfo::get):
+        * wasm/WasmMemoryInformation.h:
+        (JSC::Wasm::useFastTLS): Deleted.
+        (JSC::Wasm::useFastTLSForWasmContext): Deleted.
+        * wasm/js/WebAssemblyFunction.cpp:
+        (JSC::callWebAssemblyFunction):
+
 2017-03-30  JF Bastien  <jfbast...@apple.com>
 
         WebAssembly: fix misc JS API implementation inconsistencies

Modified: trunk/Source/_javascript_Core/heap/MarkedBlock.h (214601 => 214602)


--- trunk/Source/_javascript_Core/heap/MarkedBlock.h	2017-03-30 07:16:27 UTC (rev 214601)
+++ trunk/Source/_javascript_Core/heap/MarkedBlock.h	2017-03-30 07:29:02 UTC (rev 214602)
@@ -299,11 +299,6 @@
     bool isMarkedRaw(const void* p);
     HeapVersion markingVersion() const { return m_markingVersion; }
 
-    static ptrdiff_t offsetOfVM()
-    {
-        return OBJECT_OFFSETOF(MarkedBlock, m_vm);
-    }
-    
 private:
     static const size_t atomAlignmentMask = atomSize - 1;
 

Modified: trunk/Source/_javascript_Core/jit/AssemblyHelpers.cpp (214601 => 214602)


--- trunk/Source/_javascript_Core/jit/AssemblyHelpers.cpp	2017-03-30 07:16:27 UTC (rev 214601)
+++ trunk/Source/_javascript_Core/jit/AssemblyHelpers.cpp	2017-03-30 07:29:02 UTC (rev 214602)
@@ -33,6 +33,7 @@
 #include "LinkBuffer.h"
 
 #if ENABLE(WEBASSEMBLY)
+#include "WasmContext.h"
 #include "WasmMemoryInformation.h"
 #endif
 
@@ -777,7 +778,7 @@
 void AssemblyHelpers::loadWasmContext(GPRReg dst)
 {
 #if ENABLE(FAST_TLS_JIT)
-    if (Wasm::useFastTLSForWasmContext()) {
+    if (Wasm::useFastTLSForContext()) {
         loadFromTLSPtr(fastTLSOffsetForKey(WTF_WASM_CONTEXT_KEY), dst);
         return;
     }
@@ -788,7 +789,7 @@
 void AssemblyHelpers::storeWasmContext(GPRReg src)
 {
 #if ENABLE(FAST_TLS_JIT)
-    if (Wasm::useFastTLSForWasmContext()) {
+    if (Wasm::useFastTLSForContext()) {
         storeToTLSPtr(src, fastTLSOffsetForKey(WTF_WASM_CONTEXT_KEY));
         return;
     }
@@ -799,7 +800,7 @@
 bool AssemblyHelpers::loadWasmContextNeedsMacroScratchRegister()
 {
 #if ENABLE(FAST_TLS_JIT)
-    if (Wasm::useFastTLSForWasmContext())
+    if (Wasm::useFastTLSForContext())
         return loadFromTLSPtrNeedsMacroScratchRegister();
 #endif
     return false;
@@ -808,7 +809,7 @@
 bool AssemblyHelpers::storeWasmContextNeedsMacroScratchRegister()
 {
 #if ENABLE(FAST_TLS_JIT)
-    if (Wasm::useFastTLSForWasmContext())
+    if (Wasm::useFastTLSForContext())
         return storeToTLSPtrNeedsMacroScratchRegister();
 #endif
     return false;

Modified: trunk/Source/_javascript_Core/jsc.cpp (214601 => 214602)


--- trunk/Source/_javascript_Core/jsc.cpp	2017-03-30 07:16:27 UTC (rev 214601)
+++ trunk/Source/_javascript_Core/jsc.cpp	2017-03-30 07:29:02 UTC (rev 214602)
@@ -75,6 +75,7 @@
 #include "SuperSampler.h"
 #include "TestRunnerUtils.h"
 #include "TypeProfilerLog.h"
+#include "WasmContext.h"
 #include "WasmFaultSignalHandler.h"
 #include "WasmMemory.h"
 #include "WasmPlanInlines.h"
@@ -3224,6 +3225,13 @@
             JSArray* arguments = jsCast<JSArray*>(test->getIndexQuickly(1));
 
             MarkedArgumentBuffer boxedArgs;
+            if (!Wasm::useFastTLSForContext()) {
+                // When not using fast TLS, the code we emit expects Wasm::Context*
+                // as the first argument. These tests that this API supports don't ever
+                // use a Context. So this is just a hack to get it to not barf.
+                // We really need to remove this API.
+                boxedArgs.append(jsNumber(0xbadbeef));
+            }
             for (unsigned argIndex = 0; argIndex < arguments->length(); ++argIndex)
                 boxedArgs.append(box(exec, vm, arguments->getIndexQuickly(argIndex)));
 

Modified: trunk/Source/_javascript_Core/runtime/VM.h (214601 => 214602)


--- trunk/Source/_javascript_Core/runtime/VM.h	2017-03-30 07:16:27 UTC (rev 214601)
+++ trunk/Source/_javascript_Core/runtime/VM.h	2017-03-30 07:29:02 UTC (rev 214602)
@@ -483,11 +483,6 @@
         return OBJECT_OFFSETOF(VM, targetMachinePCForThrow);
     }
 
-    static ptrdiff_t wasmContextOffset()
-    {
-        return OBJECT_OFFSETOF(VM, wasmContext);
-    }
-
     void restorePreviousException(Exception* exception) { setException(exception); }
 
     void clearLastException() { m_lastException = nullptr; }

Modified: trunk/Source/_javascript_Core/wasm/WasmB3IRGenerator.cpp (214601 => 214602)


--- trunk/Source/_javascript_Core/wasm/WasmB3IRGenerator.cpp	2017-03-30 07:16:27 UTC (rev 214601)
+++ trunk/Source/_javascript_Core/wasm/WasmB3IRGenerator.cpp	2017-03-30 07:29:02 UTC (rev 214602)
@@ -52,6 +52,7 @@
 #include "JSWebAssemblyRuntimeError.h"
 #include "VirtualRegister.h"
 #include "WasmCallingConvention.h"
+#include "WasmContext.h"
 #include "WasmExceptionType.h"
 #include "WasmFunctionParser.h"
 #include "WasmMemory.h"
@@ -249,7 +250,7 @@
 
 Value* B3IRGenerator::materializeWasmContext(Procedure& proc, BasicBlock* block)
 {
-    if (useFastTLSForWasmContext()) {
+    if (useFastTLSForContext()) {
         PatchpointValue* patchpoint = block->appendNew<PatchpointValue>(proc, pointerType(), Origin());
         if (CCallHelpers::loadWasmContextNeedsMacroScratchRegister())
             patchpoint->clobber(RegisterSet::macroScratchRegisters());
@@ -273,7 +274,7 @@
 
 void B3IRGenerator::restoreWasmContext(Procedure& proc, BasicBlock* block, Value* arg)
 {
-    if (useFastTLSForWasmContext()) {
+    if (useFastTLSForContext()) {
         PatchpointValue* patchpoint = block->appendNew<PatchpointValue>(proc, B3::Void, Origin());
         if (CCallHelpers::storeWasmContextNeedsMacroScratchRegister())
             patchpoint->clobber(RegisterSet::macroScratchRegisters());
@@ -316,7 +317,7 @@
     m_memoryBaseGPR = pinnedRegs.baseMemoryPointer;
     m_wasmContextGPR = pinnedRegs.wasmContextPointer;
     m_proc.pinRegister(m_memoryBaseGPR);
-    if (!useFastTLSForWasmContext())
+    if (!useFastTLSForContext())
         m_proc.pinRegister(m_wasmContextGPR);
     ASSERT(!pinnedRegs.sizeRegisters[0].sizeOffset);
     m_memorySizeGPR = pinnedRegs.sizeRegisters[0].sizeRegister;
@@ -1147,6 +1148,8 @@
         jit.storePtr(reg, CCallHelpers::Address(GPRInfo::callFrameRegister, offset));
     }
 
+    GPRReg wasmContextGPR = pinnedRegs.wasmContextPointer;
+
     {
         CCallHelpers::Address calleeFrame = CCallHelpers::Address(MacroAssembler::stackPointerRegister, -static_cast<ptrdiff_t>(sizeof(CallerFrameAndPC)));
         numGPRs = 0;
@@ -1155,7 +1158,15 @@
         // we can use this as a scratch for now since we saved it above.
         GPRReg scratchReg = pinnedRegs.baseMemoryPointer;
 
-        ptrdiff_t jsOffset = CallFrameSlot::thisArgument * sizeof(void*);
+        ptrdiff_t jsOffset = CallFrameSlot::thisArgument * sizeof(EncodedJSValue);
+
+        // vmEntryToWasm passes Wasm::Context* as the first JS argument when we're
+        // not using fast TLS to hold the Wasm::Context*.
+        if (!useFastTLSForContext()) {
+            jit.loadPtr(CCallHelpers::Address(GPRInfo::callFrameRegister, jsOffset), wasmContextGPR);
+            jsOffset += sizeof(EncodedJSValue);
+        }
+
         ptrdiff_t wasmOffset = CallFrame::headerSizeInRegisters * sizeof(void*);
         for (unsigned i = 0; i < signature->argumentCount(); i++) {
             switch (signature->argument(i)) {
@@ -1201,25 +1212,15 @@
                 RELEASE_ASSERT_NOT_REACHED();
             }
 
-            jsOffset += sizeof(void*);
+            jsOffset += sizeof(EncodedJSValue);
         }
     }
 
-    // FIXME: JStoWasm wrapper should take JSWebAssemblyInstance pointer as an argument directly.
-    // https://bugs.webkit.org/show_bug.cgi?id=170182
-    GPRReg wasmContext = pinnedRegs.wasmContextPointer;
-    if (!useFastTLSForWasmContext()) {
-        jit.loadPtr(CCallHelpers::Address(GPRInfo::callFrameRegister, CallFrameSlot::callee * static_cast<int>(sizeof(Register))), wasmContext);
-        jit.andPtr(CCallHelpers::TrustedImmPtr(MarkedBlock::blockMask), wasmContext);
-        jit.loadPtr(CCallHelpers::Address(wasmContext, MarkedBlock::offsetOfVM()), wasmContext);
-        jit.loadPtr(CCallHelpers::Address(wasmContext, VM::wasmContextOffset()), wasmContext);
-    }
-
     if (!!info.memory) {
         GPRReg baseMemory = pinnedRegs.baseMemoryPointer;
 
-        if (!useFastTLSForWasmContext())
-            jit.loadPtr(CCallHelpers::Address(wasmContext, JSWebAssemblyInstance::offsetOfMemory()), baseMemory);
+        if (!useFastTLSForContext())
+            jit.loadPtr(CCallHelpers::Address(wasmContextGPR, JSWebAssemblyInstance::offsetOfMemory()), baseMemory);
         else {
             jit.loadWasmContext(baseMemory);
             jit.loadPtr(CCallHelpers::Address(baseMemory, JSWebAssemblyInstance::offsetOfMemory()), baseMemory);

Modified: trunk/Source/_javascript_Core/wasm/WasmContext.cpp (214601 => 214602)


--- trunk/Source/_javascript_Core/wasm/WasmContext.cpp	2017-03-30 07:16:27 UTC (rev 214601)
+++ trunk/Source/_javascript_Core/wasm/WasmContext.cpp	2017-03-30 07:29:02 UTC (rev 214602)
@@ -32,28 +32,28 @@
 #include <mutex>
 #include <wtf/FastTLS.h>
 
-namespace JSC {
+namespace JSC { namespace Wasm {
 
-WasmContext* loadWasmContext(VM& vm)
+Context* loadContext(VM& vm)
 {
 #if ENABLE(FAST_TLS_JIT)
-    if (Options::useWebAssemblyFastTLS())
-        return bitwise_cast<WasmContext*>(_pthread_getspecific_direct(WTF_WASM_CONTEXT_KEY));
+    if (useFastTLSForContext())
+        return bitwise_cast<Context*>(_pthread_getspecific_direct(WTF_WASM_CONTEXT_KEY));
 #endif
     // FIXME: Save this state elsewhere to allow PIC. https://bugs.webkit.org/show_bug.cgi?id=169773
     return vm.wasmContext;
 }
 
-void storeWasmContext(VM& vm, WasmContext* instance)
+void storeContext(VM& vm, Context* context)
 {
 #if ENABLE(FAST_TLS_JIT)
-    if (Options::useWebAssemblyFastTLS())
-        _pthread_setspecific_direct(WTF_WASM_CONTEXT_KEY, bitwise_cast<void*>(instance));
+    if (useFastTLSForContext())
+        _pthread_setspecific_direct(WTF_WASM_CONTEXT_KEY, bitwise_cast<void*>(context));
 #endif
     // FIXME: Save this state elsewhere to allow PIC. https://bugs.webkit.org/show_bug.cgi?id=169773
-    vm.wasmContext = instance;
+    vm.wasmContext = context;
 }
 
-} // namespace JSC
+} } // namespace JSC::Wasm
 
 #endif // ENABLE(WEBASSEMBLY)

Modified: trunk/Source/_javascript_Core/wasm/WasmContext.h (214601 => 214602)


--- trunk/Source/_javascript_Core/wasm/WasmContext.h	2017-03-30 07:16:27 UTC (rev 214601)
+++ trunk/Source/_javascript_Core/wasm/WasmContext.h	2017-03-30 07:29:02 UTC (rev 214602)
@@ -27,16 +27,36 @@
 
 #if ENABLE(WEBASSEMBLY)
 
+#include "Options.h"
+
 namespace JSC {
 
 class JSWebAssemblyInstance;
 class VM;
 
-using WasmContext = JSWebAssemblyInstance;
+namespace Wasm {
 
-WasmContext* loadWasmContext(VM&);
-void storeWasmContext(VM&, WasmContext*);
+// FIXME: We might want this to be something else at some point:
+// https://bugs.webkit.org/show_bug.cgi?id=170260
+using Context = JSWebAssemblyInstance;
 
-} // namespace JSC
+inline bool useFastTLS()
+{
+#if ENABLE(FAST_TLS_JIT)
+    return Options::useWebAssemblyFastTLS();
+#else
+    return false;
+#endif
+}
 
+inline bool useFastTLSForContext()
+{
+    return useFastTLS();
+}
+
+Context* loadContext(VM&);
+void storeContext(VM&, Context*);
+
+} } // namespace JSC::Wasm
+
 #endif // ENABLE(WEBASSEMBLY)

Modified: trunk/Source/_javascript_Core/wasm/WasmMemoryInformation.cpp (214601 => 214602)


--- trunk/Source/_javascript_Core/wasm/WasmMemoryInformation.cpp	2017-03-30 07:16:27 UTC (rev 214601)
+++ trunk/Source/_javascript_Core/wasm/WasmMemoryInformation.cpp	2017-03-30 07:29:02 UTC (rev 214602)
@@ -29,6 +29,7 @@
 #if ENABLE(WEBASSEMBLY)
 
 #include "WasmCallingConvention.h"
+#include "WasmContext.h"
 #include "WasmMemory.h"
 #include <wtf/NeverDestroyed.h>
 
@@ -64,12 +65,12 @@
         //        see: https://bugs.webkit.org/show_bug.cgi?id=162952
         Vector<unsigned> pinnedSizes = { 0 };
         unsigned numberOfPinnedRegisters = pinnedSizes.size() + 1;
-        if (!useFastTLSForWasmContext())
+        if (!useFastTLSForContext())
             ++numberOfPinnedRegisters;
         Vector<GPRReg> pinnedRegs = getPinnedRegisters(numberOfPinnedRegisters);
 
         baseMemoryPointer = pinnedRegs.takeLast();
-        if (!useFastTLSForWasmContext())
+        if (!useFastTLSForContext())
             wasmContextPointer = pinnedRegs.takeLast();
 
         ASSERT(pinnedSizes.size() == pinnedRegs.size());

Modified: trunk/Source/_javascript_Core/wasm/WasmMemoryInformation.h (214601 => 214602)


--- trunk/Source/_javascript_Core/wasm/WasmMemoryInformation.h	2017-03-30 07:16:27 UTC (rev 214601)
+++ trunk/Source/_javascript_Core/wasm/WasmMemoryInformation.h	2017-03-30 07:29:02 UTC (rev 214602)
@@ -81,20 +81,6 @@
     bool m_isImport { false };
 };
 
-inline bool useFastTLS()
-{
-#if ENABLE(FAST_TLS_JIT)
-    return Options::useWebAssemblyFastTLS();
-#else
-    return false;
-#endif
-}
-
-inline bool useFastTLSForWasmContext()
-{
-    return useFastTLS();
-}
-
 } } // namespace JSC::Wasm
 
 #endif // ENABLE(WASM)

Modified: trunk/Source/_javascript_Core/wasm/js/WebAssemblyFunction.cpp (214601 => 214602)


--- trunk/Source/_javascript_Core/wasm/js/WebAssemblyFunction.cpp	2017-03-30 07:16:27 UTC (rev 214601)
+++ trunk/Source/_javascript_Core/wasm/js/WebAssemblyFunction.cpp	2017-03-30 07:29:02 UTC (rev 214602)
@@ -82,6 +82,12 @@
     TraceScope traceScope(WebAssemblyExecuteStart, WebAssemblyExecuteEnd);
 
     Vector<JSValue> boxedArgs;
+    Wasm::Context* wasmContext = wasmFunction->instance();
+    // When we don't use fast TLS to store the context, the js
+    // entry wrapper expects the WasmContext* as the first argument.
+    if (!Wasm::useFastTLSForContext())
+        boxedArgs.append(wasmContext);
+
     for (unsigned argIndex = 0; argIndex < signature->argumentCount(); ++argIndex) {
         JSValue arg = exec->argument(argIndex);
         switch (signature->argument(argIndex)) {
@@ -122,16 +128,16 @@
     protoCallFrame.init(nullptr, wasmFunction, firstArgument, argCount, remainingArgs);
 
     // FIXME Do away with this entire function, and only use the entrypoint generated by B3. https://bugs.webkit.org/show_bug.cgi?id=166486
-    // FIXME: We would like to make loadWasmContext and storeWasmContext no-op if we use pinned registers.
-    // However, we use VM.wasmContext field to pass instance to wasm function's JS glue code.
-    // We should pass JSWebAssemblyInstance directly to vmEntryToWasm.
-    // https://bugs.webkit.org/show_bug.cgi?id=170182
-    JSWebAssemblyInstance* prevJSWebAssemblyInstance = loadWasmContext(vm);
-    storeWasmContext(vm, wasmFunction->instance());
+    Wasm::Context* prevWasmContext = Wasm::loadContext(vm);
+    Wasm::storeContext(vm, wasmContext);
     ASSERT(wasmFunction->instance());
-    ASSERT(wasmFunction->instance() == loadWasmContext(vm));
+    ASSERT(wasmFunction->instance() == Wasm::loadContext(vm));
     EncodedJSValue rawResult = vmEntryToWasm(wasmFunction->jsEntrypoint(), &vm, &protoCallFrame);
-    storeWasmContext(vm, prevJSWebAssemblyInstance);
+    // We need to make sure this is in a register or on the stack since it's stored in Vector<JSValue>.
+    // This probably isn't strictly necessary, since the WebAssemblyFunction* should keep the instance
+    // alive. But it's good hygiene.
+    wasmContext->use(); 
+    Wasm::storeContext(vm, prevWasmContext);
     RETURN_IF_EXCEPTION(scope, { });
 
     switch (signature->returnType()) {
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to