DFG variable capture analysis should work even if the variables arose through inlining
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 23 May 2012 07:29:02 +0000 (07:29 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 23 May 2012 07:29:02 +0000 (07:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=85945

Reviewed by Oliver Hunt.

Merged r116555 from dfgopt.

This just changes how the DFG queries whether a variable is captured. It does not
change any user-visible behavior.

As part of this change, I further solidified the policy that the CFA behaves in an
undefined way for captured locals and queries about their values will not yield
reliable results. This will likely be changed in the future, but for now it makes
sense.

One fun part about this change is that it recognizes that the same variable may
be both captured and not, at the same time, because their live interval spans
inlining boundaries. This only happens in the case of arguments to functions that
capture their arguments, and this change treats them with just the right touch of
conservatism: they will be treated as if captured by the caller as well as the
callee.

Finally, this also adds captured variable reasoning to the InlineCallFrame, which
I thought might be useful for later tooling.

This is perf-neutral, since it does it does not make the DFG take advantage of this
new functionality in any way. In particular, it is still the case that the DFG will
not inline functions that use arguments reflectively or that create activations.

* bytecode/CodeBlock.h:
(CodeBlock):
(JSC::CodeBlock::needsActivation):
(JSC::CodeBlock::argumentIsCaptured):
(JSC::CodeBlock::localIsCaptured):
(JSC::CodeBlock::isCaptured):
* bytecode/CodeOrigin.h:
(InlineCallFrame):
* dfg/DFGAbstractState.cpp:
(JSC::DFG::AbstractState::initialize):
(JSC::DFG::AbstractState::endBasicBlock):
(JSC::DFG::AbstractState::execute):
(JSC::DFG::AbstractState::merge):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::newVariableAccessData):
(JSC::DFG::ByteCodeParser::getLocal):
(JSC::DFG::ByteCodeParser::setLocal):
(JSC::DFG::ByteCodeParser::getArgument):
(JSC::DFG::ByteCodeParser::setArgument):
(JSC::DFG::ByteCodeParser::flushArgument):
(JSC::DFG::ByteCodeParser::parseBlock):
(JSC::DFG::ByteCodeParser::processPhiStack):
(JSC::DFG::ByteCodeParser::fixVariableAccessPredictions):
(JSC::DFG::ByteCodeParser::InlineStackEntry::InlineStackEntry):
* dfg/DFGCFGSimplificationPhase.cpp:
(CFGSimplificationPhase):
(JSC::DFG::CFGSimplificationPhase::keepOperandAlive):
(JSC::DFG::CFGSimplificationPhase::fixPossibleGetLocal):
(JSC::DFG::CFGSimplificationPhase::fixTailOperand):
* dfg/DFGCommon.h:
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::nameOfVariableAccessData):
* dfg/DFGGraph.h:
(JSC::DFG::Graph::needsActivation):
(JSC::DFG::Graph::usesArguments):
* dfg/DFGPredictionPropagationPhase.cpp:
(JSC::DFG::PredictionPropagationPhase::doRoundOfDoubleVoting):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGVariableAccessData.h:
(JSC::DFG::VariableAccessData::VariableAccessData):
(JSC::DFG::VariableAccessData::mergeIsCaptured):
(VariableAccessData):
(JSC::DFG::VariableAccessData::isCaptured):

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

15 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/CodeBlock.h
Source/JavaScriptCore/bytecode/CodeOrigin.h
Source/JavaScriptCore/dfg/DFGAbstractState.cpp
Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
Source/JavaScriptCore/dfg/DFGCFGSimplificationPhase.cpp
Source/JavaScriptCore/dfg/DFGCommon.h
Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
Source/JavaScriptCore/dfg/DFGGraph.cpp
Source/JavaScriptCore/dfg/DFGGraph.h
Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
Source/JavaScriptCore/dfg/DFGVariableAccessData.h

index 655517c..a44672c 100644 (file)
@@ -1,5 +1,87 @@
 2012-05-08  Filip Pizlo  <fpizlo@apple.com>
 
