Changeset 95754 in webkit

Timestamp:

Sep 22, 2011 3:02:24 PM (13 years ago)

Author:

fpizlo@apple.com

Message:

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):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• bytecode/CodeBlock.h (modified) (2 diffs)
• dfg/DFGByteCodeParser.cpp (modified) (2 diffs)
• dfg/DFGNode.h (modified) (1 diff)
• dfg/DFGPropagator.cpp (modified) (5 diffs)
• dfg/DFGSpeculativeJIT.cpp (modified) (1 diff)
• jit/JITArithmetic.cpp (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2011-09-22  Filip Pizlo  <fpizlo@apple.com>
+
+        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  <oliver@apple.com>
 

trunk/Source/JavaScriptCore/bytecode/CodeBlock.h

@@ -538 +538 @@
         }
         
+        bool likelyToTakeSpecialFastCase(int bytecodeOffset)
+        {
+            unsigned specialFastCaseCount = specialFastCaseProfileForBytecodeOffset(bytecodeOffset)->m_counter;
+            return specialFastCaseCount >= slowCaseThreshold();
+        }
+        
         bool likelyToTakeDeepestSlowCase(int bytecodeOffset)
         {
@@ -543 +549 @@
             unsigned specialFastCaseCount = specialFastCaseProfileForBytecodeOffset(bytecodeOffset)->m_counter;
             return (slowCaseCount - specialFastCaseCount) >= slowCaseThreshold();
+        }
+        
+        bool likelyToTakeAnySlowCase(int bytecodeOffset)
+        {
+            unsigned slowCaseCount = rareCaseProfileForBytecodeOffset(bytecodeOffset)->m_counter;
+            unsigned specialFastCaseCount = specialFastCaseProfileForBytecodeOffset(bytecodeOffset)->m_counter;
+            return (slowCaseCount + specialFastCaseCount) >= slowCaseThreshold();
         }
         

trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp

@@ -537 +537 @@
             break;
         }
+        
+        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;
@@ -917 +935 @@
             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);
         }

trunk/Source/JavaScriptCore/dfg/DFGNode.h

@@ -491 +491 @@
         case ArithMax:
         case ArithMod:
+        case ArithDiv:
         case ValueAdd:
             return true;

trunk/Source/JavaScriptCore/dfg/DFGPropagator.cpp

@@ -212 +212 @@
         }
             
-        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
@@ -423 +424 @@
         case ArithMul:
         case ArithMin:
-        case ArithMax: {
+        case ArithMax:
+        case ArithDiv: {
             PredictedType left = m_predictions[node.child1()];
             PredictedType right = m_predictions[node.child2()];
@@ -436 +438 @@
         }
             
-        case ArithDiv:
         case ArithSqrt: {
             changed |= setPrediction(makePrediction(PredictDouble, StrongPrediction));
@@ -650 +651 @@
         case ArithMin:
         case ArithMax:
-        case ArithMod: {
+        case ArithMod:
+        case ArithDiv: {
             if (!nodeCanSpeculateInteger(node.arithNodeFlags())) {
                 toDouble(node.child1());
@@ -666 +668 @@
                     toDouble(node.child1());
             }
-            break;
-        }
-            
-        case ArithDiv: {
-            toDouble(node.child1());
-            toDouble(node.child2());
             break;
         }

trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

@@ -1099 +1099 @@
 
     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());

trunk/Source/JavaScriptCore/jit/JITArithmetic.cpp

@@ -1048 +1048 @@
     }
     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);