<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[200277] trunk/Source/JavaScriptCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/200277">200277</a></dd>
<dt>Author</dt> <dd>commit-queue@webkit.org</dd>
<dt>Date</dt> <dd>2016-04-29 18:50:42 -0700 (Fri, 29 Apr 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>[JSC][ARMv7S] Arithmetic module results change when tiering up to DFG
https://bugs.webkit.org/show_bug.cgi?id=157217
rdar://problem/24733432
Patch by Benjamin Poulain <bpoulain@apple.com> on 2016-04-29
Reviewed by Mark Lam.
ARMv7's fmod() returns less accurate results than an integer division.
Since we have integer div on ARMv7s, the results start changing when
we reach DFG.
In this patch, I change our fmod slow path to behave like the fast path
on ARMv7s.
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileArithMod):
(JSC::DFG::fmodAsDFGOperation): Deleted.
* runtime/CommonSlowPaths.cpp:
(JSC::SLOW_PATH_DECL):
* runtime/MathCommon.cpp:
(JSC::isStrictInt32):
* runtime/MathCommon.h:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGSpeculativeJITcpp">trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeCommonSlowPathscpp">trunk/Source/JavaScriptCore/runtime/CommonSlowPaths.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeMathCommoncpp">trunk/Source/JavaScriptCore/runtime/MathCommon.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeMathCommonh">trunk/Source/JavaScriptCore/runtime/MathCommon.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (200276 => 200277)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-04-30 01:47:22 UTC (rev 200276)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-04-30 01:50:42 UTC (rev 200277)
</span><span class="lines">@@ -1,3 +1,27 @@
</span><ins>+2016-04-29 Benjamin Poulain <bpoulain@apple.com>
+
+ [JSC][ARMv7S] Arithmetic module results change when tiering up to DFG
+ https://bugs.webkit.org/show_bug.cgi?id=157217
+ rdar://problem/24733432
+
+ Reviewed by Mark Lam.
+
+ ARMv7's fmod() returns less accurate results than an integer division.
+ Since we have integer div on ARMv7s, the results start changing when
+ we reach DFG.
+
+ In this patch, I change our fmod slow path to behave like the fast path
+ on ARMv7s.
+
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::compileArithMod):
+ (JSC::DFG::fmodAsDFGOperation): Deleted.
+ * runtime/CommonSlowPaths.cpp:
+ (JSC::SLOW_PATH_DECL):
+ * runtime/MathCommon.cpp:
+ (JSC::isStrictInt32):
+ * runtime/MathCommon.h:
+
</ins><span class="cx"> 2016-04-29 Joseph Pecoraro <pecoraro@apple.com>
</span><span class="cx">
</span><span class="cx"> Web Inspector: Issues inspecting the inspector, pausing on breakpoints causes content to not load
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGSpeculativeJITcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp (200276 => 200277)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp 2016-04-30 01:47:22 UTC (rev 200276)
+++ trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp 2016-04-30 01:50:42 UTC (rev 200277)
</span><span class="lines">@@ -362,17 +362,6 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><del>-// 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 fmodAsDFGOperation(double x, double y)
-{
- return fmod(x, y);
-}
-#else
-#define fmodAsDFGOperation fmod
-#endif
-
</del><span class="cx"> void SpeculativeJIT::clearGenerationInfo()
</span><span class="cx"> {
</span><span class="cx"> for (unsigned i = 0; i < m_generationInfo.size(); ++i)
</span><span class="lines">@@ -4589,7 +4578,7 @@
</span><span class="cx">
</span><span class="cx"> FPRResult result(this);
</span><span class="cx">
</span><del>- callOperation(fmodAsDFGOperation, result.fpr(), op1FPR, op2FPR);
</del><ins>+ callOperation(jsMod, result.fpr(), op1FPR, op2FPR);
</ins><span class="cx">
</span><span class="cx"> doubleResult(result.fpr(), node);
</span><span class="cx"> return;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeCommonSlowPathscpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/CommonSlowPaths.cpp (200276 => 200277)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/CommonSlowPaths.cpp 2016-04-30 01:47:22 UTC (rev 200276)
+++ trunk/Source/JavaScriptCore/runtime/CommonSlowPaths.cpp 2016-04-30 01:50:42 UTC (rev 200277)
</span><span class="lines">@@ -49,6 +49,7 @@
</span><span class="cx"> #include "LLIntCommon.h"
</span><span class="cx"> #include "LLIntExceptions.h"
</span><span class="cx"> #include "LowLevelInterpreter.h"
</span><ins>+#include "MathCommon.h"
</ins><span class="cx"> #include "ObjectConstructor.h"
</span><span class="cx"> #include "ScopedArguments.h"
</span><span class="cx"> #include "StructureRareDataInlines.h"
</span><span class="lines">@@ -462,7 +463,7 @@
</span><span class="cx"> BEGIN();
</span><span class="cx"> double a = OP_C(2).jsValue().toNumber(exec);
</span><span class="cx"> double b = OP_C(3).jsValue().toNumber(exec);
</span><del>- RETURN(jsNumber(fmod(a, b)));
</del><ins>+ RETURN(jsNumber(jsMod(a, b)));
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> SLOW_PATH_DECL(slow_path_lshift)
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeMathCommoncpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/MathCommon.cpp (200276 => 200277)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/MathCommon.cpp 2016-04-30 01:47:22 UTC (rev 200276)
+++ trunk/Source/JavaScriptCore/runtime/MathCommon.cpp 2016-04-30 01:50:42 UTC (rev 200277)
</span><span class="lines">@@ -450,12 +450,53 @@
</span><span class="cx"> return mathPowInternal(x, y);
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+#if HAVE(ARM_IDIV_INSTRUCTIONS)
+static inline bool isStrictInt32(double value)
+{
+ int32_t valueAsInt32 = static_cast<int32_t>(value);
+ if (value != valueAsInt32)
+ return false;
+
+ if (!valueAsInt32) {
+ if (std::signbit(value))
+ return false;
+ }
+ return true;
+}
+#endif
+
</ins><span class="cx"> extern "C" {
</span><span class="cx"> double jsRound(double value)
</span><span class="cx"> {
</span><span class="cx"> double integer = ceil(value);
</span><span class="cx"> return integer - (integer - value > 0.5);
</span><span class="cx"> }
</span><ins>+
+#if CALLING_CONVENTION_IS_STDCALL || CPU(ARM_THUMB2)
+double jsMod(double x, double y)
+{
+#if HAVE(ARM_IDIV_INSTRUCTIONS)
+ // fmod() does not have exact results for integer on ARMv7.
+ // When DFG/FTL use IDIV, the result of op_mod can change if we use fmod().
+ //
+ // We implement here the same algorithm and conditions as the upper tier to keep
+ // a stable result when tiering up.
+ if (y) {
+ if (isStrictInt32(x) && isStrictInt32(y)) {
+ int32_t xAsInt32 = static_cast<int32_t>(x);
+ int32_t yAsInt32 = static_cast<int32_t>(y);
+ int32_t quotient = xAsInt32 / yAsInt32;
+ if (!productOverflows<int32_t>(quotient, yAsInt32)) {
+ int32_t remainder = xAsInt32 - (quotient * yAsInt32);
+ if (remainder || xAsInt32 >= 0)
+ return remainder;
+ }
+ }
+ }
+#endif
+ return fmod(x, y);
</ins><span class="cx"> }
</span><ins>+#endif
+} // extern "C"
</ins><span class="cx">
</span><span class="cx"> } // namespace JSC
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeMathCommonh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/MathCommon.h (200276 => 200277)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/MathCommon.h 2016-04-30 01:47:22 UTC (rev 200276)
+++ trunk/Source/JavaScriptCore/runtime/MathCommon.h 2016-04-30 01:50:42 UTC (rev 200277)
</span><span class="lines">@@ -27,6 +27,7 @@
</span><span class="cx"> #define MathCommon_h
</span><span class="cx">
</span><span class="cx"> #include "JITOperations.h"
</span><ins>+#include "MacroAssemblerCodeRef.h"
</ins><span class="cx"> #include <cmath>
</span><span class="cx"> #include <wtf/Optional.h>
</span><span class="cx">
</span><span class="lines">@@ -88,6 +89,14 @@
</span><span class="cx">
</span><span class="cx"> extern "C" {
</span><span class="cx"> double JIT_OPERATION jsRound(double value) REFERENCED_FROM_ASM WTF_INTERNAL;
</span><ins>+
+// 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)
+double JIT_OPERATION jsMod(double x, double y) REFERENCED_FROM_ASM WTF_INTERNAL;
+#else
+#define jsMod fmod
+#endif
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> }
</span></span></pre>
</div>
</div>
</body>
</html>