Bug 149033

Summary: Convert arguments to WebAssembly functions to the declared types
Product: WebKit Reporter: Sukolsak Sakshuwong <sukolsak>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, fpizlo, ggaren, keith_miller, saam, sukolsak
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 146064    
Attachments:
Description Flags
Patch none

Description Sukolsak Sakshuwong 2015-09-10 01:46:57 PDT 
Convert arguments to WebAssembly functions to the declared types
Comment 1 Sukolsak Sakshuwong 2015-09-10 01:59:29 PDT 
Created attachment 260915 [details]
Patch
Comment 2 Geoffrey Garen 2015-09-10 11:31:26 PDT 
Comment on attachment 260915 [details]
Patch

r=me

Can we avoid these conversions if we know that we have a WASM caller?

Why is the test case skipped?
Comment 3 WebKit Commit Bot 2015-09-10 12:01:58 PDT 
Comment on attachment 260915 [details]
Patch

Clearing flags on attachment: 260915

Committed r189582: <http://trac.webkit.org/changeset/189582>
Comment 4 WebKit Commit Bot 2015-09-10 12:02:09 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 5 Sukolsak Sakshuwong 2015-09-10 17:09:20 PDT 
Thanks for the review!

(In reply to comment #2)
> Comment on attachment 260915 [details]
> Patch
> 
> r=me
> 
> Can we avoid these conversions if we know that we have a WASM caller?

Yes, that's one of the optimizations I plan to do. Maybe exported WASM functions should have two entry points?

> Why is the test case skipped?

Because WebAssembly is not enabled by default, so loadWebAssembly() is not defined by default. I will create a patch that removes the "@skip" and uses "if (loadWebAssembly !== undefined) { ... }"
Comment 6 Geoffrey Garen 2015-09-10 17:46:44 PDT 
> > Can we avoid these conversions if we know that we have a WASM caller?
> 
> Yes, that's one of the optimizations I plan to do. Maybe exported WASM
> functions should have two entry points?

Yes -- I think we should either use two entry points or require non-WASM functions calling WASM functions to call through a thunk.

> > Why is the test case skipped?
> 
> Because WebAssembly is not enabled by default, so loadWebAssembly() is not
> defined by default. I will create a patch that removes the "@skip" and uses
> "if (loadWebAssembly !== undefined) { ... }"

Sounds good.

It seems like we have enough WASM functionality that we might want to enable WASM in trunk for testing -- even though we should leave the API for running WASM out of WebKit and Safari by default.