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] Clean up WasmToJS #8944

Conversation

Constellation
Copy link
Member

@Constellation Constellation commented Jan 22, 2023

db4b9bc

[JSC] Clean up WasmToJS
https://bugs.webkit.org/show_bug.cgi?id=250966
rdar://104523769

Reviewed by Tadeu Zagallo.

This patch fixes terrible hack in WasmToJS where we are setting JSWebAssemblyInstance to |callee| slot.

1. We should use Wasm::Callee since this is wasm code. This patch adds WasmToJSCallee and materialize this.
   We also puts Wasm::Instance* in |codeBlock| slot too so that operation function from this code can get
   Wasm::Instance*.
2. Remove indirection of types of owners. We should tightly couple JSC with wasm engine.
3. Put JSGlobalObject* in Wasm::Instance* to make WasmToJS code simpler and efficient.
4. Remove CallFrame::isAnyWasmCallee. Now all Wasm frames have Wasm::Callee*. So this hack is no longer necessary.

* JSTests/stress/sampling-profiler-wasm-name-section.js:
(platformSupportsSamplingProfiler.vm.isWasmSupported):
* Source/JavaScriptCore/bytecode/CodeBlock.cpp:
(JSC::CodeBlock::noticeIncomingCall):
* Source/JavaScriptCore/bytecode/Repatch.cpp:
(JSC::linkMonomorphicCall):
(JSC::linkVirtualFor):
(JSC::linkPolymorphicCall):
* Source/JavaScriptCore/debugger/DebuggerCallFrame.cpp:
(JSC::DebuggerCallFrame::scope):
(JSC::DebuggerCallFrame::thisValue const):
(JSC::DebuggerCallFrame::evaluateWithScopeExtension):
(JSC::DebuggerCallFrame::sourceIDForCallFrame):
* Source/JavaScriptCore/dfg/DFGDoesGCCheck.cpp:
(JSC::DFG::DoesGCCheck::verifyCanGC):
* Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.cpp:
(Inspector::functionName):
* Source/JavaScriptCore/interpreter/CallFrame.cpp:
(JSC::CallFrame::friendlyFunctionName):
(JSC::CallFrame::dump const):
(JSC::CallFrame::lexicalGlobalObjectFromWasmCallee const):
(JSC::CallFrame::isAnyWasmCallee const): Deleted.
* Source/JavaScriptCore/interpreter/CallFrame.h:
* Source/JavaScriptCore/interpreter/Interpreter.cpp:
(JSC::UnwindFunctor::notifyDebuggerOfUnwinding):
(JSC::Interpreter::unwind):
* Source/JavaScriptCore/interpreter/ShadowChicken.cpp:
(JSC::ShadowChicken::update):
* Source/JavaScriptCore/interpreter/StackVisitor.cpp:
(JSC::StackVisitor::readFrame):
(JSC::StackVisitor::readNonInlinedFrame):
(JSC::StackVisitor::Frame::calleeSaveRegistersForUnwinding):
* Source/JavaScriptCore/jit/JITExceptions.cpp:
(JSC::genericUnwind):
* Source/JavaScriptCore/jit/PolymorphicCallStubRoutine.cpp:
(JSC::PolymorphicCallStubRoutine::PolymorphicCallStubRoutine):
* Source/JavaScriptCore/jsc.cpp:
(JSC_DEFINE_HOST_FUNCTION):
* Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp:
(JSC::getCallerCodeBlock):
* Source/JavaScriptCore/runtime/VM.cpp:
(JSC::VM::throwException):
* Source/JavaScriptCore/runtime/VMTraps.cpp:
(JSC::VMTraps::invalidateCodeBlocksOnStack):
* Source/JavaScriptCore/tools/VMInspector.cpp:
(JSC::VMInspector::dumpRegisters):
* Source/JavaScriptCore/wasm/WasmCallee.cpp:
(JSC::Wasm::Callee::runWithDowncast):
(JSC::Wasm::WasmToJSCallee::WasmToJSCallee):
* Source/JavaScriptCore/wasm/WasmCallee.h:
* Source/JavaScriptCore/wasm/WasmCompilationMode.cpp:
(JSC::Wasm::makeString):
* Source/JavaScriptCore/wasm/WasmCompilationMode.h:
(JSC::Wasm::isOSREntry):
(JSC::Wasm::isAnyBBQ):
(JSC::Wasm::isAnyOMG):
* Source/JavaScriptCore/wasm/WasmGlobal.cpp:
* Source/JavaScriptCore/wasm/WasmGlobal.h:
* Source/JavaScriptCore/wasm/WasmInstance.cpp:
(JSC::Wasm::Instance::Instance):
(JSC::Wasm::Instance::create):
(JSC::Wasm::Instance::setGlobal):
(JSC::Wasm::Instance::setFunctionWrapper):
(JSC::Wasm::Instance::initElementSegment):
* Source/JavaScriptCore/wasm/WasmInstance.h:
(JSC::Wasm::Instance::setOwner):
(JSC::Wasm::Instance::owner const):
(JSC::Wasm::Instance::offsetOfGlobalObject):
(JSC::Wasm::Instance::globalObject const):
(JSC::Wasm::Instance::allocationSize):
* Source/JavaScriptCore/wasm/WasmModule.cpp:
(JSC::Wasm::Module::Module):
* Source/JavaScriptCore/wasm/WasmModule.h:
(JSC::Wasm::Module::wasmToJSCallee):
* Source/JavaScriptCore/wasm/WasmOperations.cpp:
(JSC::Wasm::JSC_DEFINE_JIT_OPERATION):
* Source/JavaScriptCore/wasm/WasmOperations.h:
* Source/JavaScriptCore/wasm/WasmOperationsInlines.h:
(JSC::Wasm::arrayNew):
(JSC::Wasm::structNew):
(JSC::Wasm::structSet):
(JSC::Wasm::throwWasmToJSException):
* Source/JavaScriptCore/wasm/WasmSlowPaths.cpp:
(JSC::LLInt::WASM_SLOW_PATH_DECL):
* Source/JavaScriptCore/wasm/WasmTable.cpp:
* Source/JavaScriptCore/wasm/WasmTable.h:
(JSC::Wasm::Table::owner const):
(JSC::Wasm::Table::setOwner):
* Source/JavaScriptCore/wasm/js/JSWebAssemblyInstance.cpp:
(JSC::JSWebAssemblyInstance::visitChildrenImpl):
(JSC::JSWebAssemblyInstance::tryCreate):
* Source/JavaScriptCore/wasm/js/JSWebAssemblyInstance.h:
* Source/JavaScriptCore/wasm/js/JSWebAssemblyModule.cpp:
(JSC::JSWebAssemblyModule::generateWasmToJSStubs):
* Source/JavaScriptCore/wasm/js/WasmToJS.cpp:
(JSC::Wasm::handleBadImportTypeUse):
(JSC::Wasm::wasmToJS):
* Source/JavaScriptCore/wasm/js/WasmToJS.h:
* Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.cpp:
(JSC::WebAssemblyModuleRecord::initializeExports):

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

