WebAssembly: JSC::link* shouldn't need a CodeBlock
authorjfbastien@apple.com <jfbastien@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 9 Dec 2016 06:52:51 +0000 (06:52 +0000)
committerjfbastien@apple.com <jfbastien@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 9 Dec 2016 06:52:51 +0000 (06:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=165591

Reviewed by Keith Miller.

JSTests:

test that wasm -> JS works, including the IC

* wasm/js-api/test_Instance.js:
(const.wasmModuleWhichImportJS):
(MonomorphicImport):
(Polyphic2Import):
(Polyphic3Import):
(VirtualImport):

Source/JavaScriptCore:

Allow linking without a CodeBlock, which WebAssembly's wasm -> JS stubs does. This needs to work for polymorphic and virtual calls. This patch adds corresponding tests for this.

* assembler/LinkBuffer.cpp:
(JSC::shouldDumpDisassemblyFor): don't look at the tier option if there isn't a CodeBlock, only look at the global one. This is a WebAssembly function, so the tier information is irrelevant.
* jit/Repatch.cpp:
(JSC::isWebAssemblyToJSCallee): this is used in the link* functions below
(JSC::linkFor):
(JSC::linkVirtualFor):
(JSC::linkPolymorphicCall):
* runtime/Options.h: add an option to change the maximum number of polymorphic calls in stubs from wasm to JS, which will come in handy when we try to tune performance or try merging some of the WebAssembly stubs
* wasm/WasmBinding.cpp:
(JSC::Wasm::importStubGenerator): remove the breakpoint since the code now works
* wasm/js/WebAssemblyToJSCallee.h:

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

JSTests/ChangeLog
JSTests/wasm/js-api/test_Instance.js
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/assembler/LinkBuffer.cpp
Source/JavaScriptCore/jit/Repatch.cpp
Source/JavaScriptCore/runtime/Options.h
Source/JavaScriptCore/wasm/WasmBinding.cpp
Source/JavaScriptCore/wasm/js/WebAssemblyToJSCallee.h

index cf603ef..2662300 100644 (file)
@@ -1,3 +1,19 @@
+2016-12-08  JF Bastien  <jfbastien@apple.com>
+
+        WebAssembly: JSC::link* shouldn't need a CodeBlock
+        https://bugs.webkit.org/show_bug.cgi?id=165591
+
+        Reviewed by Keith Miller.
+
+        test that wasm -> JS works, including the IC
+
+        * wasm/js-api/test_Instance.js:
+        (const.wasmModuleWhichImportJS):
+        (MonomorphicImport):
+        (Polyphic2Import):
+        (Polyphic3Import):
+        (VirtualImport):
+
 2016-12-08  Filip Pizlo  <fpizlo@apple.com>
 
         Green the cloop bot by raising this threshold.
index 005097b..7841d5a 100644 (file)
@@ -30,10 +30,7 @@ import Builder from '../Builder.js';
     assert.eq(result, 42);
 })();
 