+        DFG variable capture analysis should work even if the variables arose through inlining
+        https://bugs.webkit.org/show_bug.cgi?id=85945
+
+        Reviewed by Oliver Hunt.
+        
+        Merged r116555 from dfgopt.
+        
+        This just changes how the DFG queries whether a variable is captured. It does not
+        change any user-visible behavior.
+        
+        As part of this change, I further solidified the policy that the CFA behaves in an
+        undefined way for captured locals and queries about their values will not yield
+        reliable results. This will likely be changed in the future, but for now it makes
+        sense.
+        
+        One fun part about this change is that it recognizes that the same variable may
+        be both captured and not, at the same time, because their live interval spans
+        inlining boundaries. This only happens in the case of arguments to functions that
+        capture their arguments, and this change treats them with just the right touch of
+        conservatism: they will be treated as if captured by the caller as well as the 
+        callee.
+        
+        Finally, this also adds captured variable reasoning to the InlineCallFrame, which
+        I thought might be useful for later tooling.
+        
+        This is perf-neutral, since it does it does not make the DFG take advantage of this
+        new functionality in any way. In particular, it is still the case that the DFG will
+        not inline functions that use arguments reflectively or that create activations.
+
+        * bytecode/CodeBlock.h:
+        (CodeBlock):
+        (JSC::CodeBlock::needsActivation):
+        (JSC::CodeBlock::argumentIsCaptured):
+        (JSC::CodeBlock::localIsCaptured):
+        (JSC::CodeBlock::isCaptured):
+        * bytecode/CodeOrigin.h:
+        (InlineCallFrame):
+        * dfg/DFGAbstractState.cpp:
+        (JSC::DFG::AbstractState::initialize):
+        (JSC::DFG::AbstractState::endBasicBlock):
+        (JSC::DFG::AbstractState::execute):
+        (JSC::DFG::AbstractState::merge):
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::newVariableAccessData):
+        (JSC::DFG::ByteCodeParser::getLocal):
+        (JSC::DFG::ByteCodeParser::setLocal):
+        (JSC::DFG::ByteCodeParser::getArgument):
+        (JSC::DFG::ByteCodeParser::setArgument):
+        (JSC::DFG::ByteCodeParser::flushArgument):
+        (JSC::DFG::ByteCodeParser::parseBlock):
+        (JSC::DFG::ByteCodeParser::processPhiStack):
+        (JSC::DFG::ByteCodeParser::fixVariableAccessPredictions):
+        (JSC::DFG::ByteCodeParser::InlineStackEntry::InlineStackEntry):
+        * dfg/DFGCFGSimplificationPhase.cpp:
+        (CFGSimplificationPhase):
+        (JSC::DFG::CFGSimplificationPhase::keepOperandAlive):
+        (JSC::DFG::CFGSimplificationPhase::fixPossibleGetLocal):
+        (JSC::DFG::CFGSimplificationPhase::fixTailOperand):
+        * dfg/DFGCommon.h:
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::fixupNode):
+        * dfg/DFGGraph.cpp:
+        (JSC::DFG::Graph::nameOfVariableAccessData):
+        * dfg/DFGGraph.h:
+        (JSC::DFG::Graph::needsActivation):
+        (JSC::DFG::Graph::usesArguments):
+        * dfg/DFGPredictionPropagationPhase.cpp:
+        (JSC::DFG::PredictionPropagationPhase::doRoundOfDoubleVoting):
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGSpeculativeJIT32_64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGVariableAccessData.h:
+        (JSC::DFG::VariableAccessData::VariableAccessData):
+        (JSC::DFG::VariableAccessData::mergeIsCaptured):
+        (VariableAccessData):
+        (JSC::DFG::VariableAccessData::isCaptured):
+
+2012-05-08  Filip Pizlo  <fpizlo@apple.com>
+
         DFG should support op_get_argument_by_val and op_get_arguments_length
         https://bugs.webkit.org/show_bug.cgi?id=85911
 
index 38cabfb..6572b13 100644 (file)
@@ -449,6 +449,31 @@ namespace JSC {
             return m_activationRegister;
         }
         bool usesArguments() const { return m_argumentsRegister != -1; }
+        
+        bool needsActivation() const
+        {
+            return needsFullScopeChain() && codeType() != GlobalCode;
+        }
+        
+        bool argumentIsCaptured(int) const
+        {
+            return needsActivation() || usesArguments();
+        }
+        
+        bool localIsCaptured(InlineCallFrame* inlineCallFrame, int operand) const
+        {
+            if (!inlineCallFrame)
+                return operand < m_numCapturedVars;
+            
+            return inlineCallFrame->capturedVars.get(operand);
+        }
+        
+        bool isCaptured(InlineCallFrame* inlineCallFrame, int operand) const
+        {
+            if (operandIsArgument(operand))
+                return argumentIsCaptured(operandToArgument(operand));
+            return localIsCaptured(inlineCallFrame, operand);
+        }
 
         CodeType codeType() const { return m_codeType; }
 
index eda1764..034e48f 100644 (file)
@@ -28,6 +28,7 @@
 
 #include "ValueRecovery.h"
 #include "WriteBarrier.h"
+#include <wtf/BitVector.h>
 #include <wtf/StdLibExtras.h>
 #include <wtf/Vector.h>
 
@@ -92,6 +93,7 @@ struct InlineCallFrame {
     WriteBarrier<ExecutableBase> executable;
     WriteBarrier<JSFunction> callee;
     CodeOrigin caller;
+    BitVector capturedVars; // Indexed by the machine call frame's variable numbering.
     unsigned stackOffset : 31;
     bool isCall : 1;
 };
index f291b58..0ae5069 100644 (file)
@@ -111,7 +111,7 @@ void AbstractState::initialize(Graph& graph)
             continue;
         }
         
-        if (graph.argumentIsCaptured(i)) {
+        if (node.variableAccessData()->isCaptured()) {
             root->valuesAtHead.argument(i).makeTop();
             continue;
         }
@@ -147,7 +147,8 @@ void AbstractState::initialize(Graph& graph)
         root->valuesAtTail.argument(i).clear();
     }
     for (size_t i = 0; i < root->valuesAtHead.numberOfLocals(); ++i) {
-        if (graph.localIsCaptured(i))
+        NodeIndex nodeIndex = root->variablesAtHead.local(i);
+        if (nodeIndex != NoNode && graph[nodeIndex].variableAccessData()->isCaptured())
             root->valuesAtHead.local(i).makeTop();
         else
             root->valuesAtHead.local(i).clear();
@@ -195,7 +196,8 @@ bool AbstractState::endBasicBlock(MergeMode mergeMode)
             dataLog("        Merging state for argument %zu.\n", argument);
 #endif
             AbstractValue& destination = block->valuesAtTail.argument(argument);
-            if (m_graph.argumentIsCaptured(argument)) {
+            NodeIndex nodeIndex = block->variablesAtTail.argument(argument);
+            if (nodeIndex != NoNode && m_graph[nodeIndex].variableAccessData()->isCaptured()) {
                 if (!destination.isTop()) {
                     destination.makeTop();
                     changed = true;
@@ -209,7 +211,8 @@ bool AbstractState::endBasicBlock(MergeMode mergeMode)
             dataLog("        Merging state for local %zu.\n", local);
 #endif
             AbstractValue& destination = block->valuesAtTail.local(local);
-            if (m_graph.localIsCaptured(local)) {
+            NodeIndex nodeIndex = block->variablesAtTail.local(local);
+            if (nodeIndex != NoNode && m_graph[nodeIndex].variableAccessData()->isCaptured()) {
                 if (!destination.isTop()) {
                     destination.makeTop();
                     changed = true;
@@ -255,7 +258,7 @@ bool AbstractState::execute(unsigned indexInBlock)
     }
             
     case GetLocal: {
-        if (m_graph.isCaptured(node.local()))
+        if (node.variableAccessData()->isCaptured())
             forNode(nodeIndex).makeTop();
         else
             forNode(nodeIndex) = m_variables.operand(node.local());
@@ -263,7 +266,7 @@ bool AbstractState::execute(unsigned indexInBlock)
     }
         
     case SetLocal: {
-        if (m_graph.isCaptured(node.local()))
+        if (node.variableAccessData()->isCaptured())
             break;
         
         if (node.variableAccessData()->shouldUseDoubleFormat()) {
@@ -1426,7 +1429,8 @@ inline bool AbstractState::merge(BasicBlock* from, BasicBlock* to)
     
     for (size_t argument = 0; argument < from->variablesAtTail.numberOfArguments(); ++argument) {
         AbstractValue& destination = to->valuesAtHead.argument(argument);
-        if (m_graph.argumentIsCaptured(argument)) {
+        NodeIndex nodeIndex = from->variablesAtTail.argument(argument);
+        if (nodeIndex != NoNode && m_graph[nodeIndex].variableAccessData()->isCaptured()) {
             if (destination.isTop())
                 continue;
             destination.makeTop();
@@ -1438,7 +1442,8 @@ inline bool AbstractState::merge(BasicBlock* from, BasicBlock* to)
     
     for (size_t local = 0; local < from->variablesAtTail.numberOfLocals(); ++local) {
         AbstractValue& destination = to->valuesAtHead.local(local);
-        if (m_graph.localIsCaptured(local)) {
+        NodeIndex nodeIndex = from->variablesAtTail.local(local);
+        if (nodeIndex != NoNode && m_graph[nodeIndex].variableAccessData()->isCaptured()) {
             if (destination.isTop())
                 continue;
             destination.makeTop();
index c58f500..d041ff4 100644 (file)
@@ -113,11 +113,11 @@ private:
     // Add spill locations to nodes.
     void allocateVirtualRegisters();
     
-    VariableAccessData* newVariableAccessData(int operand)
+    VariableAccessData* newVariableAccessData(int operand, bool isCaptured)
     {
         ASSERT(operand < FirstConstantRegisterIndex);
         
-        m_graph.m_variableAccessData.append(VariableAccessData(static_cast<VirtualRegister>(operand)));
+        m_graph.m_variableAccessData.append(VariableAccessData(static_cast<VirtualRegister>(operand), isCaptured));
         return &m_graph.m_variableAccessData.last();
     }
     
@@ -178,6 +178,7 @@ private:
     NodeIndex getLocal(unsigned operand)
     {
         NodeIndex nodeIndex = m_currentBlock->variablesAtTail.local(operand);
+        bool isCaptured = m_codeBlock->localIsCaptured(m_inlineStackTop->m_inlineCallFrame, operand);
         
         if (nodeIndex != NoNode) {
             Node* nodePtr = &m_graph[nodeIndex];
@@ -189,6 +190,7 @@ private:
                 Node& flushChild = m_graph[nodeIndex];
                 if (flushChild.op() == Phi) {
                     VariableAccessData* variableAccessData = flushChild.variableAccessData();
+                    variableAccessData->mergeIsCaptured(isCaptured);
                     nodeIndex = injectLazyOperandPrediction(addToGraph(GetLocal, OpInfo(variableAccessData), nodeIndex));
                     m_currentBlock->variablesAtTail.local(operand) = nodeIndex;
                     return nodeIndex;
@@ -199,7 +201,9 @@ private:
             ASSERT(&m_graph[nodeIndex] == nodePtr);
             ASSERT(nodePtr->op() != Flush);
 
-            if (m_graph.localIsCaptured(operand)) {
+            nodePtr->variableAccessData()->mergeIsCaptured(isCaptured);
+                
+            if (isCaptured) {
                 // We wish to use the same variable access data as the previous access,
                 // but for all other purposes we want to issue a load since for all we
                 // know, at this stage of compilation, the local has been clobbered.
@@ -221,7 +225,7 @@ private:
         // expand m_preservedVars to cover these.
         m_preservedVars.set(operand);
         
-        VariableAccessData* variableAccessData = newVariableAccessData(operand);
+        VariableAccessData* variableAccessData = newVariableAccessData(operand, isCaptured);
         
         NodeIndex phi = addToGraph(Phi, OpInfo(variableAccessData));
         m_localPhiStack.append(PhiStackEntry(m_currentBlock, phi, operand));
@@ -234,11 +238,13 @@ private:
     }
     void setLocal(unsigned operand, NodeIndex value)
     {
-        VariableAccessData* variableAccessData = newVariableAccessData(operand);
+        bool isCaptured = m_codeBlock->localIsCaptured(m_inlineStackTop->m_inlineCallFrame, operand);
+        
+        VariableAccessData* variableAccessData = newVariableAccessData(operand, isCaptured);
         NodeIndex nodeIndex = addToGraph(SetLocal, OpInfo(variableAccessData), value);
         m_currentBlock->variablesAtTail.local(operand) = nodeIndex;
         
-        bool shouldFlush = m_graph.localIsCaptured(operand);
+        bool shouldFlush = isCaptured;
         
         if (!shouldFlush) {
             // If this is in argument position, then it should be flushed.
@@ -267,6 +273,9 @@ private:
     NodeIndex getArgument(unsigned operand)
     {
         unsigned argument = operandToArgument(operand);
+        
+        bool isCaptured = m_codeBlock->argumentIsCaptured(argument);
+        
         ASSERT(argument < m_numArguments);
         
         NodeIndex nodeIndex = m_currentBlock->variablesAtTail.argument(argument);
@@ -281,6 +290,7 @@ private:
                 Node& flushChild = m_graph[nodeIndex];
                 if (flushChild.op() == Phi) {
                     VariableAccessData* variableAccessData = flushChild.variableAccessData();
+                    variableAccessData->mergeIsCaptured(isCaptured);
                     nodeIndex = injectLazyOperandPrediction(addToGraph(GetLocal, OpInfo(variableAccessData), nodeIndex));
                     m_currentBlock->variablesAtTail.local(operand) = nodeIndex;
                     return nodeIndex;
@@ -291,6 +301,8 @@ private:
             ASSERT(&m_graph[nodeIndex] == nodePtr);
             ASSERT(nodePtr->op() != Flush);
             
+            nodePtr->variableAccessData()->mergeIsCaptured(isCaptured);
+            
             if (nodePtr->op() == SetArgument) {
                 // We're getting an argument in the first basic block; link
                 // the GetLocal to the SetArgument.
@@ -300,7 +312,7 @@ private:
                 return nodeIndex;
             }
             
-            if (m_graph.argumentIsCaptured(argument)) {
+            if (isCaptured) {
                 if (nodePtr->op() == GetLocal)
                     nodeIndex = nodePtr->child1().index();
                 return injectLazyOperandPrediction(addToGraph(GetLocal, OpInfo(nodePtr->variableAccessData()), nodeIndex));
@@ -313,7 +325,7 @@ private:
             return nodePtr->child1().index();
         }
         
-        VariableAccessData* variableAccessData = newVariableAccessData(operand);
+        VariableAccessData* variableAccessData = newVariableAccessData(operand, isCaptured);
 
         NodeIndex phi = addToGraph(Phi, OpInfo(variableAccessData));
         m_argumentPhiStack.append(PhiStackEntry(m_currentBlock, phi, argument));
@@ -327,9 +339,11 @@ private:
     void setArgument(int operand, NodeIndex value)
     {
         unsigned argument = operandToArgument(operand);
+        bool isCaptured = m_codeBlock->argumentIsCaptured(argument);
+        
         ASSERT(argument < m_numArguments);
         
-        VariableAccessData* variableAccessData = newVariableAccessData(operand);
+        VariableAccessData* variableAccessData = newVariableAccessData(operand, isCaptured);
         InlineStackEntry* stack = m_inlineStackTop;
         while (stack->m_inlineCallFrame) // find the machine stack entry.
             stack = stack->m_caller;
@@ -346,6 +360,7 @@ private:
         // some other local variable.
         
         operand = m_inlineStackTop->remapOperand(operand);
+        bool isCaptured = m_codeBlock->isCaptured(m_inlineStackTop->m_inlineCallFrame, operand);
         
         ASSERT(operand < FirstConstantRegisterIndex);
         
@@ -380,11 +395,12 @@ private:
             // This gives us guidance to see that the variable also needs to be flushed
             // for arguments, even if it already had to be flushed for other reasons.
             VariableAccessData* variableAccessData = node.variableAccessData();
+            variableAccessData->mergeIsCaptured(isCaptured);
             addToGraph(Flush, OpInfo(variableAccessData), nodeIndex);
             return variableAccessData;
         }
         
-        VariableAccessData* variableAccessData = newVariableAccessData(operand);
+        VariableAccessData* variableAccessData = newVariableAccessData(operand, isCaptured);
         NodeIndex phi = addToGraph(Phi, OpInfo(variableAccessData));
         nodeIndex = addToGraph(Flush, OpInfo(variableAccessData), phi);
         if (operandIsArgument(operand)) {
@@ -1480,7 +1496,7 @@ bool ByteCodeParser::parseBlock(unsigned limit)
     if (m_currentBlock == m_graph.m_blocks[0].get() && !m_inlineStackTop->m_inlineCallFrame) {
         m_graph.m_arguments.resize(m_numArguments);
         for (unsigned argument = 0; argument < m_numArguments; ++argument) {
-            NodeIndex setArgument = addToGraph(SetArgument, OpInfo(newVariableAccessData(argumentToOperand(argument))));
+            NodeIndex setArgument = addToGraph(SetArgument, OpInfo(newVariableAccessData(argumentToOperand(argument), m_codeBlock->argumentIsCaptured(argument))));
             m_graph.m_arguments[argument] = setArgument;
             m_currentBlock->variablesAtHead.setArgumentFirstTime(argument, setArgument);
             m_currentBlock->variablesAtTail.setArgumentFirstTime(argument, setArgument);
@@ -2463,7 +2479,7 @@ void ByteCodeParser::processPhiStack()
                 dataLog("      Did not find node, adding phi.\n");
 #endif
 
-                valueInPredecessor = insertPhiNode(OpInfo(newVariableAccessData(stackType == ArgumentPhiStack ? argumentToOperand(varNo) : static_cast<int>(varNo))), predecessorBlock);
+                valueInPredecessor = insertPhiNode(OpInfo(newVariableAccessData(stackType == ArgumentPhiStack ? argumentToOperand(varNo) : static_cast<int>(varNo), false)), predecessorBlock);
                 var = valueInPredecessor;
                 if (stackType == ArgumentPhiStack)
                     predecessorBlock->variablesAtHead.setArgumentFirstTime(varNo, valueInPredecessor);
@@ -2580,6 +2596,7 @@ void ByteCodeParser::fixVariableAccessPredictions()
     for (unsigned i = 0; i < m_graph.m_variableAccessData.size(); ++i) {
         VariableAccessData* data = &m_graph.m_variableAccessData[i];
         data->find()->predict(data->nonUnifiedPrediction());
+        data->find()->mergeIsCaptured(data->isCaptured());
     }
 }
 
@@ -2681,6 +2698,17 @@ ByteCodeParser::InlineStackEntry::InlineStackEntry(ByteCodeParser* byteCodeParse
         inlineCallFrame.caller = byteCodeParser->currentCodeOrigin();
         inlineCallFrame.arguments.resize(codeBlock->numParameters()); // Set the number of arguments including this, but don't configure the value recoveries, yet.
         inlineCallFrame.isCall = isCall(kind);
+        
+        if (inlineCallFrame.caller.inlineCallFrame)
+            inlineCallFrame.capturedVars = inlineCallFrame.caller.inlineCallFrame->capturedVars;
+        else {
+            for (int i = byteCodeParser->m_codeBlock->m_numCapturedVars; i--;)
+                inlineCallFrame.capturedVars.set(i);
+        }
+        
+        for (int i = codeBlock->m_numCapturedVars; i--;)
+            inlineCallFrame.capturedVars.set(i + inlineCallFrameStart);
+        
         byteCodeParser->m_codeBlock->inlineCallFrames().append(inlineCallFrame);
         m_inlineCallFrame = &byteCodeParser->m_codeBlock->inlineCallFrames().last();
         
index 15be9bd..be3401a 100644 (file)
@@ -254,24 +254,17 @@ private:
         m_graph.m_blocks[blockIndex].clear();
     }
     
-    NodeIndex findOperandSource(const Operands<NodeIndex, NodeIndexTraits>& variables, int operand)
-    {
-        NodeIndex nodeIndex = variables.operand(operand);
-        if (nodeIndex == NoNode)
-            return NoNode;
-        if (m_graph[nodeIndex].op() == SetLocal)
-            return m_graph[nodeIndex].child1().index();
-        return nodeIndex;
-    }
-    
     void keepOperandAlive(BasicBlock* block, CodeOrigin codeOrigin, int operand)
     {
-        if (m_graph.isCaptured(operand))
-            return;
-        NodeIndex nodeIndex = findOperandSource(block->variablesAtTail, operand);
+        NodeIndex nodeIndex = block->variablesAtTail.operand(operand);
         if (nodeIndex == NoNode)
             return;
-        if (!m_graph[nodeIndex].shouldGenerate())
+        if (m_graph[nodeIndex].variableAccessData()->isCaptured())
+            return;
+        if (m_graph[nodeIndex].op() == SetLocal)
+            nodeIndex = m_graph[nodeIndex].child1().index();
+        Node& node = m_graph[nodeIndex];
+        if (!node.shouldGenerate())
             return;
         ASSERT(m_graph[nodeIndex].op() != SetLocal);
         NodeIndex phantomNodeIndex = m_graph.size();
@@ -286,7 +279,7 @@ private:
         Node& child = m_graph[edge];
         if (child.op() != GetLocal)
             return;
-        if (m_graph.isCaptured(child.local()))
+        if (child.variableAccessData()->isCaptured())
             return;
         NodeIndex originalNodeIndex = block->variablesAtTail.operand(child.local());
         ASSERT(originalNodeIndex != NoNode);
@@ -444,19 +437,20 @@ private:
     {
         NodeIndex atSecondTail = secondBlock->variablesAtTail.operand(operand);
         
-        if (m_graph.isCaptured(operand)) {
-            // The second block did something to a variable that is captured, so reflect this.
-            firstBlock->variablesAtTail.operand(operand) = atSecondTail;
-            return;
-        }
-        
         if (atSecondTail == NoNode) {
             // If the variable is dead at the end of the second block, then do nothing; essentially
             // this means that we want the tail state to reflect whatever the first block did.
             return;
         }
-        
+
         Node& secondNode = m_graph[atSecondTail];
+        
+        if (secondNode.variableAccessData()->isCaptured()) {
+            // The second block did something to a variable that is captured, so reflect this.
+            firstBlock->variablesAtTail.operand(operand) = atSecondTail;
+            return;
+        }
+        
         switch (secondNode.op()) {
         case SetLocal:
         case Flush: {
index 76854f9..c4373d7 100644 (file)
@@ -77,9 +77,6 @@
 #define DFG_ENABLE_SUCCESS_STATS 0
 // Enable verification that the DFG is able to insert code for control flow edges.
 #define DFG_ENABLE_EDGE_CODE_VERIFICATION 0
-// Pretend that all variables in the top-level code block got captured. Great
-// for testing code gen for activations.
-#define DFG_ENABLE_ALL_VARIABLES_CAPTURED 0
 
 namespace JSC { namespace DFG {
 
index 50bb05c..bbabc00 100644 (file)
@@ -218,7 +218,7 @@ private:
         }
             
         case SetLocal: {
-            if (m_graph.isCaptured(node.local()))
+            if (node.variableAccessData()->isCaptured())
                 break;
             if (!node.variableAccessData()->shouldUseDoubleFormat())
                 break;
index ef98511..f0b5c6d 100644 (file)
@@ -65,7 +65,7 @@ const char* Graph::nameOfVariableAccessData(VariableAccessData* variableAccessDa
     if (!index)
         return "A";
 
-    static char buf[10];
+    static char buf[100];
     BoundsCheckedPointer<char> ptr(buf, sizeof(buf));
     
     while (index) {
@@ -73,6 +73,11 @@ const char* Graph::nameOfVariableAccessData(VariableAccessData* variableAccessDa
         index /= 26;
     }
     
+    if (variableAccessData->isCaptured())
+        *ptr++ = '*';
+    
+    ptr.strcat(predictionToAbbreviatedString(variableAccessData->prediction()));
+    
     *ptr++ = 0;
     
     return buf;
index c1325c5..2d025ff 100644 (file)
@@ -349,46 +349,12 @@ public:
     
     bool needsActivation() const
     {
-#if DFG_ENABLE(ALL_VARIABLES_CAPTURED)
-        return true;
-#else
         return m_codeBlock->needsFullScopeChain() && m_codeBlock->codeType() != GlobalCode;
-#endif
     }
     
     bool usesArguments() const
     {
-#if DFG_ENABLE(ALL_VARIABLES_CAPTURED)
-        return true;
-#else
         return m_codeBlock->usesArguments();
-#endif
-    }
-    
-    // Pass an argument index. Currently it's ignored, but that's somewhat
-    // of a bug.
-    bool argumentIsCaptured(int) const
-    {
-        return needsActivation() || usesArguments();
-    }
-    bool localIsCaptured(int operand) const
-    {
-#if DFG_ENABLE(ALL_VARIABLES_CAPTURED)
-        return operand < m_codeBlock->m_numVars;
-#else
-        return operand < m_codeBlock->m_numCapturedVars;
-#endif
-    }
-    
-    bool isCaptured(int operand) const
-    {
-        if (operandIsArgument(operand))
-            return argumentIsCaptured(operandToArgument(operand));
-        return localIsCaptured(operand);
-    }
-    bool isCaptured(VirtualRegister virtualRegister) const
-    {
-        return isCaptured(static_cast<int>(virtualRegister));
     }
     
     unsigned numSuccessors(BasicBlock* block)
index 1cea58e..eeb2ee6 100644 (file)
@@ -866,7 +866,7 @@ private:
             if (!variableAccessData->isRoot())
                 continue;
             if (operandIsArgument(variableAccessData->local())
-                || m_graph.isCaptured(variableAccessData->local()))
+                || variableAccessData->isCaptured())
                 continue;
             m_changed |= variableAccessData->tallyVotesForShouldUseDoubleFormat();
         }
@@ -877,7 +877,7 @@ private:
             if (!variableAccessData->isRoot())
                 continue;
             if (operandIsArgument(variableAccessData->local())
-                || m_graph.isCaptured(variableAccessData->local()))
+                || variableAccessData->isCaptured())
                 continue;
             m_changed |= variableAccessData->makePredictionForDoubleFormat();
         }
index 6953c82..165664c 100644 (file)
@@ -974,7 +974,7 @@ void SpeculativeJIT::compile(BasicBlock& block)
     ASSERT(m_arguments.size() == block.variablesAtHead.numberOfArguments());
     for (size_t i = 0; i < m_arguments.size(); ++i) {
         NodeIndex nodeIndex = block.variablesAtHead.argument(i);
-        if (nodeIndex == NoNode || m_jit.graph().argumentIsCaptured(i))
+        if (nodeIndex == NoNode || m_jit.codeBlock()->argumentIsCaptured(i))
             m_arguments[i] = ValueSource(ValueInRegisterFile);
         else
             m_arguments[i] = ValueSource::forPrediction(at(nodeIndex).variableAccessData()->prediction());
@@ -986,10 +986,12 @@ void SpeculativeJIT::compile(BasicBlock& block)
     ASSERT(m_variables.size() == block.variablesAtHead.numberOfLocals());
     for (size_t i = 0; i < m_variables.size(); ++i) {
         NodeIndex nodeIndex = block.variablesAtHead.local(i);
-        if ((nodeIndex == NoNode || !at(nodeIndex).refCount()) && !m_jit.graph().localIsCaptured(i))
-            m_variables[i] = ValueSource(SourceIsDead);
-        else if (m_jit.graph().localIsCaptured(i))
+        // FIXME: Use the variable access data, not the first node in the block.
+        // https://bugs.webkit.org/show_bug.cgi?id=87205
+        if (m_jit.codeBlock()->localIsCaptured(at(block[0]).codeOrigin.inlineCallFrame, i))
             m_variables[i] = ValueSource(ValueInRegisterFile);
+        else if (nodeIndex == NoNode || !at(nodeIndex).refCount())
+            m_variables[i] = ValueSource(SourceIsDead);
         else if (at(nodeIndex).variableAccessData()->shouldUseDoubleFormat())
             m_variables[i] = ValueSource(DoubleInRegisterFile);
         else
index e10deca..c0518d6 100644 (file)
@@ -1855,12 +1855,19 @@ void SpeculativeJIT::compile(Node& node)
         AbstractValue& value = block()->valuesAtHead.operand(node.local());
 
         // If we have no prediction for this local, then don't attempt to compile.
-        if (prediction == PredictNone || value.isClear()) {
+        if (prediction == PredictNone) {
             terminateSpeculativeExecution(InadequateCoverage, JSValueRegs(), NoNode);
             break;
         }
         
-        if (!m_jit.graph().isCaptured(node.local())) {
+        if (!node.variableAccessData()->isCaptured()) {
+            // If the CFA is tracking this variable and it found that the variable
+            // cannot have been assigned, then don't attempt to proceed.
+            if (value.isClear()) {
+                terminateSpeculativeExecution(InadequateCoverage, JSValueRegs(), NoNode);
+                break;
+            }
+            
             if (node.variableAccessData()->shouldUseDoubleFormat()) {
                 FPRTemporary result(this);
                 m_jit.loadDouble(JITCompiler::addressFor(node.local()), result.fpr());
@@ -1920,7 +1927,7 @@ void SpeculativeJIT::compile(Node& node)
 
         DataFormat format;
         if (isCellPrediction(value.m_type)
-            && !m_jit.graph().isCaptured(node.local()))
+            && !node.variableAccessData()->isCaptured())
             format = DataFormatJSCell;
         else
             format = DataFormatJS;
@@ -1965,7 +1972,7 @@ void SpeculativeJIT::compile(Node& node)
         // OSR exit, would not be visible to the old JIT in any way.
         m_codeOriginForOSR = nextNode->codeOrigin;
         
-        if (!m_jit.graph().isCaptured(node.local())) {
+        if (!node.variableAccessData()->isCaptured()) {
             if (node.variableAccessData()->shouldUseDoubleFormat()) {
                 SpeculateDoubleOperand value(this, node.child1());
                 m_jit.storeDouble(value.fpr(), JITCompiler::addressFor(node.local()));
index 03ee9ee..ce9430f 100644 (file)
@@ -1928,12 +1928,19 @@ void SpeculativeJIT::compile(Node& node)
         AbstractValue& value = block()->valuesAtHead.operand(node.local());
 
         // If we have no prediction for this local, then don't attempt to compile.
-        if (prediction == PredictNone || value.isClear()) {
+        if (prediction == PredictNone) {
             terminateSpeculativeExecution(InadequateCoverage, JSValueRegs(), NoNode);
             break;
         }
         
-        if (!m_jit.graph().isCaptured(node.local())) {
+        if (!node.variableAccessData()->isCaptured()) {
+            // If the CFA is tracking this variable and it found that the variable
+            // cannot have been assigned, then don't attempt to proceed.
+            if (value.isClear()) {
+                terminateSpeculativeExecution(InadequateCoverage, JSValueRegs(), NoNode);
+                break;
+            }
+            
             if (node.variableAccessData()->shouldUseDoubleFormat()) {
                 FPRTemporary result(this);
                 m_jit.loadDouble(JITCompiler::addressFor(node.local()), result.fpr());
@@ -1965,7 +1972,7 @@ void SpeculativeJIT::compile(Node& node)
         m_gprs.retain(result.gpr(), virtualRegister, SpillOrderJS);
 
         DataFormat format;
-        if (m_jit.graph().isCaptured(node.local()))
+        if (node.variableAccessData()->isCaptured())
             format = DataFormatJS;
         else if (isCellPrediction(value.m_type))
             format = DataFormatJSCell;
@@ -2015,7 +2022,7 @@ void SpeculativeJIT::compile(Node& node)
         // OSR exit, would not be visible to the old JIT in any way.
         m_codeOriginForOSR = nextNode->codeOrigin;
         
-        if (!m_jit.graph().isCaptured(node.local())) {
+        if (!node.variableAccessData()->isCaptured()) {
             if (node.variableAccessData()->shouldUseDoubleFormat()) {
                 SpeculateDoubleOperand value(this, node.child1());
                 m_jit.storeDouble(value.fpr(), JITCompiler::addressFor(node.local()));
index 1d99ed5..b8568d0 100644 (file)
@@ -51,12 +51,13 @@ public:
         clearVotes();
     }
     
-    VariableAccessData(VirtualRegister local)
+    VariableAccessData(VirtualRegister local, bool isCaptured)
         : m_local(local)
         , m_prediction(PredictNone)
         , m_argumentAwarePrediction(PredictNone)
         , m_flags(0)
         , m_doubleFormatState(EmptyDoubleFormatState)
+        , m_isCaptured(isCaptured)
     {
         clearVotes();
     }
@@ -72,6 +73,20 @@ public:
         return static_cast<int>(local());
     }
     
+    bool mergeIsCaptured(bool isCaptured)
+    {
+        bool newIsCaptured = m_isCaptured | isCaptured;
+        if (newIsCaptured == m_isCaptured)
+            return false;
+        m_isCaptured = newIsCaptured;
+        return true;
+    }
+    
+    bool isCaptured()
+    {
+        return m_isCaptured;
+    }
+    
     bool predict(PredictedType prediction)
     {
         VariableAccessData* self = find();
@@ -220,6 +235,8 @@ private:
     
     float m_votes[2];
     DoubleFormatState m_doubleFormatState;
+    
+    bool m_isCaptured;
 };
 
 } } // namespace JSC::DFG