Modified: trunk/Source/_javascript_Core/ChangeLog (184219 => 184220)
--- trunk/Source/_javascript_Core/ChangeLog 2015-05-12 21:24:37 UTC (rev 184219)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-05-12 21:28:43 UTC (rev 184220)
@@ -1,3 +1,47 @@
+2015-05-12 Benjamin Poulain <[email protected]>
+
+ [JSC] Make the NegZero backward propagated flags of ArithMod stricter
+ https://bugs.webkit.org/show_bug.cgi?id=144897
+
+ Reviewed by Geoffrey Garen.
+
+ The NegZero flags of ArithMod were the same as ArithDiv: both children were
+ marked as needing to handle NegativeZero.
+
+ Lucky for us, ArithMod is quite a bit different than ArithDiv.
+
+ First, the sign of the result is completely independent from
+ the sign of the divisor. A zero on the divisor always produces a NaN.
+ That's great, we can remove the NodeBytecodeNeedsNegZero
+ from the flags propagated to child2.
+
+ Second, the sign of the result is always the same as the sign of
+ the dividend. A dividend of zero produces a zero of same sign
+ unless the divisor is zero (in which case the result is NaN).
+ This is great too: we can just pass the flags we got into
+ ArithMod.
+
+ With those two out of the way, we can make a faster version of ArithRound
+ for Kraken's oscillator. Since we no longer care about negative zero,
+ rounding becomes cast<int32>(value + 0.5). This gives ~3% faster runtime
+ on the benchmark.
+
+ Unfortunatelly, most of the time is spent in FTL and the same optimization
+ does not apply well just yet: rdar://problem/20904149.
+
+ * dfg/DFGBackwardsPropagationPhase.cpp:
+ (JSC::DFG::BackwardsPropagationPhase::propagate):
+ Never add NodeBytecodeNeedsNegZero unless needed by the users of this node.
+
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::compileArithRound):
+ Faster Math.round() when negative zero is not important.
+
+ * tests/stress/arith-modulo-node-behaviors.js: Added.
+ (moduloWithNegativeZeroDividend):
+ (moduloWithUnusedNegativeZeroDividend):
+ (moduloWithNegativeZeroDivisor):
+
2015-05-12 Mark Lam <[email protected]>
Refactor MachineStackMarker.cpp so that it's easier to reason about MachineThreads::Thread.
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (184219 => 184220)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2015-05-12 21:24:37 UTC (rev 184219)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2015-05-12 21:28:43 UTC (rev 184220)
@@ -3550,18 +3550,34 @@
SpeculateDoubleOperand value(this, node->child1());
FPRReg valueFPR = value.fpr();
+
+ if (producesInteger(node->arithRoundingMode()) && !shouldCheckNegativeZero(node->arithRoundingMode())) {
+ FPRTemporary oneHalf(this);
+ GPRTemporary roundedResultAsInt32(this);
+ FPRReg _oneHalfFPR_ = oneHalf.fpr();
+ GPRReg resultGPR = roundedResultAsInt32.gpr();
+
+ static const double halfConstant = 0.5;
+ m_jit.loadDouble(MacroAssembler::TrustedImmPtr(&halfConstant), oneHalfFPR);
+ m_jit.addDouble(valueFPR, oneHalfFPR);
+
+ JITCompiler::Jump truncationFailed = m_jit.branchTruncateDoubleToInt32(oneHalfFPR, resultGPR);
+ speculationCheck(Overflow, JSValueRegs(), node, truncationFailed);
+ int32Result(resultGPR, node);
+ return;
+ }
+
flushRegisters();
FPRResult roundedResultAsDouble(this);
FPRReg resultFPR = roundedResultAsDouble.fpr();
callOperation(jsRound, resultFPR, valueFPR);
-
if (producesInteger(node->arithRoundingMode())) {
GPRTemporary roundedResultAsInt32(this);
FPRTemporary scratch(this);
FPRReg scratchFPR = scratch.fpr();
GPRReg resultGPR = roundedResultAsInt32.gpr();
JITCompiler::JumpList failureCases;
- m_jit.branchConvertDoubleToInt32(resultFPR, resultGPR, failureCases, scratchFPR, shouldCheckNegativeZero(node->arithRoundingMode()));
+ m_jit.branchConvertDoubleToInt32(resultFPR, resultGPR, failureCases, scratchFPR);
speculationCheck(Overflow, JSValueRegs(), node, failureCases);
int32Result(resultGPR, node);
Added: trunk/Source/_javascript_Core/tests/stress/arith-modulo-node-behaviors.js (0 => 184220)
--- trunk/Source/_javascript_Core/tests/stress/arith-modulo-node-behaviors.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/arith-modulo-node-behaviors.js 2015-05-12 21:28:43 UTC (rev 184220)
@@ -0,0 +1,104 @@
+// Verify that the dividend propagate the NeedsNegZero if the dividend requires it.
+function moduloWithNegativeZeroDividend(a, b, c)
+{
+ var temp = a * b;
+ return temp % c;
+}
+noInline(moduloWithNegativeZeroDividend);
+
+// Warm up with integers. The test for NegZero should not be eliminated here.
+for (var i = 1; i < 1e7; ++i) {
+ var result = moduloWithNegativeZeroDividend(i, 5, 5);
+ if (result !== 0)
+ throw "moduloWithNegativeZeroDividend(i, 5, 5), returned: " + result;
+}
+
+for (var i = 1; i < 1e7; ++i) {
+ // Produce negative zero in the multiplication.
+ var result = moduloWithNegativeZeroDividend(-i, 0, 2);
+ if (!(result === 0 && (1/result) === -Infinity))
+ throw "moduloWithNegativeZeroDividend(-i, 0, 2) failed, returned: " + result;
+
+ // A negative dividend can produce negative zero results.
+ var result = moduloWithNegativeZeroDividend(-i, 5, 5);
+ if (!(result === 0 && (1/result) === -Infinity))
+ throw "moduloWithNegativeZeroDividend(-i, 5, 5) failed, returned: " + result;
+}
+
+// Edge cases.
+for (var i = 1; i < 1e7; ++i) {
+ var result = moduloWithNegativeZeroDividend(-i, 0, Infinity);
+ if (!(result === 0 && (1/result) === -Infinity))
+ throw "moduloWithNegativeZeroDividend(-i, 0, Infinity) failed, returned: " + result;
+
+ var result = moduloWithNegativeZeroDividend(-i, 0, -Infinity);
+ if (!(result === 0 && (1/result) === -Infinity))
+ throw "moduloWithNegativeZeroDividend(-i, 0, -Infinity) failed, returned: " + result;
+
+ var result = moduloWithNegativeZeroDividend(-i, 0, NaN);
+ if (result === result)
+ throw "moduloWithNegativeZeroDividend(-i, 0, NaN) failed, returned: " + result;
+}
+
+
+// In this case, the negative zero is irrelevant. The Neg Zero check can be eliminated.
+function moduloWithUnusedNegativeZeroDividend(a, b, c)
+{
+ var temp = a * b;
+ return (temp % c) | 0;
+}
+noInline(moduloWithUnusedNegativeZeroDividend);
+
+for (var i = 1; i < 1e7; ++i) {
+ var result = moduloWithUnusedNegativeZeroDividend(i, 5, 5);
+ if (result !== 0)
+ throw "moduloWithUnusedNegativeZeroDividend(i, 5, 5), returned: " + result;
+}
+
+// Produce negative zero in the multiplication.
+for (var i = 1; i < 1e7; ++i) {
+ var result = moduloWithUnusedNegativeZeroDividend(-i, 0, 2);
+ if (!(result === 0 && (1/result) === Infinity))
+ throw "moduloWithUnusedNegativeZeroDividend(-i, 0, 2) failed, returned: " + result;
+}
+
+for (var i = 1; i < 1e7; ++i) {
+ var result = moduloWithUnusedNegativeZeroDividend(-i, 0, Infinity);
+ if (!(result === 0 && (1/result) === Infinity))
+ throw "moduloWithUnusedNegativeZeroDividend(-i, 0, Infinity) failed, returned: " + result;
+
+ var result = moduloWithUnusedNegativeZeroDividend(-i, 0, -Infinity);
+ if (!(result === 0 && (1/result) === Infinity))
+ throw "moduloWithUnusedNegativeZeroDividend(-i, 0, -Infinity) failed, returned: " + result;
+
+ var result = moduloWithUnusedNegativeZeroDividend(-i, 0, NaN);
+ if (result !== 0)
+ throw "moduloWithUnusedNegativeZeroDividend(-i, 0, NaN) failed, returned: " + result;
+}
+
+
+// The sign of the divisor is completely irrelevant. This should never fail on negative zero divisors.
+function moduloWithNegativeZeroDivisor(a, b, c)
+{
+ var temp = a * b;
+ return c % temp;
+}
+noInline(moduloWithNegativeZeroDivisor);
+
+// Warm up with integers.
+for (var i = 1; i < 1e7; ++i) {
+ var result = moduloWithNegativeZeroDivisor(i, 2, i);
+ if (result !== i)
+ throw "moduloWithNegativeZeroDividend(i, 2, i), returned: " + result;
+
+ var result = moduloWithNegativeZeroDivisor(-i, 2, i);
+ if (result !== i)
+ throw "moduloWithNegativeZeroDividend(-i, 2, i), returned: " + result;
+}
+
+// Produce negative zero in the multiplication.
+for (var i = 1; i < 1e7; ++i) {
+ var result = moduloWithNegativeZeroDivisor(-i, 0, 2);
+ if (result === result)
+ throw "moduloWithNegativeZeroDivisor(-i, 0, 2) failed, returned: " + result;
+}
\ No newline at end of file
