REGRESSION(r200208): It made 2 JSC stress tests fail on x86
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 17 May 2016 04:36:08 +0000 (04:36 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 17 May 2016 04:36:08 +0000 (04:36 +0000)
https://bugs.webkit.org/show_bug.cgi?id=157168

Reviewed by Benjamin Poulain.

The fast path in operationMathPow produces different results between x87 and the other environments.
This is because x87 calculates the double value in 80bit precision.
The situation is the following: in x86 32bit environment, floating point operations are compiled to
x87 operations by default even if we can use SSE2. But in DFG environment, we aggressively use SSE2
if the cpuid reports SSE2 is available. As a result, the implementations differ between C runtime
and DFG JIT code. The C runtime uses x87 while DFG JIT code uses SSE2. This causes a precision
problem since x87 has 80bit precision while SSE2 has 64bit precision.

In this patch, in x86 32bit environment, we use `volatile double` if the `-mfpmath=sse and -msse2 (or later)`
is not specified. This will round the x87 value into 64bit per multiplying. Note that this problem does not
occur in OS X clang 32bit environment. This is because `-mfpmath=sse` is enabled by default in OS X clang 32bit.

* b3/B3MathExtras.cpp:
(JSC::B3::powDoubleInt32):
* runtime/MathCommon.cpp:
(JSC::operationMathPow):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@200996 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/b3/B3MathExtras.cpp
Source/JavaScriptCore/runtime/MathCommon.cpp

index 91dd53e..24d671a 100644 (file)
@@ -1,3 +1,27 @@
+2016-05-16  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        REGRESSION(r200208): It made 2 JSC stress tests fail on x86
+        https://bugs.webkit.org/show_bug.cgi?id=157168
+
+        Reviewed by Benjamin Poulain.
+
+        The fast path in operationMathPow produces different results between x87 and the other environments.
+        This is because x87 calculates the double value in 80bit precision.
+        The situation is the following: in x86 32bit environment, floating point operations are compiled to
+        x87 operations by default even if we can use SSE2. But in DFG environment, we aggressively use SSE2
+        if the cpuid reports SSE2 is available. As a result, the implementations differ between C runtime
+        and DFG JIT code. The C runtime uses x87 while DFG JIT code uses SSE2. This causes a precision
+        problem since x87 has 80bit precision while SSE2 has 64bit precision.
+
+        In this patch, in x86 32bit environment, we use `volatile double` if the `-mfpmath=sse and -msse2 (or later)`
+        is not specified. This will round the x87 value into 64bit per multiplying. Note that this problem does not
+        occur in OS X clang 32bit environment. This is because `-mfpmath=sse` is enabled by default in OS X clang 32bit.
+
+        * b3/B3MathExtras.cpp:
+        (JSC::B3::powDoubleInt32):
+        * runtime/MathCommon.cpp:
+        (JSC::operationMathPow):
+
 2016-05-16  Benjamin Poulain  <bpoulain@apple.com>
 
         [JSC] "return this" in a constructor does not need a branch on isObject(this)
index 65e3276..acf8b42 100644 (file)
@@ -36,6 +36,7 @@
 #include "B3ControlValue.h"
 #include "B3UpsilonValue.h"
 #include "B3ValueInlines.h"
+#include "MathCommon.h"
 
 namespace JSC { namespace B3 {
 
@@ -50,7 +51,7 @@ std::pair<BasicBlock*, Value*> powDoubleInt32(Procedure& procedure, BasicBlock*
 
     Value* shouldGoSlowPath = start->appendNew<Value>(procedure, Above, origin,
         y,
-        start->appendNew<Const32Value>(procedure, origin, 1000));
+        start->appendNew<Const32Value>(procedure, origin, maxExponentForIntegerMathPow));
     start->appendNew<ControlValue>(
         procedure, Branch, origin,
         shouldGoSlowPath,
index 2faa464..7e2f746 100644 (file)
@@ -435,17 +435,29 @@ double JIT_OPERATION operationMathPow(double x, double y)
     }
 
     int32_t yAsInt = y;
-    if (static_cast<double>(yAsInt) == y && yAsInt > 0 && yAsInt <= maxExponentForIntegerMathPow) {
+    if (static_cast<double>(yAsInt) == y && yAsInt >= 0 && yAsInt <= maxExponentForIntegerMathPow) {
         // If the exponent is a small positive int32 integer, we do a fast exponentiation
-        double result = 1;
+
+        // Do not use x87 values for accumulation. x87 values has 80bit precision.
+        // The result produced by x87's 80bit double precision differs from the one calculated with SSE2 in DFG.
+        // Using volatile double is workaround for this problem. By specifying volatile, we expect that `result` and `xd`
+        // are stored in the stack. And at that time, we expect that they are rounded by fst/fstp[1, 2].
+        // [1]: https://gcc.gnu.org/wiki/x87note
+        // [2]: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=323
+#if !CPU(X86) || (defined(__SSE2_MATH__) && defined(__SSE2__))
+        typedef double DoubleValue;
+#else
+        typedef volatile double DoubleValue;
+#endif
+        DoubleValue result = 1;
+        DoubleValue xd = x;
         while (yAsInt) {
             if (yAsInt & 1)
-                result *= x;
-            x *= x;
+                result *= xd;
+            xd *= xd;
             yAsInt >>= 1;
         }
         return result;
-
     }
     return mathPowInternal(x, y);
 }