Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JSC] Fix jitCompileSIMDFunction when recompiling the same function with a compiled replacement in different mode #28726

Conversation

hyjorc1
Copy link
Contributor

@hyjorc1 hyjorc1 commented May 17, 2024

b53bf30

[JSC] Fix jitCompileSIMDFunction when recompiling the same function with a compiled replacement in different mode
https://bugs.webkit.org/show_bug.cgi?id=274327
rdar://128141991

Reviewed by Mark Lam and Yusuke Suzuki.

We might encounter a situation that a WASM SIMD function was previously compiled
with the fast memory in a Signaling mode, but failed and tried to recompile
the same function in a BoundsChecking mode. In that case, we should allow it
to be compiled.

To fix this issue, the compilation status should be tracked and updated for each
memory mode. This patch fixes all the potential issues among
WasmIPIntTierUpCounter.h, WasmLLIntTierUpCounter.h, and WasmTierUpCount.h.

* Source/JavaScriptCore/runtime/MemoryMode.h:
* Source/JavaScriptCore/wasm/WasmBBQPlan.cpp:
(JSC::Wasm::BBQPlan::work):
* Source/JavaScriptCore/wasm/WasmIPIntSlowPaths.cpp:
(JSC::IPInt::jitCompileAndSetHeuristics):
(JSC::IPInt::WASM_IPINT_EXTERN_CPP_DECL):
* Source/JavaScriptCore/wasm/WasmIPIntTierUpCounter.h:
(JSC::Wasm::IPIntTierUpCounter::IPIntTierUpCounter):
(JSC::Wasm::IPIntTierUpCounter::compilationStatus):
(JSC::Wasm::IPIntTierUpCounter::setCompilationStatus):
(JSC::Wasm::IPIntTierUpCounter::loopCompilationStatus):
(JSC::Wasm::IPIntTierUpCounter::setLoopCompilationStatus):
(): Deleted.
* Source/JavaScriptCore/wasm/WasmLLIntTierUpCounter.h:
(JSC::Wasm::LLIntTierUpCounter::LLIntTierUpCounter):
(JSC::Wasm::LLIntTierUpCounter::compilationStatus):
(JSC::Wasm::LLIntTierUpCounter::setCompilationStatus):
(JSC::Wasm::LLIntTierUpCounter::loopCompilationStatus):
(JSC::Wasm::LLIntTierUpCounter::setLoopCompilationStatus):
(): Deleted.
* Source/JavaScriptCore/wasm/WasmOMGPlan.cpp:
(JSC::Wasm::OMGPlan::work):
* Source/JavaScriptCore/wasm/WasmOSREntryPlan.cpp:
(JSC::Wasm::OSREntryPlan::work):
* Source/JavaScriptCore/wasm/WasmOperations.cpp:
(JSC::Wasm::triggerOMGReplacementCompile):
(JSC::Wasm::JSC_DEFINE_JIT_OPERATION):
* Source/JavaScriptCore/wasm/WasmPlan.h:
(JSC::Wasm::Plan::mode const):
* Source/JavaScriptCore/wasm/WasmSlowPaths.cpp:
(JSC::LLInt::jitCompileAndSetHeuristics):
(JSC::LLInt::jitCompileSIMDFunction):
(JSC::LLInt::WASM_SLOW_PATH_DECL):
* Source/JavaScriptCore/wasm/WasmTierUpCount.cpp:
(JSC::Wasm::TierUpCount::TierUpCount):
* Source/JavaScriptCore/wasm/WasmTierUpCount.h:
(JSC::Wasm::TierUpCount::compilationStatusForOMG):
(JSC::Wasm::TierUpCount::setCompilationStatusForOMG):
(JSC::Wasm::TierUpCount::compilationStatusForOMGForOSREntry):
(JSC::Wasm::TierUpCount::setCompilationStatusForOMGForOSREntry):
(): Deleted.

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

ea3336a

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ§ͺ wpe-wk2 βœ… πŸ§ͺ wincairo-tests
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ§ͺ api-wpe
βœ… πŸ§ͺ ios-wk2-wpt βœ… πŸ§ͺ mac-wk1 βœ… πŸ›  wpe-cairo
βœ… πŸ›  πŸ§ͺ jsc βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk2 βœ… πŸ›  gtk
βœ… πŸ›  πŸ§ͺ jsc-arm64 βœ… πŸ›  tv βœ… πŸ§ͺ mac-AS-debug-wk2 βœ… πŸ§ͺ gtk-wk2
βœ… πŸ›  tv-sim βœ… πŸ§ͺ mac-wk2-stress ❌ πŸ§ͺ api-gtk
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch βœ… πŸ›  jsc-armv7
βœ… πŸ›  watch-sim βœ… πŸ§ͺ jsc-armv7-tests

@hyjorc1 hyjorc1 requested a review from a team as a code owner May 17, 2024 19:51
@hyjorc1 hyjorc1 self-assigned this May 17, 2024
@hyjorc1 hyjorc1 added the JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues. label May 17, 2024
Copy link

@MenloDorian MenloDorian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me

Source/JavaScriptCore/wasm/WasmSlowPaths.cpp Outdated Show resolved Hide resolved
@hyjorc1 hyjorc1 force-pushed the eng/JSC-Fix-jitCompileSIMDFunction-when-recompiling-the-same-function-with-a-compiled-replacement-in-different-mode branch from d448aca to 552350a Compare May 17, 2024 20:09
@hyjorc1 hyjorc1 force-pushed the eng/JSC-Fix-jitCompileSIMDFunction-when-recompiling-the-same-function-with-a-compiled-replacement-in-different-mode branch from 552350a to 46daa9e Compare May 17, 2024 22:16
Copy link
Member

@Constellation Constellation left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me with comment.

Source/JavaScriptCore/wasm/WasmIPIntTierUpCounter.h Outdated Show resolved Hide resolved
Source/JavaScriptCore/wasm/WasmIPIntTierUpCounter.h Outdated Show resolved Hide resolved
Source/JavaScriptCore/wasm/WasmLLIntTierUpCounter.h Outdated Show resolved Hide resolved
@hyjorc1 hyjorc1 force-pushed the eng/JSC-Fix-jitCompileSIMDFunction-when-recompiling-the-same-function-with-a-compiled-replacement-in-different-mode branch from 46daa9e to 10f4081 Compare May 18, 2024 00:29
@hyjorc1 hyjorc1 force-pushed the eng/JSC-Fix-jitCompileSIMDFunction-when-recompiling-the-same-function-with-a-compiled-replacement-in-different-mode branch from 10f4081 to 69e92c9 Compare May 18, 2024 00:38
@hyjorc1 hyjorc1 force-pushed the eng/JSC-Fix-jitCompileSIMDFunction-when-recompiling-the-same-function-with-a-compiled-replacement-in-different-mode branch from 69e92c9 to 5594010 Compare May 18, 2024 00:47
Copy link
Member

@Constellation Constellation left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, not use FixedVector. This is super inefficient here since it is heap allocated.

@hyjorc1 hyjorc1 force-pushed the eng/JSC-Fix-jitCompileSIMDFunction-when-recompiling-the-same-function-with-a-compiled-replacement-in-different-mode branch from 5594010 to ea3336a Compare May 18, 2024 01:39
@hyjorc1 hyjorc1 added the merge-queue Applied to send a pull request to merge-queue label May 18, 2024
…ith a compiled replacement in different mode

https://bugs.webkit.org/show_bug.cgi?id=274327
rdar://128141991

Reviewed by Mark Lam and Yusuke Suzuki.

We might encounter a situation that a WASM SIMD function was previously compiled
with the fast memory in a Signaling mode, but failed and tried to recompile
the same function in a BoundsChecking mode. In that case, we should allow it
to be compiled.

To fix this issue, the compilation status should be tracked and updated for each
memory mode. This patch fixes all the potential issues among
WasmIPIntTierUpCounter.h, WasmLLIntTierUpCounter.h, and WasmTierUpCount.h.

* Source/JavaScriptCore/runtime/MemoryMode.h:
* Source/JavaScriptCore/wasm/WasmBBQPlan.cpp:
(JSC::Wasm::BBQPlan::work):
* Source/JavaScriptCore/wasm/WasmIPIntSlowPaths.cpp:
(JSC::IPInt::jitCompileAndSetHeuristics):
(JSC::IPInt::WASM_IPINT_EXTERN_CPP_DECL):
* Source/JavaScriptCore/wasm/WasmIPIntTierUpCounter.h:
(JSC::Wasm::IPIntTierUpCounter::IPIntTierUpCounter):
(JSC::Wasm::IPIntTierUpCounter::compilationStatus):
(JSC::Wasm::IPIntTierUpCounter::setCompilationStatus):
(JSC::Wasm::IPIntTierUpCounter::loopCompilationStatus):
(JSC::Wasm::IPIntTierUpCounter::setLoopCompilationStatus):
(): Deleted.
* Source/JavaScriptCore/wasm/WasmLLIntTierUpCounter.h:
(JSC::Wasm::LLIntTierUpCounter::LLIntTierUpCounter):
(JSC::Wasm::LLIntTierUpCounter::compilationStatus):
(JSC::Wasm::LLIntTierUpCounter::setCompilationStatus):
(JSC::Wasm::LLIntTierUpCounter::loopCompilationStatus):
(JSC::Wasm::LLIntTierUpCounter::setLoopCompilationStatus):
(): Deleted.
* Source/JavaScriptCore/wasm/WasmOMGPlan.cpp:
(JSC::Wasm::OMGPlan::work):
* Source/JavaScriptCore/wasm/WasmOSREntryPlan.cpp:
(JSC::Wasm::OSREntryPlan::work):
* Source/JavaScriptCore/wasm/WasmOperations.cpp:
(JSC::Wasm::triggerOMGReplacementCompile):
(JSC::Wasm::JSC_DEFINE_JIT_OPERATION):
* Source/JavaScriptCore/wasm/WasmPlan.h:
(JSC::Wasm::Plan::mode const):
* Source/JavaScriptCore/wasm/WasmSlowPaths.cpp:
(JSC::LLInt::jitCompileAndSetHeuristics):
(JSC::LLInt::jitCompileSIMDFunction):
(JSC::LLInt::WASM_SLOW_PATH_DECL):
* Source/JavaScriptCore/wasm/WasmTierUpCount.cpp:
(JSC::Wasm::TierUpCount::TierUpCount):
* Source/JavaScriptCore/wasm/WasmTierUpCount.h:
(JSC::Wasm::TierUpCount::compilationStatusForOMG):
(JSC::Wasm::TierUpCount::setCompilationStatusForOMG):
(JSC::Wasm::TierUpCount::compilationStatusForOMGForOSREntry):
(JSC::Wasm::TierUpCount::setCompilationStatusForOMGForOSREntry):
(): Deleted.

Canonical link: https://commits.webkit.org/278957@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/JSC-Fix-jitCompileSIMDFunction-when-recompiling-the-same-function-with-a-compiled-replacement-in-different-mode branch from ea3336a to b53bf30 Compare May 18, 2024 17:28
@webkit-commit-queue
Copy link
Collaborator

Committed 278957@main (b53bf30): https://commits.webkit.org/278957@main

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

@webkit-commit-queue webkit-commit-queue merged commit b53bf30 into WebKit:main May 18, 2024
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label May 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues.
Projects
None yet
5 participants