[webkit-changes] [112015] trunk/Source/JavaScriptCore

[fpizlo]

Title: [112015] trunk/Source/_javascript_Core

Revision

112015

Author

fpi...@apple.com

Date

2012-03-24 17:46:21 -0700 (Sat, 24 Mar 2012)

Log Message

DFG double voting may be overzealous in the case of variables that end up
being used as integers
https://bugs.webkit.org/show_bug.cgi?id=82008

Reviewed by Oliver Hunt.
        
Cleaned up propagation, making the intent more explicit in most places.
Back-propagate NodeUsedAsInt for cases where a node was used in a context
that is known to strongly prefer integers.

* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::handleCall):
(JSC::DFG::ByteCodeParser::parseBlock):
* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::dumpCodeOrigin):
(JSC::DFG::Graph::dump):
* dfg/DFGGraph.h:
(Graph):
* dfg/DFGNodeFlags.cpp:
(JSC::DFG::nodeFlagsAsString):
* dfg/DFGNodeFlags.h:
(DFG):
* dfg/DFGPredictionPropagationPhase.cpp:
(JSC::DFG::PredictionPropagationPhase::run):
(JSC::DFG::PredictionPropagationPhase::propagate):
(PredictionPropagationPhase):
(JSC::DFG::PredictionPropagationPhase::mergeDefaultFlags):
(JSC::DFG::PredictionPropagationPhase::vote):
(JSC::DFG::PredictionPropagationPhase::doRoundOfDoubleVoting):
(JSC::DFG::PredictionPropagationPhase::fixupNode):
* dfg/DFGVariableAccessData.h:
(JSC::DFG::VariableAccessData::shouldUseDoubleFormatAccordingToVote):

Modified Paths

• trunk/Source/_javascript_Core/ChangeLog
• trunk/Source/_javascript_Core/dfg/DFGGraph.cpp
• trunk/Source/_javascript_Core/dfg/DFGGraph.h
• trunk/Source/_javascript_Core/dfg/DFGNodeFlags.cpp
• trunk/Source/_javascript_Core/dfg/DFGNodeFlags.h
• trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp
• trunk/Source/_javascript_Core/dfg/DFGVariableAccessData.h

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (112014 => 112015)

--- trunk/Source/_javascript_Core/ChangeLog	2012-03-24 21:22:41 UTC (rev 112014)
+++ trunk/Source/_javascript_Core/ChangeLog	2012-03-25 00:46:21 UTC (rev 112015)
@@ -1,3 +1,38 @@
+2012-03-23  Filip Pizlo  <fpi...@apple.com>
+
+        DFG double voting may be overzealous in the case of variables that end up
+        being used as integers
+        https://bugs.webkit.org/show_bug.cgi?id=82008
+
+        Reviewed by Oliver Hunt.
+        
+        Cleaned up propagation, making the intent more explicit in most places.
+        Back-propagate NodeUsedAsInt for cases where a node was used in a context
+        that is known to strongly prefer integers.
+
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::handleCall):
+        (JSC::DFG::ByteCodeParser::parseBlock):
+        * dfg/DFGGraph.cpp:
+        (JSC::DFG::Graph::dumpCodeOrigin):
+        (JSC::DFG::Graph::dump):
+        * dfg/DFGGraph.h:
+        (Graph):
+        * dfg/DFGNodeFlags.cpp:
+        (JSC::DFG::nodeFlagsAsString):
+        * dfg/DFGNodeFlags.h:
+        (DFG):
+        * dfg/DFGPredictionPropagationPhase.cpp:
+        (JSC::DFG::PredictionPropagationPhase::run):
+        (JSC::DFG::PredictionPropagationPhase::propagate):
+        (PredictionPropagationPhase):
+        (JSC::DFG::PredictionPropagationPhase::mergeDefaultFlags):
+        (JSC::DFG::PredictionPropagationPhase::vote):
+        (JSC::DFG::PredictionPropagationPhase::doRoundOfDoubleVoting):
+        (JSC::DFG::PredictionPropagationPhase::fixupNode):
+        * dfg/DFGVariableAccessData.h:
+        (JSC::DFG::VariableAccessData::shouldUseDoubleFormatAccordingToVote):
+
 2012-03-24  Filip Pizlo  <fpi...@apple.com>
 
         DFG::Node::shouldNotSpeculateInteger() should be eliminated

