Title: [111781] trunk/Source/_javascript_Core
Revision
111781
Author
fpi...@apple.com
Date
2012-03-22 16:24:40 -0700 (Thu, 22 Mar 2012)

Log Message

DFG NodeFlags has some duplicate code and naming issues
https://bugs.webkit.org/show_bug.cgi?id=81975

Reviewed by Gavin Barraclough.
        
Removed most references to "ArithNodeFlags" since those are now just part
of the node flags. Fixed some renaming goofs (EdgedAsNum is once again
NodeUsedAsNum). Got rid of setArithNodeFlags() and mergeArithNodeFlags()
because the former was never called and the latter did the same things as
mergeFlags().

* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::makeSafe):
(JSC::DFG::ByteCodeParser::makeDivSafe):
(JSC::DFG::ByteCodeParser::handleIntrinsic):
* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::dump):
* dfg/DFGNode.h:
(JSC::DFG::Node::arithNodeFlags):
(Node):
* dfg/DFGNodeFlags.cpp:
(JSC::DFG::nodeFlagsAsString):
* dfg/DFGNodeFlags.h:
(DFG):
(JSC::DFG::nodeUsedAsNumber):
* dfg/DFGPredictionPropagationPhase.cpp:
(JSC::DFG::PredictionPropagationPhase::propagate):
(JSC::DFG::PredictionPropagationPhase::mergeDefaultArithFlags):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (111780 => 111781)


--- trunk/Source/_javascript_Core/ChangeLog	2012-03-22 23:17:44 UTC (rev 111780)
+++ trunk/Source/_javascript_Core/ChangeLog	2012-03-22 23:24:40 UTC (rev 111781)
@@ -1,3 +1,34 @@
+2012-03-22  Filip Pizlo  <fpi...@apple.com>
+
+        DFG NodeFlags has some duplicate code and naming issues
+        https://bugs.webkit.org/show_bug.cgi?id=81975
+
+        Reviewed by Gavin Barraclough.
+        
+        Removed most references to "ArithNodeFlags" since those are now just part
+        of the node flags. Fixed some renaming goofs (EdgedAsNum is once again
+        NodeUsedAsNum). Got rid of setArithNodeFlags() and mergeArithNodeFlags()
+        because the former was never called and the latter did the same things as
+        mergeFlags().
+
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::makeSafe):
+        (JSC::DFG::ByteCodeParser::makeDivSafe):
+        (JSC::DFG::ByteCodeParser::handleIntrinsic):
+        * dfg/DFGGraph.cpp:
+        (JSC::DFG::Graph::dump):
+        * dfg/DFGNode.h:
+        (JSC::DFG::Node::arithNodeFlags):
+        (Node):
+        * dfg/DFGNodeFlags.cpp:
+        (JSC::DFG::nodeFlagsAsString):
+        * dfg/DFGNodeFlags.h:
+        (DFG):
+        (JSC::DFG::nodeUsedAsNumber):
+        * dfg/DFGPredictionPropagationPhase.cpp:
+        (JSC::DFG::PredictionPropagationPhase::propagate):
+        (JSC::DFG::PredictionPropagationPhase::mergeDefaultArithFlags):
+
 2012-03-22  Eric Seidel  <e...@webkit.org>
 
         Actually move WTF files to their new home

Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (111780 => 111781)


--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp	2012-03-22 23:17:44 UTC (rev 111780)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp	2012-03-22 23:24:40 UTC (rev 111781)
@@ -709,7 +709,7 @@
         case ArithNegate:
         case ValueAdd:
         case ArithMod: // for ArithMode "MayOverflow" means we tried to divide by zero, or we saw double.
-            m_graph[nodeIndex].mergeArithNodeFlags(NodeMayOverflow);
+            m_graph[nodeIndex].mergeFlags(NodeMayOverflow);
             break;
             
         case ArithMul:
@@ -718,13 +718,13 @@
 #if DFG_ENABLE(DEBUG_VERBOSE)
                 dataLog("Making ArithMul @%u take deepest slow case.\n", nodeIndex);
 #endif
-                m_graph[nodeIndex].mergeArithNodeFlags(NodeMayOverflow | NodeMayNegZero);
+                m_graph[nodeIndex].mergeFlags(NodeMayOverflow | NodeMayNegZero);
             } else if (m_inlineStackTop->m_profiledBlock->likelyToTakeSlowCase(m_currentIndex)
                        || m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, NegativeZero)) {
 #if DFG_ENABLE(DEBUG_VERBOSE)
                 dataLog("Making ArithMul @%u take faster slow case.\n", nodeIndex);
 #endif
-                m_graph[nodeIndex].mergeArithNodeFlags(NodeMayNegZero);
+                m_graph[nodeIndex].mergeFlags(NodeMayNegZero);
             }
             break;
             
@@ -757,7 +757,7 @@
         
         // FIXME: It might be possible to make this more granular. The DFG certainly can
         // distinguish between negative zero and overflow in its exit profiles.
-        m_graph[nodeIndex].mergeArithNodeFlags(NodeMayOverflow | NodeMayNegZero);
+        m_graph[nodeIndex].mergeFlags(NodeMayOverflow | NodeMayNegZero);
         
         return nodeIndex;
     }
@@ -1297,7 +1297,7 @@
 
         NodeIndex nodeIndex = addToGraph(ArithAbs, get(registerOffset + argumentToOperand(1)));
         if (m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, Overflow))
-            m_graph[nodeIndex].mergeArithNodeFlags(NodeMayOverflow);
+            m_graph[nodeIndex].mergeFlags(NodeMayOverflow);
         set(resultOperand, nodeIndex);
         return true;
     }

Modified: trunk/Source/_javascript_Core/dfg/DFGGraph.cpp (111780 => 111781)


--- trunk/Source/_javascript_Core/dfg/DFGGraph.cpp	2012-03-22 23:17:44 UTC (rev 111780)
+++ trunk/Source/_javascript_Core/dfg/DFGGraph.cpp	2012-03-22 23:24:40 UTC (rev 111781)
@@ -176,8 +176,8 @@
         hasPrinted = !!node.child1();
     }
 
-    if (node.arithNodeFlags()) {
-        dataLog("%s%s", hasPrinted ? ", " : "", arithNodeFlagsAsString(node.arithNodeFlags()));
+    if (node.flags()) {
+        dataLog("%s%s", hasPrinted ? ", " : "", nodeFlagsAsString(node.flags()));
         hasPrinted = true;
     }
     if (node.hasVarNumber()) {

Modified: trunk/Source/_javascript_Core/dfg/DFGNode.h (111780 => 111781)


--- trunk/Source/_javascript_Core/dfg/DFGNode.h	2012-03-22 23:17:44 UTC (rev 111780)
+++ trunk/Source/_javascript_Core/dfg/DFGNode.h	2012-03-22 23:24:40 UTC (rev 111781)
@@ -317,30 +317,12 @@
     // to know if it can speculate on negative zero.
     NodeFlags arithNodeFlags()
     {
-        NodeFlags result = m_flags & NodeArithMask;
-        if (op() == ArithMul)
+        NodeFlags result = m_flags;
+        if (op() == ArithMul || op() == ArithDiv || op() == ArithMod)
             return result;
         return result & ~NodeNeedsNegZero;
     }
     
-    void setArithNodeFlag(NodeFlags newFlags)
-    {
-        ASSERT(!(newFlags & ~NodeArithMask));
-        
-        m_flags &= ~NodeArithMask;
-        m_flags |= newFlags;
-    }
-    
-    bool mergeArithNodeFlags(NodeFlags newFlags)
-    {
-        ASSERT(!(newFlags & ~NodeArithMask));
-        newFlags = m_flags | newFlags;
-        if (newFlags == m_flags)
-            return false;
-        m_flags = newFlags;
-        return true;
-    }
-    
     bool hasConstantBuffer()
     {
         return op() == NewArrayBuffer;

Modified: trunk/Source/_javascript_Core/dfg/DFGNodeFlags.cpp (111780 => 111781)


--- trunk/Source/_javascript_Core/dfg/DFGNodeFlags.cpp	2012-03-22 23:17:44 UTC (rev 111780)
+++ trunk/Source/_javascript_Core/dfg/DFGNodeFlags.cpp	2012-03-22 23:24:40 UTC (rev 111781)
@@ -32,20 +32,72 @@
 
 namespace JSC { namespace DFG {
 
-const char* arithNodeFlagsAsString(NodeFlags flags)
+const char* nodeFlagsAsString(NodeFlags flags)
 {
-    flags &= NodeArithMask;
-    
     if (!flags)
         return "<empty>";
 
-    static const int size = 64;
+    static const int size = 128;
     static char description[size];
     BoundsCheckedPointer<char> ptr(description, size);
     
     bool hasPrinted = false;
     
-    if (flags & EdgedAsNumber) {
+    if (flags & NodeResultMask) {
+        switch (flags & NodeResultMask) {
+        case NodeResultJS:
+            ptr.strcat("ResultJS");
+            break;
+        case NodeResultNumber:
+            ptr.strcat("ResultNumber");
+            break;
+        case NodeResultInt32:
+            ptr.strcat("ResultInt32");
+            break;
+        case NodeResultBoolean:
+            ptr.strcat("ResultBoolean");
+            break;
+        case NodeResultStorage:
+            ptr.strcat("ResultStorage");
+            break;
+        default:
+            ASSERT_NOT_REACHED();
+            break;
+        }
+        hasPrinted = true;
+    }
+    
+    if (flags & NodeMustGenerate) {
+        if (hasPrinted)
+            ptr.strcat("|");
+        ptr.strcat("MustGenerate");
+        hasPrinted = true;
+    }
+    
+    if (flags & NodeHasVarArgs) {
+        if (hasPrinted)
+            ptr.strcat("|");
+        ptr.strcat("HasVarArgs");
+        hasPrinted = true;
+    }
+    
+    if (flags & NodeClobbersWorld) {
+        if (hasPrinted)
+            ptr.strcat("|");
+        ptr.strcat("ClobbersWorld");
+        hasPrinted = true;
+    }
+    
+    if (flags & NodeMightClobber) {
+        if (hasPrinted)
+            ptr.strcat("|");
+        ptr.strcat("MightClobber");
+        hasPrinted = true;
+    }
+    
+    if (flags & NodeUsedAsNumber) {
+        if (hasPrinted)
+            ptr.strcat("|");
         ptr.strcat("UsedAsNum");
         hasPrinted = true;
     }

Modified: trunk/Source/_javascript_Core/dfg/DFGNodeFlags.h (111780 => 111781)


--- trunk/Source/_javascript_Core/dfg/DFGNodeFlags.h	2012-03-22 23:17:44 UTC (rev 111780)
+++ trunk/Source/_javascript_Core/dfg/DFGNodeFlags.h	2012-03-22 23:24:40 UTC (rev 111781)
@@ -36,30 +36,29 @@
 
 // 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 NodeArithMask      0xF00
-#define EdgeBottom      0x000
-#define EdgedAsNumber   0x100
-#define NodeNeedsNegZero   0x200
-#define EdgedAsMask     0x300
-#define NodeMayOverflow    0x400
-#define NodeMayNegZero     0x800
-#define NodeBehaviorMask   0xc00
+#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
 
 typedef uint16_t NodeFlags;
 
 static inline bool nodeUsedAsNumber(NodeFlags flags)
 {
-    return !!(flags & EdgedAsNumber);
+    return !!(flags & NodeUsedAsNumber);
 }
 
 static inline bool nodeCanTruncateInteger(NodeFlags flags)
@@ -88,7 +87,7 @@
     return true;
 }
 
-const char* arithNodeFlagsAsString(NodeFlags);
+const char* nodeFlagsAsString(NodeFlags);
 
 } } // namespace JSC::DFG
 

Modified: trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp (111780 => 111781)


--- trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp	2012-03-22 23:17:44 UTC (rev 111780)
+++ trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp	2012-03-22 23:24:40 UTC (rev 111781)
@@ -124,10 +124,10 @@
         NodeFlags flags = node.flags();
 
 #if DFG_ENABLE(DEBUG_PROPAGATION_VERBOSE)
-        dataLog("   %s @%u: %s ", Graph::opName(op), m_compileIndex, arithNodeFlagsAsString(flags));
+        dataLog("   %s @%u: %s ", Graph::opName(op), m_compileIndex, nodeFlagsAsString(flags));
 #endif
         
-        flags &= EdgedAsMask;
+        flags &= NodeBackPropMask;
         
         bool changed = false;
         
@@ -151,14 +151,14 @@
         case SetLocal: {
             VariableAccessData* variableAccessData = node.variableAccessData();
             changed |= variableAccessData->predict(m_graph[node.child1()].prediction());
-            changed |= m_graph[node.child1()].mergeArithNodeFlags(variableAccessData->flags());
+            changed |= m_graph[node.child1()].mergeFlags(variableAccessData->flags());
             break;
         }
             
         case Flush: {
             // Make sure that the analysis knows that flushed locals escape.
             VariableAccessData* variableAccessData = node.variableAccessData();
-            variableAccessData->mergeFlags(EdgedAsNumber | NodeNeedsNegZero);
+            variableAccessData->mergeFlags(NodeUsedAsNumber | NodeNeedsNegZero);
             break;
         }
             
@@ -221,7 +221,7 @@
             else
                 changed |= mergePrediction(PredictNumber);
             
-            changed |= m_graph[node.child1()].mergeArithNodeFlags(flags);
+            changed |= m_graph[node.child1()].mergeFlags(flags);
             break;
         }
 
@@ -245,8 +245,8 @@
             if (isNotNegZero(node.child1().index()) || isNotNegZero(node.child2().index()))
                 flags &= ~NodeNeedsNegZero;
             
-            changed |= m_graph[node.child1()].mergeArithNodeFlags(flags);
-            changed |= m_graph[node.child2()].mergeArithNodeFlags(flags);
+            changed |= m_graph[node.child1()].mergeFlags(flags);
+            changed |= m_graph[node.child2()].mergeFlags(flags);
             break;
         }
             
@@ -264,8 +264,8 @@
             if (isNotNegZero(node.child1().index()) || isNotNegZero(node.child2().index()))
                 flags &= ~NodeNeedsNegZero;
             
-            changed |= m_graph[node.child1()].mergeArithNodeFlags(flags);
-            changed |= m_graph[node.child2()].mergeArithNodeFlags(flags);
+            changed |= m_graph[node.child1()].mergeFlags(flags);
+            changed |= m_graph[node.child2()].mergeFlags(flags);
             break;
         }
             
@@ -283,8 +283,8 @@
             if (isNotZero(node.child1().index()) || isNotZero(node.child2().index()))
                 flags &= ~NodeNeedsNegZero;
             
-            changed |= m_graph[node.child1()].mergeArithNodeFlags(flags);
-            changed |= m_graph[node.child2()].mergeArithNodeFlags(flags);
+            changed |= m_graph[node.child1()].mergeFlags(flags);
+            changed |= m_graph[node.child2()].mergeFlags(flags);
             break;
         }
             
@@ -296,7 +296,7 @@
                     changed |= mergePrediction(PredictDouble);
             }
 
-            changed |= m_graph[node.child1()].mergeArithNodeFlags(flags);
+            changed |= m_graph[node.child1()].mergeFlags(flags);
             break;
             
         case ArithMin:
@@ -311,9 +311,9 @@
                     changed |= mergePrediction(PredictDouble);
             }
 
-            flags |= EdgedAsNumber;
-            changed |= m_graph[node.child1()].mergeArithNodeFlags(flags);
-            changed |= m_graph[node.child2()].mergeArithNodeFlags(flags);
+            flags |= NodeUsedAsNumber;
+            changed |= m_graph[node.child1()].mergeFlags(flags);
+            changed |= m_graph[node.child2()].mergeFlags(flags);
             break;
         }
             
@@ -334,9 +334,9 @@
             // can change the outcome. So, ArithMul always checks for overflow
             // no matter what, and always forces its inputs to check as well.
             
-            flags |= EdgedAsNumber | NodeNeedsNegZero;
-            changed |= m_graph[node.child1()].mergeArithNodeFlags(flags);
-            changed |= m_graph[node.child2()].mergeArithNodeFlags(flags);
+            flags |= NodeUsedAsNumber | NodeNeedsNegZero;
+            changed |= m_graph[node.child1()].mergeFlags(flags);
+            changed |= m_graph[node.child2()].mergeFlags(flags);
             break;
         }
             
@@ -356,7 +356,7 @@
             }
 
             flags &= ~NodeNeedsNegZero;
-            changed |= m_graph[node.child1()].mergeArithNodeFlags(flags);
+            changed |= m_graph[node.child1()].mergeFlags(flags);
             break;
         }
             
@@ -410,8 +410,8 @@
             else if (node.getHeapPrediction())
                 changed |= mergePrediction(node.getHeapPrediction());
 
-            changed |= m_graph[node.child1()].mergeArithNodeFlags(flags | EdgedAsNumber | NodeNeedsNegZero);
-            changed |= m_graph[node.child2()].mergeArithNodeFlags(flags | EdgedAsNumber);
+            changed |= m_graph[node.child1()].mergeFlags(flags | NodeUsedAsNumber | NodeNeedsNegZero);
+            changed |= m_graph[node.child2()].mergeFlags(flags | NodeUsedAsNumber);
             break;
         }
             
@@ -527,7 +527,7 @@
                 } else
                     changed |= mergePrediction(child);
             }
-            changed |= m_graph[node.child1()].mergeArithNodeFlags(flags);
+            changed |= m_graph[node.child1()].mergeFlags(flags);
             break;
         }
             
@@ -562,9 +562,9 @@
         }
         
         case PutByVal:
-            changed |= m_graph[node.child1()].mergeArithNodeFlags(flags | EdgedAsNumber | NodeNeedsNegZero);
-            changed |= m_graph[node.child2()].mergeArithNodeFlags(flags | EdgedAsNumber);
-            changed |= m_graph[node.child3()].mergeArithNodeFlags(flags | EdgedAsNumber | NodeNeedsNegZero);
+            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);
             break;
 
 #ifndef NDEBUG
@@ -617,20 +617,20 @@
     bool mergeDefaultArithFlags(Node& node, NodeFlags flags)
     {
         bool changed = false;
-        flags |= EdgedAsNumber | NodeNeedsNegZero;
+        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]].mergeArithNodeFlags(flags);
+                changed |= m_graph[m_graph.m_varArgChildren[childIdx]].mergeFlags(flags);
         } else {
             if (!node.child1())
                 return changed;
-            changed |= m_graph[node.child1()].mergeArithNodeFlags(flags);
+            changed |= m_graph[node.child1()].mergeFlags(flags);
             if (!node.child2())
                 return changed;
-            changed |= m_graph[node.child2()].mergeArithNodeFlags(flags);
+            changed |= m_graph[node.child2()].mergeFlags(flags);
             if (!node.child3())
                 return changed;
-            changed |= m_graph[node.child3()].mergeArithNodeFlags(flags);
+            changed |= m_graph[node.child3()].mergeFlags(flags);
         }
         return changed;
     }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to