Changeset 110631 in webkit

Timestamp:

Mar 13, 2012, 3:59:43 PM (15 years ago)

Author:

fpizlo@apple.com

Message:

Loads from UInt32Arrays should not result in a double up-convert if it isn't necessary
​https://bugs.webkit.org/show_bug.cgi?id=80979
<rdar://problem/11036848>

Reviewed by Oliver Hunt.

Also improved DFG IR dumping to include type information in a somewhat more
intuitive way.

• bytecode/PredictedType.cpp:

(JSC::predictionToAbbreviatedString):
(JSC):

• bytecode/PredictedType.h:

(JSC):

• dfg/DFGAbstractState.cpp:

(JSC::DFG::AbstractState::execute):

• dfg/DFGGraph.cpp:

(JSC::DFG::Graph::dump):

• dfg/DFGPredictionPropagationPhase.cpp:

(JSC::DFG::PredictionPropagationPhase::propagate):

• dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::compileUInt32ToNumber):
(JSC::DFG::SpeculativeJIT::compileGetByValOnIntTypedArray):

• dfg/DFGSpeculativeJIT.h:

(JSC::DFG::SpeculativeJIT::forwardSpeculationCheck):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• bytecode/PredictedType.cpp (modified) (1 diff)
• bytecode/PredictedType.h (modified) (1 diff)
• dfg/DFGAbstractState.cpp (modified) (1 diff)
• dfg/DFGGraph.cpp (modified) (1 diff)
• dfg/DFGPredictionPropagationPhase.cpp (modified) (1 diff)
• dfg/DFGSpeculativeJIT.cpp (modified) (2 diffs)
• dfg/DFGSpeculativeJIT.h (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2012-03-13  Filip Pizlo  <fpizlo@apple.com>
+
+        Loads from UInt32Arrays should not result in a double up-convert if it isn't necessary
+        https://bugs.webkit.org/show_bug.cgi?id=80979
+        <rdar://problem/11036848>
+
+        Reviewed by Oliver Hunt.
+        
+        Also improved DFG IR dumping to include type information in a somewhat more
+        intuitive way.
+
+        * bytecode/PredictedType.cpp:
+        (JSC::predictionToAbbreviatedString):
+        (JSC):
+        * bytecode/PredictedType.h:
+        (JSC):
+        * dfg/DFGAbstractState.cpp:
+        (JSC::DFG::AbstractState::execute):
+        * dfg/DFGGraph.cpp:
+        (JSC::DFG::Graph::dump):
+        * dfg/DFGPredictionPropagationPhase.cpp:
+        (JSC::DFG::PredictionPropagationPhase::propagate):
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileUInt32ToNumber):
+        (JSC::DFG::SpeculativeJIT::compileGetByValOnIntTypedArray):
+        * dfg/DFGSpeculativeJIT.h:
+        (JSC::DFG::SpeculativeJIT::forwardSpeculationCheck):
+
 2012-03-13  George Staikos  <staikos@webkit.org>
 

trunk/Source/JavaScriptCore/bytecode/PredictedType.cpp

@@ -167 +167 @@
 }
 
