DFG should not use or preserve Phantoms during transformations
https://bugs.webkit.org/show_bug.cgi?id=143736
Reviewed by Geoffrey Garen.
Since http://trac.webkit.org/changeset/183207 and http://trac.webkit.org/changeset/183406, it is
no longer necessary to preserve Phantoms during transformations. They are still useful just
before FixupPhase to support backwards propagation analyses. They are still inserted late in the
game in the DFG backend. But transformations don't need to worry about them. Inside a basic
block, we can be sure that so long as the IR pinpoints the place where the value becomes
available in a bytecode register (using MovHint) and so long as there is a SetLocal anytime some
other block would need the value (either for OSR or for DFG execution), then we don't need any
liveness markers.
So, this removes any places where we inserted Phantoms just for liveness during transformation
and it replaces convertToPhantom() with remove(), which just converts the node to a Check. A
Check node only keeps its children so long as those children have checks.
The fact that we no longer convertToPhantom() means that we have to be more careful when
constant-folding GetLocal. Previously we would convertToPhantom() and use the fact that
Phantom(Phi) was a valid construct. It's not valid anymore. So, when constant folding encounters
a GetLocal it needs to insert a PhantomLocal directly. This allows us to simplify
Graph::convertToConstant() a bit. Luckily, none of the other users of this method would see
GetLocals.
The only Phantom-like cruft left over after this patch is:
- Phantoms before FixupPhase. I kind of like these. It means that before FixupPhase, we can do
backwards analyses and rely on the fact that the users of a node in DFG IR are a superset of
the users of the original local's live range in bytecode. This is essential for supporting our
BackwardsPropagationPhase, which is an important optimization for things like asm.js.
- PhantomLocals and GetLocals being NodeMustGenerate. See discussion in
https://bugs.webkit.org/show_bug.cgi?id=144086. It appears that this is not as evil as the
alternatives. The best long-term plan is to simply ditch the ThreadedCPS IR entirely and have
the DFG use SSA. For now, so long as any new DFG optimizations we add are block-local and
treat GetLocal/SetLocal conservatively, this should all be sound.
This change should be perf-neutral although it does reduce the total work that the compiler
does.
* CMakeLists.txt:
* JavaScriptCore.vcxproj/JavaScriptCore.vcxproj:
* JavaScriptCore.xcodeproj/project.pbxproj:
* dfg/DFGAdjacencyList.h:
(JSC::DFG::AdjacencyList::justChecks):
* dfg/DFGArgumentsEliminationPhase.cpp:
* dfg/DFGBasicBlock.cpp:
(JSC::DFG::BasicBlock::replaceTerminal):
* dfg/DFGBasicBlock.h:
(JSC::DFG::BasicBlock::findTerminal):
* dfg/DFGCFGSimplificationPhase.cpp:
(JSC::DFG::CFGSimplificationPhase::keepOperandAlive):
(JSC::DFG::CFGSimplificationPhase::mergeBlocks):
* dfg/DFGCPSRethreadingPhase.cpp:
(JSC::DFG::CPSRethreadingPhase::CPSRethreadingPhase):
(JSC::DFG::CPSRethreadingPhase::clearVariables):
(JSC::DFG::CPSRethreadingPhase::canonicalizeFlushOrPhantomLocalFor):
(JSC::DFG::CPSRethreadingPhase::canonicalizeLocalsInBlock):
* dfg/DFGCSEPhase.cpp:
* dfg/DFGCleanUpPhase.cpp: Copied from Source/JavaScriptCore/dfg/DFGPhantomRemovalPhase.cpp.
(JSC::DFG::CleanUpPhase::CleanUpPhase):
(JSC::DFG::CleanUpPhase::run):
(JSC::DFG::performCleanUp):
(JSC::DFG::PhantomRemovalPhase::PhantomRemovalPhase): Deleted.
(JSC::DFG::PhantomRemovalPhase::run): Deleted.
(JSC::DFG::performPhantomRemoval): Deleted.
* dfg/DFGCleanUpPhase.h: Copied from Source/JavaScriptCore/dfg/DFGPhantomRemovalPhase.h.
* dfg/DFGConstantFoldingPhase.cpp:
(JSC::DFG::ConstantFoldingPhase::foldConstants):
(JSC::DFG::ConstantFoldingPhase::addBaseCheck):
(JSC::DFG::ConstantFoldingPhase::fixUpsilons):
* dfg/DFGDCEPhase.cpp:
(JSC::DFG::DCEPhase::run):
(JSC::DFG::DCEPhase::fixupBlock):
(JSC::DFG::DCEPhase::cleanVariables):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupBlock):
(JSC::DFG::FixupPhase::fixupNode):
(JSC::DFG::FixupPhase::convertStringAddUse):
(JSC::DFG::FixupPhase::attemptToMakeFastStringAdd):
(JSC::DFG::FixupPhase::checkArray):
(JSC::DFG::FixupPhase::fixIntConvertingEdge):
(JSC::DFG::FixupPhase::fixIntOrBooleanEdge):
(JSC::DFG::FixupPhase::fixDoubleOrBooleanEdge):
(JSC::DFG::FixupPhase::injectTypeConversionsInBlock):
(JSC::DFG::FixupPhase::tryToRelaxRepresentation):
(JSC::DFG::FixupPhase::injectTypeConversionsForEdge):
(JSC::DFG::FixupPhase::addRequiredPhantom): Deleted.
(JSC::DFG::FixupPhase::addPhantomsIfNecessary): Deleted.
* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::convertToConstant):
(JSC::DFG::Graph::mergeRelevantToOSR): Deleted.
* dfg/DFGGraph.h:
* dfg/DFGInsertionSet.h:
(JSC::DFG::InsertionSet::insertCheck):
* dfg/DFGIntegerCheckCombiningPhase.cpp:
(JSC::DFG::IntegerCheckCombiningPhase::handleBlock):
* dfg/DFGLICMPhase.cpp:
(JSC::DFG::LICMPhase::attemptHoist):
* dfg/DFGNode.cpp:
(JSC::DFG::Node::remove):
* dfg/DFGNode.h:
(JSC::DFG::Node::replaceWith):
(JSC::DFG::Node::convertToPhantom): Deleted.
(JSC::DFG::Node::convertToCheck): Deleted.
(JSC::DFG::Node::willHaveCodeGenOrOSR): Deleted.
* dfg/DFGNodeFlags.h:
* dfg/DFGNodeType.h:
* dfg/DFGObjectAllocationSinkingPhase.cpp:
(JSC::DFG::ObjectAllocationSinkingPhase::lowerNonReadingOperationsOnPhantomAllocations):
(JSC::DFG::ObjectAllocationSinkingPhase::handleNode):
* dfg/DFGPhantomCanonicalizationPhase.cpp: Removed.
* dfg/DFGPhantomCanonicalizationPhase.h: Removed.
* dfg/DFGPhantomRemovalPhase.cpp: Removed.
* dfg/DFGPhantomRemovalPhase.h: Removed.
* dfg/DFGPlan.cpp:
(JSC::DFG::Plan::compileInThreadImpl):
* dfg/DFGPutStackSinkingPhase.cpp:
* dfg/DFGResurrectionForValidationPhase.cpp: Removed.
* dfg/DFGResurrectionForValidationPhase.h: Removed.
* dfg/DFGSSAConversionPhase.cpp:
(JSC::DFG::SSAConversionPhase::run):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::fillSpeculateDouble):
* dfg/DFGStoreBarrierElisionPhase.cpp:
(JSC::DFG::StoreBarrierElisionPhase::elideBarrier):
* dfg/DFGStrengthReductionPhase.cpp:
(JSC::DFG::StrengthReductionPhase::handleNode):
(JSC::DFG::StrengthReductionPhase::convertToIdentityOverChild):
* dfg/DFGValidate.cpp:
(JSC::DFG::Validate::validate):
(JSC::DFG::Validate::validateCPS):
(JSC::DFG::Validate::validateSSA):
* dfg/DFGVarargsForwardingPhase.cpp:
* ftl/FTLLink.cpp:
(JSC::FTL::link):
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::LowerDFGToLLVM::compileNode):
(JSC::FTL::LowerDFGToLLVM::compileNoOp):
(JSC::FTL::LowerDFGToLLVM::compilePhantom): Deleted.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@183497 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
index cbac371..77662c6 100644
--- a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
@@ -82,10 +82,8 @@
m_block = block;
for (m_indexInBlock = 0; m_indexInBlock < block->size(); ++m_indexInBlock) {
m_currentNode = block->at(m_indexInBlock);
- addPhantomsIfNecessary();
fixupNode(m_currentNode);
}
- addPhantomsIfNecessary();
m_insertionSet.execute(block);
}
@@ -142,14 +140,12 @@
case ValueAdd: {
if (attemptToMakeIntegerAdd(node)) {
node->setOp(ArithAdd);
- node->clearFlags(NodeMustGenerate);
break;
}
if (Node::shouldSpeculateNumberOrBooleanExpectingDefined(node->child1().node(), node->child2().node())) {
fixDoubleOrBooleanEdge(node->child1());
fixDoubleOrBooleanEdge(node->child2());
node->setOp(ArithAdd);
- node->clearFlags(NodeMustGenerate);
node->setResult(NodeResultDouble);
break;
}
@@ -268,12 +264,6 @@
fixDoubleOrBooleanEdge(node->child1());
fixDoubleOrBooleanEdge(node->child2());
- // But we have to make sure that everything is phantom'd until after the
- // DoubleAsInt32 node, which occurs after the Div/Mod node that the conversions
- // will be insered on.
- addRequiredPhantom(node->child1().node());
- addRequiredPhantom(node->child2().node());
-
// We don't need to do ref'ing on the children because we're stealing them from
// the original division.
Node* newDivision = m_insertionSet.insertNode(
@@ -847,7 +837,7 @@
}
m_insertionSet.insertNode(
- m_indexInBlock, SpecNone, Phantom, node->origin,
+ m_indexInBlock, SpecNone, Check, node->origin,
Edge(node->child1().node(), OtherUse));
observeUseKindOnNode<OtherUse>(node->child1().node());
m_graph.convertToConstant(
@@ -998,7 +988,6 @@
break;
}
- case Phantom:
case Check: {
switch (node->child1().useKind()) {
case NumberUse:
@@ -1012,6 +1001,11 @@
break;
}
+ case Phantom:
+ // Phantoms are meaningless past Fixup. We recreate them on-demand in the backend.
+ node->remove();
+ break;
+
case FiatInt52: {
RELEASE_ASSERT(enableInt52());
node->convertToIdentity();
@@ -1075,7 +1069,7 @@
case IsString:
if (node->child1()->shouldSpeculateString()) {
m_insertionSet.insertNode(
- m_indexInBlock, SpecNone, Phantom, node->origin,
+ m_indexInBlock, SpecNone, Check, node->origin,
Edge(node->child1().node(), StringUse));
m_graph.convertToConstant(node, jsBoolean(true));
observeUseKindOnNode<StringUse>(node);
@@ -1085,7 +1079,7 @@
case IsObject:
if (node->child1()->shouldSpeculateObject()) {
m_insertionSet.insertNode(
- m_indexInBlock, SpecNone, Phantom, node->origin,
+ m_indexInBlock, SpecNone, Check, node->origin,
Edge(node->child1().node(), ObjectUse));
m_graph.convertToConstant(node, jsBoolean(true));
observeUseKindOnNode<ObjectUse>(node);
@@ -1158,28 +1152,28 @@
RefPtr<TypeSet> typeSet = node->typeLocation()->m_instructionTypeSet;
RuntimeTypeMask seenTypes = typeSet->seenTypes();
if (typeSet->doesTypeConformTo(TypeMachineInt)) {
- node->convertToCheck();
if (node->child1()->shouldSpeculateInt32())
fixEdge<Int32Use>(node->child1());
else
fixEdge<MachineIntUse>(node->child1());
+ node->remove();
} else if (typeSet->doesTypeConformTo(TypeNumber | TypeMachineInt)) {
- node->convertToCheck();
fixEdge<NumberUse>(node->child1());
+ node->remove();
} else if (typeSet->doesTypeConformTo(TypeString)) {
- node->convertToCheck();
fixEdge<StringUse>(node->child1());
+ node->remove();
} else if (typeSet->doesTypeConformTo(TypeBoolean)) {
- node->convertToCheck();
fixEdge<BooleanUse>(node->child1());
+ node->remove();
} else if (typeSet->doesTypeConformTo(TypeUndefined | TypeNull) && (seenTypes & TypeUndefined) && (seenTypes & TypeNull)) {
- node->convertToCheck();
fixEdge<OtherUse>(node->child1());
+ node->remove();
} else if (typeSet->doesTypeConformTo(TypeObject)) {
StructureSet set = typeSet->structureSet();
if (!set.isEmpty()) {
- node->convertToCheckStructure(m_graph.addStructureSet(set));
fixEdge<CellUse>(node->child1());
+ node->convertToCheckStructure(m_graph.addStructureSet(set));
}
}
@@ -1296,7 +1290,7 @@
// decision process much easier.
observeUseKindOnNode<StringUse>(edge.node());
m_insertionSet.insertNode(
- m_indexInBlock, SpecNone, Phantom, node->origin,
+ m_indexInBlock, SpecNone, Check, node->origin,
Edge(edge.node(), StringUse));
edge.setUseKind(KnownStringUse);
return;
@@ -1398,9 +1392,6 @@
template<UseKind leftUseKind>
bool attemptToMakeFastStringAdd(Node* node, Edge& left, Edge& right)
{
- Node* originalLeft = left.node();
- Node* originalRight = right.node();
-
ASSERT(leftUseKind == StringUse || leftUseKind == StringObjectUse || leftUseKind == StringOrStringObjectUse);
if (isStringObjectUse<leftUseKind>() && !canOptimizeStringObjectAccess(node->origin.semantic))
@@ -1435,12 +1426,6 @@
right.setNode(toString);
}
- // We're doing checks up there, so we need to make sure that the
- // *original* inputs to the addition are live up to here.
- m_insertionSet.insertNode(
- m_indexInBlock, SpecNone, Phantom, node->origin,
- Edge(originalLeft), Edge(originalRight));
-
convertToMakeRope(node);
return true;
}
@@ -1564,7 +1549,7 @@
if (arrayMode.type() == Array::String) {
m_insertionSet.insertNode(
- m_indexInBlock, SpecNone, Phantom, origin, Edge(array, StringUse));
+ m_indexInBlock, SpecNone, Check, origin, Edge(array, StringUse));
} else {
Structure* structure = arrayMode.originalArrayStructure(m_graph, origin.semantic);
@@ -1775,7 +1760,6 @@
observeUseKindOnNode(node, useKind);
edge = Edge(newNode, KnownInt32Use);
- addRequiredPhantom(node);
}
void fixIntOrBooleanEdge(Edge& edge)
@@ -1797,7 +1781,6 @@
observeUseKindOnNode(node, useKind);
edge = Edge(newNode, Int32Use);
- addRequiredPhantom(node);
}
void fixDoubleOrBooleanEdge(Edge& edge)
@@ -1819,7 +1802,6 @@
observeUseKindOnNode(node, useKind);
edge = Edge(newNode, DoubleRepUse);
- addRequiredPhantom(node);
}
void truncateConstantToInt32(Edge& edge)
@@ -1999,11 +1981,9 @@
m_block = block;
for (m_indexInBlock = 0; m_indexInBlock < block->size(); ++m_indexInBlock) {
m_currentNode = block->at(m_indexInBlock);
- addPhantomsIfNecessary();
tryToRelaxRepresentation(m_currentNode);
DFG_NODE_DO_TO_CHILDREN(m_graph, m_currentNode, injectTypeConversionsForEdge);
}
- addPhantomsIfNecessary();
m_insertionSet.execute(block);
}
@@ -2017,7 +1997,6 @@
switch (node->op()) {
case MovHint:
- case Phantom:
case Check:
DFG_NODE_DO_TO_CHILDREN(m_graph, m_currentNode, fixEdgeRepresentation);
break;
@@ -2078,8 +2057,6 @@
if (edge->hasDoubleResult())
break;
- addRequiredPhantom(edge.node());
-
if (edge->isNumberConstant()) {
result = m_insertionSet.insertNode(
m_indexInBlock, SpecBytecodeDouble, DoubleConstant, node->origin,
@@ -2102,8 +2079,6 @@
if (edge->hasInt52Result())
break;
- addRequiredPhantom(edge.node());
-
if (edge->isMachineIntConstant()) {
result = m_insertionSet.insertNode(
m_indexInBlock, SpecMachineInt, Int52Constant, node->origin,
@@ -2130,8 +2105,6 @@
if (!edge->hasDoubleResult() && !edge->hasInt52Result())
break;
- addRequiredPhantom(edge.node());
-
if (edge->hasDoubleResult()) {
result = m_insertionSet.insertNode(
m_indexInBlock, SpecBytecodeDouble, ValueRep, node->origin,
@@ -2147,32 +2120,11 @@
} }
}
- void addRequiredPhantom(Node* node)
- {
- m_requiredPhantoms.append(node);
- }
-
- void addPhantomsIfNecessary()
- {
- if (m_requiredPhantoms.isEmpty())
- return;
-
- for (unsigned i = m_requiredPhantoms.size(); i--;) {
- Node* node = m_requiredPhantoms[i];
- m_insertionSet.insertNode(
- m_indexInBlock, SpecNone, Phantom, m_currentNode->origin,
- node->defaultEdge());
- }
-
- m_requiredPhantoms.resize(0);
- }
-
BasicBlock* m_block;
unsigned m_indexInBlock;
Node* m_currentNode;
InsertionSet m_insertionSet;
bool m_profitabilityChanged;
- Vector<Node*, 3> m_requiredPhantoms;
};
bool performFixup(Graph& graph)
