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

Sukolsak Sakshuwong
Reported 2015-09-10 01:46:57 PDT
Convert arguments to WebAssembly functions to the declared types
Attachments
Patch (12.09 KB, patch)
2015-09-10 01:59 PDT, Sukolsak Sakshuwong 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Sukolsak Sakshuwong
Comment 1 2015-09-10 01:59:29 PDT
Created attachment 260915 [details] Patch
Geoffrey Garen
Comment 2 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?
WebKit Commit Bot
Comment 3 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>
WebKit Commit Bot
Comment 4 2015-09-10 12:02:09 PDT
All reviewed patches have been landed. Closing bug.
Sukolsak Sakshuwong
Comment 5 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) { ... }"
Geoffrey Garen
Comment 6 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.
Note You need to log in before you can comment on or make changes to this bug.