+const char* predictionToAbbreviatedString(PredictedType prediction)
+{
+    if (isFinalObjectPrediction(prediction))
+        return "<Final>";
+    if (isArrayPrediction(prediction))
+        return "<Array>";
+    if (isStringPrediction(prediction))
+        return "<String>";
+    if (isFunctionPrediction(prediction))
+        return "<Function>";
+    if (isByteArrayPrediction(prediction))
+        return "<Bytearray>";
+    if (isInt8ArrayPrediction(prediction))
+        return "<Int8array>";
+    if (isInt16ArrayPrediction(prediction))
+        return "<Int16array>";
+    if (isInt32ArrayPrediction(prediction))
+        return "<Int32array>";
+    if (isUint8ArrayPrediction(prediction))
+        return "<Uint8array>";
+    if (isUint16ArrayPrediction(prediction))
+        return "<Uint16array>";
+    if (isUint32ArrayPrediction(prediction))
+        return "<Uint32array>";
+    if (isFloat32ArrayPrediction(prediction))
+        return "<Float32array>";
+    if (isFloat64ArrayPrediction(prediction))
+        return "<Float64array>";
+    if (isObjectPrediction(prediction))
+        return "<Object>";
+    if (isCellPrediction(prediction))
+        return "<Cell>";
+    if (isInt32Prediction(prediction))
+        return "<Int32>";
+    if (isDoublePrediction(prediction))
+        return "<Double>";
+    if (isNumberPrediction(prediction))
+        return "<Number>";
+    if (isBooleanPrediction(prediction))
+        return "<Boolean>";
+    if (isOtherPrediction(prediction))
+        return "<Other>";
+    return "";
+}
+
 PredictedType predictionFromClassInfo(const ClassInfo* classInfo)
 {

trunk/Source/JavaScriptCore/bytecode/PredictedType.h

@@ -226 +226 @@
 
 const char* predictionToString(PredictedType value);
+const char* predictionToAbbreviatedString(PredictedType value);
 
 // Merge two predictions. Note that currently this just does left | right. It may

trunk/Source/JavaScriptCore/dfg/DFGAbstractState.cpp

@@ -515 +515 @@
             forNode(node.child1()).filter(PredictUint32Array);
             forNode(node.child2()).filter(PredictInt32);
-            forNode(nodeIndex).set(PredictDouble);
+            if (node.shouldSpeculateInteger())
+                forNode(nodeIndex).set(PredictInt32);
+            else
+                forNode(nodeIndex).set(PredictDouble);
             break;
         }

trunk/Source/JavaScriptCore/dfg/DFGGraph.cpp

@@ -165 +165 @@
             else
                 hasPrinted = true;
-            dataLog("@%u", m_varArgChildren[childIdx].index());
+            dataLog("@%u%s", m_varArgChildren[childIdx].index(), predictionToAbbreviatedString(at(childIdx).prediction()));
         }
     } else {
         if (!!node.child1())
-            dataLog("@%u", node.child1().index());
+            dataLog("@%u%s", node.child1().index(), predictionToAbbreviatedString(at(node.child1()).prediction()));
         if (!!node.child2())
-            dataLog(", @%u", node.child2().index());
+            dataLog(", @%u%s", node.child2().index(), predictionToAbbreviatedString(at(node.child2()).prediction()));
         if (!!node.child3())
-            dataLog(", @%u", node.child3().index());
+            dataLog(", @%u%s", node.child3().index(), predictionToAbbreviatedString(at(node.child3()).prediction()));
         hasPrinted = !!node.child1();
     }

trunk/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp

@@ -398 +398 @@
             
         case GetByVal: {
-            if (m_graph[node.child1()].shouldSpeculateUint32Array() || m_graph[node.child1()].shouldSpeculateFloat32Array() || m_graph[node.child1()].shouldSpeculateFloat64Array())
+            if (m_graph[node.child1()].shouldSpeculateFloat32Array() || m_graph[node.child1()].shouldSpeculateFloat64Array())
                 changed |= mergePrediction(PredictDouble);
             else if (node.getHeapPrediction())

trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

@@ -1548 +1548 @@
     // to do this because by this point, the original values necessary to compile whatever
     // operation the UInt32ToNumber originated from might be dead.
-    speculationCheck(Overflow, JSValueRegs(), NoNode, m_jit.branch32(MacroAssembler::LessThan, op1.gpr(), TrustedImm32(0)));
-        
-    // Verify that we can do roll forward.
-    // FIXME: This isn't right, since the next node in the graph may not actually be the next
-    // node in the basic block's execution sequence.
-    ASSERT(at(m_compileIndex + 1).op() == SetLocal);
-    ASSERT(at(m_compileIndex + 1).codeOrigin == node.codeOrigin);
-    ASSERT(at(m_compileIndex + 2).codeOrigin != node.codeOrigin);
-        
-    // Now do the magic.
-    OSRExit& exit = m_jit.codeBlock()->lastOSRExit();
-    Node& setLocal = at(m_compileIndex + 1);
-    exit.m_codeOrigin = at(m_compileIndex + 2).codeOrigin;
-    exit.m_lastSetOperand = setLocal.local();
-        
-    // Create the value recovery, and stuff it into the right place.
-    exit.valueRecoveryForOperand(setLocal.local()) = ValueRecovery::uint32InGPR(op1.gpr());
+    forwardSpeculationCheck(Overflow, JSValueRegs(), NoNode, m_jit.branch32(MacroAssembler::LessThan, op1.gpr(), TrustedImm32(0)), ValueRecovery::uint32InGPR(op1.gpr()));
 
     m_jit.move(op1.gpr(), result.gpr());
@@ -1761 +1745 @@
     }
     outOfBounds.link(&m_jit);
