Fix entry tier-up SIMD check in BBQ baseline JIT #10383

ddegazio · 2023-02-20T20:40:08Z

`f6adb76`

Fix entry tier-up SIMD check in BBQ baseline JIT
https://bugs.webkit.org/show_bug.cgi?id=252590
rdar://105689397

Currently, the new WebAssembly BBQ baseline JIT always passes false for the isSIMD
parameter when calling into the OMG tier-up trigger thunk. This patch makes it so
we track whether the function is a SIMD function using notifyFunctionUsesSIMD(),
and pass that into the thunk generator, ensuring we preserve any SIMD registers
used to pass arguments to the function.

Reviewed by Yusuke Suzuki.

* Source/JavaScriptCore/wasm/WasmBBQJIT.cpp:
(JSC::Wasm::BBQJIT::emitEntryTierUpCheck):
(JSC::Wasm::BBQJIT::notifyFunctionUsesSIMD):

Canonical link: https://commits.webkit.org/260562@main

701f658

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	~~🛠 ios~~	~~🛠 mac~~	~~🛠 wpe~~	~~🛠 wincairo~~
	~~🛠 ios-sim~~	✅ 🛠 mac-AS-debug	~~🛠 gtk~~
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	~~🧪 api-mac~~	~~🧪 gtk-wk2~~
	~~🧪 api-ios~~	~~🧪 mac-wk1~~	~~🧪 api-gtk~~
~~🛠 🧪 jsc~~	~~🛠 tv~~	~~🧪 mac-wk2~~	~~🛠 jsc-armv7~~
~~🛠 🧪 jsc-arm64~~	~~🛠 tv-sim~~	~~🧪 mac-AS-debug-wk2~~	~~🧪 jsc-armv7-tests~~
	~~🛠 watch~~	~~🧪 mac-wk2-stress~~	~~🛠 jsc-mips~~
	~~🛠 watch-sim~~		~~🧪 jsc-mips-tests~~
✅ 🛠 🧪 unsafe-merge

Constellation · 2023-02-20T20:42:52Z

Source/JavaScriptCore/wasm/WasmBBQJIT.cpp