-/* FIXME this currently crashes as detailed in https://bugs.webkit.org/show_bug.cgi?id=165591
-(function Import() {
-    let counter = 0;
-    const counterSetter = v => counter = v;
+const wasmModuleWhichImportJS = () => {
     const builder = (new Builder())
         .Type().End()
         .Import()
@@ -53,12 +50,89 @@ import Builder from '../Builder.js';
         .End();
     const bin = builder.WebAssembly().get();
     const module = new WebAssembly.Module(bin);
+    return module;
+};
+
+(function MonomorphicImport() {
+    let counter = 0;
+    const counterSetter = v => counter = v;
+    const module = wasmModuleWhichImportJS();
     const instance = new WebAssembly.Instance(module, { imp: { func: counterSetter } });
-    instance.exports.changeCounter(0);
-    assert.eq(counter, 42);
-    instance.exports.changeCounter(1);
-    assert.eq(counter, 43);
-    instance.exports.changeCounter(42);
-    assert.eq(counter, 84);
+    for (let i = 0; i < 4096; ++i) {
+        // Invoke this a bunch of times to make sure the IC in the wasm -> JS stub works correctly.
+        instance.exports.changeCounter(i);
+        assert.eq(counter, i + 42);
+    }
+})();
+
+(function Polyphic2Import() {
+    let counterA = 0;
+    let counterB = undefined;
+    const counterASetter = v => counterA = v;
+    const counterBSetter = v => counterB = { valueB: v };
+    const module = wasmModuleWhichImportJS();
+    const instanceA = new WebAssembly.Instance(module, { imp: { func: counterASetter } });
+    const instanceB = new WebAssembly.Instance(module, { imp: { func: counterBSetter } });
+    for (let i = 0; i < 2048; ++i) {
+        instanceA.exports.changeCounter(i);
+        assert.isA(counterA, "number");
+        assert.eq(counterA, i + 42);
+        instanceB.exports.changeCounter(i);
+        assert.isA(counterB, "object");
+        assert.eq(counterB.valueB, i + 42);
+    }
+})();
+
+(function Polyphic3Import() {
+    let counterA = 0;
+    let counterB = undefined;
+    let counterC = undefined;
+    const counterASetter = v => counterA = v;
+    const counterBSetter = v => counterB = { valueB: v };
+    const counterCSetter = v => counterC = { valueC: v };
+    const module = wasmModuleWhichImportJS();
+    const instanceA = new WebAssembly.Instance(module, { imp: { func: counterASetter } });
+    const instanceB = new WebAssembly.Instance(module, { imp: { func: counterBSetter } });
+    const instanceC = new WebAssembly.Instance(module, { imp: { func: counterCSetter } });
+    for (let i = 0; i < 2048; ++i) {
+        instanceA.exports.changeCounter(i);
+        assert.isA(counterA, "number");
+        assert.eq(counterA, i + 42);
+        instanceB.exports.changeCounter(i);
+        assert.isA(counterB, "object");
+        assert.eq(counterB.valueB, i + 42);
+        instanceC.exports.changeCounter(i);
+        assert.isA(counterC, "object");
+        assert.eq(counterC.valueC, i + 42);
+    }
+})();
+
+(function VirtualImport() {
+    const num = 10; // It's definitely going virtual at 10!
+    let counters = [0, 0, 0, 0, 0, 0, 0, 0, 0, 0];
+    const counterSetters = [
+        v => counters[0] = v,
+        v => counters[1] = v + 1,
+        v => counters[2] = v + 2,
+        v => counters[3] = v + 3,
+        v => counters[4] = v + 4,
+        v => counters[5] = v + 5,
+        v => counters[6] = v + 6,
+        v => counters[7] = v + 7,
+        v => counters[8] = v + 8,
+        v => counters[9] = v + 9,
+    ];
+    assert.eq(counters.length, num);
+    assert.eq(counterSetters.length, num);
+    const module = wasmModuleWhichImportJS();
+    let instances = [];
+    for (let i = 0; i < num; ++i)
+        instances[i] = new WebAssembly.Instance(module, { imp: { func: counterSetters[i] } });
+    for (let i = 0; i < 2048; ++i) {
+        for (let j = 0; j < num; ++j) {
+            instances[j].exports.changeCounter(i);
+            assert.isA(counters[j], "number");
+            assert.eq(counters[j], i + 42 + j);
+        }
+    }
 })();
-*/
index 9ae28fe..dce4747 100644 (file)
@@ -1,3 +1,24 @@
+2016-12-08  JF Bastien  <jfbastien@apple.com>
+
+        WebAssembly: JSC::link* shouldn't need a CodeBlock
+        https://bugs.webkit.org/show_bug.cgi?id=165591
+
+        Reviewed by Keith Miller.
+
+        Allow linking without a CodeBlock, which WebAssembly's wasm -> JS stubs does. This needs to work for polymorphic and virtual calls. This patch adds corresponding tests for this.
+
+        * assembler/LinkBuffer.cpp:
+        (JSC::shouldDumpDisassemblyFor): don't look at the tier option if there isn't a CodeBlock, only look at the global one. This is a WebAssembly function, so the tier information is irrelevant.
+        * jit/Repatch.cpp:
+        (JSC::isWebAssemblyToJSCallee): this is used in the link* functions below
+        (JSC::linkFor):
+        (JSC::linkVirtualFor):
+        (JSC::linkPolymorphicCall):
+        * runtime/Options.h: add an option to change the maximum number of polymorphic calls in stubs from wasm to JS, which will come in handy when we try to tune performance or try merging some of the WebAssembly stubs
+        * wasm/WasmBinding.cpp:
+        (JSC::Wasm::importStubGenerator): remove the breakpoint since the code now works
+        * wasm/js/WebAssemblyToJSCallee.h:
+
 2016-12-08  Filip Pizlo  <fpizlo@apple.com>
 
         MultiPutByOffset should get a barrier if it transitions