Modified: trunk/Source/_javascript_Core/dfg/DFGGraph.cpp (112014 => 112015)

--- trunk/Source/_javascript_Core/dfg/DFGGraph.cpp	2012-03-24 21:22:41 UTC (rev 112014)
+++ trunk/Source/_javascript_Core/dfg/DFGGraph.cpp	2012-03-25 00:46:21 UTC (rev 112015)
@@ -84,13 +84,13 @@
         dataLog(" ");
 }
 
-void Graph::dumpCodeOrigin(NodeIndex nodeIndex)
+void Graph::dumpCodeOrigin(NodeIndex prevNodeIndex, NodeIndex nodeIndex)
 {
-    if (!nodeIndex)
+    if (prevNodeIndex == NoNode)
         return;
     
     Node& currentNode = at(nodeIndex);
-    Node& previousNode = at(nodeIndex - 1);
+    Node& previousNode = at(prevNodeIndex);
     if (previousNode.codeOrigin.inlineCallFrame == currentNode.codeOrigin.inlineCallFrame)
         return;
     
@@ -131,7 +131,6 @@
         --refCount;
     }
     
-    dumpCodeOrigin(nodeIndex);
     printWhiteSpace((node.codeOrigin.inlineDepth() - 1) * 2);
 
     // Example/explanation of dataflow dump output
