Inserting a node into the DFG graph should not require five lines of code
https://bugs.webkit.org/show_bug.cgi?id=107381
Reviewed by Sam Weinig.
This adds fairly comprehensive support for inserting a node into a DFG graph in one
method call. A common example of this is:
m_insertionSet.insertNode(indexInBlock, DontRefChildren, DontRefNode, SpecNone, ForceOSRExit, codeOrigin);
The arguments to insert() specify what reference counting you need to have happen
(RefChildren => recursively refs all children, RefNode => non-recursively refs the node
that was created), the prediction to set (SpecNone is a common default), followed by
the arguments to the Node() constructor. InsertionSet::insertNode() and similar methods
(Graph::addNode() and BasicBlock::appendNode()) all use a common variadic template
function macro from DFGVariadicFunction.h. Also, all of these methods will automatically
non-recursively ref() the node being created if the flags say NodeMustGenerate.
In all, this new mechanism retains the flexibility of the old approach (you get to
manage ref counts yourself, albeit in less code) while ensuring that most code that adds
nodes to the graph now needs less code to do it.
In the future, we should revisit the reference counting methodology in the DFG: we could
do like most compilers and get rid of it entirely, or we could make it automatic. This
patch doesn't attempt to make any such major changes, and only seeks to simplify the
technique we were already using (manual ref counting).
* GNUmakefile.list.am:
* JavaScriptCore.xcodeproj/project.pbxproj:
* bytecode/Operands.h:
(JSC::dumpOperands):
* dfg/DFGAdjacencyList.h:
(AdjacencyList):
(JSC::DFG::AdjacencyList::kind):
* dfg/DFGArgumentsSimplificationPhase.cpp:
(JSC::DFG::ArgumentsSimplificationPhase::run):
* dfg/DFGBasicBlock.h:
(DFG):
(BasicBlock):
* dfg/DFGBasicBlockInlines.h: Added.
(DFG):
* dfg/DFGCFGSimplificationPhase.cpp:
(JSC::DFG::CFGSimplificationPhase::run):
(JSC::DFG::CFGSimplificationPhase::keepOperandAlive):
* dfg/DFGCommon.h:
* dfg/DFGConstantFoldingPhase.cpp:
(JSC::DFG::ConstantFoldingPhase::ConstantFoldingPhase):
(JSC::DFG::ConstantFoldingPhase::foldConstants):
(JSC::DFG::ConstantFoldingPhase::addStructureTransitionCheck):
(JSC::DFG::ConstantFoldingPhase::paintUnreachableCode):
(ConstantFoldingPhase):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::FixupPhase):
(JSC::DFG::FixupPhase::fixupBlock):
(JSC::DFG::FixupPhase::fixupNode):
(FixupPhase):
(JSC::DFG::FixupPhase::checkArray):
(JSC::DFG::FixupPhase::blessArrayOperation):
(JSC::DFG::FixupPhase::injectInt32ToDoubleNode):
* dfg/DFGGraph.h:
(JSC::DFG::Graph::ref):
(Graph):
* dfg/DFGInsertionSet.h:
(DFG):
(JSC::DFG::Insertion::Insertion):
(JSC::DFG::Insertion::element):
(Insertion):
(JSC::DFG::InsertionSet::InsertionSet):
(JSC::DFG::InsertionSet::insert):
(InsertionSet):
(JSC::DFG::InsertionSet::execute):
* dfg/DFGNode.h:
(JSC::DFG::Node::Node):
(Node):
* dfg/DFGStructureCheckHoistingPhase.cpp:
(JSC::DFG::StructureCheckHoistingPhase::run):
* dfg/DFGVariadicFunction.h: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@140275 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
index 448ef18..bbff1b6 100644
--- a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
@@ -39,6 +39,7 @@
public:
FixupPhase(Graph& graph)
: Phase(graph, "fixup")
+ , m_insertionSet(graph)
{
}
@@ -60,7 +61,7 @@
m_compileIndex = block->at(m_indexInBlock);
fixupNode(m_graph[m_compileIndex]);
}
- m_insertionSet.execute(*block);
+ m_insertionSet.execute(block);
}
void fixupNode(Node& node)
@@ -96,12 +97,10 @@
m_graph[node.child1()].prediction(),
m_graph[m_compileIndex].prediction());
if (arrayMode.supportsLength() && arrayProfile->hasDefiniteStructure()) {
- m_graph.ref(nodePtr->child1());
- Node checkStructure(CheckStructure, nodePtr->codeOrigin, OpInfo(m_graph.addStructureSet(arrayProfile->expectedStructure())), nodePtr->child1().index());
- checkStructure.ref();
- NodeIndex checkStructureIndex = m_graph.size();
- m_graph.append(checkStructure);
- m_insertionSet.append(m_indexInBlock, checkStructureIndex);
+ m_insertionSet.insertNode(
+ m_indexInBlock, RefChildren, DontRefNode, SpecNone, CheckStructure,
+ nodePtr->codeOrigin, OpInfo(m_graph.addStructureSet(arrayProfile->expectedStructure())),
+ nodePtr->child1().index());
nodePtr = &m_graph[m_compileIndex];
}
} else {
@@ -322,19 +321,14 @@
break;
injectInt32ToDoubleNode(0);
injectInt32ToDoubleNode(1);
+
+ // We don't need to do ref'ing on the children because we're stealing them from
+ // the original division.
+ NodeIndex newDivisionIndex = m_insertionSet.insertNode(
+ m_indexInBlock, DontRefChildren, RefNode, SpecDouble, m_graph[m_compileIndex]);
- Node& oldDivision = m_graph[m_compileIndex];
-
- Node newDivision = oldDivision;
- newDivision.setRefCount(2);
- newDivision.predict(SpecDouble);
- NodeIndex newDivisionIndex = m_graph.size();
-
- oldDivision.setOp(DoubleAsInt32);
- oldDivision.children.initialize(Edge(newDivisionIndex, DoubleUse), Edge(), Edge());
-
- m_graph.append(newDivision);
- m_insertionSet.append(m_indexInBlock, newDivisionIndex);
+ m_graph[m_compileIndex].setOp(DoubleAsInt32);
+ m_graph[m_compileIndex].children.initialize(Edge(newDivisionIndex, DoubleUse), Edge(), Edge());
break;
}
@@ -423,28 +417,13 @@
#endif
}
- NodeIndex addNode(const Node& node, bool shouldGenerate)
- {
- NodeIndex nodeIndex = m_graph.size();
- m_graph.append(node);
- m_insertionSet.append(m_indexInBlock, nodeIndex);
- if (shouldGenerate)
- m_graph[nodeIndex].ref();
- return nodeIndex;
- }
-
NodeIndex checkArray(ArrayMode arrayMode, CodeOrigin codeOrigin, NodeIndex array, NodeIndex index, bool (*storageCheck)(const ArrayMode&) = canCSEStorage, bool shouldGenerate = true)
{
ASSERT(arrayMode.isSpecific());
- m_graph.ref(array);
-
Structure* structure = arrayMode.originalArrayStructure(m_graph, codeOrigin);
if (arrayMode.doesConversion()) {
- if (index != NoNode)
- m_graph.ref(index);
-
if (structure) {
if (m_indexInBlock > 0) {
// If the previous node was a CheckStructure inserted because of stuff
@@ -456,44 +435,42 @@
previousNode.setOpAndDefaultFlags(Phantom);
}
- Node arrayify(ArrayifyToStructure, codeOrigin, OpInfo(structure), OpInfo(arrayMode.asWord()), array, index);
- arrayify.ref();
- NodeIndex arrayifyIndex = m_graph.size();
- m_graph.append(arrayify);
- m_insertionSet.append(m_indexInBlock, arrayifyIndex);
+ m_insertionSet.insertNode(
+ m_indexInBlock, RefChildren, DontRefNode, SpecNone, ArrayifyToStructure, codeOrigin,
+ OpInfo(structure), OpInfo(arrayMode.asWord()), array, index);
} else {
- Node arrayify(Arrayify, codeOrigin, OpInfo(arrayMode.asWord()), array, index);
- arrayify.ref();
- NodeIndex arrayifyIndex = m_graph.size();
- m_graph.append(arrayify);
- m_insertionSet.append(m_indexInBlock, arrayifyIndex);
+ m_insertionSet.insertNode(
+ m_indexInBlock, RefChildren, DontRefNode, SpecNone, Arrayify, codeOrigin,
+ OpInfo(arrayMode.asWord()), array, index);
}
} else {
if (structure) {
- Node checkStructure(CheckStructure, codeOrigin, OpInfo(m_graph.addStructureSet(structure)), array);
- checkStructure.ref();
- NodeIndex checkStructureIndex = m_graph.size();
- m_graph.append(checkStructure);
- m_insertionSet.append(m_indexInBlock, checkStructureIndex);
+ m_insertionSet.insertNode(
+ m_indexInBlock, RefChildren, DontRefNode, SpecNone, CheckStructure, codeOrigin,
+ OpInfo(m_graph.addStructureSet(structure)), array);
} else {
- Node checkArray(CheckArray, codeOrigin, OpInfo(arrayMode.asWord()), array);
- checkArray.ref();
- NodeIndex checkArrayIndex = m_graph.size();
- m_graph.append(checkArray);
- m_insertionSet.append(m_indexInBlock, checkArrayIndex);
+ m_insertionSet.insertNode(
+ m_indexInBlock, RefChildren, DontRefNode, SpecNone, CheckArray, codeOrigin,
+ OpInfo(arrayMode.asWord()), array);
}
}
if (!storageCheck(arrayMode))
return NoNode;
- if (shouldGenerate)
- m_graph.ref(array);
+ if (arrayMode.usesButterfly()) {
+ return m_insertionSet.insertNode(
+ m_indexInBlock,
+ shouldGenerate ? RefChildren : DontRefChildren,
+ shouldGenerate ? RefNode : DontRefNode,
+ SpecNone, GetButterfly, codeOrigin, array);
+ }
- if (arrayMode.usesButterfly())
- return addNode(Node(GetButterfly, codeOrigin, array), shouldGenerate);
-
- return addNode(Node(GetIndexedPropertyStorage, codeOrigin, OpInfo(arrayMode.asWord()), array), shouldGenerate);
+ return m_insertionSet.insertNode(
+ m_indexInBlock,
+ shouldGenerate ? RefChildren : DontRefChildren,
+ shouldGenerate ? RefNode : DontRefNode,
+ SpecNone, GetIndexedPropertyStorage, codeOrigin, OpInfo(arrayMode.asWord()), array);
}
void blessArrayOperation(Edge base, Edge index, unsigned storageChildIdx)
@@ -505,11 +482,9 @@
switch (nodePtr->arrayMode().type()) {
case Array::ForceExit: {
- Node forceExit(ForceOSRExit, nodePtr->codeOrigin);
- forceExit.ref();
- NodeIndex forceExitIndex = m_graph.size();
- m_graph.append(forceExit);
- m_insertionSet.append(m_indexInBlock, forceExitIndex);
+ m_insertionSet.insertNode(
+ m_indexInBlock, DontRefChildren, DontRefNode, SpecNone, ForceOSRExit,
+ nodePtr->codeOrigin);
return;
}
@@ -564,33 +539,24 @@
void injectInt32ToDoubleNode(unsigned childIndex)
{
- Node& source = m_graph[m_compileIndex];
- Edge& edge = m_graph.child(source, childIndex);
-
- NodeIndex resultIndex = (NodeIndex)m_graph.size();
+ NodeIndex resultIndex = m_insertionSet.insertNode(
+ m_indexInBlock, DontRefChildren, RefNode, SpecDouble, Int32ToDouble,
+ m_graph[m_compileIndex].codeOrigin,
+ m_graph.child(m_graph[m_compileIndex], childIndex).index());
#if DFG_ENABLE(DEBUG_PROPAGATION_VERBOSE)
- dataLogF("(replacing @%u->@%u with @%u->@%u) ",
- m_compileIndex, edge.index(), m_compileIndex, resultIndex);
+ dataLogF(
+ "(replacing @%u->@%u with @%u->@%u) ",
+ m_compileIndex, m_graph.child(m_graph[m_compileIndex], childIndex).index(), m_compileIndex, resultIndex);
#endif
-
- // Fix the edge up here because it's a reference that will be clobbered by
- // the append() below.
- NodeIndex oldIndex = edge.index();
- edge = Edge(resultIndex, DoubleUse);
- m_graph.append(Node(Int32ToDouble, source.codeOrigin, oldIndex));
- m_insertionSet.append(m_indexInBlock, resultIndex);
-
- Node& int32ToDouble = m_graph[resultIndex];
- int32ToDouble.predict(SpecDouble);
- int32ToDouble.ref();
+ m_graph.child(m_graph[m_compileIndex], childIndex) = Edge(resultIndex, DoubleUse);
}
BasicBlock* m_block;
unsigned m_indexInBlock;
NodeIndex m_compileIndex;
- InsertionSet<NodeIndex> m_insertionSet;
+ InsertionSet m_insertionSet;
};
bool performFixup(Graph& graph)
