Bug 148838

Summary: Implement the relational instructions in WebAssembly
Product: WebKit Reporter: Sukolsak Sakshuwong <sukolsak>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, fpizlo, ggaren, mark.lam, saam, sukolsak
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 146064, 148882    
Attachments:
Description Flags
Patch
none
Patch for landing none

Description Sukolsak Sakshuwong 2015-09-04 15:48:11 PDT
Implement the relational instructions for WebAssembly files generated by pack-asmjs <https://github.com/WebAssembly/polyfill-prototype-1>.
Comment 1 Sukolsak Sakshuwong 2015-09-04 17:25:01 PDT
Created attachment 260658 [details]
Patch
Comment 2 Saam Barati 2015-09-06 13:20:31 PDT
Comment on attachment 260658 [details]
Patch

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

r=me with comment

> Source/JavaScriptCore/tests/stress/wasm-relational.js:119
> +var module = loadWebAssembly("wasm-relational.wasm");

I think it'd be nicer to have a directory in stress called "wasm" that holds all the
wasm binaries were testing. Do we have other tests checked in that follow the
model in this patch? Maybe it's worth changing those.  

Also, would it be possible to write the tests such that
we can write all wasm tests in JS, and have the loadWebAssembly do the
translation on the fly?
Comment 3 Saam Barati 2015-09-06 13:21:29 PDT
Doing the JS =>wasm translation on the fly will make
converting to newer wasm formats much easier.
Comment 4 Sukolsak Sakshuwong 2015-09-06 15:02:15 PDT
Thanks.

(In reply to comment #2)
> Comment on attachment 260658 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=260658&action=review
> 
> r=me with comment
> 
> > Source/JavaScriptCore/tests/stress/wasm-relational.js:119
> > +var module = loadWebAssembly("wasm-relational.wasm");
> 
> I think it'd be nicer to have a directory in stress called "wasm" that holds
> all the
> wasm binaries were testing. Do we have other tests checked in that follow the
> model in this patch? Maybe it's worth changing those.  

Will do.

> Also, would it be possible to write the tests such that
> we can write all wasm tests in JS, and have the loadWebAssembly do the
> translation on the fly?

(In reply to comment #3)
> Doing the JS =>wasm translation on the fly will make
> converting to newer wasm formats much easier.

We will have to include the source code of pack-asmjs (~3,800 lines) and the asm.js parser that it uses (~2,900 lines.) It's probably not worth the time though, considering that eventually, we will have to implement the text format <https://github.com/WebAssembly/design/blob/master/TextFormat.md>. It will be a better way to write tests, because we will have more control over what pack-asmjs generates. asm.js is only a stopgap until the spec is finalized.
Comment 5 Sukolsak Sakshuwong 2015-09-06 15:40:38 PDT
Created attachment 260720 [details]
Patch for landing
Comment 6 WebKit Commit Bot 2015-09-06 23:18:02 PDT
Comment on attachment 260720 [details]
Patch for landing

Clearing flags on attachment: 260720

Committed r189458: <http://trac.webkit.org/changeset/189458>
Comment 7 WebKit Commit Bot 2015-09-06 23:18:06 PDT
All reviewed patches have been landed.  Closing bug.