Bug 148945

Summary: Implement the arithmetic instructions for doubles in WebAssembly
Product: WebKit Reporter: Sukolsak Sakshuwong <sukolsak>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, fpizlo, ggaren, keith_miller, sukolsak
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 148913    
Bug Blocks: 146064    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch for ARM 1
none
Patch for ARM 2
none
Update to ToT
none
Remove maxFrameExtentForSlowPathCall
none
Patch none

Sukolsak Sakshuwong
Reported 2015-09-07 14:43:54 PDT
Implement the arithmetic instructions for doubles (float64) in WebAssembly.
Attachments
Patch (28.37 KB, patch)
2015-09-07 15:36 PDT, Sukolsak Sakshuwong 		no flags
DetailsFormatted DiffDiff
Patch (29.08 KB, patch)
2015-09-08 20:47 PDT, Sukolsak Sakshuwong 		no flags
DetailsFormatted DiffDiff
Patch (29.21 KB, patch)
2015-09-09 02:34 PDT, Sukolsak Sakshuwong 		no flags
DetailsFormatted DiffDiff
Patch (18.69 KB, patch)
2015-09-11 01:06 PDT, Sukolsak Sakshuwong 		no flags
DetailsFormatted DiffDiff
Patch for ARM 1 (17.31 KB, patch)
2015-09-17 21:19 PDT, Sukolsak Sakshuwong 		no flags
DetailsFormatted DiffDiff
Patch for ARM 2 (17.64 KB, patch)
2015-09-17 21:19 PDT, Sukolsak Sakshuwong 		no flags
DetailsFormatted DiffDiff
Update to ToT (18.28 KB, patch)
2015-09-18 16:15 PDT, Sukolsak Sakshuwong 		no flags
DetailsFormatted DiffDiff
Remove maxFrameExtentForSlowPathCall (17.73 KB, text/plain)
2015-09-18 16:25 PDT, Sukolsak Sakshuwong 		no flags
Details
Patch (17.73 KB, patch)
2015-09-18 16:26 PDT, Sukolsak Sakshuwong 		no flags
DetailsFormatted DiffDiff
Show Obsolete (8) View All Add attachment proposed patch, testcase, etc.
Sukolsak Sakshuwong
Comment 1 2015-09-07 15:36:03 PDT
Created attachment 260749 [details] Patch
Sukolsak Sakshuwong
Comment 2 2015-09-08 20:47:28 PDT
Created attachment 260828 [details] Patch
Sukolsak Sakshuwong
Comment 3 2015-09-09 02:34:42 PDT
Created attachment 260851 [details] Patch
Sukolsak Sakshuwong
Comment 4 2015-09-11 01:06:00 PDT
Created attachment 260994 [details] Patch
Geoffrey Garen
Comment 5 2015-09-11 10:52:47 PDT
Comment on attachment 260994 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=260994&action=review > Source/JavaScriptCore/wasm/WASMFunctionCompiler.h:77 > +// On Windows we need to wrap fmod; on other platforms we can call it directly. > +// On ARMv7 we assert that all function pointers have to low bit set (point to thumb code). > +#if CALLING_CONVENTION_IS_STDCALL || CPU(ARM_THUMB2) > +static double JIT_OPERATION operationFmod(double x, double y) to low bit => the low bit I don't understand this comment. Why do we need to wrap fmod on Windows? Your comment says that we need to wrap fmod on Windows and not other platforms, but your code tests for all stdcall and/or thumb2 platforms. Why is it relevant that ARMv7 code is thumb code?
Sukolsak Sakshuwong
Comment 6 2015-09-17 21:19:20 PDT
Created attachment 261480 [details] Patch for ARM 1
Sukolsak Sakshuwong
Comment 7 2015-09-17 21:19:43 PDT
Created attachment 261481 [details] Patch for ARM 2
Sukolsak Sakshuwong
Comment 8 2015-09-18 14:03:01 PDT
Comment on attachment 261480 [details] Patch for ARM 1 Michael Saboff has tested this patch on both ARMv7 and ARM64. This patch doesn't use a wrapper for fmod. It passes every WASM tests.
Sukolsak Sakshuwong
Comment 9 2015-09-18 16:15:08 PDT
Created attachment 261541 [details] Update to ToT
Sukolsak Sakshuwong
Comment 10 2015-09-18 16:20:44 PDT
(In reply to comment #5) > Comment on attachment 260994 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=260994&action=review > > > Source/JavaScriptCore/wasm/WASMFunctionCompiler.h:77 > > +// On Windows we need to wrap fmod; on other platforms we can call it directly. > > +// On ARMv7 we assert that all function pointers have to low bit set (point to thumb code). > > +#if CALLING_CONVENTION_IS_STDCALL || CPU(ARM_THUMB2) > > +static double JIT_OPERATION operationFmod(double x, double y) > > to low bit => the low bit > > I don't understand this comment. Why do we need to wrap fmod on Windows? > Your comment says that we need to wrap fmod on Windows and not other > platforms, but your code tests for all stdcall and/or thumb2 platforms. Why > is it relevant that ARMv7 code is thumb code? I copied this code from dfg/DFGSpeculativeJIT.cpp. My guess is that fmod was written in non-thumb code. I have removed the wrapper for fmod and tested this patch on ARMv7 and ARM64 with the help of Michael. It passes every WASM test.
Sukolsak Sakshuwong
Comment 11 2015-09-18 16:25:40 PDT
Created attachment 261542 [details] Remove maxFrameExtentForSlowPathCall
Sukolsak Sakshuwong
Comment 12 2015-09-18 16:26:39 PDT
Created attachment 261543 [details] Patch
Geoffrey Garen
Comment 13 2015-09-18 16:44:27 PDT
Comment on attachment 261543 [details] Patch r=me
WebKit Commit Bot
Comment 14 2015-09-18 17:31:21 PDT
Comment on attachment 261543 [details] Patch Clearing flags on attachment: 261543 Committed r190002: <http://trac.webkit.org/changeset/190002>
WebKit Commit Bot
Comment 15 2015-09-18 17:31:26 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.