Skip to content

Enable FTL on Windows#30580

Merged
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
iangrunert:ig/enable-ftl-jit-win
Jul 9, 2024
Merged

Enable FTL on Windows#30580
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
iangrunert:ig/enable-ftl-jit-win

Conversation

@iangrunert

@iangrunert iangrunert commented Jul 8, 2024

Copy link
Copy Markdown
Contributor

dc849c8

Enable FTL on Windows
https://bugs.webkit.org/show_bug.cgi?id=145366

Reviewed by Yusuke Suzuki.

Disabling BBQ and OMG JIT for now, there's some edge cases which are
currently broken.

Fixes to testmasm, broken by previous sysv abi work.

B3 float mod tests were broken on Windows due to differences between
fmod(double, double) used within MathCommon fmodDouble, and the
fmod(float, float) used within the tests. Using fmodl in the tests so
they match behaviour with compiled B3.

The countLeadingZero implementation for testClz* tests had an off-by-one
error on Windows - using builtin __lzcnt64 and __lzcnt instead.

Had to precommit stack memory for testCallFunctionWithHellaArguments3 to
avoid accessing stack memory past the stack guard page.

* Source/JavaScriptCore/assembler/ProbeContext.cpp:
(JSC::Probe::flushDirtyStackPages):
* Source/JavaScriptCore/assembler/ProbeContext.h:
* Source/JavaScriptCore/assembler/testmasm.cpp:
(JSC::invoke):
(JSC::testStoreBaseIndex):
* Source/JavaScriptCore/b3/B3LowerToAir.cpp:
* Source/JavaScriptCore/b3/B3Validate.cpp:
* Source/JavaScriptCore/b3/air/opcode_generator.rb:
* Source/JavaScriptCore/b3/air/testair.cpp:
* Source/JavaScriptCore/b3/testb3.h:
(invoke):
* Source/JavaScriptCore/b3/testb3_2.cpp:
(testModArgFloat):
(testModArgsFloat):
(testModArgImmFloat):
(testModImmArgFloat):
(testModImmsFloat):
* Source/JavaScriptCore/b3/testb3_3.cpp:
(countLeadingZero):
* Source/JavaScriptCore/b3/testb3_4.cpp:
(testLoadFromFramePointer):
* Source/JavaScriptCore/b3/testb3_5.cpp:
(preCommitStackMemory):
(testCallFunctionWithHellaArguments3):
* Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileCompareStrictEq):
* Source/JavaScriptCore/jit/RegisterSet.cpp:
(JSC::RegisterSetBuilder::ftlCalleeSaveRegisters):
* Source/JavaScriptCore/llint/LowLevelInterpreter.asm:
* Source/JavaScriptCore/runtime/Options.cpp:
(JSC::Options::notifyOptionsChanged):
* Source/JavaScriptCore/wasm/WasmBBQJIT.cpp:
(JSC::Wasm::BBQJITImpl::RegisterBinding::none):
* Source/JavaScriptCore/wasm/WasmBBQJIT.h:
* Source/JavaScriptCore/wasm/WasmBBQJIT64.cpp:
(JSC::Wasm::BBQJITImpl::BBQJIT::addSIMDV_V):
* Source/JavaScriptCore/wasm/WasmIRGeneratorHelpers.h:
(JSC::Wasm::buildEntryBufferForCatchSIMD):
(JSC::Wasm::buildEntryBufferForCatchNoSIMD):
* Source/JavaScriptCore/wasm/WasmOMGIRGenerator.cpp:
* Source/cmake/OptionsJSCOnly.cmake:
* Source/cmake/OptionsWin.cmake:

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

1f59e73

Misc iOS, visionOS, 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 ✅ 🛠 vision 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv ✅ 🧪 jsc-armv7-tests
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@iangrunert iangrunert requested a review from a team as a code owner July 8, 2024 21:37
Comment thread Source/JavaScriptCore/b3/testb3_3.cpp Outdated
Comment on lines 1693 to 1697

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should make countLeadingZero work on Windows too.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

And we should explore why this is not correctly working on Windows.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The countLeadingZero implementation for testClz* tests had an off-by-one
error on Windows - using builtin __lzcnt64 and __lzcnt instead.

Using __lzcnt64 etc. is not right. We should know why this does not work on Windows, and we need to fix it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed - inside countLeadingZero we were left shifting a long (1L) instead of an unsigned long long (1ull) by the bitcount.

