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/DFGAbstractState.cpp b/Source/JavaScriptCore/dfg/DFGAbstractState.cpp
index 43b5a03..aad9786 100644
--- a/Source/JavaScriptCore/dfg/DFGAbstractState.cpp
+++ b/Source/JavaScriptCore/dfg/DFGAbstractState.cpp
@@ -887,12 +887,10 @@
forNode(nodeIndex).makeTop();
break;
case Array::String:
- forNode(node.child1()).filter(SpecString);
forNode(node.child2()).filter(SpecInt32);
forNode(nodeIndex).set(SpecString);
break;
case Array::Arguments:
- forNode(node.child1()).filter(SpecArguments);
forNode(node.child2()).filter(SpecInt32);
forNode(nodeIndex).makeTop();
break;
@@ -900,42 +898,34 @@
case Array::JSArrayOutOfBounds:
// FIXME: We should have more conservative handling of the out-of-bounds
// case.
- forNode(node.child1()).filter(SpecCell);
forNode(node.child2()).filter(SpecInt32);
forNode(nodeIndex).makeTop();
break;
case Array::Int8Array:
- forNode(node.child1()).filter(SpecInt8Array);
forNode(node.child2()).filter(SpecInt32);
forNode(nodeIndex).set(SpecInt32);
break;
case Array::Int16Array:
- forNode(node.child1()).filter(SpecInt16Array);
forNode(node.child2()).filter(SpecInt32);
forNode(nodeIndex).set(SpecInt32);
break;
case Array::Int32Array:
- forNode(node.child1()).filter(SpecInt32Array);
forNode(node.child2()).filter(SpecInt32);
forNode(nodeIndex).set(SpecInt32);
break;
case Array::Uint8Array:
- forNode(node.child1()).filter(SpecUint8Array);
forNode(node.child2()).filter(SpecInt32);
forNode(nodeIndex).set(SpecInt32);
break;
case Array::Uint8ClampedArray:
- forNode(node.child1()).filter(SpecUint8ClampedArray);
forNode(node.child2()).filter(SpecInt32);
forNode(nodeIndex).set(SpecInt32);
break;
case Array::Uint16Array:
- forNode(node.child1()).filter(SpecUint16Array);
forNode(node.child2()).filter(SpecInt32);
forNode(nodeIndex).set(SpecInt32);
break;
case Array::Uint32Array:
- forNode(node.child1()).filter(SpecUint32Array);
forNode(node.child2()).filter(SpecInt32);
if (node.shouldSpeculateInteger())
forNode(nodeIndex).set(SpecInt32);
@@ -943,12 +933,10 @@
forNode(nodeIndex).set(SpecDouble);
break;
case Array::Float32Array:
- forNode(node.child1()).filter(SpecFloat32Array);
forNode(node.child2()).filter(SpecInt32);
forNode(nodeIndex).set(SpecDouble);
break;
case Array::Float64Array:
- forNode(node.child1()).filter(SpecFloat64Array);
forNode(node.child2()).filter(SpecInt32);
forNode(nodeIndex).set(SpecDouble);
break;
@@ -959,7 +947,6 @@
case PutByVal:
case PutByValAlias: {
node.setCanExit(true);
- Edge child1 = m_graph.varArgChild(node, 0);
Edge child2 = m_graph.varArgChild(node, 1);
Edge child3 = m_graph.varArgChild(node, 2);
switch (modeForPut(node.arrayMode())) {
@@ -970,20 +957,16 @@
clobberWorld(node.codeOrigin, indexInBlock);
break;
case Array::JSArray:
- forNode(child1).filter(SpecCell);
forNode(child2).filter(SpecInt32);
break;
case Array::JSArrayOutOfBounds:
- forNode(child1).filter(SpecCell);
forNode(child2).filter(SpecInt32);
clobberWorld(node.codeOrigin, indexInBlock);
break;
case Array::Arguments:
- forNode(child1).filter(SpecArguments);
forNode(child2).filter(SpecInt32);
break;
case Array::Int8Array:
- forNode(child1).filter(SpecInt8Array);
forNode(child2).filter(SpecInt32);
if (m_graph[child3].shouldSpeculateInteger())
forNode(child3).filter(SpecInt32);
@@ -991,7 +974,6 @@
forNode(child3).filter(SpecNumber);
break;
case Array::Int16Array:
- forNode(child1).filter(SpecInt16Array);
forNode(child2).filter(SpecInt32);
if (m_graph[child3].shouldSpeculateInteger())
forNode(child3).filter(SpecInt32);
@@ -999,7 +981,6 @@
forNode(child3).filter(SpecNumber);
break;
case Array::Int32Array:
- forNode(child1).filter(SpecInt32Array);
forNode(child2).filter(SpecInt32);
if (m_graph[child3].shouldSpeculateInteger())
forNode(child3).filter(SpecInt32);
@@ -1007,7 +988,6 @@
forNode(child3).filter(SpecNumber);
break;
case Array::Uint8Array:
- forNode(child1).filter(SpecUint8Array);
forNode(child2).filter(SpecInt32);
if (m_graph[child3].shouldSpeculateInteger())
forNode(child3).filter(SpecInt32);
@@ -1015,7 +995,6 @@
forNode(child3).filter(SpecNumber);
break;
case Array::Uint8ClampedArray:
- forNode(child1).filter(SpecUint8ClampedArray);
forNode(child2).filter(SpecInt32);
if (m_graph[child3].shouldSpeculateInteger())
forNode(child3).filter(SpecInt32);
@@ -1023,7 +1002,6 @@
forNode(child3).filter(SpecNumber);
break;
case Array::Uint16Array:
- forNode(child1).filter(SpecUint16Array);
forNode(child2).filter(SpecInt32);
if (m_graph[child3].shouldSpeculateInteger())
forNode(child3).filter(SpecInt32);
@@ -1031,7 +1009,6 @@
forNode(child3).filter(SpecNumber);
break;
case Array::Uint32Array:
- forNode(child1).filter(SpecUint32Array);
forNode(child2).filter(SpecInt32);
if (m_graph[child3].shouldSpeculateInteger())
forNode(child3).filter(SpecInt32);
@@ -1039,12 +1016,10 @@
forNode(child3).filter(SpecNumber);
break;
case Array::Float32Array:
- forNode(child1).filter(SpecFloat32Array);
forNode(child2).filter(SpecInt32);
forNode(child3).filter(SpecNumber);
break;
case Array::Float64Array:
- forNode(child1).filter(SpecFloat64Array);
forNode(child2).filter(SpecInt32);
forNode(child3).filter(SpecNumber);
break;
@@ -1057,13 +1032,11 @@
case ArrayPush:
node.setCanExit(true);
- forNode(node.child1()).filter(SpecCell);
forNode(nodeIndex).set(SpecNumber);
break;
case ArrayPop:
node.setCanExit(true);
- forNode(node.child1()).filter(SpecCell);
forNode(nodeIndex).makeTop();
break;
@@ -1347,80 +1320,8 @@
break;
case GetArrayLength:
- switch (node.arrayMode()) {
- case Array::Undecided:
- ASSERT_NOT_REACHED();
- break;
- case Array::ForceExit:
- m_isValid = false;
- break;
- case Array::Generic:
- ASSERT_NOT_REACHED();
- break;
- case Array::String:
- node.setCanExit(!isStringSpeculation(forNode(node.child1()).m_type));
- forNode(node.child1()).filter(SpecString);
- forNode(nodeIndex).set(SpecInt32);
- break;
- case Array::JSArray:
- node.setCanExit(true);
- forNode(node.child1()).filter(SpecCell);
- forNode(nodeIndex).set(SpecInt32);
- break;
- case Array::JSArrayOutOfBounds:
- ASSERT_NOT_REACHED();
- break;
- case Array::Arguments:
- node.setCanExit(true);
- forNode(node.child1()).filter(SpecArguments);
- forNode(nodeIndex).set(SpecInt32);
- break;
- case Array::Int8Array:
- node.setCanExit(!isInt8ArraySpeculation(forNode(node.child1()).m_type));
- forNode(node.child1()).filter(SpecInt8Array);
- forNode(nodeIndex).set(SpecInt32);
- break;
- case Array::Int16Array:
- node.setCanExit(!isInt16ArraySpeculation(forNode(node.child1()).m_type));
- forNode(node.child1()).filter(SpecInt16Array);
- forNode(nodeIndex).set(SpecInt32);
- break;
- case Array::Int32Array:
- node.setCanExit(!isInt32ArraySpeculation(forNode(node.child1()).m_type));
- forNode(node.child1()).filter(SpecInt32Array);
- forNode(nodeIndex).set(SpecInt32);
- break;
- case Array::Uint8Array:
- node.setCanExit(!isUint8ArraySpeculation(forNode(node.child1()).m_type));
- forNode(node.child1()).filter(SpecUint8Array);
- forNode(nodeIndex).set(SpecInt32);
- break;
- case Array::Uint8ClampedArray:
- node.setCanExit(!isUint8ClampedArraySpeculation(forNode(node.child1()).m_type));
- forNode(node.child1()).filter(SpecUint8ClampedArray);
- forNode(nodeIndex).set(SpecInt32);
- break;
- case Array::Uint16Array:
- node.setCanExit(!isUint16ArraySpeculation(forNode(node.child1()).m_type));
- forNode(node.child1()).filter(SpecUint16Array);
- forNode(nodeIndex).set(SpecInt32);
- break;
- case Array::Uint32Array:
- node.setCanExit(!isUint32ArraySpeculation(forNode(node.child1()).m_type));
- forNode(node.child1()).filter(SpecUint32Array);
- forNode(nodeIndex).set(SpecInt32);
- break;
- case Array::Float32Array:
- node.setCanExit(!isFloat32ArraySpeculation(forNode(node.child1()).m_type));
- forNode(node.child1()).filter(SpecFloat32Array);
- forNode(nodeIndex).set(SpecInt32);
- break;
- case Array::Float64Array:
- node.setCanExit(!isFloat64ArraySpeculation(forNode(node.child1()).m_type));
- forNode(node.child1()).filter(SpecFloat64Array);
- forNode(nodeIndex).set(SpecInt32);
- break;
- }
+ node.setCanExit(true); // Lies, but it's true for the common case of JSArray, so it's good enough.
+ forNode(nodeIndex).set(SpecInt32);
break;
case CheckStructure:
@@ -1492,8 +1393,13 @@
forNode(node.child1()).filter(SpecCell);
forNode(nodeIndex).clear(); // The result is not a JS value.
break;
- case GetIndexedPropertyStorage: {
- node.setCanExit(true); // Lies, but this is (almost) always followed by GetByVal, which does exit. So no point in trying to be more precise.
+ case CheckArray: {
+ if (modeAlreadyChecked(forNode(node.child1()), node.arrayMode())) {
+ m_foundConstants = true;
+ node.setCanExit(false);
+ break;
+ }
+ node.setCanExit(true); // Lies, but this is followed by operations (like GetByVal) that always exit, so there is no point in us trying to be clever here.
switch (node.arrayMode()) {
case Array::String:
forNode(node.child1()).filter(SpecString);
@@ -1504,6 +1410,9 @@
// CFA tracking of array mode speculations, but we don't have that, yet.
forNode(node.child1()).filter(SpecCell);
break;
+ case Array::Arguments:
+ forNode(node.child1()).filter(SpecArguments);
+ break;
case Array::Int8Array:
forNode(node.child1()).filter(SpecInt8Array);
break;
@@ -1535,6 +1444,19 @@
ASSERT_NOT_REACHED();
break;
}
+ break;
+ }
+ case GetIndexedPropertyStorage: {
+ switch (node.arrayMode()) {
+ case Array::String:
+ // Strings are weird - we may spec fail if the string was a rope. That is of course
+ // stupid, and we should fix that, but for now let's at least be honest about it.
+ node.setCanExit(true);
+ break;
+ default:
+ node.setCanExit(false);
+ break;
+ }
forNode(nodeIndex).clear();
break;
}
