+2017-04-18 Mark Lam <mark.lam@apple.com>
+
+ r211670 broke double to int conversion.
+ https://bugs.webkit.org/show_bug.cgi?id=170961
+ <rdar://problem/31687696>
+
+ Reviewed by Yusuke Suzuki.
+
+ * microbenchmarks/double-to-int32.js: Added.
+ * stress/to-int32-sensible2.js: Added.
+
2017-04-18 Oleksandr Skachkov <gskachkov@gmail.com>
[ES6]. Implement Annex B.3.3 function hoisting rules for eval
--- /dev/null
+// This microbenchmarks validates that the fix in https://webkit.org/b/170961
+// does not regress the performance gains from r211670: <http://trac.webkit.org/changeset/211670>.
+// r211670 reduces the size of operationToInt32SensibleSlow() for handling double numbers with
+// binary exponent 31. Hence, this microbenchmark stresses doubleToIn32 conversion on
+// numbers with binary exponents in the vicinity of 31.
+
+let doubleValues = [
+ 2.147483648e8, // exp = 27
+ 2.147483648e9, // exp = 31
+ 2.147483648e10, // exp = 34
+];
+
+function test(q, r, s, t, u, v, w, x, y, z) {
+ // Do a lot of double to int32 conversions to weigh more on the conversion.
+ return q|0 + r|0 + s|0 + t|0 + u|0 + v|0 + w|0 + x|0 + y|0 + z|0;
+}
+noInline(test);
+
+var result = 0;
+let length = doubleValues.length;
+for (var i = 0; i < 1000000; ++i) {
+ for (var j = 0; j < length; j++) {
+ var value = doubleValues[j];
+ result |= test(value, value, value, value, value, value, value, value, value, value);
+ }
+}
+
+if (result != -1932735284) {
+ throw "Bad result: " + result;
+}
--- /dev/null
+//@ runFTLNoCJIT
+
+var testCases = [
+ { value: -Number.MAX_VALUE, expected: 0 },
+ { value: Number.MAX_VALUE, expected: 0 },
+ { value: -Number.MIN_VALUE, expected: 0 },
+ { value: Number.MIN_VALUE, expected: 0 },
+ { value: -Infinity, expected: 0 },
+ { value: Infinity, expected: 0 },
+ { value: NaN, expected: 0 },
+ { value: -NaN, expected: 0 },
+ { value: 0, expected: 0 },
+ { value: -0, expected: 0 },
+ { value: 1, expected: 1 },
+ { value: -1, expected: -1 },
+ { value: 5, expected: 5 },
+ { value: -5, expected: -5 },
+
+ { value: 0x80000001, expected: -2147483647 },
+ { value: 0x80000000, expected: -2147483648 },
+ { value: 0x7fffffff, expected: 2147483647 },
+ { value: 0x7ffffffe, expected: 2147483646 },
+
+ { value: -2147483647, expected: -2147483647 },
+ { value: -2147483648, expected: -2147483648 },
+ { value: -2147483649, expected: 2147483647 },
+ { value: -2147483650, expected: 2147483646 },
+
+ { value: 2147483646, expected: 2147483646 },
+ { value: 2147483647, expected: 2147483647 },
+ { value: 2147483648, expected: -2147483648 },
+ { value: 2147483649, expected: -2147483647 },
+];
+
+var numFailures = 0;
+for (var i = 0; i < testCases.length; i++) {
+ try {
+ var testCase = testCases[i];
+ var test = new Function("x", "y", "y = " + i + "; return x|0;");
+ noInline(test);
+
+ for (var k = 0; k < 10000; ++k) {
+ actual = test(testCase.value);
+ if (actual != testCase.expected)
+ throw ("FAILED: x|0 where x = " + testCase.value + ": expected " + testCase.expected + ", actual " + actual);
+ }
+ } catch (e) {
+ print(e);
+ numFailures++;
+ }
+}
+
+if (numFailures)
+ throw("Found " + numFailures + " failures");
+
+2017-04-18 Mark Lam <mark.lam@apple.com>
+
+ r211670 broke double to int conversion.
+ https://bugs.webkit.org/show_bug.cgi?id=170961
+ <rdar://problem/31687696>
+
+ Reviewed by Yusuke Suzuki.
+
+ This is because operationToInt32SensibleSlow() assumes that left shifts of greater
+ than 31 bits on an 31-bit value will produce a 0. However, the spec says that
+ "if the value of the right operand is negative or is greater or equal to the
+ number of bits in the promoted left operand, the behavior is undefined."
+ See http://en.cppreference.com/w/cpp/language/operator_arithmetic#Bitwise_shift_operators.
+
+ This patch fixes this by restoring the check to prevent a shift of greater than
+ 31 bits. It also consolidates the optimization in operationToInt32SensibleSlow()
+ back into toInt32() so that we don't have 2 copies of the same code with only a
+ slight variation.
+
+ JSC benchmarks shows that performance is neutral with this patch.
+
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::compileValueToInt32):
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::sensibleDoubleToInt32):
+ * runtime/MathCommon.cpp:
+ (JSC::operationToInt32SensibleSlow): Deleted.
+ * runtime/MathCommon.h:
+ (JSC::toInt32):
+
2017-04-18 Oleksandr Skachkov <gskachkov@gmail.com>
[ES6]. Implement Annex B.3.3 function hoisting rules for eval
GPRReg gpr = result.gpr();
JITCompiler::Jump notTruncatedToInteger = m_jit.branchTruncateDoubleToInt32(fpr, gpr, JITCompiler::BranchIfTruncateFailed);
- addSlowPathGenerator(slowPathCall(notTruncatedToInteger, this,
- hasSensibleDoubleToInt() ? operationToInt32SensibleSlow : operationToInt32, NeedToSpill, ExceptionCheckRequirement::CheckNotNeeded, gpr, fpr));
+ addSlowPathGenerator(slowPathCall(notTruncatedToInteger, this, operationToInt32, NeedToSpill, ExceptionCheckRequirement::CheckNotNeeded, gpr, fpr));
int32Result(gpr, node);
return;
LBasicBlock lastNext = m_out.appendTo(slowPath, continuation);
ValueFromBlock slowResult = m_out.anchor(
- m_out.call(Int32, m_out.operation(operationToInt32SensibleSlow), doubleValue));
+ m_out.call(Int32, m_out.operation(operationToInt32), doubleValue));
m_out.jump(continuation);
m_out.appendTo(continuation, lastNext);
return JSC::toInt32(value);
}
-int32_t JIT_OPERATION operationToInt32SensibleSlow(double number)
-{
- // This function is specialized `operationToInt32` for the slow case of
- // the sensible double-to-int32 operation. It is available in x86.
- //
- // In the sensible double-to-int32, first we attempt to truncate the
- // double value to int32 by using cvttsd2si_rr.
- // According to the Intel's manual, cvttsd2si perform the following truncate
- // operation.
- //
- // If src = NaN, +-Inf, or |(src)rz| > 0x7fffffff and (src)rz != 0x80000000,
- // the result becomes 0x80000000. Otherwise, the operation succeeds.
- // Note that ()rz is rouding towards zero.
- //
- // We call this slow case function when the above cvttsd2si fails. We check
- // this condition by performing `result == 0x80000000`. So this function only
- // accepts the following numbers.
- //
- // NaN, +-Inf, |(src)rz| > 0x7fffffff.
- //
- // As a result, the exp of the double is always >= 31.
- // This condition simplifies and speeds up the toInt32 implementation.
- int64_t bits = WTF::bitwise_cast<int64_t>(number);
- int32_t exp = (static_cast<int32_t>(bits >> 52) & 0x7ff) - 0x3ff;
-
- // If exponent < 0 there will be no bits to the left of the decimal point
- // after rounding; if the exponent is > 83 then no bits of precision can be
- // left in the low 32-bit range of the result (IEEE-754 doubles have 52 bits
- // of fractional precision).
- // Note this case handles 0, -0, and all infinite, NaN, & denormal value.
-
- // If exp < 0, truncate operation succeeds. So this function does not
- // encounter that case. If exp > 83, it means exp >= 84. In that case,
- // the following operation produces 0 for the result.
- ASSERT(exp >= 0);
-
- // Select the appropriate 32-bits from the floating point mantissa. If the
- // exponent is 52 then the bits we need to select are already aligned to the
- // lowest bits of the 64-bit integer representation of the number, no need
- // to shift. If the exponent is greater than 52 we need to shift the value
- // left by (exp - 52), if the value is less than 52 we need to shift right
- // accordingly.
- int32_t result = (exp > 52)
- ? static_cast<int32_t>(bits << (exp - 52))
- : static_cast<int32_t>(bits >> (52 - exp));
-
- // IEEE-754 double precision values are stored omitting an implicit 1 before
- // the decimal point; we need to reinsert this now. We may also the shifted
- // invalid bits into the result that are not a part of the mantissa (the sign
- // and exponent bits from the floatingpoint representation); mask these out.
- //
- // The important observation is that exp is always >= 31. So the above case
- // is needed to be cared only when the exp == 31.
- ASSERT(exp >= 31);
- if (exp == 31) {
- int32_t missingOne = 1 << exp;
- result &= (missingOne - 1);
- result += missingOne;
- }
-
- // If the input value was negative (we could test either 'number' or 'bits',
- // but testing 'bits' is likely faster) invert the result appropriately.
- return bits < 0 ? -result : result;
-}
-
#if HAVE(ARM_IDIV_INSTRUCTIONS)
static inline bool isStrictInt32(double value)
{
#pragma once
+#include "CPU.h"
#include <cmath>
#include <wtf/Optional.h>
const int32_t maxExponentForIntegerMathPow = 1000;
double JIT_OPERATION operationMathPow(double x, double y) WTF_INTERNAL;
int32_t JIT_OPERATION operationToInt32(double) WTF_INTERNAL;
-int32_t JIT_OPERATION operationToInt32SensibleSlow(double) WTF_INTERNAL;
inline constexpr double maxSafeInteger()
{
// left in the low 32-bit range of the result (IEEE-754 doubles have 52 bits
// of fractional precision).
// Note this case handles 0, -0, and all infinite, NaN, & denormal value.
- if (exp < 0 || exp > 83)
+
+ // We need to check exp > 83 because:
+ // 1. exp may be used as a left shift value below in (exp - 52), and
+ // 2. Left shift amounts that exceed 31 results in undefined behavior. See:
+ // http://en.cppreference.com/w/cpp/language/operator_arithmetic#Bitwise_shift_operators
+ //
+ // Using an unsigned comparison here also gives us a exp < 0 check for free.
+ if (static_cast<uint32_t>(exp) > 83u)
return 0;
// Select the appropriate 32-bits from the floating point mantissa. If the
// the decimal point; we need to reinsert this now. We may also the shifted
// invalid bits into the result that are not a part of the mantissa (the sign
// and exponent bits from the floatingpoint representation); mask these out.
- if (exp < 32) {
+ if (hasSensibleDoubleToInt() && (exp == 31)) {
+ // This is an optimization for when toInt32() is called in the slow path
+ // of a JIT operation. Currently, this optimization is only applicable for
+ // x86 ports.
+ //
+ // On x86, the fast path does a sensible double-to-int32 conversion, by
+ // first attempting to truncate the double value to int32 using the
+ // cvttsd2si_rr instruction. According to Intel's manual, cvttsd2si performs
+ // the following truncate operation:
+ //
+ // If src = NaN, +-Inf, or |(src)rz| > 0x7fffffff and (src)rz != 0x80000000,
+ // then the result becomes 0x80000000. Otherwise, the operation succeeds.
+ //
+ // Note that the ()rz notation means rounding towards zero.
+ // We'll call the slow case function only when the above cvttsd2si fails. The
+ // JIT code checks for fast path failure by checking if result == 0x80000000.
+ // Hence, the slow path will only see the following possible set of numbers:
+ //
+ // NaN, +-Inf, or |(src)rz| > 0x7fffffff.
+ //
+ // As a result, the exp of the double is always >= 31. We can take advantage
+ // of this by specifically checking for (exp == 31) and give the compiler a
+ // chance to constant fold the operations below.
+ const int32_t missingOne = 1 << exp;
+ result &= missingOne - 1;
+ result += missingOne;
+ } else if (exp < 32) {
int32_t missingOne = 1 << exp;
result &= missingOne - 1;
result += missingOne;
git://git.webkit.org / WebKit-https.git / commitdiff