Changeset 271186 in webkit

Timestamp:

Jan 5, 2021, 7:03:43 PM (6 years ago)

Author:

ysuzuki@apple.com

Message:

[JSC] DFG/FTL DirectCall need to respect Wasm IC
​https://bugs.webkit.org/show_bug.cgi?id=220339

Reviewed by Saam Barati.

We found that Wasm IC for fast calls are not used in several places.

1. LLInt calls
2. DFG/FTL DirectCall
3. Virtual calls

We noticed this because of r271112. r271112 made wasm function loading from exports constant-folded in DFG/FTL.
And it emits DirectCall instead of Call in DFG/FTL. Then, we missed Wasm IC and get large performance regression
in JetStream2 richard-wasm.

In this patch, we use Wasm IC as much as possible. The key thing of this wasm IC is that it relies on callee.
So, if the place is just checking Executable, then we should not go to that IC. Fortunately, the above three checks
callee before using code pointer obtained for Wasm IC.

1. LLInt call fast path first checks callee.
2. DFG/FTL DirectCall requires callee is constant for wasm functions.
3. Virtual calls are not storing generated codePtr.

• dfg/DFGOperations.cpp:

(JSC::DFG::JSC_DEFINE_JIT_OPERATION):

• jit/JITOperations.cpp:

(JSC::virtualForWithFunction):

• llint/LLIntSlowPaths.cpp:

(JSC::LLInt::setUpCall):

Location:

trunk

Files:

• JSTests/stress/sampling-profiler-wasm-name-section.js (modified) (1 diff)
• JSTests/stress/sampling-profiler-wasm.js (modified) (1 diff)
• JSTests/stress/sampling-profiler/samplingProfiler.js (modified) (1 diff)
• JSTests/wasm/function-tests/nameSection.js (modified) (1 diff)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/dfg/DFGOperations.cpp (modified) (1 diff)
• Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp (modified) (2 diffs)
• Source/JavaScriptCore/jit/JITOperations.cpp (modified) (1 diff)
• Source/JavaScriptCore/llint/LLIntSlowPaths.cpp (modified) (2 diffs)
• Source/JavaScriptCore/runtime/Intrinsic.cpp (modified) (1 diff)
• Source/JavaScriptCore/runtime/Intrinsic.h (modified) (1 diff)
• Source/JavaScriptCore/wasm/js/WebAssemblyFunction.cpp (modified) (1 diff)

trunk/JSTests/stress/sampling-profiler-wasm-name-section.js

@@ -70 +70 @@
         return instance.exports._parrot(1);
     };
-    runTest(wasmEntry, ["_silly", "(unknown)", "<?>.wasm-function[_eggs]", "<?>.wasm-function[_bacon]", "<?>.wasm-function[_spam]", "<?>.wasm-function[_parrot]", "wasm-stub", "24", "wasmEntry"]);
+    runTest(wasmEntry, ["_silly", "(unknown)", "<?>.wasm-function[_eggs]", "<?>.wasm-function[_bacon]", "<?>.wasm-function[_spam]", "<?>.wasm-function[_parrot]", "(unknown)", "wasmEntry"]);
 }

trunk/JSTests/stress/sampling-profiler-wasm.js

@@ -9 +9 @@
         return instance.exports.loop(10000000);
     };
-    runTest(wasmEntry, ["<?>.wasm-function[0]", "wasm-stub", "0", "wasmEntry"]);
+    runTest(wasmEntry, ["<?>.wasm-function[0]", "(unknown)", "wasmEntry"]);
 }

trunk/JSTests/stress/sampling-profiler/samplingProfiler.js

@@ -44 +44 @@
     
     let node = tree;
+    let prev = null;
     for (let i = stackTrace.length; i--; ) {
+        prev = node;
         node = node.children[stackTrace[i]];
         if (!node) {
             if (verbose)
-                print("failing on " + i + " : " + stackTrace[i]);
+                print("failing on " + i + " : " + stackTrace[i], " ", JSON.stringify(Object.getOwnPropertyNames(prev.children)));
             return false;
         }

trunk/JSTests/wasm/function-tests/nameSection.js

@@ -70 +70 @@
 assert.eq(stacktrace[4], "<?>.wasm-function[_spam]@[wasm code]");
 assert.eq(stacktrace[5], "<?>.wasm-function[_parrot]@[wasm code]");
-assert.eq(stacktrace[6], "wasm-stub@[wasm code]"); // wasm entry
+assert.eq(stacktrace[6], "wasm-stub@[native code]"); // wasm entry

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2021-01-05  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] DFG/FTL DirectCall need to respect Wasm IC
+        https://bugs.webkit.org/show_bug.cgi?id=220339
+
+        Reviewed by Saam Barati.
+
+        We found that Wasm IC for fast calls are not used in several places.
+
+            1. LLInt calls
+            2. DFG/FTL DirectCall
+            3. Virtual calls
+
+        We noticed this because of r271112. r271112 made wasm function loading from exports constant-folded in DFG/FTL.
+        And it emits DirectCall instead of Call in DFG/FTL. Then, we missed Wasm IC and get large performance regression
+        in JetStream2 richard-wasm.
+
+        In this patch, we use Wasm IC as much as possible. The key thing of this wasm IC is that it relies on callee.
+        So, if the place is just checking Executable, then we should not go to that IC. Fortunately, the above three checks
+        callee before using code pointer obtained for Wasm IC.
+
+            1. LLInt call fast path first checks callee.
+            2. DFG/FTL DirectCall requires callee is constant for wasm functions.
+            3. Virtual calls are not storing generated codePtr.
+
+        * dfg/DFGOperations.cpp:
+        (JSC::DFG::JSC_DEFINE_JIT_OPERATION):
+        * jit/JITOperations.cpp:
+        (JSC::virtualForWithFunction):
+        * llint/LLIntSlowPaths.cpp:
+        (JSC::LLInt::setUpCall):
+
 2021-01-05  Yusuke Suzuki  <ysuzuki@apple.com>
 

