Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (153280 => 153281)
--- trunk/Source/_javascript_Core/ChangeLog 2013-07-25 04:04:57 UTC (rev 153280)
+++ trunk/Source/_javascript_Core/ChangeLog 2013-07-25 04:04:59 UTC (rev 153281)
@@ -1,5 +1,45 @@
2013-07-18 Filip Pizlo <[email protected]>
+ fourthTier: DFG shouldn't create CheckStructures for array accesses except if the ArrayMode implies an original array access
+ https://bugs.webkit.org/show_bug.cgi?id=118867
+
+ Reviewed by Mark Hahnenberg.
+
+ This allows us to kill off a bunch of code in the parser, in fixup, and to simplify
+ ArrayProfile.
+
+ It also makes it easier to ask any array-using node how to create its type check.
+
+ Doing this required fixing a bug in LowLevelInterpreter64, where it was storing into
+ an array profile, thinking that it was storing into a value profile. Reshuffling the
+ fields in ArrayProfile revealed this.
+
+ * bytecode/ArrayProfile.cpp:
+ (JSC::ArrayProfile::computeUpdatedPrediction):
+ (JSC::ArrayProfile::briefDescriptionWithoutUpdating):
+ * bytecode/ArrayProfile.h:
+ (JSC::ArrayProfile::ArrayProfile):
+ (ArrayProfile):
+ * bytecode/CodeBlock.cpp:
+ (JSC::CodeBlock::updateAllArrayPredictions):
+ (JSC::CodeBlock::updateAllPredictions):
+ * bytecode/CodeBlock.h:
+ (CodeBlock):
+ (JSC::CodeBlock::updateAllArrayPredictions):
+ * dfg/DFGArrayMode.h:
+ (ArrayMode):
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::getArrayModeConsideringSlowPath):
+ (JSC::DFG::ByteCodeParser::parseBlock):
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::fixupNode):
+ (FixupPhase):
+ (JSC::DFG::FixupPhase::checkArray):
+ (JSC::DFG::FixupPhase::blessArrayOperation):
+ * llint/LowLevelInterpreter64.asm:
+
+2013-07-18 Filip Pizlo <[email protected]>
+
fourthTier: CFA should consider live-at-head for clobbering and dumping
https://bugs.webkit.org/show_bug.cgi?id=118857
Modified: trunk/Source/_javascript_Core/bytecode/ArrayProfile.cpp (153280 => 153281)
--- trunk/Source/_javascript_Core/bytecode/ArrayProfile.cpp 2013-07-25 04:04:57 UTC (rev 153280)
+++ trunk/Source/_javascript_Core/bytecode/ArrayProfile.cpp 2013-07-25 04:04:59 UTC (rev 153281)
@@ -74,38 +74,24 @@
out.print("ArrayWithSlowPutArrayStorage");
}
-void ArrayProfile::computeUpdatedPrediction(const ConcurrentJITLocker& locker, CodeBlock* codeBlock, OperationInProgress operation)
+void ArrayProfile::computeUpdatedPrediction(const ConcurrentJITLocker&, CodeBlock* codeBlock)
{
- if (m_lastSeenStructure) {
- m_observedArrayModes |= arrayModeFromStructure(m_lastSeenStructure);
-
- if (!m_didPerformFirstRunPruning
- && hasTwoOrMoreBitsSet(m_observedArrayModes)) {
- m_observedArrayModes = arrayModeFromStructure(m_lastSeenStructure);
- m_expectedStructure = 0;
- m_didPerformFirstRunPruning = true;
- }
-
- m_mayInterceptIndexedAccesses |=
- m_lastSeenStructure->typeInfo().interceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero();
- if (!codeBlock->globalObject()->isOriginalArrayStructure(m_lastSeenStructure))
- m_usesOriginalArrayStructures = false;
- if (!structureIsPolymorphic(locker)) {
- if (!m_expectedStructure)
- m_expectedStructure = m_lastSeenStructure;
- else if (m_expectedStructure != m_lastSeenStructure)
- m_expectedStructure = polymorphicStructure();
- }
- m_lastSeenStructure = 0;
+ if (!m_lastSeenStructure)
+ return;
+
+ m_observedArrayModes |= arrayModeFromStructure(m_lastSeenStructure);
+
+ if (!m_didPerformFirstRunPruning
+ && hasTwoOrMoreBitsSet(m_observedArrayModes)) {
+ m_observedArrayModes = arrayModeFromStructure(m_lastSeenStructure);
+ m_didPerformFirstRunPruning = true;
}
- if (hasTwoOrMoreBitsSet(m_observedArrayModes))
- m_expectedStructure = polymorphicStructure();
-
- if (operation == Collection
- && expectedStructure(locker)
- && !Heap::isMarked(m_expectedStructure))
- m_expectedStructure = polymorphicStructure();
+ m_mayInterceptIndexedAccesses |=
+ m_lastSeenStructure->typeInfo().interceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero();
+ if (!codeBlock->globalObject()->isOriginalArrayStructure(m_lastSeenStructure))
+ m_usesOriginalArrayStructures = false;
+ m_lastSeenStructure = 0;
}
CString ArrayProfile::briefDescription(const ConcurrentJITLocker& locker, CodeBlock* codeBlock)
@@ -114,7 +100,7 @@
return briefDescriptionWithoutUpdating(locker);
}
-CString ArrayProfile::briefDescriptionWithoutUpdating(const ConcurrentJITLocker& locker)
+CString ArrayProfile::briefDescriptionWithoutUpdating(const ConcurrentJITLocker&)
{
StringPrintStream out;
@@ -127,18 +113,6 @@
hasPrinted = true;
}
- if (structureIsPolymorphic(locker)) {
- if (hasPrinted)
- out.print(", ");
- out.print("struct = TOP");
- hasPrinted = true;
- } else if (m_expectedStructure) {
- if (hasPrinted)
- out.print(", ");
- out.print("struct = ", RawPointer(m_expectedStructure));
- hasPrinted = true;
- }
-
if (m_mayStoreToHole) {
if (hasPrinted)
out.print(", ");
Modified: trunk/Source/_javascript_Core/bytecode/ArrayProfile.h (153280 => 153281)
--- trunk/Source/_javascript_Core/bytecode/ArrayProfile.h 2013-07-25 04:04:57 UTC (rev 153280)
+++ trunk/Source/_javascript_Core/bytecode/ArrayProfile.h 2013-07-25 04:04:59 UTC (rev 153281)
@@ -136,7 +136,6 @@
ArrayProfile()
: m_bytecodeOffset(std::numeric_limits<unsigned>::max())
, m_lastSeenStructure(0)
- , m_expectedStructure(0)
, m_mayStoreToHole(false)
, m_outOfBounds(false)
, m_mayInterceptIndexedAccesses(false)
@@ -149,7 +148,6 @@
ArrayProfile(unsigned bytecodeOffset)
: m_bytecodeOffset(bytecodeOffset)
, m_lastSeenStructure(0)
- , m_expectedStructure(0)
, m_mayStoreToHole(false)
, m_outOfBounds(false)
, m_mayInterceptIndexedAccesses(false)
@@ -171,22 +169,8 @@
m_lastSeenStructure = structure;
}
- void computeUpdatedPrediction(const ConcurrentJITLocker&, CodeBlock*, OperationInProgress = NoOperation);
+ void computeUpdatedPrediction(const ConcurrentJITLocker&, CodeBlock*);
- Structure* expectedStructure(const ConcurrentJITLocker& locker) const
- {
- if (structureIsPolymorphic(locker))
- return 0;
- return m_expectedStructure;
- }
- bool structureIsPolymorphic(const ConcurrentJITLocker&) const
- {
- return m_expectedStructure == polymorphicStructure();
- }
- bool hasDefiniteStructure(const ConcurrentJITLocker& locker) const
- {
- return !structureIsPolymorphic(locker) && m_expectedStructure;
- }
ArrayModes observedArrayModes(const ConcurrentJITLocker&) const { return m_observedArrayModes; }
bool mayInterceptIndexedAccesses(const ConcurrentJITLocker&) const { return m_mayInterceptIndexedAccesses; }
@@ -205,7 +189,6 @@
unsigned m_bytecodeOffset;
Structure* m_lastSeenStructure;
- Structure* m_expectedStructure;
bool m_mayStoreToHole; // This flag may become overloaded to indicate other special cases that were encountered during array access, as it depends on indexing type. Since we currently have basically just one indexing type (two variants of ArrayStorage), this flag for now just means exactly what its name implies.
bool m_outOfBounds;
bool m_mayInterceptIndexedAccesses : 1;
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp (153280 => 153281)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2013-07-25 04:04:57 UTC (rev 153280)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2013-07-25 04:04:59 UTC (rev 153281)
@@ -3204,12 +3204,12 @@
updateAllPredictionsAndCountLiveness(operation, ignoredValue1, ignoredValue2);
}
-void CodeBlock::updateAllArrayPredictions(OperationInProgress operation)
+void CodeBlock::updateAllArrayPredictions()
{
ConcurrentJITLocker locker(m_lock);
for (unsigned i = m_arrayProfiles.size(); i--;)
- m_arrayProfiles[i].computeUpdatedPrediction(locker, this, operation);
+ m_arrayProfiles[i].computeUpdatedPrediction(locker, this);
// Don't count these either, for similar reasons.
for (unsigned i = m_arrayAllocationProfiles.size(); i--;)
@@ -3219,7 +3219,7 @@
void CodeBlock::updateAllPredictions(OperationInProgress operation)
{
updateAllValueProfilePredictions(operation);
- updateAllArrayPredictions(operation);
+ updateAllArrayPredictions();
}
bool CodeBlock::shouldOptimizeNow()
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.h (153280 => 153281)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.h 2013-07-25 04:04:57 UTC (rev 153280)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.h 2013-07-25 04:04:59 UTC (rev 153281)
@@ -885,12 +885,12 @@
#if ENABLE(VALUE_PROFILER)
bool shouldOptimizeNow();
void updateAllValueProfilePredictions(OperationInProgress = NoOperation);
- void updateAllArrayPredictions(OperationInProgress = NoOperation);
+ void updateAllArrayPredictions();
void updateAllPredictions(OperationInProgress = NoOperation);
#else
bool updateAllPredictionsAndCheckIfShouldOptimizeNow() { return false; }
void updateAllValueProfilePredictions(OperationInProgress = NoOperation) { }
- void updateAllArrayPredictions(OperationInProgress = NoOperation) { }
+ void updateAllArrayPredictions() { }
void updateAllPredictions(OperationInProgress = NoOperation) { }
#endif
Modified: trunk/Source/_javascript_Core/dfg/DFGArrayMode.h (153280 => 153281)
--- trunk/Source/_javascript_Core/dfg/DFGArrayMode.h 2013-07-25 04:04:57 UTC (rev 153280)
+++ trunk/Source/_javascript_Core/dfg/DFGArrayMode.h 2013-07-25 04:04:59 UTC (rev 153281)
@@ -345,24 +345,6 @@
Structure* originalArrayStructure(Graph&, const CodeOrigin&) const;
Structure* originalArrayStructure(Graph&, Node*) const;
- bool benefitsFromStructureCheck() const
- {
- switch (type()) {
- case Array::SelectUsingPredictions:
- // It might benefit from structure checks! If it ends up not benefiting, we can just
- // remove it. The FixupPhase does this: if it finds a CheckStructure just before an
- // array op and it had turned that array op into either generic or conversion mode,
- // it will remove the CheckStructure.
- return true;
- case Array::Unprofiled:
- case Array::ForceExit:
- case Array::Generic:
- return false;
- default:
- return conversion() == Array::AsIs;
- }
- }
-
bool doesConversion() const
{
return conversion() != Array::AsIs;
Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (153280 => 153281)
--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2013-07-25 04:04:57 UTC (rev 153280)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2013-07-25 04:04:59 UTC (rev 153281)
@@ -815,7 +815,7 @@
return getArrayMode(profile, Array::Read);
}
- ArrayMode getArrayModeAndEmitChecks(ArrayProfile* profile, Array::Action action, Node* base)
+ ArrayMode getArrayModeConsideringSlowPath(ArrayProfile* profile, Array::Action action)
{
ConcurrentJITLocker locker(m_inlineStackTop->m_profiledBlock->m_lock);
@@ -824,7 +824,7 @@
#if DFG_ENABLE(DEBUG_PROPAGATION_VERBOSE)
if (m_inlineStackTop->m_profiledBlock->numberOfRareCaseProfiles())
dataLogF("Slow case profile for bc#%u: %u\n", m_currentIndex, m_inlineStackTop->m_profiledBlock->rareCaseProfileForBytecodeOffset(m_currentIndex)->m_counter);
- dataLogF("Array profile for bc#%u: %p%s%s, %u\n", m_currentIndex, profile->expectedStructure(), profile->structureIsPolymorphic(locker) ? " (polymorphic)" : "", profile->mayInterceptIndexedAccesses(locker) ? " (may intercept)" : "", profile->observedArrayModes(locker));
+ dataLogF("Array profile for bc#%u: %u %s%s\n", m_currentIndex, profile->observedArrayModes(locker), profile->structureIsPolymorphic(locker) ? " (polymorphic)" : "", profile->mayInterceptIndexedAccesses(locker) ? " (may intercept)" : "");
#endif
bool makeSafe =
@@ -833,11 +833,6 @@
ArrayMode result = ArrayMode::fromObserved(locker, profile, action, makeSafe);
- if (profile->hasDefiniteStructure(locker)
- && result.benefitsFromStructureCheck()
- && !m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, BadCache))
- addToGraph(CheckStructure, OpInfo(m_graph.addStructureSet(profile->expectedStructure(locker))), base);
-
return result;
}
@@ -2328,7 +2323,7 @@
SpeculatedType prediction = getPrediction();
Node* base = get(currentInstruction[2].u.operand);
- ArrayMode arrayMode = getArrayModeAndEmitChecks(currentInstruction[4].u.arrayProfile, Array::Read, base);
+ ArrayMode arrayMode = getArrayModeConsideringSlowPath(currentInstruction[4].u.arrayProfile, Array::Read);
Node* property = get(currentInstruction[3].u.operand);
Node* getByVal = addToGraph(GetByVal, OpInfo(arrayMode.asWord()), OpInfo(prediction), base, property);
set(currentInstruction[1].u.operand, getByVal);
@@ -2339,7 +2334,7 @@
case op_put_by_val: {
Node* base = get(currentInstruction[1].u.operand);
- ArrayMode arrayMode = getArrayModeAndEmitChecks(currentInstruction[4].u.arrayProfile, Array::Write, base);
+ ArrayMode arrayMode = getArrayModeConsideringSlowPath(currentInstruction[4].u.arrayProfile, Array::Write);
Node* property = get(currentInstruction[2].u.operand);
Node* value = get(currentInstruction[3].u.operand);
Modified: trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (153280 => 153281)
--- trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2013-07-25 04:04:57 UTC (rev 153280)
+++ trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2013-07-25 04:04:59 UTC (rev 153281)
@@ -748,12 +748,6 @@
arrayProfile->computeUpdatedPrediction(locker, profiledBlock);
arrayMode = ArrayMode::fromObserved(locker, arrayProfile, Array::Read, false);
arrayMode = arrayMode.refine(node->child1()->prediction(), node->prediction());
- if (arrayMode.supportsLength() && arrayProfile->hasDefiniteStructure(locker)) {
- m_insertionSet.insertNode(
- m_indexInBlock, SpecNone, CheckStructure, node->codeOrigin,
- OpInfo(m_graph.addStructureSet(arrayProfile->expectedStructure(locker))),
- node->child1());
- }
} else
arrayMode = arrayMode.refine(node->child1()->prediction(), node->prediction());
@@ -1220,25 +1214,6 @@
m_insertionSet.execute(block);
}
- void findAndRemoveUnnecessaryStructureCheck(Node* array, const CodeOrigin& codeOrigin)
- {
- for (unsigned index = m_indexInBlock; index--;) {
- Node* previousNode = m_block->at(index);
- if (previousNode->codeOrigin != codeOrigin)
- return;
-
- if (previousNode->op() != CheckStructure)
- continue;
-
- if (previousNode->child1() != array)
- continue;
-
- previousNode->child1() = Edge();
- previousNode->convertToPhantom();
- return; // Assume we were smart enough to only insert one CheckStructure on the array.
- }
- }
-
Node* checkArray(ArrayMode arrayMode, const CodeOrigin& codeOrigin, Node* array, Node* index, bool (*storageCheck)(const ArrayMode&) = canCSEStorage)
{
ASSERT(arrayMode.isSpecific());
@@ -1249,13 +1224,6 @@
if (arrayMode.doesConversion()) {
if (structure) {
- if (m_indexInBlock > 0) {
- // If the previous node was a CheckStructure inserted because of stuff
- // that the array profile told us, then remove it, since we're going to be
- // doing arrayification instead.
- findAndRemoveUnnecessaryStructureCheck(array, codeOrigin);
- }
-
m_insertionSet.insertNode(
m_indexInBlock, SpecNone, ArrayifyToStructure, codeOrigin,
OpInfo(structure), OpInfo(arrayMode.asWord()), Edge(array, CellUse), indexEdge);
@@ -1306,7 +1274,6 @@
return;
case Array::Generic:
- findAndRemoveUnnecessaryStructureCheck(base.node(), node->codeOrigin);
return;
default: {
