Bug 220412 – [JSC] Implement a B3::ValueRep replacement for wasm-llint

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Xan Lopez]

This is used unconditionally by WebAssembly (even at the interpreter level), so we need to move it somewhere else if we want a B3-less implementation. It's a self-contained class mostly tasked with representing how a value is laid out. See bug #220365.

[Xan Lopez]

Created attachment 417175 [details]
WIP patch.

WIP.

A couple things:
- There's a node type in DFG also named ValueRep. In some cases I had to add extra namespace information to avoid ambiguities.
- See the changed comment in ValueRep.h wrt ValueRecovery. It seems to me we could try to create a class that could be used by the wasm code using ValueRep, either based on ValueRecovery or merging bits from both. In any case probably moving stuff to bytecode/ first is a good idea.

[Xan Lopez]

After some talk today offline we'll try to come up with a class that WasmCallingConvention can use just for its needs, to remove the B3::ValueRep dependency on LLInt code. Possibly will need o translator to B3::ValueRep for the B3/Air generators in wasm/, but that should be simple? We'll see.

[Xan Lopez]

Created attachment 417453 [details]
WIP patch.

New WIP patch.

This just takes B3::ValueRep, removes everything that Wasm/LLInt does not use (most of it), renames it to WasmValueLocation and uses it where needed. I added a method to convert a WasmValueLocation to a B3::ValueRep and use it in the B3/Air generators, which actually do need to use the full B3::ValueRep.

[Saam Barati]

Comment on attachment 417453 [details]
WIP patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=417453&action=review

> Source/JavaScriptCore/wasm/WasmValueLocation.h:65
> +    static ValueLocation stack(intptr_t offsetFromFP)

do we really need this and stackArgument?

[Xan Lopez]

(In reply to Saam Barati from comment #4)
> Comment on attachment 417453 [details]
> WIP patch.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=417453&action=review
> 
> > Source/JavaScriptCore/wasm/WasmValueLocation.h:65
> > +    static ValueLocation stack(intptr_t offsetFromFP)
> 
> do we really need this and stackArgument?

All three 'kinds' left in the class (reg, stack, stackArgument) are used in WasmCallingConvention (see marshallLocationImpl), if that's what you are asking. If you mean whether the class *really* needs that, not sure. The code seems sensible enough, but it's the first time I work on it.

[Radar WebKit Bug Importer]

<rdar://problem/73196482>

[Xan Lopez]

Created attachment 417624 [details]
WasmValueLocation, v1

v1, add the Mac build bits and fill in the ChangeLog.

[Xan Lopez]

Created attachment 417625 [details]
WasmValueLocation, v2

v2, try again with the Mac build.

[Xan Lopez]

Created attachment 417626 [details]
WasmValueLocation, v3

v3, try to fix the Watch build.

[Yusuke Suzuki]

Comment on attachment 417626 [details]
WasmValueLocation, v3

r=me

[EWS]

Committed r271500: <https://trac.webkit.org/changeset/271500>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 417626 [details].