Skip to content

Commit

Permalink
Refactor OMG call patchpoint and tail call patchpoint
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=273823

Reviewed by Yusuke Suzuki.

The wasm tail calls feature does not currently work when the new frame
overlaps the old frame in interesting ways, so the tail calls tests
were disabled. Then, OMG inlining broke tail calls, causing an assertion failure.

This patch does not fix tail calls, but it does fix that assertion.

It also refactors both versions of create[Tail]CallPatchpoint to look
the same. This makes it a little nicer to read, but more importantly,
it makes it easier for a follow-up patch to fix OMG tail calls.

The main reason for this change is so that the follow-up patch is easier to read.

* Source/JavaScriptCore/wasm/WasmOMGIRGenerator.cpp:
(JSC::Wasm::OMGIRGenerator::emitIndirectCall):
(JSC::Wasm::OMGIRGenerator::createCallPatchpoint):
(JSC::Wasm::OMGIRGenerator::createTailCallPatchpoint):
(JSC::Wasm::OMGIRGenerator::addCall):

Canonical link: https://commits.webkit.org/279055@main
  • Loading branch information
justinmichaud committed May 21, 2024
1 parent 041231a commit c883116
Show file tree
Hide file tree
Showing 5 changed files with 160 additions and 85 deletions.
58 changes: 58 additions & 0 deletions JSTests/wasm/stress/cc-int-to-int-tail-call.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
//@ skip
//@ requireOptions("--useIPIntWrappers=1", "--useWebAssemblyTailCalls=1")
import { instantiate } from "../wabt-wrapper.js"
import * as assert from "../assert.js"

let wat = `
(module
(type $sig_test (func (param i32) (result i32)))
(table $t 1 funcref)
(elem (i32.const 0) $test)
(func $test (export "test") (param $x i32) (result i32)
(i32.add (local.get $x) (i32.const 42))
)
(func (export "test_with_call") (param $x i32) (result i32)
(return_call $test (i32.add (local.get $x) (i32.const 1337)))
)
(func (export "test_with_call_indirect") (param $x i32) (result i32)
(local.get $x)
(i32.const 98)
i32.add
(return_call_indirect $t (type $sig_test) (i32.const 0))
)
)
`

async function test() {
const instance = await instantiate(wat, {}, { simd: true, tail_call: true })
const { test, test_with_call, test_with_call_indirect } = instance.exports

for (let i = 0; i < 10000; ++i) {
assert.eq(test(5), 42 + 5)
assert.eq(test(), 42 + 0)
assert.eq(test(null), 42 + 0)
assert.eq(test({ }), 42 + 0)
assert.eq(test({ }, 10), 42 + 0)
assert.eq(test(20.1, 10), 42 + 20)
assert.eq(test(10, 20.1), 42 + 10)
assert.eq(test_with_call(5), 42 + 5 + 1337)
assert.eq(test_with_call(), 42 + 0 + 1337)
assert.eq(test_with_call(null), 42 + 0 + 1337)
assert.eq(test_with_call({ }), 42 + 0 + 1337)
assert.eq(test_with_call({ }, 10), 42 + 0 + 1337)
assert.eq(test_with_call(20.1, 10), 42 + 20 + 1337)
assert.eq(test_with_call(10, 20.1), 42 + 10 + 1337)
assert.eq(test_with_call_indirect(5), 42 + 5 + 98)
assert.eq(test_with_call_indirect(), 42 + 0 + 98)
assert.eq(test_with_call_indirect(null), 42 + 0 + 98)
assert.eq(test_with_call_indirect({ }), 42 + 0 + 98)
assert.eq(test_with_call_indirect({ }, 10), 42 + 0 + 98)
assert.eq(test_with_call_indirect(20.1, 10), 42 + 20 + 98)
assert.eq(test_with_call_indirect(10, 20.1), 42 + 10 + 98)
}
}

await assert.asyncTest(test())
2 changes: 1 addition & 1 deletion Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1249,7 +1249,7 @@ class StrengthReductionPhase : public Phase {
// FIXME: Support wasm IC.
// DirectCall to wasm function has suboptimal implementation. We avoid using DirectCall if we know that function is a wasm function.
// https://bugs.webkit.org/show_bug.cgi?id=220339
if (executable->intrinsic() == WasmFunctionIntrinsic) {
if (executable->intrinsic() == WasmFunctionIntrinsic && !Options::forceICFailure()) {
if (m_node->op() != Call) // FIXME: We should support tail-call.
break;
if (!function)
Expand Down
14 changes: 8 additions & 6 deletions Source/JavaScriptCore/runtime/Options.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -825,18 +825,20 @@ void Options::notifyOptionsChanged()
if (Options::forceAllFunctionsToUseSIMD() && !Options::useWebAssemblySIMD())
Options::forceAllFunctionsToUseSIMD() = false;

if (Options::useWebAssemblySIMD() && !(Options::useWasmLLInt() || Options::useWasmIPInt())) {
// The LLInt is responsible for discovering if functions use SIMD.
// If we can't run using it, then we should be conservative.
Options::forceAllFunctionsToUseSIMD() = true;
}

if (Options::useWebAssemblyTailCalls()) {
// The single-pass BBQ JIT doesn't support these features currently, so we should use a different
// BBQ backend if any of them are enabled. We should remove these limitations as support for each
// is added.
// FIXME: Add WASM tail calls support to single-pass BBQ JIT. https://bugs.webkit.org/show_bug.cgi?id=253192
Options::useBBQJIT() = false;
Options::useWasmLLInt() = true;
Options::wasmLLIntTiersUpToBBQ() = false;
}

if (Options::useWebAssemblySIMD() && !(Options::useWasmLLInt() || Options::useWasmIPInt())) {
// The LLInt is responsible for discovering if functions use SIMD.
// If we can't run using it, then we should be conservative.
Options::forceAllFunctionsToUseSIMD() = true;
}
}

Expand Down
Loading

0 comments on commit c883116

Please sign in to comment.