Skip to content

Conversation

@takikawa
Copy link
Contributor

@takikawa takikawa commented Aug 28, 2023

9a518a5

Re-land [WASM-Function-References] Fix block signature parsing for reftypes https://bugs.webkit.org/show_bug.cgi?id=247383

Reviewed by Justin Michaud.

This is a re-land of this patch that removes a perf issue introduced in
the previous attempts.

Adds a fast path for parsing potentially nested blocks with simple (one byte)
type signatures. Also changes BlockSignature definition in order to avoid
redundant calls to as<FunctionSignature>().

Also fixes the error case when a block signature refers to a non-function type.

* JSTests/wasm/function-references/block_signature.js: Added.
(module):
(async blockSignatureTest):
* JSTests/wasm/gc-spec-tests/type-equivalence.wast.js:
* JSTests/wasm/gc-spec-tests/type-subtyping.wast.js:
* JSTests/wasm/gc/block.js: Added.
(testBlockType):
* Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:
(JSC::Wasm::B3IRGenerator::ControlData::ControlData):
(JSC::Wasm::B3IRGenerator::ControlData::hasNonVoidresult const):
(JSC::Wasm::B3IRGenerator::ControlData::branchTargetArity const):
(JSC::Wasm::B3IRGenerator::ControlData::branchTargetType const):
(JSC::Wasm::B3IRGenerator::toB3ResultType):
(JSC::Wasm::B3IRGenerator::addLoop):
(JSC::Wasm::B3IRGenerator::addElseToUnreachable):
(JSC::Wasm::B3IRGenerator::endBlock):
(JSC::Wasm::B3IRGenerator::addEndToUnreachable):
* Source/JavaScriptCore/wasm/WasmBBQJIT.cpp:
(JSC::Wasm::BBQJIT::ControlData::ControlData):
(JSC::Wasm::BBQJIT::ControlData::branchTargetArity const):
(JSC::Wasm::BBQJIT::ControlData::branchTargetType const):
(JSC::Wasm::BBQJIT::ControlData::argumentType const):
(JSC::Wasm::BBQJIT::addBlock):
(JSC::Wasm::BBQJIT::addLoop):
(JSC::Wasm::BBQJIT::addIf):
(JSC::Wasm::BBQJIT::addElse):
(JSC::Wasm::BBQJIT::addElseToUnreachable):
(JSC::Wasm::BBQJIT::addTry):
(JSC::Wasm::BBQJIT::addEndToUnreachable):
* Source/JavaScriptCore/wasm/WasmCallingConvention.h:
(JSC::Wasm::WasmCallingConvention::callInformationFor const):
* Source/JavaScriptCore/wasm/WasmFormat.h:
(JSC::Wasm::isValueType):
* Source/JavaScriptCore/wasm/WasmFunctionParser.h:
(JSC::Wasm::splitStack):
(JSC::Wasm::FunctionParser<Context>::parseBody):
(JSC::Wasm::FunctionParser<Context>::unify):
(JSC::Wasm::FunctionParser<Context>::parseNestedBlocksEagerly):
(JSC::Wasm::FunctionParser<Context>::parseExpression):
(JSC::Wasm::FunctionParser<Context>::switchToBlock):
* Source/JavaScriptCore/wasm/WasmIPIntGenerator.cpp:
(JSC::Wasm::IPIntControlType::branchTargetType const):
(JSC::Wasm::IPIntControlType::branchTargetArity const):
(JSC::Wasm::IPIntGenerator::addElse):
(JSC::Wasm::IPIntGenerator::addEndToUnreachable):
(JSC::Wasm::IPIntGenerator::endTopLevel):
* Source/JavaScriptCore/wasm/WasmLLIntGenerator.cpp:
(JSC::Wasm::LLIntGenerator::ControlType::loop):
(JSC::Wasm::LLIntGenerator::ControlType::block):
(JSC::Wasm::LLIntGenerator::ControlType::if_):
(JSC::Wasm::LLIntGenerator::ControlType::createTry):
(JSC::Wasm::LLIntGenerator::ControlType::branchTargetArity const):
(JSC::Wasm::LLIntGenerator::ControlType::branchTargetType const):
(JSC::Wasm::LLIntGenerator::addElseToUnreachable):
(JSC::Wasm::LLIntGenerator::addReturn):
(JSC::Wasm::LLIntGenerator::addEndToUnreachable):
(JSC::Wasm::LLIntGenerator::endTopLevel):
* Source/JavaScriptCore/wasm/WasmParser.h:
(JSC::Wasm::Parser<SuccessType>::parseBlockSignature):
(JSC::Wasm::Parser<SuccessType>::parseReftypeSignature):
* Source/JavaScriptCore/wasm/WasmTypeDefinition.cpp:
(JSC::Wasm::TypeInformation::TypeInformation):
* Source/JavaScriptCore/wasm/WasmTypeDefinition.h:
(JSC::Wasm::TypeInformation::thunkFor const):

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

bca8358

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

@takikawa takikawa requested a review from a team as a code owner August 28, 2023 21:41
@takikawa takikawa self-assigned this Aug 28, 2023
@takikawa takikawa added the WebAssembly For bugs in JavaScript WebAssembly label Aug 28, 2023
@takikawa takikawa changed the title Re-land [WASM-Function-References] Fix block signature parsing for reftypes Re-land [WASM-Function-References] Fix block signature parsing for reftypes https://bugs.webkit.org/show_bug.cgi?id=247383 Nov 22, 2023
@takikawa takikawa force-pushed the eng/WASM-Function-References-Fix-block-signature-parsing-for-reftypes branch from 97a7f7f to df44828 Compare November 22, 2023 07:05
Copy link
Contributor

@justinmichaud justinmichaud 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 some tiny suggestions to make the control flow more clear

@takikawa takikawa force-pushed the eng/WASM-Function-References-Fix-block-signature-parsing-for-reftypes branch from df44828 to bca8358 Compare November 28, 2023 21:07
@takikawa takikawa added the safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks label Nov 28, 2023
@webkit-ews-buildbot
Copy link
Collaborator

Failed ios-wk2-wpt, api-gtk checks. Please resolve failures and re-apply safe-merge-queue label.

Rejecting #17146 from merge queue.

@webkit-ews-buildbot webkit-ews-buildbot added merging-blocked Applied to prevent a change from being merged and removed safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks labels Nov 29, 2023
@webkit-ews-buildbot
Copy link
Collaborator

Safe-Merge-Queue: Build #4713.

@takikawa takikawa added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels Nov 29, 2023
…ftypes https://bugs.webkit.org/show_bug.cgi?id=247383

Reviewed by Justin Michaud.

This is a re-land of this patch that removes a perf issue introduced in
the previous attempts.

Adds a fast path for parsing potentially nested blocks with simple (one byte)
type signatures. Also changes BlockSignature definition in order to avoid
redundant calls to as<FunctionSignature>().

Also fixes the error case when a block signature refers to a non-function type.