On debug builds this was using a 32 bit signed long, on release builds a 64 bit signed long. That gave us the off-by-one on release builds.

Comment thread Source/JavaScriptCore/b3/testb3_3.cpp Outdated
Comment on lines 1708 to 1712

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ditto

Comment thread Source/JavaScriptCore/b3/testb3_3.cpp Outdated
Comment on lines 1723 to 1727

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ditto

Comment thread Source/JavaScriptCore/b3/testb3_3.cpp Outdated
Comment on lines 1738 to 1742

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ditto

@iangrunert iangrunert force-pushed the ig/enable-ftl-jit-win branch from b31a80e to 0f4852f Compare July 9, 2024 14:45
@iangrunert iangrunert force-pushed the ig/enable-ftl-jit-win branch from 0f4852f to 1f59e73 Compare July 9, 2024 14:47

@Constellation Constellation left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

r=me

@Constellation Constellation added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jul 9, 2024
https://bugs.webkit.org/show_bug.cgi?id=145366

Reviewed by Yusuke Suzuki.

Disabling BBQ and OMG JIT for now, there's some edge cases which are
currently broken.

Fixes to testmasm, broken by previous sysv abi work.

B3 float mod tests were broken on Windows due to differences between
fmod(double, double) used within MathCommon fmodDouble, and the
fmod(float, float) used within the tests. Using fmodl in the tests so
they match behaviour with compiled B3.

The countLeadingZero implementation for testClz* tests had an off-by-one
error on Windows - using builtin __lzcnt64 and __lzcnt instead.

Had to precommit stack memory for testCallFunctionWithHellaArguments3 to
avoid accessing stack memory past the stack guard page.

* Source/JavaScriptCore/assembler/ProbeContext.cpp:
(JSC::Probe::flushDirtyStackPages):
* Source/JavaScriptCore/assembler/ProbeContext.h:
* Source/JavaScriptCore/assembler/testmasm.cpp:
(JSC::invoke):
(JSC::testStoreBaseIndex):
* Source/JavaScriptCore/b3/B3LowerToAir.cpp:
* Source/JavaScriptCore/b3/B3Validate.cpp:
* Source/JavaScriptCore/b3/air/opcode_generator.rb:
* Source/JavaScriptCore/b3/air/testair.cpp:
* Source/JavaScriptCore/b3/testb3.h:
(invoke):
* Source/JavaScriptCore/b3/testb3_2.cpp:
(testModArgFloat):
(testModArgsFloat):
(testModArgImmFloat):
(testModImmArgFloat):
(testModImmsFloat):
* Source/JavaScriptCore/b3/testb3_3.cpp:
(countLeadingZero):
* Source/JavaScriptCore/b3/testb3_4.cpp:
(testLoadFromFramePointer):
* Source/JavaScriptCore/b3/testb3_5.cpp:
(preCommitStackMemory):
(testCallFunctionWithHellaArguments3):
* Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileCompareStrictEq):
* Source/JavaScriptCore/jit/RegisterSet.cpp:
(JSC::RegisterSetBuilder::ftlCalleeSaveRegisters):
* Source/JavaScriptCore/llint/LowLevelInterpreter.asm:
* Source/JavaScriptCore/runtime/Options.cpp:
(JSC::Options::notifyOptionsChanged):
* Source/JavaScriptCore/wasm/WasmBBQJIT.cpp:
(JSC::Wasm::BBQJITImpl::RegisterBinding::none):
* Source/JavaScriptCore/wasm/WasmBBQJIT.h:
* Source/JavaScriptCore/wasm/WasmBBQJIT64.cpp:
(JSC::Wasm::BBQJITImpl::BBQJIT::addSIMDV_V):
* Source/JavaScriptCore/wasm/WasmIRGeneratorHelpers.h:
(JSC::Wasm::buildEntryBufferForCatchSIMD):
(JSC::Wasm::buildEntryBufferForCatchNoSIMD):
* Source/JavaScriptCore/wasm/WasmOMGIRGenerator.cpp:
* Source/cmake/OptionsJSCOnly.cmake:
* Source/cmake/OptionsWin.cmake:

Canonical link: https://commits.webkit.org/280777@main
@webkit-commit-queue

Copy link
Copy Markdown
Collaborator

Committed 280777@main (dc849c8): https://commits.webkit.org/280777@main

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

@webkit-commit-queue webkit-commit-queue merged commit dc849c8 into WebKit:main Jul 9, 2024
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jul 9, 2024
@iangrunert iangrunert self-assigned this Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants