Bug 190512 - [WebAssembly] I64 arguments / return value check should be moved from callWebAssemblyFunction to JSToWasm wrapper
Summary: [WebAssembly] I64 arguments / return value check should be moved from callWeb...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-10-11 22:38 PDT by Yusuke Suzuki
Modified: 2018-11-19 08:50 PST (History)
7 users (show)

See Also:

Attachments
Patch (10.93 KB, patch)
2018-10-11 22:53 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (10.92 KB, patch)
2018-10-11 23:31 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews107 for mac-sierra-wk2 (3.41 MB, application/zip)
2018-10-12 00:34 PDT, EWS Watchlist 		no flags Details
Patch (11.06 KB, patch)
2018-10-12 07:11 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (10.54 KB, patch)
2018-10-12 09:12 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (10.91 KB, patch)
2018-11-01 06:21 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (9.97 KB, patch)
2018-11-01 08:06 PDT, Yusuke Suzuki 		keith_miller: review+
Details | Formatted Diff | Diff
Show Obsolete (6) 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 Yusuke Suzuki 2018-10-11 22:38:05 PDT 
[WebAssembly] I64 arguments / return value check should be moved from callWebAssemblyFunction to JSToWasm wrapper
Comment 1 Yusuke Suzuki 2018-10-11 22:53:01 PDT 
Created attachment 352139 [details]
Patch
Comment 2 Yusuke Suzuki 2018-10-11 23:27:38 PDT 
The goal of this is eliminating callWebAssemblyFunction, making the current wrapper the same to JS JITCode entry, calling wasm code smoothly from the rest of the world.
Comment 3 Yusuke Suzuki 2018-10-11 23:31:41 PDT 
Created attachment 352142 [details]
Patch
Comment 4 EWS Watchlist 2018-10-12 00:34:54 PDT 
Comment on attachment 352142 [details]
Patch

Attachment 352142 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/9548028

New failing tests:
css3/filters/backdrop/add-remove-add-backdrop-filter.html
Comment 5 EWS Watchlist 2018-10-12 00:34:56 PDT 
Created attachment 352149 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 6 EWS Watchlist 2018-10-12 01:00:42 PDT 
Comment on attachment 352142 [details]
Patch

Attachment 352142 [details] did not pass jsc-ews (mac):
Output: https://webkit-queues.webkit.org/results/9548138

New failing tests:
wasm.yaml/wasm/function-tests/i64-from-js-exceptions.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/function-import-return-value.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/i64-from-js-exceptions.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/function-import-return-value.js.wasm-slow-memory
wasm.yaml/wasm/function-tests/function-import-return-value.js.default-wasm
wasm.yaml/wasm/function-tests/i64-from-js-exceptions.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/function-import-return-value.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/i64-from-js-exceptions.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/function-import-return-value.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/i64-from-js-exceptions.js.default-wasm
wasm.yaml/wasm/function-tests/i64-from-js-exceptions.js.wasm-slow-memory
wasm.yaml/wasm/function-tests/function-import-return-value.js.wasm-eager-jettison
apiTests
Comment 7 Yusuke Suzuki 2018-10-12 07:11:57 PDT 
Created attachment 352166 [details]
Patch
Comment 8 EWS Watchlist 2018-10-12 08:45:03 PDT 
Comment on attachment 352166 [details]
Patch

Attachment 352166 [details] did not pass jsc-ews (mac):
Output: https://webkit-queues.webkit.org/results/9551414

New failing tests:
wasm.yaml/wasm/function-tests/i64-from-js-exceptions.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/function-import-return-value.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/i64-from-js-exceptions.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/function-import-return-value.js.wasm-slow-memory
wasm.yaml/wasm/function-tests/function-import-return-value.js.default-wasm
wasm.yaml/wasm/function-tests/i64-from-js-exceptions.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/function-import-return-value.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/i64-from-js-exceptions.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/function-import-return-value.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/i64-from-js-exceptions.js.default-wasm
wasm.yaml/wasm/function-tests/i64-from-js-exceptions.js.wasm-slow-memory
wasm.yaml/wasm/function-tests/function-import-return-value.js.wasm-eager-jettison
apiTests
Comment 9 Yusuke Suzuki 2018-10-12 09:12:58 PDT 
Created attachment 352171 [details]
Patch
Comment 10 Yusuke Suzuki 2018-11-01 06:21:19 PDT 
Created attachment 353594 [details]
Patch
Comment 11 Yusuke Suzuki 2018-11-01 08:06:22 PDT 
Created attachment 353600 [details]
Patch
Comment 12 Yusuke Suzuki 2018-11-11 23:58:55 PST 
Ping?
Comment 13 Yusuke Suzuki 2018-11-18 22:52:07 PST 
Ping?
Comment 14 Keith Miller 2018-11-19 08:42:59 PST 
Comment on attachment 353600 [details]
Patch

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

> Source/JavaScriptCore/wasm/js/WebAssemblyFunction.cpp:84
> +            arg = JSValue::decode(0);

Nit I think you can just do:

arg = JSValue()
Comment 15 Yusuke Suzuki 2018-11-19 08:48:05 PST 
Comment on attachment 353600 [details]
Patch

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

>> Source/JavaScriptCore/wasm/js/WebAssemblyFunction.cpp:84
>> +            arg = JSValue::decode(0);
> 
> Nit I think you can just do:
> 
> arg = JSValue()

Nice, fixed.
Comment 16 Yusuke Suzuki 2018-11-19 08:49:48 PST 
Committed r238376: <https://trac.webkit.org/changeset/238376>
Comment 17 Radar WebKit Bug Importer 2018-11-19 08:50:58 PST 
<rdar://problem/46169779>