* JSTests/wasm/function-references/block_signature.js: Added.
(module):
(async blockSignatureTest):
* JSTests/wasm/gc-spec-tests/type-equivalence.wast.js:
* JSTests/wasm/gc-spec-tests/type-subtyping.wast.js:
* JSTests/wasm/gc/block.js: Added.
(testBlockType):
* Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:
(JSC::Wasm::B3IRGenerator::ControlData::ControlData):
(JSC::Wasm::B3IRGenerator::ControlData::hasNonVoidresult const):
(JSC::Wasm::B3IRGenerator::ControlData::branchTargetArity const):
(JSC::Wasm::B3IRGenerator::ControlData::branchTargetType const):
(JSC::Wasm::B3IRGenerator::toB3ResultType):
(JSC::Wasm::B3IRGenerator::addLoop):
(JSC::Wasm::B3IRGenerator::addElseToUnreachable):
(JSC::Wasm::B3IRGenerator::endBlock):
(JSC::Wasm::B3IRGenerator::addEndToUnreachable):
* Source/JavaScriptCore/wasm/WasmBBQJIT.cpp:
(JSC::Wasm::BBQJIT::ControlData::ControlData):
(JSC::Wasm::BBQJIT::ControlData::branchTargetArity const):
(JSC::Wasm::BBQJIT::ControlData::branchTargetType const):
(JSC::Wasm::BBQJIT::ControlData::argumentType const):
(JSC::Wasm::BBQJIT::addBlock):
(JSC::Wasm::BBQJIT::addLoop):
(JSC::Wasm::BBQJIT::addIf):
(JSC::Wasm::BBQJIT::addElse):
(JSC::Wasm::BBQJIT::addElseToUnreachable):
(JSC::Wasm::BBQJIT::addTry):
(JSC::Wasm::BBQJIT::addEndToUnreachable):
* Source/JavaScriptCore/wasm/WasmCallingConvention.h:
(JSC::Wasm::WasmCallingConvention::callInformationFor const):
* Source/JavaScriptCore/wasm/WasmFormat.h:
(JSC::Wasm::isValueType):
* Source/JavaScriptCore/wasm/WasmFunctionParser.h:
(JSC::Wasm::splitStack):
(JSC::Wasm::FunctionParser<Context>::parseBody):
(JSC::Wasm::FunctionParser<Context>::unify):
(JSC::Wasm::FunctionParser<Context>::parseNestedBlocksEagerly):
(JSC::Wasm::FunctionParser<Context>::parseExpression):
(JSC::Wasm::FunctionParser<Context>::switchToBlock):
* Source/JavaScriptCore/wasm/WasmIPIntGenerator.cpp:
(JSC::Wasm::IPIntControlType::branchTargetType const):
(JSC::Wasm::IPIntControlType::branchTargetArity const):
(JSC::Wasm::IPIntGenerator::addElse):
(JSC::Wasm::IPIntGenerator::addEndToUnreachable):
(JSC::Wasm::IPIntGenerator::endTopLevel):
* Source/JavaScriptCore/wasm/WasmLLIntGenerator.cpp:
(JSC::Wasm::LLIntGenerator::ControlType::loop):
(JSC::Wasm::LLIntGenerator::ControlType::block):
(JSC::Wasm::LLIntGenerator::ControlType::if_):
(JSC::Wasm::LLIntGenerator::ControlType::createTry):
(JSC::Wasm::LLIntGenerator::ControlType::branchTargetArity const):
(JSC::Wasm::LLIntGenerator::ControlType::branchTargetType const):
(JSC::Wasm::LLIntGenerator::addElseToUnreachable):
(JSC::Wasm::LLIntGenerator::addReturn):
(JSC::Wasm::LLIntGenerator::addEndToUnreachable):
(JSC::Wasm::LLIntGenerator::endTopLevel):
* Source/JavaScriptCore/wasm/WasmParser.h:
(JSC::Wasm::Parser<SuccessType>::parseBlockSignature):
(JSC::Wasm::Parser<SuccessType>::parseReftypeSignature):
* Source/JavaScriptCore/wasm/WasmTypeDefinition.cpp:
(JSC::Wasm::TypeInformation::TypeInformation):
* Source/JavaScriptCore/wasm/WasmTypeDefinition.h:
(JSC::Wasm::TypeInformation::thunkFor const):

Canonical link: https://commits.webkit.org/271262@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/WASM-Function-References-Fix-block-signature-parsing-for-reftypes branch from bca8358 to 9a518a5 Compare November 29, 2023 06:47
@webkit-commit-queue
Copy link
Collaborator

Committed 271262@main (9a518a5): https://commits.webkit.org/271262@main

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

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Nov 29, 2023
@webkit-commit-queue webkit-commit-queue merged commit 9a518a5 into WebKit:main Nov 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WebAssembly For bugs in JavaScript WebAssembly

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants