Summary: | Implement the arithmetic instructions for doubles in WebAssembly | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Sukolsak Sakshuwong <sukolsak> | ||||||||||||||||||||
Component: | JavaScriptCore | Assignee: | 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
Sukolsak Sakshuwong
2015-09-07 14:43:54 PDT
Created attachment 260749 [details]
Patch
Created attachment 260828 [details]
Patch
Created attachment 260851 [details]
Patch
Created attachment 260994 [details]
Patch
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? Created attachment 261480 [details]
Patch for ARM 1
Created attachment 261481 [details]
Patch for ARM 2
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.
Created attachment 261541 [details]
Update to ToT
(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. Created attachment 261542 [details]
Remove maxFrameExtentForSlowPathCall
Created attachment 261543 [details]
Patch
Comment on attachment 261543 [details]
Patch
r=me
Comment on attachment 261543 [details] Patch Clearing flags on attachment: 261543 Committed r190002: <http://trac.webkit.org/changeset/190002> All reviewed patches have been landed. Closing bug. |