Bug 148838 - Implement the relational instructions in WebAssembly
Summary: Implement the relational instructions in WebAssembly
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 146064 148882
  Show dependency treegraph
 
Reported: 2015-09-04 15:48 PDT by Sukolsak Sakshuwong
Modified: 2015-09-06 23:18 PDT (History)
6 users (show)

See Also:


Attachments
Patch (12.58 KB, patch)
2015-09-04 17:25 PDT, Sukolsak Sakshuwong
no flags Details | Formatted Diff | Diff
Patch for landing (16.51 KB, patch)
2015-09-06 15:40 PDT, Sukolsak Sakshuwong
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.