DFG DCE might eliminate checks unsoundly
https://bugs.webkit.org/show_bug.cgi?id=109389
Source/JavaScriptCore:
Reviewed by Oliver Hunt.
This gets rid of all eager reference counting, and does all dead code elimination
in one phase - the DCEPhase. This phase also sets up the node reference counts,
which are then used not just for DCE but also register allocation and stack slot
allocation.
Doing this required a number of surgical changes in places that previously relied
on always having liveness information. For example, the structure check hoisting
phase must now consult whether a VariableAccessData is profitable for unboxing to
make sure that it doesn't try to do hoisting on set SetLocals. The arguments
simplification phase employs its own light-weight liveness analysis. Both phases
previously just used reference counts.
The largest change is that now, dead nodes get turned into Phantoms. Those
Phantoms will retain those child edges that are not proven. This ensures that any
type checks performed by a dead node remain even after the node is killed. On the
other hand, this Phantom conversion means that we need special handling for
SetLocal. I decided to make the four forms of SetLocal explicit:
MovHint(@a, rK): Just indicates that node @a contains the value that would have
now been placed into virtual register rK. Does not actually cause @a to be
stored into rK. This would have previously been a dead SetLocal with @a
being live. MovHints are always dead.
ZombieHint(rK): Indicates that at this point, register rK will contain a dead
value and OSR should put Undefined into it. This would have previously been
a dead SetLocal with @a being dead also. ZombieHints are always dead.
MovHintAndCheck(@a, rK): Identical to MovHint except @a is also type checked,
according to whatever UseKind the edge to @a has. The type check is always a
forward exit. MovHintAndChecks are always live, since they are
NodeMustGenerate. Previously this would have been a dead SetLocal with a
live @a, and the check would have disappeared. This is one of the bugs that
this patch solves.
SetLocal(@a, rK): This still does exactly what it does now, if the SetLocal is
live.
Basically this patch makes it so that dead SetLocals eventually decay to MovHint,
ZombieHint, or MovHintAndCheck depending on the situation. If the child @a is
also dead, then you get a ZombieHint. If the child @a is live but the SetLocal
has a type check and @a's type hasn't been proven to have that type then you get
a MovHintAndCheck. Otherwise you get a MovHint.
This is performance neutral.
* CMakeLists.txt:
* GNUmakefile.list.am:
* JavaScriptCore.xcodeproj/project.pbxproj:
* Target.pri:
* dfg/DFGAbstractState.cpp:
(JSC::DFG::AbstractState::executeEffects):
(JSC::DFG::AbstractState::mergeStateAtTail):
* dfg/DFGArgumentsSimplificationPhase.cpp:
(JSC::DFG::ArgumentsSimplificationPhase::run):
(ArgumentsSimplificationPhase):
(JSC::DFG::ArgumentsSimplificationPhase::removeArgumentsReferencingPhantomChild):
* dfg/DFGBasicBlock.h:
(BasicBlock):
* dfg/DFGBasicBlockInlines.h:
(DFG):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::addToGraph):
(JSC::DFG::ByteCodeParser::insertPhiNode):
(JSC::DFG::ByteCodeParser::emitFunctionChecks):
* dfg/DFGCFAPhase.cpp:
(JSC::DFG::CFAPhase::run):
* dfg/DFGCFGSimplificationPhase.cpp:
(JSC::DFG::CFGSimplificationPhase::run):
(JSC::DFG::CFGSimplificationPhase::keepOperandAlive):
* dfg/DFGCPSRethreadingPhase.cpp:
(JSC::DFG::CPSRethreadingPhase::run):
(JSC::DFG::CPSRethreadingPhase::addPhiSilently):
* dfg/DFGCSEPhase.cpp:
(JSC::DFG::CSEPhase::eliminateIrrelevantPhantomChildren):
(JSC::DFG::CSEPhase::setReplacement):
(JSC::DFG::CSEPhase::performNodeCSE):
* dfg/DFGCommon.cpp:
(WTF::printInternal):
(WTF):
* dfg/DFGCommon.h:
(WTF):
* dfg/DFGConstantFoldingPhase.cpp:
(JSC::DFG::ConstantFoldingPhase::foldConstants):
(JSC::DFG::ConstantFoldingPhase::addStructureTransitionCheck):
(JSC::DFG::ConstantFoldingPhase::paintUnreachableCode):
* dfg/DFGDCEPhase.cpp: Added.
(DFG):
(DCEPhase):
(JSC::DFG::DCEPhase::DCEPhase):
(JSC::DFG::DCEPhase::run):
(JSC::DFG::DCEPhase::findTypeCheckRoot):
(JSC::DFG::DCEPhase::countEdge):
(JSC::DFG::DCEPhase::eliminateIrrelevantPhantomChildren):
(JSC::DFG::performDCE):
* dfg/DFGDCEPhase.h: Added.
(DFG):
* dfg/DFGDriver.cpp:
(JSC::DFG::compile):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
(JSC::DFG::FixupPhase::checkArray):
(JSC::DFG::FixupPhase::blessArrayOperation):
(JSC::DFG::FixupPhase::fixIntEdge):
(JSC::DFG::FixupPhase::injectInt32ToDoubleNode):
(JSC::DFG::FixupPhase::truncateConstantToInt32):
* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::Graph):
(JSC::DFG::Graph::dump):
(DFG):
* dfg/DFGGraph.h:
(JSC::DFG::Graph::changeChild):
(JSC::DFG::Graph::changeEdge):
(JSC::DFG::Graph::compareAndSwap):
(JSC::DFG::Graph::clearAndDerefChild):
(JSC::DFG::Graph::performSubstitution):
(JSC::DFG::Graph::performSubstitutionForEdge):
(Graph):
(JSC::DFG::Graph::substitute):
* dfg/DFGInsertionSet.h:
(InsertionSet):
* dfg/DFGNode.h:
(JSC::DFG::Node::Node):
(JSC::DFG::Node::convertToConstant):
(JSC::DFG::Node::convertToGetLocalUnlinked):
(JSC::DFG::Node::containsMovHint):
(Node):
(JSC::DFG::Node::hasVariableAccessData):
(JSC::DFG::Node::willHaveCodeGenOrOSR):
* dfg/DFGNodeType.h:
(DFG):
* dfg/DFGPredictionPropagationPhase.cpp:
(JSC::DFG::PredictionPropagationPhase::propagate):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::convertLastOSRExitToForward):
(JSC::DFG::SpeculativeJIT::compileMovHint):
(JSC::DFG::SpeculativeJIT::compileMovHintAndCheck):
(DFG):
(JSC::DFG::SpeculativeJIT::compileInlineStart):
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT.h:
(SpeculativeJIT):
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGStructureCheckHoistingPhase.cpp:
(JSC::DFG::StructureCheckHoistingPhase::run):
(JSC::DFG::StructureCheckHoistingPhase::shouldConsiderForHoisting):
(StructureCheckHoistingPhase):
* dfg/DFGValidate.cpp:
(JSC::DFG::Validate::validate):
LayoutTests:
Reviewed by Oliver Hunt.
* fast/js/dfg-arguments-osr-exit-multiple-blocks-before-exit-expected.txt: Added.
* fast/js/dfg-arguments-osr-exit-multiple-blocks-before-exit.html: Added.
* fast/js/dfg-arguments-osr-exit-multiple-blocks-expected.txt: Added.
* fast/js/dfg-arguments-osr-exit-multiple-blocks.html: Added.
* fast/js/jsc-test-list:
* fast/js/script-tests/dfg-arguments-osr-exit-multiple-blocks-before-exit.js: Added.
(baz):
(foo):
(bar):
* fast/js/script-tests/dfg-arguments-osr-exit-multiple-blocks.js: Added.
(baz):
(foo):
(bar):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@144862 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
index 887dac3..0cc0f77 100644
--- a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
@@ -197,7 +197,7 @@
// 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(
- m_indexInBlock, DontRefChildren, RefNode, SpecDouble, *node);
+ m_indexInBlock, SpecDouble, *node);
node->setOp(DoubleAsInt32);
node->children.initialize(Edge(newDivision, KnownNumberUse), Edge(), Edge());
@@ -518,8 +518,6 @@
if (ok) {
Edge newChildEdge = logicalNot->child1();
if (newChildEdge->hasBooleanResult()) {
- m_graph.ref(newChildEdge);
- m_graph.deref(logicalNot);
node->children.setChild1(newChildEdge);
BlockIndex toBeTaken = node->notTakenBlockIndex();
@@ -556,8 +554,7 @@
// would have already exited by now, but insert a forced exit just to
// be safe.
m_insertionSet.insertNode(
- m_indexInBlock, DontRefChildren, DontRefNode, SpecNone, ForceOSRExit,
- node->codeOrigin);
+ m_indexInBlock, SpecNone, ForceOSRExit, node->codeOrigin);
}
break;
case ALL_INT32_INDEXING_TYPES:
@@ -638,8 +635,8 @@
node->child1()->prediction(), node->prediction());
if (arrayMode.supportsLength() && arrayProfile->hasDefiniteStructure()) {
m_insertionSet.insertNode(
- m_indexInBlock, RefChildren, DontRefNode, SpecNone, CheckStructure,
- node->codeOrigin, OpInfo(m_graph.addStructureSet(arrayProfile->expectedStructure())),
+ m_indexInBlock, SpecNone, CheckStructure, node->codeOrigin,
+ OpInfo(m_graph.addStructureSet(arrayProfile->expectedStructure())),
node->child1());
}
} else
@@ -650,10 +647,9 @@
ASSERT(node->flags() & NodeMustGenerate);
node->clearFlags(NodeMustGenerate | NodeClobbersWorld);
setUseKindAndUnboxIfProfitable<KnownCellUse>(node->child1());
- m_graph.deref(node);
node->setArrayMode(arrayMode);
- Node* storage = checkArray(arrayMode, node->codeOrigin, node->child1().node(), 0, lengthNeedsStorage, node->shouldGenerate());
+ Node* storage = checkArray(arrayMode, node->codeOrigin, node->child1().node(), 0, lengthNeedsStorage);
if (!storage)
break;
@@ -730,6 +726,9 @@
case PhantomPutStructure:
case GetIndexedPropertyStorage:
case LastNodeType:
+ case MovHint:
+ case MovHintAndCheck:
+ case ZombieHint:
RELEASE_ASSERT_NOT_REACHED();
break;
@@ -837,7 +836,7 @@
m_insertionSet.execute(block);
}
- Node* checkArray(ArrayMode arrayMode, CodeOrigin codeOrigin, Node* array, Node* index, bool (*storageCheck)(const ArrayMode&) = canCSEStorage, bool shouldGenerate = true)
+ Node* checkArray(ArrayMode arrayMode, CodeOrigin codeOrigin, Node* array, Node* index, bool (*storageCheck)(const ArrayMode&) = canCSEStorage)
{
ASSERT(arrayMode.isSpecific());
@@ -858,21 +857,21 @@
}
m_insertionSet.insertNode(
- m_indexInBlock, RefChildren, DontRefNode, SpecNone, ArrayifyToStructure, codeOrigin,
+ m_indexInBlock, SpecNone, ArrayifyToStructure, codeOrigin,
OpInfo(structure), OpInfo(arrayMode.asWord()), Edge(array, CellUse), indexEdge);
} else {
m_insertionSet.insertNode(
- m_indexInBlock, RefChildren, DontRefNode, SpecNone, Arrayify, codeOrigin,
+ m_indexInBlock, SpecNone, Arrayify, codeOrigin,
OpInfo(arrayMode.asWord()), Edge(array, CellUse), indexEdge);
}
} else {
if (structure) {
m_insertionSet.insertNode(
- m_indexInBlock, RefChildren, DontRefNode, SpecNone, CheckStructure, codeOrigin,
+ m_indexInBlock, SpecNone, CheckStructure, codeOrigin,
OpInfo(m_graph.addStructureSet(structure)), Edge(array, CellUse));
} else {
m_insertionSet.insertNode(
- m_indexInBlock, RefChildren, DontRefNode, SpecNone, CheckArray, codeOrigin,
+ m_indexInBlock, SpecNone, CheckArray, codeOrigin,
OpInfo(arrayMode.asWord()), Edge(array, CellUse));
}
}
@@ -882,17 +881,12 @@
if (arrayMode.usesButterfly()) {
return m_insertionSet.insertNode(
- m_indexInBlock,
- shouldGenerate ? RefChildren : DontRefChildren,
- shouldGenerate ? RefNode : DontRefNode,
- SpecNone, GetButterfly, codeOrigin, Edge(array, KnownCellUse));
+ m_indexInBlock, SpecNone, GetButterfly, codeOrigin, Edge(array, KnownCellUse));
}
return m_insertionSet.insertNode(
- m_indexInBlock,
- shouldGenerate ? RefChildren : DontRefChildren,
- shouldGenerate ? RefNode : DontRefNode,
- SpecNone, GetIndexedPropertyStorage, codeOrigin, OpInfo(arrayMode.asWord()), Edge(array, KnownCellUse));
+ m_indexInBlock, SpecNone, GetIndexedPropertyStorage, codeOrigin,
+ OpInfo(arrayMode.asWord()), Edge(array, KnownCellUse));
}
void blessArrayOperation(Edge base, Edge index, Edge& storageChild)
@@ -902,8 +896,7 @@
switch (node->arrayMode().type()) {
case Array::ForceExit: {
m_insertionSet.insertNode(
- m_indexInBlock, DontRefChildren, DontRefNode, SpecNone, ForceOSRExit,
- node->codeOrigin);
+ m_indexInBlock, SpecNone, ForceOSRExit, node->codeOrigin);
return;
}
@@ -982,7 +975,6 @@
return;
}
- Edge oldEdge = edge;
Edge newEdge = node->child1();
if (newEdge.useKind() != Int32Use) {
@@ -992,8 +984,10 @@
ASSERT(newEdge->shouldSpeculateInteger());
- m_graph.ref(newEdge);
- m_graph.deref(oldEdge);
+ // Completely kill the ValueToInt32. We wouldn't have to do crazy things like this
+ // if it weren't for https://bugs.webkit.org/show_bug.cgi?id=111238.
+ node->setOpAndDefaultFlags(Nop);
+ node->child1() = Edge();
edge = newEdge;
}
@@ -1014,7 +1008,7 @@
void injectInt32ToDoubleNode(Edge& edge, UseKind useKind = NumberUse, SpeculationDirection direction = BackwardSpeculation)
{
Node* result = m_insertionSet.insertNode(
- m_indexInBlock, DontRefChildren, RefNode, SpecDouble,
+ m_indexInBlock, SpecDouble,
direction == BackwardSpeculation ? Int32ToDouble : ForwardInt32ToDouble,
m_currentNode->codeOrigin, Edge(edge.node(), NumberUse));
@@ -1039,9 +1033,8 @@
value = jsNumber(JSC::toInt32(value.asNumber()));
ASSERT(value.isInt32());
edge.setNode(m_insertionSet.insertNode(
- m_indexInBlock, DontRefChildren, RefNode, SpecInt32, JSConstant,
- m_currentNode->codeOrigin, OpInfo(codeBlock()->addOrFindConstant(value))));
- m_graph.deref(oldNode);
+ m_indexInBlock, SpecInt32, JSConstant, m_currentNode->codeOrigin,
+ OpInfo(codeBlock()->addOrFindConstant(value))));
}
void truncateConstantsIfNecessary(Node* node, AddSpeculationMode mode)