@@ -5733,13 +5733,13 @@ class BBQJIT {
        Jump tierUp = m_jit.branchAdd32(CCallHelpers::PositiveOrZero, TrustedImm32(TierUpCount::functionEntryIncrement()), Address(m_scratchGPR));
        MacroAssembler::Label tierUpResume = m_jit.label();
        auto functionIndex = m_functionIndex;
-        addLatePath([tierUp, tierUpResume, functionIndex](BBQJIT&, CCallHelpers& jit) {
+        bool isSIMD = m_isSIMD;


We should get this when late path is executed. notifyFunctionUsesSIMD is not called yet when emitEntryTierUpCheck is called.

It should be called before addTopLevel, so it'll be true if any of our arguments or locals are SIMD (which I think is all we need to handle entry tier-up correctly?). But I do agree it's a better, less fragile solution to move the capture (although capturing members is a pain). I'll fix it up.

It is possible that body can include SIMD operations and it generates V128 temp (not local).

Should be resolved in the latest push!

Constellation · 2023-02-20T20:43:08Z

Source/JavaScriptCore/wasm/WasmBBQJIT.cpp

            tierUp.link(&jit);
            jit.move(TrustedImm32(functionIndex), GPRInfo::argumentGPR1);
            MacroAssembler::Call call = jit.nearCall();
            jit.jump(tierUpResume);

-            bool isSIMD = false; // TODO: Support SIMD


So, here, let's get m_isSIMD from BBQJIT.

So,

bool isSIMD = generator.m_isSIMD;

and capture this bool in jit.addLinkTask's lambda.
generator should be BBQJIT& which is passed to addLatePath's lambda.

webkit-early-warning-system · 2023-02-20T20:49:14Z

EWS run on previous version of this PR (hash 10436be)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	⏳ 🛠 ios	⏳ 🛠 mac	⏳ 🛠 wpe	⏳ 🛠 wincairo
⏳ 🧪 bindings	⏳ 🛠 ios-sim	⏳ 🛠 mac-AS-debug	⏳ 🛠 gtk
✅ 🧪 webkitperl	⏳ 🧪 ios-wk2	⏳ 🧪 api-mac	⏳ 🧪 gtk-wk2
	⏳ 🧪 api-ios	⏳ 🧪 mac-wk1	⏳ 🧪 api-gtk
⏳ 🛠 🧪 jsc	⏳ 🛠 tv	⏳ 🧪 mac-wk2	⏳ 🛠 jsc-armv7
⏳ 🛠 🧪 jsc-arm64	⏳ 🛠 tv-sim	⏳ 🧪 mac-AS-debug-wk2	⏳ 🧪 jsc-armv7-tests
	⏳ 🛠 watch	⏳ 🧪 mac-wk2-stress	⏳ 🛠 jsc-mips
	⏳ 🛠 watch-sim		⏳ 🧪 jsc-mips-tests

Constellation · 2023-02-20T20:52:07Z

Source/JavaScriptCore/wasm/WasmBBQJIT.cpp

+            jit.addLinkTask([=, this] (LinkBuffer& linkBuffer) {
+                MacroAssembler::repatchNearCall(linkBuffer.locationOfNearCall<NoPtrTag>(call), CodeLocationLabel<JITThunkPtrTag>(Thunks::singleton().stub(triggerOMGEntryTierUpThunkGenerator(this->m_isSIMD)).code()));


I think we cannot use this in LinkTask since BBQJIT is already destroyed at that time.
So, see https://github.com/WebKit/WebKit/pull/10383/files#r1112305847, let's get isSIMD at that time, and pass it here.

Oh good catch! Sorry, makes sense.

Constellation · 2023-02-20T20:52:34Z

Source/JavaScriptCore/wasm/WasmBBQJIT.cpp

@@ -5733,15 +5733,14 @@ class BBQJIT {
        Jump tierUp = m_jit.branchAdd32(CCallHelpers::PositiveOrZero, TrustedImm32(TierUpCount::functionEntryIncrement()), Address(m_scratchGPR));
        MacroAssembler::Label tierUpResume = m_jit.label();
        auto functionIndex = m_functionIndex;
-        addLatePath([tierUp, tierUpResume, functionIndex](BBQJIT&, CCallHelpers& jit) {
+        addLatePath([tierUp, tierUpResume, functionIndex, this](BBQJIT&, CCallHelpers& jit) {


Instead of capturing this, use BBQJIT&'s parameter.

This is probably best. Added it in the latest push.

webkit-early-warning-system · 2023-02-20T20:52:45Z

EWS run on previous version of this PR (hash 44c6953)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	⏳ 🛠 ios	⏳ 🛠 mac	❌ 🛠 wpe	⏳ 🛠 wincairo
	⏳ 🛠 ios-sim	✅ 🛠 mac-AS-debug	⏳ 🛠 gtk
✅ 🧪 webkitperl	⏳ 🧪 ios-wk2	⏳ 🧪 api-mac	⏳ 🧪 gtk-wk2
	⏳ 🧪 api-ios	⏳ 🧪 mac-wk1	⏳ 🧪 api-gtk
⏳ 🛠 🧪 jsc	⏳ 🛠 tv	⏳ 🧪 mac-wk2	⏳ 🛠 jsc-armv7
⏳ 🛠 🧪 jsc-arm64	⏳ 🛠 tv-sim	⏳ 🧪 mac-AS-debug-wk2	⏳ 🧪 jsc-armv7-tests
	⏳ 🛠 watch	⏳ 🧪 mac-wk2-stress	⏳ 🛠 jsc-mips
	⏳ 🛠 watch-sim		⏳ 🧪 jsc-mips-tests

webkit-early-warning-system · 2023-02-20T21:03:12Z

EWS run on current version of this PR (hash 701f658)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	~~🛠 ios~~	~~🛠 mac~~	~~🛠 wpe~~	~~🛠 wincairo~~
	~~🛠 ios-sim~~	✅ 🛠 mac-AS-debug	~~🛠 gtk~~
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	~~🧪 api-mac~~	~~🧪 gtk-wk2~~
	~~🧪 api-ios~~	~~🧪 mac-wk1~~	~~🧪 api-gtk~~
~~🛠 🧪 jsc~~	~~🛠 tv~~	~~🧪 mac-wk2~~	~~🛠 jsc-armv7~~
~~🛠 🧪 jsc-arm64~~	~~🛠 tv-sim~~	~~🧪 mac-AS-debug-wk2~~	~~🧪 jsc-armv7-tests~~
	~~🛠 watch~~	~~🧪 mac-wk2-stress~~	~~🛠 jsc-mips~~
	~~🛠 watch-sim~~		~~🧪 jsc-mips-tests~~
✅ 🛠 🧪 unsafe-merge

Constellation

r=me

Constellation · 2023-02-20T21:14:19Z

Land it since new BBQ is not tested yet.

https://bugs.webkit.org/show_bug.cgi?id=252590 rdar://105689397 Currently, the new WebAssembly BBQ baseline JIT always passes false for the isSIMD parameter when calling into the OMG tier-up trigger thunk. This patch makes it so we track whether the function is a SIMD function using notifyFunctionUsesSIMD(), and pass that into the thunk generator, ensuring we preserve any SIMD registers used to pass arguments to the function. Reviewed by Yusuke Suzuki. * Source/JavaScriptCore/wasm/WasmBBQJIT.cpp: (JSC::Wasm::BBQJIT::emitEntryTierUpCheck): (JSC::Wasm::BBQJIT::notifyFunctionUsesSIMD): Canonical link: https://commits.webkit.org/260562@main

webkit-commit-queue · 2023-02-20T21:17:04Z

Committed 260562@main (f6adb76): https://commits.webkit.org/260562@main

Reviewed commits have been landed. Closing PR #10383 and removing active labels.

ddegazio requested a review from a team as a code owner February 20, 2023 20:40

ddegazio self-assigned this Feb 20, 2023

ddegazio added the WebAssembly For bugs in JavaScript WebAssembly label Feb 20, 2023

Constellation requested changes Feb 20, 2023

View reviewed changes

ddegazio force-pushed the eng/Fix-entry-tier-up-SIMD-check-in-BBQ-baseline-JIT branch from 10436be to 44c6953 Compare February 20, 2023 20:50

Constellation reviewed Feb 20, 2023

View reviewed changes

ddegazio force-pushed the eng/Fix-entry-tier-up-SIMD-check-in-BBQ-baseline-JIT branch from 44c6953 to 701f658 Compare February 20, 2023 21:02

Constellation approved these changes Feb 20, 2023

View reviewed changes

Constellation added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Feb 20, 2023

webkit-early-warning-system force-pushed the eng/Fix-entry-tier-up-SIMD-check-in-BBQ-baseline-JIT branch from 701f658 to f6adb76 Compare February 20, 2023 21:16

webkit-early-warning-system merged commit f6adb76 into WebKit:main Feb 20, 2023

webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Feb 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix entry tier-up SIMD check in BBQ baseline JIT #10383

Fix entry tier-up SIMD check in BBQ baseline JIT #10383

ddegazio commented Feb 20, 2023 •

edited by webkit-early-warning-system

Constellation Feb 20, 2023

ddegazio Feb 20, 2023

Constellation Feb 20, 2023

ddegazio Feb 20, 2023

Constellation Feb 20, 2023

Constellation Feb 20, 2023

webkit-early-warning-system commented Feb 20, 2023 •

edited

Constellation Feb 20, 2023 •

edited

ddegazio Feb 20, 2023

Constellation Feb 20, 2023

ddegazio Feb 20, 2023

webkit-early-warning-system commented Feb 20, 2023 •

edited

webkit-early-warning-system commented Feb 20, 2023 •

edited

Constellation left a comment

Constellation commented Feb 20, 2023

webkit-commit-queue commented Feb 20, 2023

		jit.addLinkTask([=, this] (LinkBuffer& linkBuffer) {
		MacroAssembler::repatchNearCall(linkBuffer.locationOfNearCall<NoPtrTag>(call), CodeLocationLabel<JITThunkPtrTag>(Thunks::singleton().stub(triggerOMGEntryTierUpThunkGenerator(this->m_isSIMD)).code()));

Fix entry tier-up SIMD check in BBQ baseline JIT #10383

Fix entry tier-up SIMD check in BBQ baseline JIT #10383

Conversation

ddegazio commented Feb 20, 2023 • edited by webkit-early-warning-system

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webkit-early-warning-system commented Feb 20, 2023 • edited

Constellation Feb 20, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webkit-early-warning-system commented Feb 20, 2023 • edited

webkit-early-warning-system commented Feb 20, 2023 • edited

Constellation left a comment

Choose a reason for hiding this comment

Constellation commented Feb 20, 2023

webkit-commit-queue commented Feb 20, 2023

ddegazio commented Feb 20, 2023 •

edited by webkit-early-warning-system

webkit-early-warning-system commented Feb 20, 2023 •

edited

Constellation Feb 20, 2023 •

edited

webkit-early-warning-system commented Feb 20, 2023 •

edited

webkit-early-warning-system commented Feb 20, 2023 •

edited