fourthTier: DFG shouldn't create CheckStructures for array accesses except if the...
authoroliver@apple.com <oliver@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 25 Jul 2013 04:04:59 +0000 (04:04 +0000)
committeroliver@apple.com <oliver@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 25 Jul 2013 04:04:59 +0000 (04:04 +0000)
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:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@153281 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/ArrayProfile.cpp
Source/JavaScriptCore/bytecode/ArrayProfile.h
Source/JavaScriptCore/bytecode/CodeBlock.cpp
Source/JavaScriptCore/bytecode/CodeBlock.h
Source/JavaScriptCore/dfg/DFGArrayMode.h
Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
Source/JavaScriptCore/dfg/DFGFixupPhase.cpp

index 6aca70b..b1868b2 100644 (file)
@@ -1,5 +1,45 @@
 2013-07-18  Filip Pizlo  <fpizlo@apple.com>
 
+        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  <fpizlo@apple.com>
+
         fourthTier: CFA should consider live-at-head for clobbering and dumping
         https://bugs.webkit.org/show_bug.cgi?id=118857
 
index 04e9d69..269499c 100644 (file)
@@ -74,38 +74,24 @@ void dumpArrayModes(PrintStream& out, ArrayModes arrayModes)
         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 @@ CString ArrayProfile::briefDescription(const ConcurrentJITLocker& locker, CodeBl
     return briefDescriptionWithoutUpdating(locker);
 }
 
-CString ArrayProfile::briefDescriptionWithoutUpdating(const ConcurrentJITLocker& locker)
+CString ArrayProfile::briefDescriptionWithoutUpdating(const ConcurrentJITLocker&)
 {
     StringPrintStream out;
     
@@ -127,18 +113,6 @@ CString ArrayProfile::briefDescriptionWithoutUpdating(const ConcurrentJITLocker&
         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(", ");
index 42f0b0a..c23230e 100644 (file)
@@ -136,7 +136,6 @@ public:
     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 @@ public:
     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 @@ public:
         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 @@ private:
     
     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;
index c458d5a..99ce9c4 100644 (file)
@@ -3204,12 +3204,12 @@ void CodeBlock::updateAllValueProfilePredictions(OperationInProgress operation)
     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::updateAllArrayPredictions(OperationInProgress operation)
 void CodeBlock::updateAllPredictions(OperationInProgress operation)
 {
     updateAllValueProfilePredictions(operation);
-    updateAllArrayPredictions(operation);
+    updateAllArrayPredictions();
 }
 
 bool CodeBlock::shouldOptimizeNow()
index 9d7371e..01e52f1 100644 (file)
@@ -885,12 +885,12 @@ public:
 #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
 
index d7b30ab..5fbb67d 100644 (file)
@@ -345,24 +345,6 @@ public:
     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;
index c1d1d89..abb1b8a 100644 (file)
@@ -815,7 +815,7 @@ private:
         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 @@ private:
 #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 @@ private:
         
         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 @@ bool ByteCodeParser::parseBlock(unsigned limit)
             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 @@ bool ByteCodeParser::parseBlock(unsigned limit)
         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);
index 24e29f4..ffb26d2 100644 (file)
@@ -748,12 +748,6 @@ private:
                 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 @@ private:
         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 @@ private:
         
         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 @@ private:
             return;
             
         case Array::Generic:
-            findAndRemoveUnnecessaryStructureCheck(base.node(), node->codeOrigin);
             return;
             
         default: {