327ecd4

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  πŸ§ͺ win
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ›  gtk βœ… πŸ›  wincairo
βœ… πŸ§ͺ 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
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch-sim βœ… πŸ§ͺ jsc-mips-tests

@Constellation Constellation self-assigned this Jan 22, 2023
@Constellation Constellation added the JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues. label Jan 22, 2023
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 22, 2023
@Constellation Constellation removed the merging-blocked Applied to prevent a change from being merged label Jan 22, 2023
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 22, 2023
@Constellation Constellation removed the merging-blocked Applied to prevent a change from being merged label Jan 22, 2023
Copy link
Member

@tadeuzagallo tadeuzagallo left a comment

Choose a reason for hiding this comment

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

This is great, so much better!

@Constellation Constellation added the merge-queue Applied to send a pull request to merge-queue label Jan 23, 2023
https://bugs.webkit.org/show_bug.cgi?id=250966
rdar://104523769

Reviewed by Tadeu Zagallo.

This patch fixes terrible hack in WasmToJS where we are setting JSWebAssemblyInstance to |callee| slot.

1. We should use Wasm::Callee since this is wasm code. This patch adds WasmToJSCallee and materialize this.
   We also puts Wasm::Instance* in |codeBlock| slot too so that operation function from this code can get
   Wasm::Instance*.
2. Remove indirection of types of owners. We should tightly couple JSC with wasm engine.
3. Put JSGlobalObject* in Wasm::Instance* to make WasmToJS code simpler and efficient.
4. Remove CallFrame::isAnyWasmCallee. Now all Wasm frames have Wasm::Callee*. So this hack is no longer necessary.