index 326d20d..9010967 100644 (file)
@@ -39,7 +39,7 @@ namespace JSC {
 
 bool shouldDumpDisassemblyFor(CodeBlock* codeBlock)
 {
-    if (JITCode::isOptimizingJIT(codeBlock->jitType()) && Options::dumpDFGDisassembly())
+    if (codeBlock && JITCode::isOptimizingJIT(codeBlock->jitType()) && Options::dumpDFGDisassembly())
         return true;
     return Options::dumpDisassembly();
 }
index 924f80a..d4f98f8 100644 (file)
@@ -44,6 +44,7 @@
 #include "JIT.h"
 #include "JITInlines.h"
 #include "JSCInlines.h"
+#include "JSWebAssembly.h"
 #include "LinkBuffer.h"
 #include "PolymorphicAccess.h"
 #include "ScopedArguments.h"
@@ -556,32 +557,60 @@ static void linkSlowFor(VM* vm, CallLinkInfo& callLinkInfo)
     callLinkInfo.setSlowStub(createJITStubRoutine(virtualThunk, *vm, nullptr, true));
 }
 
+static bool isWebAssemblyToJSCallee(VM& vm, JSCell* callee)
+{
+#if ENABLE(WEBASSEMBLY)
+    // The WebAssembly -> JS stub sets it caller frame's callee to a singleton which lives on the VM.
+    return callee == vm.webAssemblyToJSCallee.get();
+#else
+    UNUSED_PARAM(vm);
+    UNUSED_PARAM(callee);
+    return false;
+#endif // ENABLE(WEBASSEMBLY)
+}
+
+static JSCell* webAssemblyOwner(VM& vm)
+{
+#if ENABLE(WEBASSEMBLY)
+    // Each WebAssembly.Instance shares the stubs from their WebAssembly.Module, which are therefore the appropriate owner.
+    return vm.topJSWebAssemblyInstance->module();
+#else
+    UNUSED_PARAM(vm);
+    RELEASE_ASSERT_NOT_REACHED();
+    return nullptr;
+#endif // ENABLE(WEBASSEMBLY)
+}
+
 void linkFor(
     ExecState* exec, CallLinkInfo& callLinkInfo, CodeBlock* calleeCodeBlock,
     JSFunction* callee, MacroAssemblerCodePtr codePtr)
 {
     ASSERT(!callLinkInfo.stub());
-    
-    CodeBlock* callerCodeBlock = exec->callerFrame()->codeBlock();
 
-    VM* vm = callerCodeBlock->vm();
-    
+    CallFrame* callerFrame = exec->callerFrame();
+    VM& vm = callerFrame->vm();
+    CodeBlock* callerCodeBlock = callerFrame->codeBlock();
+
+    // WebAssembly -> JS stubs don't have a valid CodeBlock.
+    JSCell* owner = isWebAssemblyToJSCallee(vm, callerFrame->callee()) ? webAssemblyOwner(vm) : callerCodeBlock;
+    ASSERT(owner);
+
     ASSERT(!callLinkInfo.isLinked());
-    callLinkInfo.setCallee(exec->callerFrame()->vm(), callerCodeBlock, callee);
-    callLinkInfo.setLastSeenCallee(exec->callerFrame()->vm(), callerCodeBlock, callee);
+    callLinkInfo.setCallee(vm, owner, callee);
+    callLinkInfo.setLastSeenCallee(vm, owner, callee);
     if (shouldDumpDisassemblyFor(callerCodeBlock))
         dataLog("Linking call in ", *callerCodeBlock, " at ", callLinkInfo.codeOrigin(), " to ", pointerDump(calleeCodeBlock), ", entrypoint at ", codePtr, "\n");
     MacroAssembler::repatchNearCall(callLinkInfo.hotPathOther(), CodeLocationLabel(codePtr));
-    
+
     if (calleeCodeBlock)
-        calleeCodeBlock->linkIncomingCall(exec->callerFrame(), &callLinkInfo);
-    
+        calleeCodeBlock->linkIncomingCall(callerFrame, &callLinkInfo);
+
     if (callLinkInfo.specializationKind() == CodeForCall && callLinkInfo.allowStubs()) {
-        linkSlowFor(vm, callLinkInfo, linkPolymorphicCallThunkGenerator);
+        linkSlowFor(&vm, callLinkInfo, linkPolymorphicCallThunkGenerator);
         return;
     }
     
-    linkSlowFor(vm, callLinkInfo);
+    linkSlowFor(&vm, callLinkInfo);
 }
 
 void linkDirectFor(
@@ -645,18 +674,18 @@ void unlinkFor(VM& vm, CallLinkInfo& callLinkInfo)
     revertCall(&vm, callLinkInfo, vm.getCTIStub(linkCallThunkGenerator));
 }
 
-void linkVirtualFor(
-    ExecState* exec, CallLinkInfo& callLinkInfo)
+void linkVirtualFor(ExecState* exec, CallLinkInfo& callLinkInfo)
 {
-    CodeBlock* callerCodeBlock = exec->callerFrame()->codeBlock();
-    VM* vm = callerCodeBlock->vm();
+    CallFrame* callerFrame = exec->callerFrame();
+    VM& vm = callerFrame->vm();
+    CodeBlock* callerCodeBlock = callerFrame->codeBlock();
 
     if (shouldDumpDisassemblyFor(callerCodeBlock))
-        dataLog("Linking virtual call at ", *callerCodeBlock, " ", exec->callerFrame()->codeOrigin(), "\n");
-    
-    MacroAssemblerCodeRef virtualThunk = virtualThunkFor(vm, callLinkInfo);
-    revertCall(vm, callLinkInfo, virtualThunk);
-    callLinkInfo.setSlowStub(createJITStubRoutine(virtualThunk, *vm, nullptr, true));
+        dataLog("Linking virtual call at ", *callerCodeBlock, " ", callerFrame->codeOrigin(), "\n");
+
+    MacroAssemblerCodeRef virtualThunk = virtualThunkFor(&vm, callLinkInfo);
+    revertCall(&vm, callLinkInfo, virtualThunk);
+    callLinkInfo.setSlowStub(createJITStubRoutine(virtualThunk, vm, nullptr, true));
 }
 
 namespace {
@@ -677,10 +706,16 @@ void linkPolymorphicCall(
         linkVirtualFor(exec, callLinkInfo);
         return;
     }
-    
-    CodeBlock* callerCodeBlock = exec->callerFrame()->codeBlock();
-    VM* vm = callerCodeBlock->vm();
-    
+
+    CallFrame* callerFrame = exec->callerFrame();
+    VM& vm = callerFrame->vm();
+    CodeBlock* callerCodeBlock = callerFrame->codeBlock();
+    bool isWebAssembly = isWebAssemblyToJSCallee(vm, callerFrame->callee());
+
+    // WebAssembly -> JS stubs don't have a valid CodeBlock.
+    JSCell* owner = isWebAssembly ? webAssemblyOwner(vm) : callerCodeBlock;
+    ASSERT(owner);
+
     CallVariantList list;
     if (PolymorphicCallStubRoutine* stub = callLinkInfo.stub())
         list = stub->variants();
@@ -709,7 +744,7 @@ void linkPolymorphicCall(
     // Figure out what our cases are.
     for (CallVariant variant : list) {
         CodeBlock* codeBlock;
-        if (variant.executable()->isHostFunction())
+        if (isWebAssembly || variant.executable()->isHostFunction())
             codeBlock = nullptr;
         else {
             ExecutableBase* executable = variant.executable();
@@ -727,10 +762,13 @@ void linkPolymorphicCall(
     
     // If we are over the limit, just use a normal virtual call.
     unsigned maxPolymorphicCallVariantListSize;
-    if (callerCodeBlock->jitType() == JITCode::topTierJIT())
+    if (isWebAssembly)
+        maxPolymorphicCallVariantListSize = Options::maxPolymorphicCallVariantListSizeForWebAssemblyToJS();
+    else if (callerCodeBlock->jitType() == JITCode::topTierJIT())
         maxPolymorphicCallVariantListSize = Options::maxPolymorphicCallVariantListSizeForTopTier();
     else
         maxPolymorphicCallVariantListSize = Options::maxPolymorphicCallVariantListSize();
+
     if (list.size() > maxPolymorphicCallVariantListSize) {
         linkVirtualFor(exec, callLinkInfo);
         return;
@@ -738,7 +776,7 @@ void linkPolymorphicCall(
     
     GPRReg calleeGPR = static_cast<GPRReg>(callLinkInfo.calleeGPR());
     
-    CCallHelpers stubJit(vm, callerCodeBlock);
+    CCallHelpers stubJit(&vm, callerCodeBlock);
     
     CCallHelpers::JumpList slowPath;
     
@@ -787,7 +825,7 @@ void linkPolymorphicCall(
     Vector<CallToCodePtr> calls(callCases.size());
     std::unique_ptr<uint32_t[]> fastCounts;
     
-    if (callerCodeBlock->jitType() != JITCode::topTierJIT())
+    if (!isWebAssembly && callerCodeBlock->jitType() != JITCode::topTierJIT())
         fastCounts = std::make_unique<uint32_t[]>(callCases.size());
     
     for (size_t i = 0; i < callCases.size(); ++i) {
@@ -884,7 +922,7 @@ void linkPolymorphicCall(
     stubJit.restoreReturnAddressBeforeReturn(GPRInfo::regT4);
     AssemblyHelpers::Jump slow = stubJit.jump();
         
-    LinkBuffer patchBuffer(*vm, stubJit, callerCodeBlock, JITCompilationCanFail);
+    LinkBuffer patchBuffer(vm, stubJit, owner, JITCompilationCanFail);
     if (patchBuffer.didFailToAllocate()) {
         linkVirtualFor(exec, callLinkInfo);
         return;
@@ -898,19 +936,19 @@ void linkPolymorphicCall(
         patchBuffer.link(
             callToCodePtr.call, FunctionPtr(isTailCall ? callToCodePtr.codePtr.dataLocation() : callToCodePtr.codePtr.executableAddress()));
     }
-    if (JITCode::isOptimizingJIT(callerCodeBlock->jitType()))
+    if (isWebAssembly || JITCode::isOptimizingJIT(callerCodeBlock->jitType()))
         patchBuffer.link(done, callLinkInfo.callReturnLocation().labelAtOffset(0));
     else
         patchBuffer.link(done, callLinkInfo.hotPathOther().labelAtOffset(0));
-    patchBuffer.link(slow, CodeLocationLabel(vm->getCTIStub(linkPolymorphicCallThunkGenerator).code()));
+    patchBuffer.link(slow, CodeLocationLabel(vm.getCTIStub(linkPolymorphicCallThunkGenerator).code()));
     
     auto stubRoutine = adoptRef(*new PolymorphicCallStubRoutine(
         FINALIZE_CODE_FOR(
             callerCodeBlock, patchBuffer,
             ("Polymorphic call stub for %s, return point %p, targets %s",
-                toCString(*callerCodeBlock).data(), callLinkInfo.callReturnLocation().labelAtOffset(0).executableAddress(),
+                isWebAssembly ? "WebAssembly" : toCString(*callerCodeBlock).data(), callLinkInfo.callReturnLocation().labelAtOffset(0).executableAddress(),
                 toCString(listDump(callCases)).data())),
-        *vm, callerCodeBlock, exec->callerFrame(), callLinkInfo, callCases,
+        vm, owner, exec->callerFrame(), callLinkInfo, callCases,
         WTFMove(fastCounts)));
     
     MacroAssembler::replaceWithJump(
@@ -919,7 +957,7 @@ void linkPolymorphicCall(
     // The original slow path is unreachable on 64-bits, but still
     // reachable on 32-bits since a non-cell callee will always
     // trigger the slow path
-    linkSlowFor(vm, callLinkInfo);
+    linkSlowFor(&vm, callLinkInfo);
     
     // If there had been a previous stub routine, that one will die as soon as the GC runs and sees
     // that it's no longer on stack.
index 5a8926d..d3f09d3 100644 (file)
@@ -232,6 +232,7 @@ typedef const char* optionString;
     v(bool, usePolymorphicCallInliningForNonStubStatus, false, Normal, nullptr) \
     v(unsigned, maxPolymorphicCallVariantListSize, 15, Normal, nullptr) \
     v(unsigned, maxPolymorphicCallVariantListSizeForTopTier, 5, Normal, nullptr) \
+    v(unsigned, maxPolymorphicCallVariantListSizeForWebAssemblyToJS, 5, Normal, nullptr) \
     v(unsigned, maxPolymorphicCallVariantsForInlining, 5, Normal, nullptr) \
     v(unsigned, frequentCallThreshold, 2, Normal, nullptr) \
     v(double, minimumCallToKnownRate, 0.51, Normal, nullptr) \
index 136c65c..5ffc45d 100644 (file)
@@ -49,7 +49,6 @@ WasmToJSStub importStubGenerator(VM* vm, Bag<CallLinkInfo>& callLinkInfos, Signa
     ASSERT(!jsCC.m_fprArgs.size());
 
     jit.emitFunctionPrologue();
-    jit.breakpoint(); // FIXME make calling to JavaScript work. https://bugs.webkit.org/show_bug.cgi?id=165591
     jit.store64(JIT::TrustedImm32(0), JIT::Address(GPRInfo::callFrameRegister, CallFrameSlot::codeBlock * static_cast<int>(sizeof(Register)))); // FIXME Stop using 0 as codeBlocks. https://bugs.webkit.org/show_bug.cgi?id=165321
     jit.storePtr(JIT::TrustedImmPtr(vm->webAssemblyToJSCallee.get()), JIT::Address(GPRInfo::callFrameRegister, CallFrameSlot::callee * static_cast<int>(sizeof(Register))));
 
index a34e3d1..496c885 100644 (file)
@@ -45,7 +45,7 @@ public:
 
 private:
     void finishCreation(VM&);
-    WebAssemblyToJSCallee(VM&, Structure* structure);
+    WebAssemblyToJSCallee(VM&, Structure*);
 };
 
 } // namespace JSC