@@ -263,6 +262,7 @@
 
 void Graph::dump()
 {
+    NodeIndex lastNodeIndex = NoNode;
     for (size_t b = 0; b < m_blocks.size(); ++b) {
         BasicBlock* block = m_blocks[b].get();
         dataLog("Block #%u (bc#%u): %s%s\n", (int)b, block->bytecodeBegin, block->isReachable ? "" : " (skipped)", block->isOSRTarget ? " (OSR target)" : "");
@@ -281,8 +281,11 @@
         dataLog("  var links: ");
         dumpOperands(block->variablesAtHead, WTF::dataFile());
         dataLog("\n");
-        for (size_t i = 0; i < block->size(); ++i)
+        for (size_t i = 0; i < block->size(); ++i) {
+            dumpCodeOrigin(lastNodeIndex, block->at(i));
             dump(block->at(i));
+            lastNodeIndex = block->at(i);
+        }
         dataLog("  vars after: ");
         if (block->cfaHasVisited)
             dumpOperands(block->valuesAtTail, WTF::dataFile());

Modified: trunk/Source/_javascript_Core/dfg/DFGGraph.h (112014 => 112015)

--- trunk/Source/_javascript_Core/dfg/DFGGraph.h	2012-03-24 21:22:41 UTC (rev 112014)
+++ trunk/Source/_javascript_Core/dfg/DFGGraph.h	2012-03-25 00:46:21 UTC (rev 112015)
@@ -143,7 +143,7 @@
 
     // Dump the code origin of the given node as a diff from the code origin of the
     // preceding node.
-    void dumpCodeOrigin(NodeIndex);
+    void dumpCodeOrigin(NodeIndex, NodeIndex);
 
     BlockIndex blockIndexForBytecodeOffset(Vector<BlockIndex>& blocks, unsigned bytecodeBegin);
 

Modified: trunk/Source/_javascript_Core/dfg/DFGNodeFlags.cpp (112014 => 112015)

--- trunk/Source/_javascript_Core/dfg/DFGNodeFlags.cpp	2012-03-24 21:22:41 UTC (rev 112014)
+++ trunk/Source/_javascript_Core/dfg/DFGNodeFlags.cpp	2012-03-25 00:46:21 UTC (rev 112015)
@@ -123,6 +123,13 @@
         hasPrinted = true;
     }
     
+    if (flags & NodeUsedAsInt) {
+        if (hasPrinted)
+            ptr.strcat("|");
+        ptr.strcat("UsedAsInt");
+        hasPrinted = true;
+    }
+    
     *ptr++ = 0;
     
     return description;

Modified: trunk/Source/_javascript_Core/dfg/DFGNodeFlags.h (112014 => 112015)

--- trunk/Source/_javascript_Core/dfg/DFGNodeFlags.h	2012-03-24 21:22:41 UTC (rev 112014)
+++ trunk/Source/_javascript_Core/dfg/DFGNodeFlags.h	2012-03-25 00:46:21 UTC (rev 112015)
@@ -36,23 +36,28 @@
 
 // Entries in the NodeType enum (below) are composed of an id, a result type (possibly none)
 // and some additional informative flags (must generate, is constant, etc).
-#define NodeResultMask             0xF
-#define NodeResultJS               0x1
-#define NodeResultNumber           0x2
-#define NodeResultInt32            0x3
-#define NodeResultBoolean          0x4
-#define NodeResultStorage          0x5
-#define NodeMustGenerate          0x10 // set on nodes that have side effects, and may not trivially be removed by DCE.
-#define NodeHasVarArgs            0x20
-#define NodeClobbersWorld         0x40
-#define NodeMightClobber          0x80
-#define NodeBackPropMask         0x300
-#define NodeUseBottom            0x000
-#define NodeUsedAsNumber         0x100
-#define NodeNeedsNegZero         0x200
-#define NodeBehaviorMask         0xC00
-#define NodeMayOverflow          0x400
-#define NodeMayNegZero           0x800
+#define NodeResultMask              0xF
+#define NodeResultJS                0x1
+#define NodeResultNumber            0x2
+#define NodeResultInt32             0x3
+#define NodeResultBoolean           0x4
+#define NodeResultStorage           0x5
+                                
+#define NodeMustGenerate           0x10 // set on nodes that have side effects, and may not trivially be removed by DCE.
+#define NodeHasVarArgs             0x20
+#define NodeClobbersWorld          0x40
+#define NodeMightClobber           0x80
+                                
+#define NodeBehaviorMask          0x300
+#define NodeMayOverflow           0x100
+#define NodeMayNegZero            0x200
+                                
+#define NodeBackPropMask         0x1C00
+#define NodeUseBottom             0x000
+#define NodeUsedAsNumber          0x400 // The result of this computation may be used in a context that observes fractional results.
+#define NodeNeedsNegZero          0x800 // The result of this computation may be used in a context that observes -0.
+#define NodeUsedAsValue           (NodeUsedAsNumber | NodeNeedsNegZero)
+#define NodeUsedAsInt            0x1000 // The result of this computation is known to be used in a context that prefers, but does not require, integer values.
 
 typedef uint16_t NodeFlags;
 

Modified: trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp (112014 => 112015)

--- trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp	2012-03-24 21:22:41 UTC (rev 112014)
+++ trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp	2012-03-25 00:46:21 UTC (rev 112015)
@@ -45,8 +45,8 @@
 #if DFG_ENABLE(DEBUG_PROPAGATION_VERBOSE)
         m_count = 0;
 #endif
-        // Two stage process: first propagate predictions, then propagate while doing double voting.
-        
+        // 1) propagate predictions
+
         do {
             m_changed = false;
             
@@ -64,6 +64,8 @@
             propagateBackward();
         } while (m_changed);
         
+        // 2) repropagate predictions while doing double voting.
+
         do {
             m_changed = false;
             doRoundOfDoubleVoting();
@@ -119,14 +121,12 @@
             return;
         
         NodeType op = node.op();
-        NodeFlags flags = node.flags();
+        NodeFlags flags = node.flags() & NodeBackPropMask;
 
 #if DFG_ENABLE(DEBUG_PROPAGATION_VERBOSE)
         dataLog("   %s @%u: %s ", Graph::opName(op), m_compileIndex, nodeFlagsAsString(flags));
 #endif
         
-        flags &= NodeBackPropMask;
-        
         bool changed = false;
         
         switch (op) {
@@ -156,7 +156,7 @@
         case Flush: {
             // Make sure that the analysis knows that flushed locals escape.
             VariableAccessData* variableAccessData = node.variableAccessData();
-            variableAccessData->mergeFlags(NodeUsedAsNumber | NodeNeedsNegZero);
+            changed |= variableAccessData->mergeFlags(NodeUsedAsValue);
             break;
         }
             
@@ -165,36 +165,47 @@
         case BitXor:
         case BitRShift:
         case BitLShift:
-        case BitURShift:
+        case BitURShift: {
+            changed |= setPrediction(PredictInt32);
+            flags |= NodeUsedAsInt;
+            flags &= ~(NodeUsedAsNumber | NodeNeedsNegZero);
+            changed |= m_graph[node.child1()].mergeFlags(flags);
+            changed |= m_graph[node.child2()].mergeFlags(flags);
+            break;
+        }
+            
         case ValueToInt32: {
             changed |= setPrediction(PredictInt32);
+            flags |= NodeUsedAsInt;
+            flags &= ~(NodeUsedAsNumber | NodeNeedsNegZero);
+            changed |= m_graph[node.child1()].mergeFlags(flags);
             break;
         }
             
         case ArrayPop: {
-            if (node.getHeapPrediction())
-                changed |= mergePrediction(node.getHeapPrediction());
+            changed |= mergePrediction(node.getHeapPrediction());
+            changed |= mergeDefaultFlags(node);
             break;
         }
 
         case ArrayPush: {
-            if (node.getHeapPrediction())
-                changed |= mergePrediction(node.getHeapPrediction());
-            changed |= mergeDefaultArithFlags(node, flags);
+            changed |= mergePrediction(node.getHeapPrediction());
+            changed |= m_graph[node.child1()].mergeFlags(NodeUsedAsValue);
+            changed |= m_graph[node.child2()].mergeFlags(NodeUsedAsValue);
             break;
         }
 
         case RegExpExec:
         case RegExpTest: {
-            if (node.getHeapPrediction())
-                changed |= mergePrediction(node.getHeapPrediction());
-            changed |= mergeDefaultArithFlags(node, flags);
+            changed |= mergePrediction(node.getHeapPrediction());
+            changed |= mergeDefaultFlags(node);
             break;
         }
 
         case StringCharCodeAt: {
             changed |= mergePrediction(PredictInt32);
-            changed |= mergeDefaultArithFlags(node, flags);
+            changed |= m_graph[node.child1()].mergeFlags(NodeUsedAsValue);
+            changed |= m_graph[node.child2()].mergeFlags(NodeUsedAsNumber | NodeUsedAsInt);
             break;
         }
 
@@ -203,13 +214,16 @@
             PredictedType right = m_graph[node.child2()].prediction();
             
             if (left && right) {
-                if (isInt32Prediction(mergePredictions(left, right)) && nodeCanSpeculateInteger(node.arithNodeFlags()))
+                if (isInt32Prediction(mergePredictions(left, right))
+                    && nodeCanSpeculateInteger(node.arithNodeFlags()))
                     changed |= mergePrediction(PredictInt32);
                 else
                     changed |= mergePrediction(PredictDouble);
             }
             
-            changed |= mergeDefaultArithFlags(node, flags);
+            flags |= NodeUsedAsValue;
+            changed |= m_graph[node.child1()].mergeFlags(flags);
+            changed |= m_graph[node.child2()].mergeFlags(flags);
             break;
         }
             
@@ -303,7 +317,8 @@
             PredictedType right = m_graph[node.child2()].prediction();
             
             if (left && right) {
-                if (isInt32Prediction(mergePredictions(left, right)) && nodeCanSpeculateInteger(node.arithNodeFlags()))
+                if (isInt32Prediction(mergePredictions(left, right))
+                    && nodeCanSpeculateInteger(node.arithNodeFlags()))
                     changed |= mergePrediction(PredictInt32);
                 else
                     changed |= mergePrediction(PredictDouble);
@@ -321,7 +336,8 @@
             PredictedType right = m_graph[node.child2()].prediction();
             
             if (left && right) {
-                if (isInt32Prediction(mergePredictions(left, right)) && nodeCanSpeculateInteger(node.arithNodeFlags()))
+                if (isInt32Prediction(mergePredictions(left, right))
+                    && nodeCanSpeculateInteger(node.arithNodeFlags()))
                     changed |= mergePrediction(PredictInt32);
                 else
                     changed |= mergePrediction(PredictDouble);
@@ -340,18 +356,16 @@
             
         case ArithSqrt: {
             changed |= setPrediction(PredictDouble);
-            changed |= mergeDefaultArithFlags(node, flags);
+            changed |= m_graph[node.child1()].mergeFlags(flags | NodeUsedAsValue);
             break;
         }
             
         case ArithAbs: {
             PredictedType child = m_graph[node.child1()].prediction();
-            if (child) {
-                if (nodeCanSpeculateInteger(node.arithNodeFlags()))
-                    changed |= mergePrediction(child);
-                else
-                    changed |= setPrediction(PredictDouble);
-            }
+            if (nodeCanSpeculateInteger(node.arithNodeFlags()))
+                changed |= mergePrediction(child);
+            else
+                changed |= setPrediction(PredictDouble);
 
             flags &= ~NodeNeedsNegZero;
             changed |= m_graph[node.child1()].mergeFlags(flags);
@@ -367,71 +381,55 @@
         case CompareStrictEq:
         case InstanceOf: {
             changed |= setPrediction(PredictBoolean);
-            changed |= mergeDefaultArithFlags(node, flags);
+            changed |= mergeDefaultFlags(node);
             break;
         }
             
         case GetById: {
-            if (node.getHeapPrediction())
-                changed |= mergePrediction(node.getHeapPrediction());
-            else if (codeBlock()->identifier(node.identifierNumber()) == globalData().propertyNames->length) {
-                // If there is no prediction from value profiles, check if we might be
-                // able to infer the type ourselves.
-                bool isArray = isArrayPrediction(m_graph[node.child1()].prediction());
-                bool isString = isStringPrediction(m_graph[node.child1()].prediction());
-                bool isByteArray = m_graph[node.child1()].shouldSpeculateByteArray();
-                bool isInt8Array = m_graph[node.child1()].shouldSpeculateInt8Array();
-                bool isInt16Array = m_graph[node.child1()].shouldSpeculateInt16Array();
-                bool isInt32Array = m_graph[node.child1()].shouldSpeculateInt32Array();
-                bool isUint8Array = m_graph[node.child1()].shouldSpeculateUint8Array();
-                bool isUint8ClampedArray = m_graph[node.child1()].shouldSpeculateUint8ClampedArray();
-                bool isUint16Array = m_graph[node.child1()].shouldSpeculateUint16Array();
-                bool isUint32Array = m_graph[node.child1()].shouldSpeculateUint32Array();
-                bool isFloat32Array = m_graph[node.child1()].shouldSpeculateFloat32Array();
-                bool isFloat64Array = m_graph[node.child1()].shouldSpeculateFloat64Array();
-                if (isArray || isString || isByteArray || isInt8Array || isInt16Array || isInt32Array || isUint8Array || isUint8ClampedArray || isUint16Array || isUint32Array || isFloat32Array || isFloat64Array)
-                    changed |= mergePrediction(PredictInt32);
-            }
-            changed |= mergeDefaultArithFlags(node, flags);
+            changed |= mergePrediction(node.getHeapPrediction());
+            changed |= mergeDefaultFlags(node);
             break;
         }
             
         case GetByIdFlush:
-            if (node.getHeapPrediction())
-                changed |= mergePrediction(node.getHeapPrediction());
-            changed |= mergeDefaultArithFlags(node, flags);
+            changed |= mergePrediction(node.getHeapPrediction());
+            changed |= mergeDefaultFlags(node);
             break;
             
         case GetByVal: {
-            if (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())
+            else
                 changed |= mergePrediction(node.getHeapPrediction());
 
-            changed |= m_graph[node.child1()].mergeFlags(flags | NodeUsedAsNumber | NodeNeedsNegZero);
-            changed |= m_graph[node.child2()].mergeFlags(flags | NodeUsedAsNumber);
+            changed |= m_graph[node.child1()].mergeFlags(NodeUsedAsValue);
+            changed |= m_graph[node.child2()].mergeFlags(NodeUsedAsNumber | NodeUsedAsInt);
             break;
         }
             
         case GetPropertyStorage: 
         case GetIndexedPropertyStorage: {
             changed |= setPrediction(PredictOther);
-            changed |= mergeDefaultArithFlags(node, flags);
+            changed |= mergeDefaultFlags(node);
             break;
         }
 
         case GetByOffset: {
-            if (node.getHeapPrediction())
-                changed |= mergePrediction(node.getHeapPrediction());
-            changed |= mergeDefaultArithFlags(node, flags);
+            changed |= mergePrediction(node.getHeapPrediction());
+            changed |= mergeDefaultFlags(node);
             break;
         }
             
         case Call:
         case Construct: {
-            if (node.getHeapPrediction())
-                changed |= mergePrediction(node.getHeapPrediction());
-            changed |= mergeDefaultArithFlags(node, flags);
+            changed |= mergePrediction(node.getHeapPrediction());
+            for (unsigned childIdx = node.firstChild();
+                 childIdx < node.firstChild() + node.numChildren();
+                 ++childIdx) {
+                Edge edge = m_graph.m_varArgChildren[childIdx];
+                changed |= m_graph[edge].mergeFlags(NodeUsedAsValue);
+            }
             break;
         }
             
@@ -444,20 +442,20 @@
                 }
                 changed |= mergePrediction(prediction);
             }
-            changed |= mergeDefaultArithFlags(node, flags);
+            changed |= mergeDefaultFlags(node);
             break;
         }
             
         case GetGlobalVar: {
             PredictedType prediction = m_graph.getGlobalVarPrediction(node.varNumber());
-            if (prediction)
-                changed |= mergePrediction(prediction);
+            changed |= mergePrediction(prediction);
             break;
         }
             
         case PutGlobalVar: {
-            changed |= m_graph.predictGlobalVar(node.varNumber(), m_graph[node.child1()].prediction());
-            changed |= mergeDefaultArithFlags(node, flags);
+            changed |= m_graph.predictGlobalVar(
+                node.varNumber(), m_graph[node.child1()].prediction());
+            changed |= m_graph[node.child1()].mergeFlags(NodeUsedAsValue);
             break;
         }
             
@@ -467,8 +465,7 @@
         case ResolveBaseStrictPut:
         case ResolveGlobal: {
             PredictedType prediction = node.getHeapPrediction();
-            if (prediction)
-                changed |= mergePrediction(prediction);
+            changed |= mergePrediction(prediction);
             break;
         }
             
@@ -485,14 +482,23 @@
         case CreateThis:
         case NewObject: {
             changed |= setPrediction(PredictFinalObject);
-            changed |= mergeDefaultArithFlags(node, flags);
+            changed |= mergeDefaultFlags(node);
             break;
         }
             
-        case NewArray:
+        case NewArray: {
+            changed |= setPrediction(PredictArray);
+            for (unsigned childIdx = node.firstChild();
+                 childIdx < node.firstChild() + node.numChildren();
+                 ++childIdx) {
+                Edge edge = m_graph.m_varArgChildren[childIdx];
+                changed |= m_graph[edge].mergeFlags(NodeUsedAsValue);
+            }
+            break;
+        }
+            
         case NewArrayBuffer: {
             changed |= setPrediction(PredictArray);
-            changed |= mergeDefaultArithFlags(node, flags);
             break;
         }
             
@@ -501,10 +507,19 @@
             break;
         }
         
-        case StringCharAt:
+        case StringCharAt: {
+            changed |= setPrediction(PredictString);
+            changed |= m_graph[node.child1()].mergeFlags(NodeUsedAsValue);
+            changed |= m_graph[node.child2()].mergeFlags(NodeUsedAsNumber | NodeUsedAsInt);
+            break;
+        }
+            
         case StrCat: {
             changed |= setPrediction(PredictString);
-            changed |= mergeDefaultArithFlags(node, flags);
+            for (unsigned childIdx = node.firstChild();
+                 childIdx < node.firstChild() + node.numChildren();
+                 ++childIdx)
+                changed |= m_graph[m_graph.m_varArgChildren[childIdx]].mergeFlags(NodeUsedAsNumber);
             break;
         }
             
@@ -521,7 +536,8 @@
                 } else if (child & PredictObjectMask) {
                     // Objects get turned into strings. So if the input has hints of objectness,
                     // the output will have hinsts of stringiness.
-                    changed |= mergePrediction(mergePredictions(child & ~PredictObjectMask, PredictString));
+                    changed |= mergePrediction(
+                        mergePredictions(child & ~PredictObjectMask, PredictString));
                 } else
                     changed |= mergePrediction(child);
             }
