Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (111354 => 111355)
--- trunk/Source/_javascript_Core/ChangeLog 2012-03-20 05:04:58 UTC (rev 111354)
+++ trunk/Source/_javascript_Core/ChangeLog 2012-03-20 05:15:50 UTC (rev 111355)
@@ -1,3 +1,30 @@
+2012-03-19 Filip Pizlo <[email protected]>
+
+ Division optimizations fail to infer cases of truncated division and
+ mishandle -2147483648/-1
+ https://bugs.webkit.org/show_bug.cgi?id=81428
+ <rdar://problem/11067382>
+
+ Reviewed by Oliver Hunt.
+
+ If you're a division over integers and you're only used as an integer, then you're
+ an integer division and remainder checks become unnecessary. If you're dividing
+ -2147483648 by -1, don't crash.
+
+ * assembler/MacroAssemblerX86Common.h:
+ (MacroAssemblerX86Common):
+ (JSC::MacroAssemblerX86Common::add32):
+ * dfg/DFGSpeculativeJIT.cpp:
+ (DFG):
+ (JSC::DFG::SpeculativeJIT::compileIntegerArithDivForX86):
+ * dfg/DFGSpeculativeJIT.h:
+ (SpeculativeJIT):
+ * dfg/DFGSpeculativeJIT32_64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * dfg/DFGSpeculativeJIT64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * llint/LowLevelInterpreter64.asm:
+
2012-03-19 Benjamin Poulain <[email protected]>
Simplify SmallStrings
Modified: trunk/Source/_javascript_Core/assembler/MacroAssemblerX86Common.h (111354 => 111355)
--- trunk/Source/_javascript_Core/assembler/MacroAssemblerX86Common.h 2012-03-20 05:04:58 UTC (rev 111354)
+++ trunk/Source/_javascript_Core/assembler/MacroAssemblerX86Common.h 2012-03-20 05:15:50 UTC (rev 111355)
@@ -129,6 +129,11 @@
{
m_assembler.addl_rm(src, dest.offset, dest.base);
}
+
+ void add32(TrustedImm32 imm, RegisterID src, RegisterID dest)
+ {
+ m_assembler.leal_mr(imm.m_value, src, dest);
+ }
void and32(RegisterID src, RegisterID dest)
{
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (111354 => 111355)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2012-03-20 05:04:58 UTC (rev 111354)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2012-03-20 05:15:50 UTC (rev 111355)
@@ -2450,6 +2450,82 @@
doubleResult(result.fpr(), m_compileIndex);
}
+#if CPU(X86) || CPU(X86_64)
+void SpeculativeJIT::compileIntegerArithDivForX86(Node& node)
+{
+ SpeculateIntegerOperand op1(this, node.child1());
+ SpeculateIntegerOperand op2(this, node.child2());
+ GPRTemporary eax(this, X86Registers::eax);
+ GPRTemporary edx(this, X86Registers::edx);
+ GPRReg op1GPR = op1.gpr();
+ GPRReg op2GPR = op2.gpr();
+
+ GPRReg op2TempGPR;
+ GPRReg temp;
+ if (op2GPR == X86Registers::eax || op2GPR == X86Registers::edx) {
+ op2TempGPR = allocate();
+ temp = op2TempGPR;
+ } else {
+ op2TempGPR = InvalidGPRReg;
+ if (op1GPR == X86Registers::eax)
+ temp = X86Registers::edx;
+ else
+ temp = X86Registers::eax;
+ }
+
+ ASSERT(temp != op1GPR);
+ ASSERT(temp != op2GPR);
+
+ m_jit.add32(JITCompiler::TrustedImm32(1), op2GPR, temp);
+
+ JITCompiler::Jump safeDenominator = m_jit.branch32(JITCompiler::Above, temp, JITCompiler::TrustedImm32(1));
+
+ JITCompiler::Jump done;
+ if (nodeUsedAsNumber(node.arithNodeFlags())) {
+ speculationCheck(Overflow, JSValueRegs(), NoNode, m_jit.branchTest32(JITCompiler::Zero, op2GPR));
+ speculationCheck(Overflow, JSValueRegs(), NoNode, m_jit.branch32(JITCompiler::Equal, op1GPR, TrustedImm32(-2147483648)));
+ } else {
+ JITCompiler::Jump zero = m_jit.branchTest32(JITCompiler::Zero, op2GPR);
+ JITCompiler::Jump notNeg2ToThe31 = m_jit.branch32(JITCompiler::Equal, op1GPR, TrustedImm32(-2147483648));
+ zero.link(&m_jit);
+ m_jit.move(TrustedImm32(0), eax.gpr());
+ done = m_jit.jump();
+ notNeg2ToThe31.link(&m_jit);
+ }
+
+ safeDenominator.link(&m_jit);
+
+ // If the user cares about negative zero, then speculate that we're not about
+ // to produce negative zero.
+ if (!nodeCanIgnoreNegativeZero(node.arithNodeFlags())) {
+ MacroAssembler::Jump numeratorNonZero = m_jit.branchTest32(MacroAssembler::NonZero, op1GPR);
+ speculationCheck(NegativeZero, JSValueRegs(), NoNode, m_jit.branch32(MacroAssembler::LessThan, op2GPR, TrustedImm32(0)));
+ numeratorNonZero.link(&m_jit);
+ }
+
+ if (op2TempGPR != InvalidGPRReg) {
+ m_jit.move(op2GPR, op2TempGPR);
+ op2GPR = op2TempGPR;
+ }
+
+ m_jit.move(op1GPR, eax.gpr());
+ m_jit.assembler().cdq();
+ m_jit.assembler().idivl_r(op2GPR);
+
+ if (op2TempGPR != InvalidGPRReg)
+ unlock(op2TempGPR);
+
+ // Check that there was no remainder. If there had been, then we'd be obligated to
+ // produce a double result instead.
+ if (nodeUsedAsNumber(node.arithNodeFlags()))
+ speculationCheck(Overflow, JSValueRegs(), NoNode, m_jit.branchTest32(JITCompiler::NonZero, edx.gpr()));
+ else
+ done.link(&m_jit);
+
+ integerResult(eax.gpr(), m_compileIndex);
+}
+#endif // CPU(X86) || CPU(X86_64)
+
void SpeculativeJIT::compileArithMod(Node& node)
{
if (!at(node.child1()).shouldNotSpeculateInteger() && !at(node.child2()).shouldNotSpeculateInteger()
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h (111354 => 111355)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2012-03-20 05:04:58 UTC (rev 111354)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2012-03-20 05:15:50 UTC (rev 111355)
@@ -1737,6 +1737,9 @@
void compileArithSub(Node&);
void compileArithNegate(Node&);
void compileArithMul(Node&);
+#if CPU(X86) || CPU(X86_64)
+ void compileIntegerArithDivForX86(Node&);
+#endif
void compileArithMod(Node&);
void compileSoftModulo(Node&);
void compileGetTypedArrayLength(const TypedArrayDescriptor&, Node&, bool needsSpeculationCheck);
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (111354 => 111355)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2012-03-20 05:04:58 UTC (rev 111354)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2012-03-20 05:15:50 UTC (rev 111355)
@@ -1899,43 +1899,7 @@
case ArithDiv: {
if (Node::shouldSpeculateInteger(at(node.child1()), at(node.child2())) && node.canSpeculateInteger()) {
#if CPU(X86)
- SpeculateIntegerOperand op1(this, node.child1());
- SpeculateIntegerOperand op2(this, node.child2());
- GPRReg op1GPR = op1.gpr();
- GPRReg op2GPR = op2.gpr();
-
- speculationCheck(Overflow, JSValueRegs(), NoNode, m_jit.branchTest32(JITCompiler::Zero, op2GPR));
-
- // If the user cares about negative zero, then speculate that we're not about
- // to produce negative zero.
- if (!nodeCanIgnoreNegativeZero(node.arithNodeFlags())) {
- MacroAssembler::Jump numeratorNonZero = m_jit.branchTest32(MacroAssembler::NonZero, op1GPR);
- speculationCheck(NegativeZero, JSValueRegs(), NoNode, m_jit.branch32(MacroAssembler::LessThan, op2GPR, TrustedImm32(0)));
- numeratorNonZero.link(&m_jit);
- }
-
- GPRTemporary eax(this, X86Registers::eax);
- GPRTemporary edx(this, X86Registers::edx);
-
- GPRReg temp2 = InvalidGPRReg;
- if (op2GPR == X86Registers::eax || op2GPR == X86Registers::edx) {
- temp2 = allocate();
- m_jit.move(op2GPR, temp2);
- op2GPR = temp2;
- }
-
- m_jit.move(op1GPR, eax.gpr());
- m_jit.assembler().cdq();
- m_jit.assembler().idivl_r(op2GPR);
-
- if (temp2 != InvalidGPRReg)
- unlock(temp2);
-
- // Check that there was no remainder. If there had been, then we'd be obligated to
- // produce a double result instead.
- speculationCheck(Overflow, JSValueRegs(), NoNode, m_jit.branchTest32(JITCompiler::NonZero, edx.gpr()));
-
- integerResult(eax.gpr(), m_compileIndex);
+ compileIntegerArithDivForX86(node);
#else // CPU(X86) -> so non-X86 code follows
SpeculateDoubleOperand op1(this, node.child1());
SpeculateDoubleOperand op2(this, node.child2());
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (111354 => 111355)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2012-03-20 05:04:58 UTC (rev 111354)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2012-03-20 05:15:50 UTC (rev 111355)
@@ -1984,42 +1984,7 @@
case ArithDiv: {
if (Node::shouldSpeculateInteger(at(node.child1()), at(node.child2())) && node.canSpeculateInteger()) {
- SpeculateIntegerOperand op1(this, node.child1());
- SpeculateIntegerOperand op2(this, node.child2());
- GPRTemporary eax(this, X86Registers::eax);
- GPRTemporary edx(this, X86Registers::edx);
- GPRReg op1GPR = op1.gpr();
- GPRReg op2GPR = op2.gpr();
-
- speculationCheck(Overflow, JSValueRegs(), NoNode, m_jit.branchTest32(JITCompiler::Zero, op2GPR));
-
- // If the user cares about negative zero, then speculate that we're not about
- // to produce negative zero.
- if (!nodeCanIgnoreNegativeZero(node.arithNodeFlags())) {
- MacroAssembler::Jump numeratorNonZero = m_jit.branchTest32(MacroAssembler::NonZero, op1GPR);
- speculationCheck(NegativeZero, JSValueRegs(), NoNode, m_jit.branch32(MacroAssembler::LessThan, op2GPR, TrustedImm32(0)));
- numeratorNonZero.link(&m_jit);
- }
-
- GPRReg temp2 = InvalidGPRReg;
- if (op2GPR == X86Registers::eax || op2GPR == X86Registers::edx) {
- temp2 = allocate();
- m_jit.move(op2GPR, temp2);
- op2GPR = temp2;
- }
-
- m_jit.move(op1GPR, eax.gpr());
- m_jit.assembler().cdq();
- m_jit.assembler().idivl_r(op2GPR);
-
- if (temp2 != InvalidGPRReg)
- unlock(temp2);
-
- // Check that there was no remainder. If there had been, then we'd be obligated to
- // produce a double result instead.
- speculationCheck(Overflow, JSValueRegs(), NoNode, m_jit.branchTest32(JITCompiler::NonZero, edx.gpr()));
-
- integerResult(eax.gpr(), m_compileIndex);
+ compileIntegerArithDivForX86(node);
break;
}
Modified: trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm (111354 => 111355)
--- trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm 2012-03-20 05:04:58 UTC (rev 111354)
+++ trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm 2012-03-20 05:15:50 UTC (rev 111355)
@@ -594,6 +594,9 @@
macro (left, right, slow, index)
# Assume t3 is scratchable.
btiz left, slow
+ bineq left, -1, .notNeg2TwoThe31DivByNeg1
+ bieq right, -2147483648, .slow
+ .notNeg2TwoThe31DivByNeg1:
btinz right, .intOK
bilt left, 0, slow
.intOK: