Bug 220585

Summary: [JSC] Implement a B3::Compilation replacement for wasm-llint
Product: WebKit Reporter: Xan Lopez <xan.lopez>
Component: JavaScriptCoreAssignee: 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 Flags
WIP.
none
WIP.
none
Move B3::Compilation to jit/, v1
ysuzuki: review+, ysuzuki: commit-queue-
Move B3::Compilation to jit/, v2 none

Description Xan Lopez 2021-01-13 03:57:46 PST
This is used by WasmLLIntPlan, which prevents us from compiling Wasm without B3 support.
Comment 1 Xan Lopez 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.
Comment 2 Xan Lopez 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).
Comment 3 Xan Lopez 2021-01-15 09:13:01 PST
Created attachment 417707 [details]
WIP.
Comment 4 Xan Lopez 2021-01-16 02:24:34 PST
Created attachment 417764 [details]
Move B3::Compilation to jit/, v1

Try the build bots, v1
Comment 5 Yusuke Suzuki 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.
Comment 6 Xan Lopez 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).
Comment 7 EWS 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].
Comment 8 Radar WebKit Bug Importer 2021-01-18 16:25:26 PST
<rdar://problem/73332409>