@@ -541,6 +557,7 @@
             break;
         }
             
+        case PutByValAlias:
         case GetArrayLength:
         case GetByteArrayLength:
         case GetInt8ArrayLength:
@@ -560,33 +577,45 @@
         }
         
         case PutByVal:
-            changed |= m_graph[node.child1()].mergeFlags(flags | NodeUsedAsNumber | NodeNeedsNegZero);
-            changed |= m_graph[node.child2()].mergeFlags(flags | NodeUsedAsNumber);
-            changed |= m_graph[node.child3()].mergeFlags(flags | NodeUsedAsNumber | NodeNeedsNegZero);
+            changed |= m_graph[node.child1()].mergeFlags(NodeUsedAsValue);
+            changed |= m_graph[node.child2()].mergeFlags(NodeUsedAsNumber | NodeUsedAsInt);
+            changed |= m_graph[node.child3()].mergeFlags(NodeUsedAsValue);
             break;
 
+        case PutScopedVar:
+        case Return:
+        case Throw:
+            changed |= m_graph[node.child1()].mergeFlags(NodeUsedAsValue);
+            break;
+
+        case PutById:
+        case PutByIdDirect:
+            changed |= m_graph[node.child1()].mergeFlags(NodeUsedAsValue);
+            changed |= m_graph[node.child2()].mergeFlags(NodeUsedAsValue);
+            break;
+
+        case PutByOffset:
+            changed |= m_graph[node.child1()].mergeFlags(NodeUsedAsValue);
+            changed |= m_graph[node.child3()].mergeFlags(NodeUsedAsValue);
+            break;
+            
+        case Phi:
+            break;
+
 #ifndef NDEBUG
         // These get ignored because they don't return anything.
