From de08b63c9a1325dc9c2816f98df0a4896ec1689a Mon Sep 17 00:00:00 2001 From: "fpizlo@apple.com" Date: Thu, 22 Sep 2011 22:02:24 +0000 Subject: [PATCH] DFG JIT should support integer division https://bugs.webkit.org/show_bug.cgi?id=68597 Reviewed by Darin Adler. This adds support for ArithDiv speculating integer, and speculating that the result is integer (i.e. remainder = 0). This is a 4% win on Kraken and a 1% loss on V8. * bytecode/CodeBlock.h: * dfg/DFGByteCodeParser.cpp: (JSC::DFG::ByteCodeParser::makeDivSafe): (JSC::DFG::ByteCodeParser::parseBlock): * dfg/DFGNode.h: (JSC::DFG::Node::hasArithNodeFlags): * dfg/DFGPropagator.cpp: (JSC::DFG::Propagator::propagateArithNodeFlags): (JSC::DFG::Propagator::propagateNodePredictions): (JSC::DFG::Propagator::fixupNode): * dfg/DFGSpeculativeJIT.cpp: (JSC::DFG::SpeculativeJIT::compile): * jit/JITArithmetic.cpp: (JSC::JIT::emit_op_div): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@95754 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- Source/JavaScriptCore/ChangeLog | 27 ++++++++++++++++++ Source/JavaScriptCore/bytecode/CodeBlock.h | 13 +++++++++ Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp | 20 ++++++++++++- Source/JavaScriptCore/dfg/DFGNode.h | 1 + Source/JavaScriptCore/dfg/DFGPropagator.cpp | 16 ++++------- Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp | 37 +++++++++++++++++++++++++ Source/JavaScriptCore/jit/JITArithmetic.cpp | 29 ++++++++++++++++++- 7 files changed, 131 insertions(+), 12 deletions(-) diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog index 3b64160..a955cb2 100644 --- a/Source/JavaScriptCore/ChangeLog +++ b/Source/JavaScriptCore/ChangeLog @@ -1,3 +1,30 @@ +2011-09-22 Filip Pizlo + + DFG JIT should support integer division + https://bugs.webkit.org/show_bug.cgi?id=68597 + + Reviewed by Darin Adler. + + This adds support for ArithDiv speculating integer, and speculating + that the result is integer (i.e. remainder = 0). + + This is a 4% win on Kraken and a 1% loss on V8. + + * bytecode/CodeBlock.h: + * dfg/DFGByteCodeParser.cpp: + (JSC::DFG::ByteCodeParser::makeDivSafe): + (JSC::DFG::ByteCodeParser::parseBlock): + * dfg/DFGNode.h: + (JSC::DFG::Node::hasArithNodeFlags): + * dfg/DFGPropagator.cpp: + (JSC::DFG::Propagator::propagateArithNodeFlags): + (JSC::DFG::Propagator::propagateNodePredictions): + (JSC::DFG::Propagator::fixupNode): + * dfg/DFGSpeculativeJIT.cpp: + (JSC::DFG::SpeculativeJIT::compile): + * jit/JITArithmetic.cpp: + (JSC::JIT::emit_op_div): + 2011-09-22 Oliver Hunt Implement put_scoped_var in the DFG jit diff --git a/Source/JavaScriptCore/bytecode/CodeBlock.h b/Source/JavaScriptCore/bytecode/CodeBlock.h index 5a577d0..687f749 100644 --- a/Source/JavaScriptCore/bytecode/CodeBlock.h +++ b/Source/JavaScriptCore/bytecode/CodeBlock.h @@ -537,6 +537,12 @@ namespace JSC { return WTF::genericBinarySearch(m_specialFastCaseProfiles, m_specialFastCaseProfiles.size(), bytecodeOffset); } + bool likelyToTakeSpecialFastCase(int bytecodeOffset) + { + unsigned specialFastCaseCount = specialFastCaseProfileForBytecodeOffset(bytecodeOffset)->m_counter; + return specialFastCaseCount >= slowCaseThreshold(); + } + bool likelyToTakeDeepestSlowCase(int bytecodeOffset) { unsigned slowCaseCount = rareCaseProfileForBytecodeOffset(bytecodeOffset)->m_counter; @@ -544,6 +550,13 @@ namespace JSC { return (slowCaseCount - specialFastCaseCount) >= slowCaseThreshold(); } + bool likelyToTakeAnySlowCase(int bytecodeOffset) + { + unsigned slowCaseCount = rareCaseProfileForBytecodeOffset(bytecodeOffset)->m_counter; + unsigned specialFastCaseCount = specialFastCaseProfileForBytecodeOffset(bytecodeOffset)->m_counter; + return (slowCaseCount + specialFastCaseCount) >= slowCaseThreshold(); + } + void resetRareCaseProfiles(); #endif diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp index 097cca8..fa653e4 100644 --- a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp +++ b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp @@ -540,6 +540,24 @@ private: return nodeIndex; } + NodeIndex makeDivSafe(NodeIndex nodeIndex) + { + ASSERT(m_graph[nodeIndex].op == ArithDiv); + + // The main slow case counter for op_div in the old JIT counts only when + // the operands are not numbers. We don't care about that since we already + // have speculations in place that take care of that separately. We only + // care about when the outcome of the division is not an integer, which + // is what the special fast case counter tells us. + + if (!m_profiledBlock->likelyToTakeSpecialFastCase(m_currentIndex)) + return nodeIndex; + + m_graph[nodeIndex].mergeArithNodeFlags(NodeMayOverflow | NodeMayNegZero); + + return nodeIndex; + } + JSGlobalData* m_globalData; CodeBlock* m_codeBlock; CodeBlock* m_profiledBlock; @@ -916,7 +934,7 @@ bool ByteCodeParser::parseBlock(unsigned limit) case op_div: { NodeIndex op1 = getToNumber(currentInstruction[2].u.operand); NodeIndex op2 = getToNumber(currentInstruction[3].u.operand); - set(currentInstruction[1].u.operand, addToGraph(ArithDiv, op1, op2)); + set(currentInstruction[1].u.operand, makeDivSafe(addToGraph(ArithDiv, OpInfo(NodeUseBottom), op1, op2))); NEXT_OPCODE(op_div); } diff --git a/Source/JavaScriptCore/dfg/DFGNode.h b/Source/JavaScriptCore/dfg/DFGNode.h index 335208b..9f6cd9d 100644 --- a/Source/JavaScriptCore/dfg/DFGNode.h +++ b/Source/JavaScriptCore/dfg/DFGNode.h @@ -490,6 +490,7 @@ struct Node { case ArithMin: case ArithMax: case ArithMod: + case ArithDiv: case ValueAdd: return true; default: diff --git a/Source/JavaScriptCore/dfg/DFGPropagator.cpp b/Source/JavaScriptCore/dfg/DFGPropagator.cpp index cd49e55..c99345e 100644 --- a/Source/JavaScriptCore/dfg/DFGPropagator.cpp +++ b/Source/JavaScriptCore/dfg/DFGPropagator.cpp @@ -211,7 +211,8 @@ private: break; } - case ArithMul: { + case ArithMul: + case ArithDiv: { // As soon as a multiply happens, we can easily end up in the part // of the double domain where the point at which you do truncation // can change the outcome. So, ArithMul always checks for overflow @@ -422,7 +423,8 @@ private: case ArithSub: case ArithMul: case ArithMin: - case ArithMax: { + case ArithMax: + case ArithDiv: { PredictedType left = m_predictions[node.child1()]; PredictedType right = m_predictions[node.child2()]; @@ -435,7 +437,6 @@ private: break; } - case ArithDiv: case ArithSqrt: { changed |= setPrediction(makePrediction(PredictDouble, StrongPrediction)); break; @@ -649,7 +650,8 @@ private: case ArithMul: case ArithMin: case ArithMax: - case ArithMod: { + case ArithMod: + case ArithDiv: { if (!nodeCanSpeculateInteger(node.arithNodeFlags())) { toDouble(node.child1()); toDouble(node.child2()); @@ -668,12 +670,6 @@ private: break; } - case ArithDiv: { - toDouble(node.child1()); - toDouble(node.child2()); - break; - } - case ArithAbs: { if (!nodeCanSpeculateInteger(node.arithNodeFlags())) { toDouble(node.child1()); diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp index a64ed95..77d238d 100644 --- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp +++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp @@ -1098,6 +1098,43 @@ void SpeculativeJIT::compile(Node& node) } case ArithDiv: { + if (shouldSpeculateInteger(node.child1(), node.child2()) && nodeCanSpeculateInteger(node.arithNodeFlags())) { + 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(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(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); + + // Check that there was no remainder. If there had been, then we'd be obligated to + // produce a double result instead. + speculationCheck(m_jit.branchTest32(JITCompiler::NonZero, edx.gpr())); + + integerResult(eax.gpr(), m_compileIndex); + break; + } + SpeculateDoubleOperand op1(this, node.child1()); SpeculateDoubleOperand op2(this, node.child2()); FPRTemporary result(this, op1); diff --git a/Source/JavaScriptCore/jit/JITArithmetic.cpp b/Source/JavaScriptCore/jit/JITArithmetic.cpp index 38b020d..3167f5e 100644 --- a/Source/JavaScriptCore/jit/JITArithmetic.cpp +++ b/Source/JavaScriptCore/jit/JITArithmetic.cpp @@ -1047,10 +1047,37 @@ void JIT::emit_op_div(Instruction* currentInstruction) skipDoubleLoad.link(this); } divDouble(fpRegT1, fpRegT0); - + +#if ENABLE(DFG_JIT) + // Is the result actually an integer? The DFG JIT would really like to know. If it's + // not an integer, we increment a count. If this together with the slow case counter + // are below threshold then the DFG JIT will compile this division with a specualtion + // that the remainder is zero. + + // As well, there are cases where a double result here would cause an important field + // in the heap to sometimes have doubles in it, resulting in double predictions getting + // propagated to a use site where it might cause damage (such as the index to an array + // access). So if we are DFG compiling anything in the program, we want this code to + // ensure that it produces integers whenever possible. + + // FIXME: This will fail to convert to integer if the result is zero. We should + // distinguish between positive zero and negative zero here. + + JumpList notInteger; + branchConvertDoubleToInt32(fpRegT0, regT0, notInteger, fpRegT1); + // If we've got an integer, we might as well make that the result of the division. + emitFastArithReTagImmediate(regT0, regT0); + Jump isInteger = jump(); + notInteger.link(this); + add32(Imm32(1), AbsoluteAddress(&m_codeBlock->addSpecialFastCaseProfile(m_bytecodeOffset)->m_counter)); + moveDoubleToPtr(fpRegT0, regT0); + subPtr(tagTypeNumberRegister, regT0); + isInteger.link(this); +#else // Double result. moveDoubleToPtr(fpRegT0, regT0); subPtr(tagTypeNumberRegister, regT0); +#endif emitPutVirtualRegister(dst, regT0); } -- 2.7.4