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):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@155593 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index d71fa10..3e2fd3c 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,5 +1,34 @@
2013-09-11 Filip Pizlo <fpizlo@apple.com>
+ 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):
+
+2013-09-11 Filip Pizlo <fpizlo@apple.com>
+
FixupPhase's setUseKindAndUnboxBlahbittyblah and fixDoubleEdge methods should be merged and given intuitive names
https://bugs.webkit.org/show_bug.cgi?id=121202
diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
index f474f61..d03f63e 100644
--- a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
@@ -907,6 +907,8 @@
break;
#endif
}
+
+ DFG_NODE_DO_TO_CHILDREN(m_graph, node, observeUntypedEdge);
#if DFG_ENABLE(DEBUG_PROPAGATION_VERBOSE)
if (!(node->flags() & NodeHasVarArgs)) {
@@ -917,6 +919,13 @@
#endif
}
+ void observeUntypedEdge(Node*, Edge& edge)
+ {
+ if (edge.useKind() != UntypedUse)
+ return;
+ fixEdge<UntypedUse>(edge);
+ }
+
template<UseKind useKind>
void createToString(Node* node, Edge& edge)
{
@@ -1280,6 +1289,8 @@
template<UseKind useKind>
void observeUseKindOnNode(Node* node)
{
+ if (useKind == UntypedUse)
+ return;
observeUseKindOnNode(node, useKind);
}
@@ -1326,8 +1337,13 @@
}
}
- // 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)
{