-        case PutScopedVar:
         case DFG::Jump:
         case Branch:
         case Breakpoint:
-        case Return:
         case CheckHasInstance:
-        case Phi:
-        case Throw:
         case ThrowReferenceError:
         case ForceOSRExit:
         case SetArgument:
-        case PutByValAlias:
-        case PutById:
-        case PutByIdDirect:
         case CheckStructure:
         case CheckFunction:
         case PutStructure:
-        case PutByOffset:
         case TearOffActivation:
-            changed |= mergeDefaultArithFlags(node, flags);
+            changed |= mergeDefaultFlags(node);
             break;
             
         // These gets ignored because it doesn't do anything.
@@ -600,7 +629,7 @@
             break;
 #else
         default:
-            changed |= mergeDefaultArithFlags(node, flags);
+            changed |= mergeDefaultFlags(node);
             break;
 #endif
         }
@@ -611,24 +640,25 @@
         
         m_changed |= changed;
     }
-    
-    bool mergeDefaultArithFlags(Node& node, NodeFlags flags)
+        
+    bool mergeDefaultFlags(Node& node)
     {
         bool changed = false;
-        flags |= NodeUsedAsNumber | NodeNeedsNegZero;
         if (node.flags() & NodeHasVarArgs) {
-            for (unsigned childIdx = node.firstChild(); childIdx < node.firstChild() + node.numChildren(); childIdx++)
-                changed |= m_graph[m_graph.m_varArgChildren[childIdx]].mergeFlags(flags);
+            for (unsigned childIdx = node.firstChild();
+                 childIdx < node.firstChild() + node.numChildren();
+                 childIdx++)
+                changed |= m_graph[m_graph.m_varArgChildren[childIdx]].mergeFlags(NodeUsedAsValue);
         } else {
             if (!node.child1())
                 return changed;
-            changed |= m_graph[node.child1()].mergeFlags(flags);
+            changed |= m_graph[node.child1()].mergeFlags(NodeUsedAsValue);
             if (!node.child2())
                 return changed;
-            changed |= m_graph[node.child2()].mergeFlags(flags);
+            changed |= m_graph[node.child2()].mergeFlags(NodeUsedAsValue);
             if (!node.child3())
                 return changed;
-            changed |= m_graph[node.child3()].mergeFlags(flags);
+            changed |= m_graph[node.child3()].mergeFlags(NodeUsedAsValue);
         }
         return changed;
     }
