Array type checks and storage accesses should be uniformly represented and available to CSE
https://bugs.webkit.org/show_bug.cgi?id=95013
Reviewed by Oliver Hunt.
This uniformly breaks up all array accesses into up to three parts:
1) The type check, using a newly introduced CheckArray node, in addition to possibly
a CheckStructure node. We were already inserting the CheckStructure prior to this
patch. The CheckArray node will be automatically eliminated if the thing it was
checking for had already been checked for, either intentionally (a CheckStructure
inserted based on the array profile of this access) or accidentally (some checks,
typically a CheckStructure, inserted for some unrelated operations). The
CheckArray node may not be inserted if the array type is non-specific (Generic or
ForceExit).
2) The storage load using GetIndexedPropertyStorage. Previously, this only worked for
GetByVal. Now it works for all array accesses. The storage load may not be
inserted if the mode of array access does not permit CSE of storage loads (like
non-specific modes or Arguments).
3) The access itself: one of GetByVal, PutByVal, PutByValAlias, ArrayPush, ArrayPop,
GetArrayLength, StringCharAt, or StringCharCodeAt.
This means that the type check can be subjected to CSE even if the CFA isn't smart
enough to reason about it (yet!). It also means that the storage load can always be
subjected to CSE; previously CSE on storage load only worked for array loads and not
other forms of access. Finally, it removes the bizarre behavior that
GetIndexedPropertyStorage previously had: previously, it was responsible for the type
check in some cases, but not others; this made reasoning about the CFA really
confusing.
This change also disables late refinement of array mode, since I decided that
supporting that feature is both confusing and likely unprofitable. The array modes are
now locked in in the first fixup run after prediction propagation. Of course,
refinements from Generic to something else would not have been a problem; we could
reenable those if we thought we really needed to.
* dfg/DFGAbstractState.cpp:
(JSC::DFG::AbstractState::execute):
* dfg/DFGArgumentsSimplificationPhase.cpp:
(JSC::DFG::ArgumentsSimplificationPhase::run):
* dfg/DFGArrayMode.cpp:
(JSC::DFG::fromStructure):
(DFG):
(JSC::DFG::refineArrayMode):
* dfg/DFGArrayMode.h:
(DFG):
(JSC::DFG::modeIsJSArray):
(JSC::DFG::lengthNeedsStorage):
(JSC::DFG::modeIsSpecific):
(JSC::DFG::modeSupportsLength):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::ByteCodeParser):
(JSC::DFG::ByteCodeParser::getArrayMode):
(ByteCodeParser):
(JSC::DFG::ByteCodeParser::getArrayModeAndEmitChecks):
(JSC::DFG::ByteCodeParser::handleIntrinsic):
(JSC::DFG::ByteCodeParser::parseBlock):
* dfg/DFGCFGSimplificationPhase.cpp:
(JSC::DFG::CFGSimplificationPhase::mergeBlocks):
* dfg/DFGCSEPhase.cpp:
(JSC::DFG::CSEPhase::CSEPhase):
(JSC::DFG::CSEPhase::checkStructureElimination):
(CSEPhase):
(JSC::DFG::CSEPhase::checkArrayElimination):
(JSC::DFG::CSEPhase::getIndexedPropertyStorageLoadElimination):
(JSC::DFG::CSEPhase::performNodeCSE):
(JSC::DFG::performCSE):
* dfg/DFGCSEPhase.h:
(DFG):
* dfg/DFGCommon.h:
* dfg/DFGConstantFoldingPhase.cpp:
(JSC::DFG::ConstantFoldingPhase::foldConstants):
* dfg/DFGDriver.cpp:
(JSC::DFG::compile):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
(JSC::DFG::FixupPhase::checkArray):
(FixupPhase):
(JSC::DFG::FixupPhase::blessArrayOperation):
* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::Graph):
(DFG):
(JSC::DFG::Graph::dump):
(JSC::DFG::Graph::collectGarbage):
* dfg/DFGGraph.h:
(Graph):
(JSC::DFG::Graph::vote):
(JSC::DFG::Graph::substitute):
* dfg/DFGNode.h:
(JSC::DFG::Node::hasArrayMode):
(JSC::DFG::Node::setArrayMode):
* dfg/DFGNodeType.h:
(DFG):
* dfg/DFGOperations.cpp:
* dfg/DFGPhase.h:
(DFG):
* dfg/DFGPredictionPropagationPhase.cpp:
(JSC::DFG::PredictionPropagationPhase::propagate):
(JSC::DFG::PredictionPropagationPhase::mergeDefaultFlags):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::checkArray):
(JSC::DFG::SpeculativeJIT::useChildren):
(JSC::DFG::SpeculativeJIT::compilePutByValForIntTypedArray):
(JSC::DFG::SpeculativeJIT::compilePutByValForFloatTypedArray):
(JSC::DFG::SpeculativeJIT::compileGetIndexedPropertyStorage):
(JSC::DFG::SpeculativeJIT::compileGetArrayLength):
* 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):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@126715 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
index fe7cae8..7700b4b 100644
--- a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
@@ -74,6 +74,9 @@
switch (op) {
case GetById: {
+ if (m_graph.m_fixpointState > BeforeFixpoint)
+ break;
+
Node* nodePtr = &node;
if (!isInt32Speculation(m_graph[m_compileIndex].prediction()))
@@ -90,8 +93,7 @@
fromObserved(arrayProfile->observedArrayModes(), false),
m_graph[node.child1()].prediction(),
m_graph[m_compileIndex].prediction());
- if (modeSupportsLength(arrayMode)
- && arrayProfile->hasDefiniteStructure()) {
+ if (modeSupportsLength(arrayMode) && arrayProfile->hasDefiniteStructure()) {
m_graph.ref(nodePtr->child1());
Node checkStructure(CheckStructure, nodePtr->codeOrigin, OpInfo(m_graph.addStructureSet(arrayProfile->expectedStructure())), nodePtr->child1().index());
checkStructure.ref();
@@ -113,17 +115,16 @@
nodePtr->clearFlags(NodeMustGenerate);
m_graph.deref(m_compileIndex);
nodePtr->setArrayMode(arrayMode);
+
+ NodeIndex storage = checkArray(arrayMode, nodePtr->codeOrigin, nodePtr->child1().index(), lengthNeedsStorage, nodePtr->shouldGenerate());
+ if (storage == NoNode)
+ break;
+
+ nodePtr = &m_graph[m_compileIndex];
+ nodePtr->children.child2() = Edge(storage);
break;
}
case GetIndexedPropertyStorage: {
- node.setArrayMode(
- refineArrayMode(
- node.arrayMode(),
- m_graph[node.child1()].prediction(),
- m_graph[node.child2()].prediction()));
- // Predictions should only become more, rather than less, refined. Hence
- // if we were ever able to CSE the storage pointer for this operation,
- // then we should always continue to be able to do so.
ASSERT(canCSEStorage(node.arrayMode()));
break;
}
@@ -136,30 +137,19 @@
m_graph[node.child1()].prediction(),
m_graph[node.child2()].prediction()));
- if (canCSEStorage(node.arrayMode())) {
- if (node.child3()) {
- ASSERT(m_graph[node.child3()].op() == GetIndexedPropertyStorage);
- ASSERT(modesCompatibleForStorageLoad(m_graph[node.child3()].arrayMode(), node.arrayMode()));
- } else {
- // Make sure we don't use the node reference after we do the append.
- Node getIndexedPropertyStorage(
- GetIndexedPropertyStorage, node.codeOrigin, OpInfo(node.arrayMode()),
- node.child1().index(), node.child2().index());
- NodeIndex getIndexedPropertyStorageIndex = m_graph.size();
- node.children.child3() = Edge(getIndexedPropertyStorageIndex);
- m_graph.append(getIndexedPropertyStorage);
- m_graph.ref(getIndexedPropertyStorageIndex); // Once because it's MustGenerate.
- m_graph.ref(getIndexedPropertyStorageIndex); // And again because it's referenced from the GetByVal.
- m_insertionSet.append(m_indexInBlock, getIndexedPropertyStorageIndex);
- }
- } else {
- // See above. Continued fixup of the graph should not regress our ability
- // to speculate.
- ASSERT(!node.child3());
- }
+ blessArrayOperation(node.child1(), 2);
break;
}
+ case ArrayPush: {
+ blessArrayOperation(node.child1(), 2);
+ break;
+ }
+
+ case ArrayPop: {
+ blessArrayOperation(node.child1(), 1);
+ }
+
case ValueToInt32: {
if (m_graph[node.child1()].shouldSpeculateNumber()
&& node.mustGenerate()) {
@@ -330,11 +320,18 @@
Edge child1 = m_graph.varArgChild(node, 0);
Edge child2 = m_graph.varArgChild(node, 1);
Edge child3 = m_graph.varArgChild(node, 2);
+
node.setArrayMode(
refineArrayMode(
- node.arrayMode(), m_graph[child1].prediction(), m_graph[child2].prediction()));
+ node.arrayMode(),
+ m_graph[child1].prediction(),
+ m_graph[child2].prediction()));
- switch (modeForPut(node.arrayMode())) {
+ blessArrayOperation(child1, 3);
+
+ Node* nodePtr = &m_graph[m_compileIndex];
+
+ switch (modeForPut(nodePtr->arrayMode())) {
case Array::Int8Array:
case Array::Int16Array:
case Array::Int32Array:
@@ -368,6 +365,59 @@
#endif
}
+ NodeIndex checkArray(Array::Mode arrayMode, CodeOrigin codeOrigin, NodeIndex array, bool (*storageCheck)(Array::Mode) = canCSEStorage, bool shouldGenerate = true)
+ {
+ ASSERT(modeIsSpecific(arrayMode));
+
+ m_graph.ref(array);
+ Node checkArray(CheckArray, codeOrigin, OpInfo(arrayMode), array);
+ checkArray.ref();
+ NodeIndex checkArrayIndex = m_graph.size();
+ m_graph.append(checkArray);
+ m_insertionSet.append(m_indexInBlock, checkArrayIndex);
+
+ if (!storageCheck(arrayMode))
+ return NoNode;
+
+ if (shouldGenerate)
+ m_graph.ref(array);
+ Node getIndexedPropertyStorage(
+ GetIndexedPropertyStorage, codeOrigin, OpInfo(arrayMode), array);
+ if (shouldGenerate)
+ getIndexedPropertyStorage.ref();
+ NodeIndex getIndexedPropertyStorageIndex = m_graph.size();
+ m_graph.append(getIndexedPropertyStorage);
+ m_insertionSet.append(m_indexInBlock, getIndexedPropertyStorageIndex);
+
+ return getIndexedPropertyStorageIndex;
+ }
+
+ void blessArrayOperation(Edge base, unsigned storageChildIdx)
+ {
+ if (m_graph.m_fixpointState > BeforeFixpoint)
+ return;
+
+ Node* nodePtr = &m_graph[m_compileIndex];
+
+ if (nodePtr->arrayMode() == Array::ForceExit) {
+ Node forceExit(ForceOSRExit, nodePtr->codeOrigin);
+ forceExit.ref();
+ NodeIndex forceExitIndex = m_graph.size();
+ m_graph.append(forceExit);
+ m_insertionSet.append(m_indexInBlock, forceExitIndex);
+ return;
+ }
+
+ if (!modeIsSpecific(nodePtr->arrayMode()))
+ return;
+
+ NodeIndex storage = checkArray(nodePtr->arrayMode(), nodePtr->codeOrigin, base.index());
+ if (storage == NoNode)
+ return;
+
+ m_graph.child(m_graph[m_compileIndex], storageChildIdx) = Edge(storage);
+ }
+
void fixIntEdge(Edge& edge)
{
Node& node = m_graph[edge];
