RESOLVED FIXED 220585
[JSC] Implement a B3::Compilation replacement for wasm-llint
https://bugs.webkit.org/show_bug.cgi?id=220585
Summary [JSC] Implement a B3::Compilation replacement for wasm-llint
Xan Lopez
Reported 2021-01-13 03:57:46 PST
This is used by WasmLLIntPlan, which prevents us from compiling Wasm without B3 support.
Attachments
WIP. (40.04 KB, patch)
2021-01-15 08:55 PST, Xan Lopez
no flags
WIP. (40.89 KB, patch)
2021-01-15 09:13 PST, Xan Lopez
no flags
Move B3::Compilation to jit/, v1 (58.43 KB, patch)
2021-01-16 02:24 PST, Xan Lopez
ysuzuki: review+
ysuzuki: commit-queue-
Move B3::Compilation to jit/, v2 (58.94 KB, patch)
2021-01-18 03:26 PST, Xan Lopez
no flags
Xan Lopez
Comment 1 2021-01-13 04:05:50 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.
Xan Lopez
Comment 2 2021-01-15 08:55:09 PST
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).
Xan Lopez
Comment 3 2021-01-15 09:13:01 PST
Xan Lopez
Comment 4 2021-01-16 02:24:34 PST
Created attachment 417764 [details] Move B3::Compilation to jit/, v1 Try the build bots, v1
Yusuke Suzuki
Comment 5 2021-01-16 14:47:45 PST
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.
Xan Lopez
Comment 6 2021-01-18 03:26:16 PST
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).
EWS
Comment 7 2021-01-18 16:24:57 PST
Committed r271594: <https://trac.webkit.org/changeset/271594> All reviewed patches have been landed. Closing bug and clearing flags on attachment 417827 [details].
Radar WebKit Bug Importer
Comment 8 2021-01-18 16:25:26 PST
Note You need to log in before you can comment on or make changes to this bug.