* JSTests/stress/sampling-profiler-wasm-name-section.js:
(platformSupportsSamplingProfiler.vm.isWasmSupported):
* Source/JavaScriptCore/bytecode/CodeBlock.cpp:
(JSC::CodeBlock::noticeIncomingCall):
* Source/JavaScriptCore/bytecode/Repatch.cpp:
(JSC::linkMonomorphicCall):
(JSC::linkVirtualFor):
(JSC::linkPolymorphicCall):
* Source/JavaScriptCore/debugger/DebuggerCallFrame.cpp:
(JSC::DebuggerCallFrame::scope):
(JSC::DebuggerCallFrame::thisValue const):
(JSC::DebuggerCallFrame::evaluateWithScopeExtension):
(JSC::DebuggerCallFrame::sourceIDForCallFrame):
* Source/JavaScriptCore/dfg/DFGDoesGCCheck.cpp:
(JSC::DFG::DoesGCCheck::verifyCanGC):
* Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.cpp:
(Inspector::functionName):
* Source/JavaScriptCore/interpreter/CallFrame.cpp:
(JSC::CallFrame::friendlyFunctionName):
(JSC::CallFrame::dump const):
(JSC::CallFrame::lexicalGlobalObjectFromWasmCallee const):
(JSC::CallFrame::isAnyWasmCallee const): Deleted.
* Source/JavaScriptCore/interpreter/CallFrame.h:
* Source/JavaScriptCore/interpreter/Interpreter.cpp:
(JSC::UnwindFunctor::notifyDebuggerOfUnwinding):
(JSC::Interpreter::unwind):
* Source/JavaScriptCore/interpreter/ShadowChicken.cpp:
(JSC::ShadowChicken::update):
* Source/JavaScriptCore/interpreter/StackVisitor.cpp:
(JSC::StackVisitor::readFrame):
(JSC::StackVisitor::readNonInlinedFrame):
(JSC::StackVisitor::Frame::calleeSaveRegistersForUnwinding):
* Source/JavaScriptCore/jit/JITExceptions.cpp:
(JSC::genericUnwind):
* Source/JavaScriptCore/jit/PolymorphicCallStubRoutine.cpp:
(JSC::PolymorphicCallStubRoutine::PolymorphicCallStubRoutine):
* Source/JavaScriptCore/jsc.cpp:
(JSC_DEFINE_HOST_FUNCTION):
* Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp:
(JSC::getCallerCodeBlock):
* Source/JavaScriptCore/runtime/VM.cpp:
(JSC::VM::throwException):
* Source/JavaScriptCore/runtime/VMTraps.cpp:
(JSC::VMTraps::invalidateCodeBlocksOnStack):
* Source/JavaScriptCore/tools/VMInspector.cpp:
(JSC::VMInspector::dumpRegisters):
* Source/JavaScriptCore/wasm/WasmCallee.cpp:
(JSC::Wasm::Callee::runWithDowncast):
(JSC::Wasm::WasmToJSCallee::WasmToJSCallee):
* Source/JavaScriptCore/wasm/WasmCallee.h:
* Source/JavaScriptCore/wasm/WasmCompilationMode.cpp:
(JSC::Wasm::makeString):
* Source/JavaScriptCore/wasm/WasmCompilationMode.h:
(JSC::Wasm::isOSREntry):
(JSC::Wasm::isAnyBBQ):
(JSC::Wasm::isAnyOMG):
* Source/JavaScriptCore/wasm/WasmGlobal.cpp:
* Source/JavaScriptCore/wasm/WasmGlobal.h:
* Source/JavaScriptCore/wasm/WasmInstance.cpp:
(JSC::Wasm::Instance::Instance):
(JSC::Wasm::Instance::create):
(JSC::Wasm::Instance::setGlobal):
(JSC::Wasm::Instance::setFunctionWrapper):
(JSC::Wasm::Instance::initElementSegment):
* Source/JavaScriptCore/wasm/WasmInstance.h:
(JSC::Wasm::Instance::setOwner):
(JSC::Wasm::Instance::owner const):
(JSC::Wasm::Instance::offsetOfGlobalObject):
(JSC::Wasm::Instance::globalObject const):
(JSC::Wasm::Instance::allocationSize):
* Source/JavaScriptCore/wasm/WasmModule.cpp:
(JSC::Wasm::Module::Module):
* Source/JavaScriptCore/wasm/WasmModule.h:
(JSC::Wasm::Module::wasmToJSCallee):
* Source/JavaScriptCore/wasm/WasmOperations.cpp:
(JSC::Wasm::JSC_DEFINE_JIT_OPERATION):
* Source/JavaScriptCore/wasm/WasmOperations.h:
* Source/JavaScriptCore/wasm/WasmOperationsInlines.h:
(JSC::Wasm::arrayNew):
(JSC::Wasm::structNew):
(JSC::Wasm::structSet):
(JSC::Wasm::throwWasmToJSException):
* Source/JavaScriptCore/wasm/WasmSlowPaths.cpp:
(JSC::LLInt::WASM_SLOW_PATH_DECL):
* Source/JavaScriptCore/wasm/WasmTable.cpp:
* Source/JavaScriptCore/wasm/WasmTable.h:
(JSC::Wasm::Table::owner const):
(JSC::Wasm::Table::setOwner):
* Source/JavaScriptCore/wasm/js/JSWebAssemblyInstance.cpp:
(JSC::JSWebAssemblyInstance::visitChildrenImpl):
(JSC::JSWebAssemblyInstance::tryCreate):
* Source/JavaScriptCore/wasm/js/JSWebAssemblyInstance.h:
* Source/JavaScriptCore/wasm/js/JSWebAssemblyModule.cpp:
(JSC::JSWebAssemblyModule::generateWasmToJSStubs):
* Source/JavaScriptCore/wasm/js/WasmToJS.cpp:
(JSC::Wasm::handleBadImportTypeUse):
(JSC::Wasm::wasmToJS):
* Source/JavaScriptCore/wasm/js/WasmToJS.h:
* Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.cpp:
(JSC::WebAssemblyModuleRecord::initializeExports):

Canonical link: https://commits.webkit.org/259216@main
@webkit-commit-queue
Copy link
Collaborator

Committed 259216@main (db4b9bc): https://commits.webkit.org/259216@main

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

@webkit-early-warning-system webkit-early-warning-system merged commit db4b9bc into WebKit:main Jan 23, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jan 23, 2023
@Constellation Constellation deleted the eng/JSC-Clean-up-WasmToJS branch January 23, 2023 18:53
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