Summary: | [JSC] Implement a B3::Compilation replacement for wasm-llint | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Xan Lopez <xan.lopez> | ||||||||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | annulen, ews-watchlist, gyuyoung.kim, keith_miller, mark.lam, msaboff, ryuan.choi, saam, sergio, tzagallo, webkit-bug-importer, xan.lopez, ysuzuki | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 220365 | ||||||||||||
Attachments: |
|
Description
Xan Lopez
2021-01-13 03:57:46 PST
B3::Compilation is basically: - A MacroAssemblerCodeRef. - A vector of opaque "compilation byproducts". This, incidentally, is never used by WasmLLInt (it's always set to null). I see a few options: - We could move this to jit/. This would remove the dependency from B3, but would still be a sort of layering violation, since it would make LLInt code depend on jit/ (which is still the case in other places, for now). - We could move this even further down the stack? - We could make WasmLLInt use something else when B3 is disabled, and keep the code as-is when B3 is enabled. I suppose the cleanest solutions in terms of the end result would be either #2 or #1, but comments are welcome. Created attachment 417704 [details]
WIP.
WIP patch.
Having a closer look at the code it seems to me there's so much stuff depending on JIT atm that there's no point in trying to put B3Compilation elsewhere. Since it fits nicely there, let's start with that. WIP patch attached, 100% mechanical (I've left the JSC namespace in front of classes so I can grep easily, but it could be removed in the final patch).
Created attachment 417707 [details]
WIP.
Created attachment 417764 [details]
Move B3::Compilation to jit/, v1
Try the build bots, v1
Comment on attachment 417764 [details] Move B3::Compilation to jit/, v1 View in context: https://bugs.webkit.org/attachment.cgi?id=417764&action=review r=me > Source/JavaScriptCore/b3/air/testair.cpp:88 > +std::unique_ptr<JSC::Compilation> compile(B3::Procedure& proc) JSC:: of JSC::Compilation is not necessary. > Source/JavaScriptCore/b3/air/testair.cpp:95 > + return makeUnique<JSC::Compilation>( JSC:: is not necessary. > Source/JavaScriptCore/b3/air/testair.cpp:100 > +T invoke(const JSC::Compilation& code, Arguments... arguments) Ditto. > Source/JavaScriptCore/ftl/FTLJITCode.h:55 > + void initializeB3Byproducts(std::unique_ptr<JSC::OpaqueByproducts>); JSC:: of JSC::OpaqueByproducts is not necessary. > Source/JavaScriptCore/ftl/FTLJITCode.h:80 > + std::unique_ptr<JSC::OpaqueByproducts> m_b3Byproducts; Ditto. > Source/JavaScriptCore/wasm/WasmB3IRGenerator.h:50 > + std::unique_ptr<JSC::OpaqueByproducts> wasmEntrypointByproducts; Ditto > Source/JavaScriptCore/wasm/WasmBBQPlan.cpp:113 > + function->entrypoint.compilation = makeUnique<JSC::Compilation>( Ditto > Source/JavaScriptCore/wasm/WasmBBQPlan.cpp:229 > + m_wasmInternalFunctions[functionIndex]->entrypoint.compilation = makeUnique<JSC::Compilation>( Ditto. > Source/JavaScriptCore/wasm/WasmBBQPlan.cpp:241 > + embedderToWasmInternalFunction->entrypoint.compilation = makeUnique<JSC::Compilation>( Ditto > Source/JavaScriptCore/wasm/WasmFormat.h:341 > + std::unique_ptr<JSC::Compilation> compilation; Ditto. > Source/JavaScriptCore/wasm/WasmLLIntPlan.cpp:155 > + function->entrypoint.compilation = makeUnique<JSC::Compilation>( Ditto. > Source/JavaScriptCore/wasm/WasmOMGForOSREntryPlan.cpp:92 > + omgEntrypoint.compilation = makeUnique<JSC::Compilation>( Ditto > Source/JavaScriptCore/wasm/WasmOMGPlan.cpp:89 > + omgEntrypoint.compilation = makeUnique<JSC::Compilation>( Ditto. Created attachment 417827 [details]
Move B3::Compilation to jit/, v2
v2, remove unnecessary JSC:: namespace (which also found a redundant and mistaken B3::Compilation declaration left).
Committed r271594: <https://trac.webkit.org/changeset/271594> All reviewed patches have been landed. Closing bug and clearing flags on attachment 417827 [details]. |