Bug 210302 - Add support for the Wasm i32 sign-extension-ops proposal
Summary: Add support for the Wasm i32 sign-extension-ops proposal
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebAssembly (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sergey Rubanov
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-04-09 13:28 PDT by Ben Smith
Modified: 2020-11-17 15:49 PST (History)
12 users (show)

See Also:

Attachments
Patch (12.93 KB, patch)
2020-11-14 13:50 PST, Sergey Rubanov 		no flags Details | Formatted Diff | Diff
Patch (13.19 KB, patch)
2020-11-14 15:35 PST, Sergey Rubanov 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (13.19 KB, patch)
2020-11-14 15:58 PST, Sergey Rubanov 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (13.19 KB, patch)
2020-11-14 16:14 PST, Sergey Rubanov 		no flags Details | Formatted Diff | Diff
Patch (12.91 KB, patch)
2020-11-14 19:13 PST, Sergey Rubanov 		no flags Details | Formatted Diff | Diff
Patch (24.42 KB, patch)
2020-11-15 11:17 PST, Sergey Rubanov 		ysuzuki: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (24.43 KB, patch)
2020-11-16 08:58 PST, Sergey Rubanov 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (47.89 KB, patch)
2020-11-16 09:30 PST, Sergey Rubanov 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (47.89 KB, patch)
2020-11-16 10:00 PST, Sergey Rubanov 		no flags Details | Formatted Diff | Diff
Show Obsolete (5) View All Add an attachment (proposed patch, testcase, etc.)

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