-    if (elementSize < 4 || signedness == SignedTypedArray)
+    if (elementSize < 4 || signedness == SignedTypedArray) {
         integerResult(resultReg, m_compileIndex);
-    else {
-        FPRTemporary fresult(this);
-        m_jit.convertInt32ToDouble(resultReg, fresult.fpr());
-        JITCompiler::Jump positive = m_jit.branch32(MacroAssembler::GreaterThanOrEqual, resultReg, TrustedImm32(0));
-        m_jit.addDouble(JITCompiler::AbsoluteAddress(&AssemblyHelpers::twoToThe32), fresult.fpr());
-        positive.link(&m_jit);
-        doubleResult(fresult.fpr(), m_compileIndex);
-    }
+        return;
+    }
+    
+    ASSERT(elementSize == 4 && signedness == UnsignedTypedArray);
+    if (node.shouldSpeculateInteger()) {
+        forwardSpeculationCheck(Overflow, JSValueRegs(), NoNode, m_jit.branch32(MacroAssembler::LessThan, resultReg, TrustedImm32(0)), ValueRecovery::uint32InGPR(resultReg));
+        integerResult(resultReg, m_compileIndex);
+        return;
+    }
+    
+    FPRTemporary fresult(this);
+    m_jit.convertInt32ToDouble(resultReg, fresult.fpr());
+    JITCompiler::Jump positive = m_jit.branch32(MacroAssembler::GreaterThanOrEqual, resultReg, TrustedImm32(0));
+    m_jit.addDouble(JITCompiler::AbsoluteAddress(&AssemblyHelpers::twoToThe32), fresult.fpr());
+    positive.link(&m_jit);
+    doubleResult(fresult.fpr(), m_compileIndex);
 }
 

trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h

@@ -1824 +1824 @@
         speculationCheck(kind, jsValueSource, nodeUse.index(), jumpToFail, recovery);
     }
+    void forwardSpeculationCheck(ExitKind kind, JSValueSource jsValueSource, NodeIndex nodeIndex, MacroAssembler::Jump jumpToFail, const ValueRecovery& valueRecovery)
+    {
+        speculationCheck(kind, jsValueSource, nodeIndex, jumpToFail);
+        
+        Node& setLocal = at(m_jit.graph().m_blocks[m_block]->at(m_indexInBlock + 1));
+        Node& nextNode = at(m_jit.graph().m_blocks[m_block]->at(m_indexInBlock + 2));
+        ASSERT(setLocal.op() == SetLocal);
+        ASSERT(setLocal.codeOrigin == at(m_compileIndex).codeOrigin);
+        ASSERT(nextNode.codeOrigin != at(m_compileIndex).codeOrigin);
+        
+        OSRExit& exit = m_jit.codeBlock()->lastOSRExit();
+        exit.m_codeOrigin = nextNode.codeOrigin;
+        exit.m_lastSetOperand = setLocal.local();
+        
+        exit.valueRecoveryForOperand(setLocal.local()) = valueRecovery;
+    }
 
     // Called when we statically determine that a speculation will fail.