Bug 189498

Summary: [WebAssembly] Move type conversion code of JSToWasm return type to JS wasm wrapper
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: New BugsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, mark.lam, msaboff, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch saam: review+

Yusuke Suzuki
Reported 2018-09-11 05:00:05 PDT
[WebAssembly] Move type conversion code of JSToWasm return type to JS wasm wrapper
Attachments
Patch (6.07 KB, patch)
2018-09-11 05:03 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (6.05 KB, patch)
2018-09-11 05:06 PDT, Yusuke Suzuki 		saam: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Yusuke Suzuki
Comment 1 2018-09-11 05:03:10 PDT
Created attachment 349390 [details] Patch
Yusuke Suzuki
Comment 2 2018-09-11 05:06:34 PDT
Created attachment 349391 [details] Patch
Yusuke Suzuki
Comment 3 2018-09-11 05:56:56 PDT
Eventually I would like to merge these code into wrapper side.
Yusuke Suzuki
Comment 4 2018-09-26 01:38:46 PDT
Ping?
Saam Barati
Comment 5 2018-09-30 11:46:50 PDT
Comment on attachment 349391 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=349391&action=review r=me > Source/JavaScriptCore/wasm/js/WebAssemblyFunction.cpp:164 > + return rawResult; Is there any value in keeping the above code in some form for a debug assert?
Saam Barati
Comment 6 2018-09-30 11:48:26 PDT
Comment on attachment 349391 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=349391&action=review > Source/JavaScriptCore/wasm/js/JSToWasm.cpp:223 > + auto isNaN = jit.branchDouble(CCallHelpers::DoubleNotEqualOrUnordered, FPRInfo::returnValueFPR, FPRInfo::returnValueFPR); We should really just add something in AssemblyHelpers sometime in the future for branchIfNaN
Yusuke Suzuki
Comment 7 2018-10-01 02:24:04 PDT
Comment on attachment 349391 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=349391&action=review >> Source/JavaScriptCore/wasm/js/JSToWasm.cpp:223 >> + auto isNaN = jit.branchDouble(CCallHelpers::DoubleNotEqualOrUnordered, FPRInfo::returnValueFPR, FPRInfo::returnValueFPR); > > We should really just add something in AssemblyHelpers sometime in the future for branchIfNaN Sounds fine. I'll add this in a separate patch. >> Source/JavaScriptCore/wasm/js/WebAssemblyFunction.cpp:164 >> + return rawResult; > > Is there any value in keeping the above code in some form for a debug assert? Eventually, I would like to remove this entire function to merge this functionality into the generated code. So I think we should add assertions in the generated code instead of this function.
Yusuke Suzuki
Comment 8 2018-10-01 02:25:46 PDT
Committed r236651: <https://trac.webkit.org/changeset/236651>
Radar WebKit Bug Importer
Comment 9 2018-10-01 02:26:28 PDT
<rdar://problem/44904397>
Note You need to log in before you can comment on or make changes to this bug.