Bug 210302

Summary: Add support for the Wasm i32 sign-extension-ops proposal
Product: WebKit Reporter: Ben Smith <binji>
Component: WebAssemblyAssignee: Sergey Rubanov <chi187>
Status: RESOLVED FIXED    
Severity: Normal CC: chi187, cryze92, dschuff, ews-watchlist, fpizlo, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=218990
Attachments:
Description Flags
Patch
none
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
none
Patch
none
Patch
ysuzuki: review+, ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch none

Description Ben Smith 2020-04-09 13:28:26 PDT 
The sign-extension-ops proposal recently became standardized as a phase 5 proposal, and was merged into the spec repo: https://github.com/WebAssembly/spec/pull/1144
Comment 1 Radar WebKit Bug Importer 2020-04-09 15:46:14 PDT 
<rdar://problem/61545666>
Comment 2 Keith Miller 2020-04-09 17:10:15 PDT 
Yikes! We never did this? Whoops... 

Thanks for the report!
Comment 3 Sergey Rubanov 2020-11-14 13:50:54 PST 
Created attachment 414143 [details]
Patch
Comment 4 EWS Watchlist 2020-11-14 13:52:05 PST 
This patch modifies one of the wasm.json files. Please ensure that any changes in one have been mirrored to the other. You can find the wasm.json files at "Source/JavaScriptCore/wasm/wasm.json" and "JSTests/wasm/wasm.json".
Comment 5 Sergey Rubanov 2020-11-14 13:53:11 PST 
This patch implements only sign extensions to i32 for now.

It passes spect tests from https://bugs.webkit.org/show_bug.cgi?id=218940

i62 is WIP
Comment 6 Sergey Rubanov 2020-11-14 15:35:33 PST 
Created attachment 414145 [details]
Patch
Comment 7 Sergey Rubanov 2020-11-14 15:58:37 PST 
Created attachment 414147 [details]
Patch
Comment 8 Sergey Rubanov 2020-11-14 16:14:16 PST 
Created attachment 414148 [details]
Patch
Comment 9 Sergey Rubanov 2020-11-14 19:13:47 PST 
Created attachment 414150 [details]
Patch
Comment 10 Sergey Rubanov 2020-11-15 11:17:18 PST 
Created attachment 414173 [details]
Patch
Comment 11 Yusuke Suzuki 2020-11-16 08:43:05 PST 
Comment on attachment 414173 [details]
Patch

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

r=me Nice.

> Source/JavaScriptCore/ChangeLog:3
> +        Add support for the Wasm sign-extension-ops proposal

Can you change the bug title + ChangeLog titles to clarify this implements i32 sign-extension-ops and it is not including i64 ops?
And, can you open a bug for i64 sign-extension-ops, and change JSTests/wasm/spec-tests/i64.wast.js's URL to the new one?
Comment 12 Sergey Rubanov 2020-11-16 08:58:29 PST 
Created attachment 414239 [details]
Patch
Comment 13 Sergey Rubanov 2020-11-16 09:11:47 PST 
Bug for i64 sign-extension-ops proposal
https://bugs.webkit.org/show_bug.cgi?id=218990
Comment 14 Sergey Rubanov 2020-11-16 09:30:22 PST 
Created attachment 414242 [details]
Patch
Comment 15 Yusuke Suzuki 2020-11-16 09:34:29 PST 
Comment on attachment 414242 [details]
Patch

OK, for GTK and WPE builds, I ensured that GTK / WPE build work on my Linux (Debian unstable) box, so I think this is build-related stale, and clean-build will fix the issue.
And we know that the code in this patch is not changed from r+ patch. I'll put cq+ here.
Comment 16 EWS 2020-11-16 09:35:01 PST 
ChangeLog entry in JSTests/ChangeLog contains OOPS!.
Comment 17 Sergey Rubanov 2020-11-16 10:00:16 PST 
Created attachment 414245 [details]
Patch
Comment 18 EWS 2020-11-17 15:49:25 PST 
Committed r269929: <https://trac.webkit.org/changeset/269929>

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