trunk/Source/JavaScriptCore/dfg/DFGOperations.cpp

@@ -3551 +3551 @@
     MacroAssemblerCodePtr<JSEntryPtrTag> codePtr;
     CodeBlock* codeBlock = nullptr;
-    if (executable->isHostFunction())
-        codePtr = executable->entrypointFor(kind, MustCheckArity);
-    else {
+    if (executable->isHostFunction()) {
+        // jsToWasmICCodePtr assumes that callee is always the same since DirectCall does not check callee.
+        // But for wasm functions, we already ensured that callee is constant when emitting DirectCall.
+        codePtr = jsToWasmICCodePtr(vm, kind, callee);
+        if (!codePtr)
+            codePtr = executable->entrypointFor(kind, MustCheckArity);
+    } else {
         FunctionExecutable* functionExecutable = static_cast<FunctionExecutable*>(executable);
 

trunk/Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp

@@ -918 +918 @@
             Edge callee = m_graph.varArgChild(m_node, 0);
             CallVariant callVariant;
-            if (JSFunction* function = callee->dynamicCastConstant<JSFunction*>(vm())) {
+            JSFunction* function = callee->dynamicCastConstant<JSFunction*>(vm());
+            if (function) {
                 executable = function->executable();
                 callVariant = CallVariant(function);
@@ -928 +929 @@
             if (!executable)
                 break;
+
+            // If this is wasm function, and callee is not an constant,
+            // we should not use DirectCall since it will emit Wasm IC based on the assumption that the callee is constant.
+            // Currently, there is no way to reach to this condition (since no function-allocation node generates WebAssemblyFunction),
+            // but this is good guard for the future extension.
+            if (executable->intrinsic() == WasmFunctionIntrinsic) {
+                if (!function)
+                    break;
+            }
             
             if (FunctionExecutable* functionExecutable = jsDynamicCast<FunctionExecutable*>(vm(), executable)) {

trunk/Source/JavaScriptCore/jit/JITOperations.cpp

@@ -1390 +1390 @@
         }
     }
-    return encodeResult(executable->entrypointFor(
-        kind, MustCheckArity).executableAddress(),
+
+    MacroAssemblerCodePtr<JSEntryPtrTag> codePtr;
+    if (executable->isHostFunction())
+        codePtr = jsToWasmICCodePtr(vm, kind, function);
+    if (!codePtr)
+        codePtr = executable->entrypointFor(kind, MustCheckArity);
+
+    return encodeResult(codePtr.executableAddress(),
         reinterpret_cast<void*>(callLinkInfo->callMode() == CallMode::Tail ? ReuseTheFrame : KeepTheFrame));
 }

trunk/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp

@@ -61 +61 @@
 #include "ProtoCallFrameInlines.h"
 #include "RegExpObject.h"
+#include "Repatch.h"
 #include "ShadowChicken.h"
 #include "SuperSampler.h"
@@ -1737 +1738 @@
     MacroAssemblerCodePtr<JSEntryPtrTag> codePtr;
     CodeBlock* codeBlock = nullptr;
-    if (executable->isHostFunction())
-        codePtr = executable->entrypointFor(kind, MustCheckArity);
-    else {
+    if (executable->isHostFunction()) {
+#if ENABLE(JIT)
+        codePtr = jsToWasmICCodePtr(vm, kind, callee);
+#endif
+        if (!codePtr)
+            codePtr = executable->entrypointFor(kind, MustCheckArity);
+    } else {
         FunctionExecutable* functionExecutable = static_cast<FunctionExecutable*>(executable);
 

trunk/Source/JavaScriptCore/runtime/Intrinsic.cpp

@@ -338 +338 @@
     case DataViewSetFloat64:
         return "DataViewSetFloat64";
+    case WasmFunctionIntrinsic:
+        return "WasmFunctionIntrinsic";
     }
     RELEASE_ASSERT_NOT_REACHED();

trunk/Source/JavaScriptCore/runtime/Intrinsic.h

@@ -193 +193 @@
     DataViewSetFloat32,
     DataViewSetFloat64,
+
+    WasmFunctionIntrinsic,
 };
 

trunk/Source/JavaScriptCore/wasm/js/WebAssemblyFunction.cpp

@@ -435 +435 @@
 WebAssemblyFunction* WebAssemblyFunction::create(VM& vm, JSGlobalObject* globalObject, Structure* structure, unsigned length, const String& name, JSWebAssemblyInstance* instance, Wasm::Callee& jsEntrypoint, Wasm::WasmToWasmImportableFunction::LoadLocation wasmToWasmEntrypointLoadLocation, Wasm::SignatureIndex signatureIndex)
 {
-    NativeExecutable* executable = vm.getHostFunction(callWebAssemblyFunction, NoIntrinsic, callHostFunctionAsConstructor, nullptr, name);
+    NativeExecutable* executable = vm.getHostFunction(callWebAssemblyFunction, WasmFunctionIntrinsic, callHostFunctionAsConstructor, nullptr, name);
     WebAssemblyFunction* function = new (NotNull, allocateCell<WebAssemblyFunction>(vm.heap)) WebAssemblyFunction(vm, executable, globalObject, structure, jsEntrypoint, wasmToWasmEntrypointLoadLocation, signatureIndex);
     function->finishCreation(vm, executable, length, name, instance);