@@ -650,7 +680,7 @@
         for (m_compileIndex = m_graph.size(); m_compileIndex-- > 0;)
             propagate(m_graph[m_compileIndex]);
     }
-
+    
     void vote(Edge nodeUse, VariableAccessData::Ballot ballot)
     {
         switch (m_graph[nodeUse].op()) {
@@ -669,7 +699,9 @@
     void vote(Node& node, VariableAccessData::Ballot ballot)
     {
         if (node.flags() & NodeHasVarArgs) {
-            for (unsigned childIdx = node.firstChild(); childIdx < node.firstChild() + node.numChildren(); childIdx++)
+            for (unsigned childIdx = node.firstChild();
+                 childIdx < node.firstChild() + node.numChildren();
+                 childIdx++)
                 vote(m_graph.m_varArgChildren[childIdx], ballot);
             return;
         }
@@ -724,7 +756,9 @@
                 
                 VariableAccessData::Ballot ballot;
                 
-                if (isNumberPrediction(left) && isNumberPrediction(right) && !(Node::shouldSpeculateInteger(m_graph[node.child1()], m_graph[node.child1()]) && node.canSpeculateInteger()))
+                if (isNumberPrediction(left) && isNumberPrediction(right)
+                    && !(Node::shouldSpeculateInteger(m_graph[node.child1()], m_graph[node.child1()])
+                         && node.canSpeculateInteger()))
                     ballot = VariableAccessData::VoteDouble;
                 else
                     ballot = VariableAccessData::VoteValue;
@@ -736,7 +770,8 @@
                 
             case ArithAbs:
                 VariableAccessData::Ballot ballot;
-                if (!(m_graph[node.child1()].shouldSpeculateInteger() && node.canSpeculateInteger()))
+                if (!(m_graph[node.child1()].shouldSpeculateInteger()
+                      && node.canSpeculateInteger()))
                     ballot = VariableAccessData::VoteDouble;
                 else
                     ballot = VariableAccessData::VoteValue;

Modified: trunk/Source/_javascript_Core/dfg/DFGVariableAccessData.h (112014 => 112015)

--- trunk/Source/_javascript_Core/dfg/DFGVariableAccessData.h	2012-03-24 21:22:41 UTC (rev 112014)
+++ trunk/Source/_javascript_Core/dfg/DFGVariableAccessData.h	2012-03-25 00:46:21 UTC (rev 112015)
@@ -104,8 +104,32 @@
     
     bool shouldUseDoubleFormatAccordingToVote()
     {
+        // We don't support this facility for arguments, yet.
         // FIXME: make this work for arguments.
-        return !operandIsArgument(operand()) && ((isNumberPrediction(prediction()) && doubleVoteRatio() >= Options::doubleVoteRatioForDoubleFormat) || isDoublePrediction(prediction()));
+        if (operandIsArgument(operand()))
+            return false;
+        
+        // If the variable is not a number prediction, then this doesn't
+        // make any sense.
+        if (!isNumberPrediction(prediction()))
+            return false;
+        
+        // If the variable is predicted to hold only doubles, then it's a
+        // no-brainer: it should be formatted as a double.
+        if (isDoublePrediction(prediction()))
+            return true;
+        
+        // If the variable is known to be used as an integer, then be safe -
+        // don't force it to be a double.
+        if (flags() & NodeUsedAsInt)
+            return false;
+        
+        // If the variable has been voted to become a double, then make it a
+        // double.
+        if (doubleVoteRatio() >= Options::doubleVoteRatioForDoubleFormat)
+            return true;
+        
+        return false;
     }
     
     bool shouldUseDoubleFormat()

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes