Changeset 155593 in webkit for trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp

Timestamp:

Sep 11, 2013, 10:46:49 PM (12 years ago)

Author:

[email protected]

Message:

FixupPhase should always call fixEdge() exactly once for every edge
​https://bugs.webkit.org/show_bug.cgi?id=121211

Reviewed by Geoffrey Garen.

Previously we only call fixEdge() on edges that we want to make typed. UntypedUse
edges don't get fixEdge() called. This makes it difficult to add functionality in
fixEdge() that runs for UntypedUses. It's difficult to remember to call fixEdge()
for every edge that we don't want to turn into a typed edge; in an alternative
universe where we did this, it would mean that every case in FixupPhase would
have to make a fixEdge() call for *every* edge even ones that it doesn't want to
modify.

This patch takes a different path. fixEdge() must never be called explicitly with
UntypedUse. fixEdge() should be used to set the UseKind of edges. Consequently,
all that FixupPhase has to do is call fixEdge<UntypedUse>(edge) for every edge
that was still UntypedUse after we are done processing a node.

This is cheap and easy to implement and ought to be easy to maintain. We won't
have a need to call fixEdge<UntypedUse>(edge) explicitly, so depending on that is
only natural.

• dfg/DFGFixupPhase.cpp:

(JSC::DFG::FixupPhase::fixupNode):
(JSC::DFG::FixupPhase::observeUntypedEdge):
(JSC::DFG::FixupPhase::observeUseKindOnNode):

File:

• trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp (modified) (4 diffs)

trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp

@@ -908 +908 @@
 #endif
         }
+        
+        DFG_NODE_DO_TO_CHILDREN(m_graph, node, observeUntypedEdge);
 
 #if DFG_ENABLE(DEBUG_PROPAGATION_VERBOSE)
@@ -916 +918 @@
         dataLogF("\n");
 #endif
+    }
+    
+    void observeUntypedEdge(Node*, Edge& edge)
+    {
+        if (edge.useKind() != UntypedUse)
+            return;
+        fixEdge<UntypedUse>(edge);
     }
     
@@ -1281 +1290 @@
     void observeUseKindOnNode(Node* node)
     {
+        if (useKind == UntypedUse)
+            return;
         observeUseKindOnNode(node, useKind);
     }
@@ -1327 +1338 @@
     }
     
-    // Set the use kind of the edge. In the future (https://bugs.webkit.org/show_bug.cgi?id=110433),
-    // this can be used to notify the GetLocal that the variable is profitable to unbox.
+    // Set the use kind of the edge and perform any actions that need to be done for
+    // that use kind, like inserting intermediate conversion nodes. Never call this
+    // with useKind = UntypedUse explicitly; edges have UntypedUse implicitly and any
+    // edge that survives fixup and still has UntypedUse will have this method called
+    // from observeUntypedEdge(). Also, make sure that if you do change the type of an
+    // edge, you either call fixEdge() or perform the equivalent functionality
+    // yourself. Obviously, you should have a really good reason if you do the latter.
     template<UseKind useKind>
     void fixEdge(Edge& edge, SpeculationDirection direction = BackwardSpeculation)