- Revision
- 216363
- Author
- [email protected]
- Date
- 2017-05-08 01:24:36 -0700 (Mon, 08 May 2017)
Log Message
Merge r215516 - r211670 broke double to int conversion.
https://bugs.webkit.org/show_bug.cgi?id=170961
Reviewed by Mark Lam.
In this patch, we take a template parameter way.
While it reduces duplicate code, it effectively produces
optimized code for operationToInt32SensibleSlow,
and fixes kraken pbkdf2 regression on Linux.
And this patch also fixes undefined behavior by changing
int32_t to uint32_t. If exp is 31, missingOne is 1 << 31,
INT32_MIN. Thus missingOne - 1 will cause int32_t overflow,
and it is an undefined behavior.
* runtime/MathCommon.cpp:
(JSC::operationToInt32SensibleSlow):
* runtime/MathCommon.h:
(JSC::toInt32Internal):
(JSC::toInt32):
Modified Paths
Diff
Modified: releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/ChangeLog (216362 => 216363)
--- releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/ChangeLog 2017-05-08 08:24:26 UTC (rev 216362)
+++ releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/ChangeLog 2017-05-08 08:24:36 UTC (rev 216363)
@@ -1,3 +1,26 @@
+2017-04-19 Yusuke Suzuki <[email protected]>
+
+ r211670 broke double to int conversion.
+ https://bugs.webkit.org/show_bug.cgi?id=170961
+
+ Reviewed by Mark Lam.
+
+ In this patch, we take a template parameter way.
+ While it reduces duplicate code, it effectively produces
+ optimized code for operationToInt32SensibleSlow,
+ and fixes kraken pbkdf2 regression on Linux.
+
+ And this patch also fixes undefined behavior by changing
+ int32_t to uint32_t. If exp is 31, missingOne is 1 << 31,
+ INT32_MIN. Thus missingOne - 1 will cause int32_t overflow,
+ and it is an undefined behavior.
+
+ * runtime/MathCommon.cpp:
+ (JSC::operationToInt32SensibleSlow):
+ * runtime/MathCommon.h:
+ (JSC::toInt32Internal):
+ (JSC::toInt32):
+
2017-04-18 Mark Lam <[email protected]>
r211670 broke double to int conversion.
Modified: releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (216362 => 216363)
--- releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2017-05-08 08:24:26 UTC (rev 216362)
+++ releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2017-05-08 08:24:36 UTC (rev 216363)
@@ -2208,7 +2208,8 @@
GPRReg gpr = result.gpr();
JITCompiler::Jump notTruncatedToInteger = m_jit.branchTruncateDoubleToInt32(fpr, gpr, JITCompiler::BranchIfTruncateFailed);
- addSlowPathGenerator(slowPathCall(notTruncatedToInteger, this, operationToInt32, NeedToSpill, ExceptionCheckRequirement::CheckNotNeeded, gpr, fpr));
+ addSlowPathGenerator(slowPathCall(notTruncatedToInteger, this,
+ hasSensibleDoubleToInt() ? operationToInt32SensibleSlow : operationToInt32, NeedToSpill, ExceptionCheckRequirement::CheckNotNeeded, gpr, fpr));
int32Result(gpr, node);
return;
Modified: releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (216362 => 216363)
--- releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2017-05-08 08:24:26 UTC (rev 216362)
+++ releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2017-05-08 08:24:36 UTC (rev 216363)
@@ -11676,7 +11676,7 @@
LBasicBlock lastNext = m_out.appendTo(slowPath, continuation);
ValueFromBlock slowResult = m_out.anchor(
- m_out.call(Int32, m_out.operation(operationToInt32), doubleValue));
+ m_out.call(Int32, m_out.operation(operationToInt32SensibleSlow), doubleValue));
m_out.jump(continuation);
m_out.appendTo(continuation, lastNext);
Modified: releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/runtime/MathCommon.cpp (216362 => 216363)
--- releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/runtime/MathCommon.cpp 2017-05-08 08:24:26 UTC (rev 216362)
+++ releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/runtime/MathCommon.cpp 2017-05-08 08:24:36 UTC (rev 216363)
@@ -467,6 +467,11 @@
return JSC::toInt32(value);
}
+int32_t JIT_OPERATION operationToInt32SensibleSlow(double number)
+{
+ return toInt32Internal<ToInt32Mode::AfterSensibleConversionAttempt>(number);
+}
+
#if HAVE(ARM_IDIV_INSTRUCTIONS)
static inline bool isStrictInt32(double value)
{
Modified: releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/runtime/MathCommon.h (216362 => 216363)
--- releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/runtime/MathCommon.h 2017-05-08 08:24:26 UTC (rev 216362)
+++ releases/WebKitGTK/webkit-2.16/Source/_javascript_Core/runtime/MathCommon.h 2017-05-08 08:24:36 UTC (rev 216363)
@@ -25,7 +25,6 @@
#pragma once
-#include "CPU.h"
#include <cmath>
#include <wtf/Optional.h>
@@ -34,6 +33,7 @@
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()
{
@@ -73,9 +73,14 @@
//
// The operation can be described as round towards zero, then select the 32 least
// bits of the resulting value in 2s-complement representation.
-ALWAYS_INLINE int32_t toInt32(double number)
+enum ToInt32Mode {
+ Generic,
+ AfterSensibleConversionAttempt,
+};
+template<ToInt32Mode Mode>
+ALWAYS_INLINE int32_t toInt32Internal(double number)
{
- int64_t bits = WTF::bitwise_cast<int64_t>(number);
+ uint64_t bits = WTF::bitwise_cast<uint64_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
@@ -99,51 +104,63 @@
// 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));
+ uint32_t result = (exp > 52)
+ ? static_cast<uint32_t>(bits << (exp - 52))
+ : static_cast<uint32_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.
- 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 = "" +-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;
+ // Note that missingOne should be held as uint32_t since ((1 << 31) - 1) causes
+ // int32_t overflow.
+ if (Mode == ToInt32Mode::AfterSensibleConversionAttempt) {
+ if (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. This optimization offers 5% performance improvement in
+ // kraken-crypto-pbkdf2.
+ //
+ // 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 = "" +-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 constexpr uint32_t missingOne = 1U << 31;
+ result &= missingOne - 1;
+ result += missingOne;
+ }
+ } else {
+ if (exp < 32) {
+ const uint32_t missingOne = 1U << 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;
+ return static_cast<int64_t>(bits) < 0 ? -static_cast<int32_t>(result) : static_cast<int32_t>(result);
}
+ALWAYS_INLINE int32_t toInt32(double number)
+{
+ return toInt32Internal<ToInt32Mode::Generic>(number);
+}
+
// This implements ToUInt32, defined in ECMA-262 9.6.
inline uint32_t toUInt32(double number)
{
