-
Notifications
You must be signed in to change notification settings - Fork 1.8k
[JSC] Introduce B3 WasmRefCast / WasmRefTest values #56950
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] Introduce B3 WasmRefCast / WasmRefTest values #56950
Conversation
|
EWS run on previous version of this PR (hash eda956d) Details
|
eda956d to
d973293
Compare
|
EWS run on previous version of this PR (hash d973293) Details |
Safer C++ Build #75642 (d973293)❌ Found 1 failing file with 1 issue. Please address these issues before landing. See WebKit Guidelines for Safer C++ Programming. |
d973293 to
d2d56b4
Compare
|
EWS run on previous version of this PR (hash d2d56b4) Details
|
d2d56b4 to
7f80ffb
Compare
|
EWS run on previous version of this PR (hash 7f80ffb) Details |
7f80ffb to
a743328
Compare
|
EWS run on previous version of this PR (hash a743328) Details
|
a743328 to
f7d8aeb
Compare
|
EWS run on previous version of this PR (hash f7d8aeb) Details
|
f7d8aeb to
5ff9619
Compare
|
EWS run on previous version of this PR (hash 5ff9619) Details |
5ff9619 to
7056696
Compare
|
EWS run on previous version of this PR (hash 7056696) Details
|
7056696 to
ea12212
Compare
|
EWS run on previous version of this PR (hash ea12212) Details |
kmiller68
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r=me if you get rid of the extra subclasses for B3::WasmRefTypeCheckValue
| BasicBlock* before = m_blockInsertionSet.splitForward(m_block, m_index, &m_insertionSet); | ||
| BasicBlock* continuation = m_block; | ||
| m_value->replaceWithIdentity(emitRefTestOrCast(typeCheck, before, continuation)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like most of the time for Casts we wouldn't have to create the continuation. Might be worth a FIXME to only split the block if we're actually inserting new control flow.
| HasRTT = 1 << 5, // Whether m_targetRTT is non-null (vs using targetHeapType) | ||
| }; | ||
|
|
||
| class WasmRefTypeCheckValue : public Value { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like we can just have WasmRefCast and WasmRefTest share the same WasmRefTypeCheckValue and get rid of the specialized subclasses. Those subclasses don't have anything interesting in them and I think we mostly use different Values subclasses when we need extra fields, which isn't the case here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's nice, changed.
| }; | ||
|
|
||
| if (cast->allowNull()) { | ||
| // This is really common in J2CL, Java to wasm compiler. When you write a code in Java like, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be worth mentioning that this comment is from JS3, it's possible future versions of J2CL in JS4+ lower code differently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's good. Changed.
ea12212 to
013402d
Compare
|
EWS run on previous version of this PR (hash 013402d) Details |
013402d to
b515c3a
Compare
|
EWS run on previous version of this PR (hash b515c3a) Details
|
b515c3a to
77bc15b
Compare
|
EWS run on current version of this PR (hash 77bc15b) Details
|
https://bugs.webkit.org/show_bug.cgi?id=305912 rdar://168566739 Reviewed by Keith Miller. We introduce WasmRefCast and WasmRefTest B3 values. This is another high-level B3 nodes for WasmGC. This enables data-flow analysis for wasm GC operations in B3 finally. The generated code is literally just moved from OMG IR generator to B3LowerMacros. We introduce ValueKey Value::key CSE support for these values. Based on this high-level semantics, we start using it in ReduceStrength. We make WasmStructGet, WasmStructSet can remove trapping bits based on input's WasmRefCast. And WasmRefCast can make convert itself to non-nullable based on subsequent values in the same basic block. * Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj: * Source/JavaScriptCore/Sources.txt: * Source/JavaScriptCore/b3/B3Kind.h: (JSC::B3::Kind::hasTraps const): * Source/JavaScriptCore/b3/B3LowerMacros.cpp: * Source/JavaScriptCore/b3/B3Opcode.h: * Source/JavaScriptCore/b3/B3ReduceStrength.cpp: * Source/JavaScriptCore/b3/B3Validate.cpp: * Source/JavaScriptCore/b3/B3Value.cpp: (JSC::B3::Value::effects const): (JSC::B3::Value::key const): * Source/JavaScriptCore/b3/B3Value.h: * Source/JavaScriptCore/b3/B3ValueInlines.h: * Source/JavaScriptCore/b3/B3ValueKey.cpp: (JSC::B3::ValueKey::materialize const): * Source/JavaScriptCore/b3/B3ValueKey.h: * Source/JavaScriptCore/b3/B3ValueKeyInlines.h: (JSC::B3::ValueKey::ValueKey): * Source/JavaScriptCore/b3/B3WasmRefTypeCheckValue.cpp: Added. * Source/JavaScriptCore/b3/B3WasmRefTypeCheckValue.h: Added. * Source/JavaScriptCore/wasm/WasmOMGIRGenerator.cpp: (JSC::Wasm::OMGIRGenerator::emitRefTestOrCast): (JSC::Wasm::OMGIRGenerator::emitCheckOrBranchForCast): Deleted. * Source/JavaScriptCore/wasm/WasmTypeDefinition.cpp: (JSC::Wasm::RTT::RTT): (JSC::Wasm::RTT::tryCreate): (JSC::Wasm::TypeInformation::createCanonicalRTTForType): * Source/JavaScriptCore/wasm/WasmTypeDefinition.h: Canonical link: https://commits.webkit.org/306061@main
77bc15b to
ea11204
Compare
|
Committed 306061@main (ea11204): https://commits.webkit.org/306061@main Reviewed commits have been landed. Closing PR #56950 and removing active labels. |
🛠 ios-apple
ea11204
77bc15b
🛠 wpe🛠 win🧪 wpe-wk2🧪 win-tests🧪 ios-wk2🧪 api-mac🧪 api-wpe🧪 ios-wk2-wpt🧪 api-mac-debug🧪 api-ios🧪 mac-wk1🛠 🧪 jsc-debug-arm64🧪 mac-wk2🧪 mac-AS-debug-wk2🧪 vision-wk2🧪 mac-wk2-stress🛠 playstation🛠 tv🧪 mac-intel-wk2🛠 jsc-armv7🛠 mac-safer-cpp🧪 jsc-armv7-tests🛠 watch🛠 watch-sim