DFG should constant fold GetScope, and accesses to the scope register in the ByteCode...
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Mar 2015 05:26:54 +0000 (05:26 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Mar 2015 05:26:54 +0000 (05:26 +0000)
https://bugs.webkit.org/show_bug.cgi?id=106202

Rubber stamped by Benjamin Poulain.

This fixes a bug discovered by working on https://bugs.webkit.org/show_bug.cgi?id=142229,
which was in turn discovered by working on https://bugs.webkit.org/show_bug.cgi?id=141174.
Our way of dealing with scopes known to be constant is very sketchy, and only really works
when a function is inlined. When it is, we pretend that every load of the scopeRegister sees
a constant. But this breaks the DFG's tracking of the liveness of the scopeRegister. The way
this worked made us miss oppportunities for optimizing based on a constant scope, and it also
meant that in some cases - particularly like when we inline code that uses NewFuction and
friends, as I do in bug 142229 - it makes OSR exit think that the scope is dead even though
it's most definitely alive and it's a constant.

The problem here is that we were doing too many optimizations in the ByteCodeParser, and not
later. Later optimization phases know how to preserve OSR exit liveness. They're actually
really good at it. Also, later phases know how to infer that any variable is a constant no
matter how that constant arose - rather than the inlining-specific thing in ByteCodeParser.

This changes the ByteCodeParser to largely avoid doing constant folding on the scope, except
making the GetScope operation itself a constant. This is a compilation-time hack for small
functions, and it doesn't break the loads of local variables - so OSR exit liveness still
sees that the scopeRegister is in use. This then adds a vastly more powerful GetScope and
GetClosureVar constant folder in the AbstractInterpreter. This handles most general cases
including those that arise in complex control flow. This will catch cases where the scope
is constant for any number of reasons. Basically anytime that the callee is inferred constant
this will give us a constant scope. Also, we still have the parse-time constant folding of
ResolveScope based on the reentry watchpoint, which luckily did the right thing with respect
to OSR exit liveness (it splats a Phantom on its inputs, and it produces a constant result
which is then set() normally).

This appears to be a broad speed-up, albeit a small one. But mainly it unblocks bug 142229,
which then should unblock bug 141174.

* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::get):
(JSC::DFG::ByteCodeParser::getLocal):
(JSC::DFG::ByteCodeParser::parseBlock):
(JSC::DFG::ByteCodeParser::parse):
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* dfg/DFGDoesGC.cpp:
(JSC::DFG::doesGC):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::tryGetConstantClosureVar):
(JSC::DFG::Graph::tryGetRegisters):
(JSC::DFG::Graph::tryGetActivation): Deleted.
* dfg/DFGGraph.h:
* dfg/DFGNode.h:
(JSC::DFG::Node::hasVariableWatchpointSet):
(JSC::DFG::Node::hasSymbolTable): Deleted.
(JSC::DFG::Node::symbolTable): Deleted.
* dfg/DFGNodeType.h:
* dfg/DFGPredictionPropagationPhase.cpp:
(JSC::DFG::PredictionPropagationPhase::propagate):
* dfg/DFGSafeToExecute.h:
(JSC::DFG::safeToExecute):
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGWatchpointCollectionPhase.cpp:
(JSC::DFG::WatchpointCollectionPhase::handle):
* ftl/FTLCapabilities.cpp:
(JSC::FTL::canCompile):
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::LowerDFGToLLVM::compileNode):
(JSC::FTL::LowerDFGToLLVM::compileGetClosureVar):
* runtime/SymbolTable.cpp:
(JSC::SymbolTable::visitChildren):
(JSC::SymbolTable::localToEntry):
(JSC::SymbolTable::entryFor):
* runtime/SymbolTable.h:
(JSC::SymbolTable::add):
(JSC::SymbolTable::set):
* tests/stress/function-expression-exit.js: Added.
* tests/stress/function-reentry-infer-on-self.js: Added.
(thingy):
* tests/stress/goofy-function-reentry-incorrect-inference.js: Added.

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

22 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
Source/JavaScriptCore/dfg/DFGClobberize.h
Source/JavaScriptCore/dfg/DFGDoesGC.cpp
Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
Source/JavaScriptCore/dfg/DFGGraph.cpp
Source/JavaScriptCore/dfg/DFGGraph.h
Source/JavaScriptCore/dfg/DFGNode.h
Source/JavaScriptCore/dfg/DFGNodeType.h
Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
Source/JavaScriptCore/dfg/DFGSafeToExecute.h
Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
Source/JavaScriptCore/dfg/DFGWatchpointCollectionPhase.cpp
Source/JavaScriptCore/ftl/FTLCapabilities.cpp
Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp
Source/JavaScriptCore/runtime/SymbolTable.cpp
Source/JavaScriptCore/runtime/SymbolTable.h
Source/JavaScriptCore/tests/stress/function-expression-exit.js [new file with mode: 0644]
Source/JavaScriptCore/tests/stress/function-reentry-infer-on-self.js [new file with mode: 0644]
Source/JavaScriptCore/tests/stress/goofy-function-reentry-incorrect-inference.js [new file with mode: 0644]

index bd2eaf8..afd3899 100644 (file)
@@ -1,3 +1,90 @@
+2015-03-03  Filip Pizlo  <fpizlo@apple.com>
+
+        DFG should constant fold GetScope, and accesses to the scope register in the ByteCodeParser should not pretend that it's a constant as that breaks OSR exit liveness tracking
+        https://bugs.webkit.org/show_bug.cgi?id=106202
+
+        Rubber stamped by Benjamin Poulain.
+        
+        This fixes a bug discovered by working on https://bugs.webkit.org/show_bug.cgi?id=142229,
+        which was in turn discovered by working on https://bugs.webkit.org/show_bug.cgi?id=141174.
+        Our way of dealing with scopes known to be constant is very sketchy, and only really works
+        when a function is inlined. When it is, we pretend that every load of the scopeRegister sees
+        a constant. But this breaks the DFG's tracking of the liveness of the scopeRegister. The way
+        this worked made us miss oppportunities for optimizing based on a constant scope, and it also
+        meant that in some cases - particularly like when we inline code that uses NewFuction and
+        friends, as I do in bug 142229 - it makes OSR exit think that the scope is dead even though
+        it's most definitely alive and it's a constant.
+        
+        The problem here is that we were doing too many optimizations in the ByteCodeParser, and not
+        later. Later optimization phases know how to preserve OSR exit liveness. They're actually
+        really good at it. Also, later phases know how to infer that any variable is a constant no
+        matter how that constant arose - rather than the inlining-specific thing in ByteCodeParser.
+        
+        This changes the ByteCodeParser to largely avoid doing constant folding on the scope, except
+        making the GetScope operation itself a constant. This is a compilation-time hack for small
+        functions, and it doesn't break the loads of local variables - so OSR exit liveness still
+        sees that the scopeRegister is in use. This then adds a vastly more powerful GetScope and
+        GetClosureVar constant folder in the AbstractInterpreter. This handles most general cases
+        including those that arise in complex control flow. This will catch cases where the scope
+        is constant for any number of reasons. Basically anytime that the callee is inferred constant
+        this will give us a constant scope. Also, we still have the parse-time constant folding of
+        ResolveScope based on the reentry watchpoint, which luckily did the right thing with respect
+        to OSR exit liveness (it splats a Phantom on its inputs, and it produces a constant result
+        which is then set() normally).
+        
+        This appears to be a broad speed-up, albeit a small one. But mainly it unblocks bug 142229,
+        which then should unblock bug 141174.
+
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::get):
+        (JSC::DFG::ByteCodeParser::getLocal):
+        (JSC::DFG::ByteCodeParser::parseBlock):
+        (JSC::DFG::ByteCodeParser::parse):
+        * dfg/DFGClobberize.h:
+        (JSC::DFG::clobberize):
+        * dfg/DFGDoesGC.cpp:
+        (JSC::DFG::doesGC):
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::fixupNode):
+        * dfg/DFGGraph.cpp:
+        (JSC::DFG::Graph::tryGetConstantClosureVar):
+        (JSC::DFG::Graph::tryGetRegisters):
+        (JSC::DFG::Graph::tryGetActivation): Deleted.
+        * dfg/DFGGraph.h:
+        * dfg/DFGNode.h:
+        (JSC::DFG::Node::hasVariableWatchpointSet):
+        (JSC::DFG::Node::hasSymbolTable): Deleted.
+        (JSC::DFG::Node::symbolTable): Deleted.
+        * dfg/DFGNodeType.h:
+        * dfg/DFGPredictionPropagationPhase.cpp:
+        (JSC::DFG::PredictionPropagationPhase::propagate):
+        * dfg/DFGSafeToExecute.h:
+        (JSC::DFG::safeToExecute):
+        * dfg/DFGSpeculativeJIT32_64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGWatchpointCollectionPhase.cpp:
+        (JSC::DFG::WatchpointCollectionPhase::handle):
+        * ftl/FTLCapabilities.cpp:
+        (JSC::FTL::canCompile):
+        * ftl/FTLLowerDFGToLLVM.cpp:
+        (JSC::FTL::LowerDFGToLLVM::compileNode):
+        (JSC::FTL::LowerDFGToLLVM::compileGetClosureVar):
+        * runtime/SymbolTable.cpp:
+        (JSC::SymbolTable::visitChildren):
+        (JSC::SymbolTable::localToEntry):
+        (JSC::SymbolTable::entryFor):
+        * runtime/SymbolTable.h:
+        (JSC::SymbolTable::add):
+        (JSC::SymbolTable::set):
+        * tests/stress/function-expression-exit.js: Added.
+        * tests/stress/function-reentry-infer-on-self.js: Added.
+        (thingy):
+        * tests/stress/goofy-function-reentry-incorrect-inference.js: Added.
+
 2015-03-03  Anders Carlsson  <andersca@apple.com>
 
         Remove unused compression code
index 1495c97..90f98aa 100644 (file)
@@ -1329,7 +1329,6 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
             m_graph, m_codeBlock->globalObjectFor(node->origin.semantic)->activationStructure());
         break;
         
-    case FunctionReentryWatchpoint:
     case TypedArrayWatchpoint:
         break;
     
@@ -1450,7 +1449,13 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
         break;
     }
         
-    case GetScope: // FIXME: We could get rid of these if we know that the JSFunction is a constant. https://bugs.webkit.org/show_bug.cgi?id=106202
+    case GetScope:
+        if (JSValue base = forNode(node->child1()).m_value) {
+            if (JSFunction* function = jsDynamicCast<JSFunction*>(base)) {
+                setConstant(node, *m_graph.freeze(function->scope()));
+                break;
+            }
+        }
         forNode(node).setType(SpecObjectOther);
         break;
 
@@ -1469,6 +1474,10 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
         break;
 
     case GetClosureVar:
+        if (JSValue value = m_graph.tryGetConstantClosureVar(forNode(node->child1()), VirtualRegister(node->varNumber()))) {
+            setConstant(node, *m_graph.freeze(value));
+            break;
+        }
         forNode(node).makeHeapTop();
         break;
             
@@ -1968,7 +1977,6 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
         forNode(node).makeHeapTop();
         break;
         
-    case VariableWatchpoint:
     case VarInjectionWatchpoint:
     case PutGlobalVar:
     case NotifyWrite:
index fce954f..04e66fa 100644 (file)
@@ -277,8 +277,6 @@ private:
                 JSFunction* callee = inlineCallFrame()->calleeConstant();
                 if (operand.offset() == JSStack::Callee)
                     return weakJSConstant(callee);
-                if (operand == m_inlineStackTop->m_codeBlock->scopeRegister())
-                    return weakJSConstant(callee->scope());
             }
         } else if (operand.offset() == JSStack::Callee)
             return addToGraph(GetCallee);
@@ -347,16 +345,6 @@ private:
     {
         unsigned local = operand.toLocal();
 
-        if (local < m_localWatchpoints.size()) {
-            if (VariableWatchpointSet* set = m_localWatchpoints[local]) {
-                if (JSValue value = set->inferredValue()) {
-                    addToGraph(FunctionReentryWatchpoint, OpInfo(m_codeBlock->symbolTable()));
-                    addToGraph(VariableWatchpoint, OpInfo(set));
-                    return weakJSConstant(value);
-                }
-            }
-        }
-
         Node* node = m_currentBlock->variablesAtTail.local(local);
         bool isCaptured = m_codeBlock->isCaptured(operand, inlineCallFrame());
         
@@ -891,8 +879,6 @@ private:
 
     HashMap<ConstantBufferKey, unsigned> m_constantBufferCache;
     
-    Vector<VariableWatchpointSet*, 16> m_localWatchpoints;
-    
     struct InlineStackEntry {
         ByteCodeParser* m_byteCodeParser;
         
@@ -3432,7 +3418,7 @@ bool ByteCodeParser::parseBlock(unsigned limit)
                 JSLexicalEnvironment* lexicalEnvironment = currentInstruction[6].u.lexicalEnvironment.get();
                 if (lexicalEnvironment
                     && lexicalEnvironment->symbolTable()->m_functionEnteredOnce.isStillValid()) {
-                    addToGraph(FunctionReentryWatchpoint, OpInfo(lexicalEnvironment->symbolTable()));
+                    m_graph.watchpoints().addLazily(lexicalEnvironment->symbolTable()->m_functionEnteredOnce);
                     addToGraph(Phantom, getDirect(m_inlineStackTop->remapOperand(VirtualRegister(currentInstruction[2].u.operand))));
                     set(VirtualRegister(dst), weakJSConstant(lexicalEnvironment));
                     break;
@@ -3491,7 +3477,8 @@ bool ByteCodeParser::parseBlock(unsigned limit)
             case GlobalVar:
             case GlobalVarWithVarInjectionChecks: {
                 addToGraph(Phantom, get(VirtualRegister(scope)));
-                SymbolTableEntry entry = globalObject->symbolTable()->get(uid);
+                ConcurrentJITLocker locker(globalObject->symbolTable()->m_lock);
+                SymbolTableEntry entry = globalObject->symbolTable()->get(locker, uid);
                 VariableWatchpointSet* watchpointSet = entry.watchpointSet();
                 JSValue inferredValue =
                     watchpointSet ? watchpointSet->inferredValue() : JSValue();
@@ -3501,7 +3488,7 @@ bool ByteCodeParser::parseBlock(unsigned limit)
                     break;
                 }
                 
-                addToGraph(VariableWatchpoint, OpInfo(watchpointSet));
+                m_graph.watchpoints().addLazily(watchpointSet);
                 set(VirtualRegister(dst), weakJSConstant(inferredValue));
                 break;
             }
@@ -3509,25 +3496,16 @@ bool ByteCodeParser::parseBlock(unsigned limit)
             case ClosureVar:
             case ClosureVarWithVarInjectionChecks: {
                 Node* scopeNode = get(VirtualRegister(scope));
-                if (JSLexicalEnvironment* lexicalEnvironment = m_graph.tryGetActivation(scopeNode)) {
-                    SymbolTable* symbolTable = lexicalEnvironment->symbolTable();
-                    ConcurrentJITLocker locker(symbolTable->m_lock);
-                    SymbolTable::Map::iterator iter = symbolTable->find(locker, uid);
-                    ASSERT(iter != symbolTable->end(locker));
-                    VariableWatchpointSet* watchpointSet = iter->value.watchpointSet();
-                    if (watchpointSet) {
-                        if (JSValue value = watchpointSet->inferredValue()) {
-                            addToGraph(Phantom, scopeNode);
-                            addToGraph(VariableWatchpoint, OpInfo(watchpointSet));
-                            set(VirtualRegister(dst), weakJSConstant(value));
-                            break;
-                        }
-                    }
+                if (JSValue value = m_graph.tryGetConstantClosureVar(scopeNode, VirtualRegister(operand))) {
+                    addToGraph(Phantom, scopeNode);
+                    set(VirtualRegister(dst), weakJSConstant(value));
+                    break;
                 }
                 SpeculatedType prediction = getPrediction();
                 set(VirtualRegister(dst),
-                    addToGraph(GetClosureVar, OpInfo(operand), OpInfo(prediction), 
-                        addToGraph(GetClosureRegisters, scopeNode)));
+                    addToGraph(
+                        GetClosureVar, OpInfo(operand), OpInfo(prediction),
+                        scopeNode, addToGraph(GetClosureRegisters, scopeNode)));
                 break;
             }
             case Dynamic:
@@ -3643,7 +3621,17 @@ bool ByteCodeParser::parseBlock(unsigned limit)
         }
             
         case op_get_scope: {
-            set(VirtualRegister(currentInstruction[1].u.operand), addToGraph(GetScope, get(VirtualRegister(JSStack::Callee))));
+            // Help the later stages a bit by doing some small constant folding here. Note that this
+            // only helps for the first basic block. It's extremely important not to constant fold
+            // loads from the scope register later, as that would prevent the DFG from tracking the
+            // bytecode-level liveness of the scope register.
+            Node* callee = get(VirtualRegister(JSStack::Callee));
+            Node* result;
+            if (JSFunction* function = callee->dynamicCastConstant<JSFunction*>())
+                result = weakJSConstant(function->scope());
+            else
+                result = addToGraph(GetScope, callee);
+            set(VirtualRegister(currentInstruction[1].u.operand), result);
             NEXT_OPCODE(op_get_scope);
         }
             
@@ -4128,22 +4116,6 @@ bool ByteCodeParser::parse()
             m_dfgCodeBlock->getStubInfoMap(m_dfgStubInfos);
     }
     
-    if (m_codeBlock->captureCount()) {
-        SymbolTable* symbolTable = m_codeBlock->symbolTable();
-        ConcurrentJITLocker locker(symbolTable->m_lock);
-        SymbolTable::Map::iterator iter = symbolTable->begin(locker);
-        SymbolTable::Map::iterator end = symbolTable->end(locker);
-        for (; iter != end; ++iter) {
-            VariableWatchpointSet* set = iter->value.watchpointSet();
-            if (!set)
-                continue;
-            size_t index = static_cast<size_t>(VirtualRegister(iter->value.getIndex()).toLocal());
-            while (m_localWatchpoints.size() <= index)
-                m_localWatchpoints.append(nullptr);
-            m_localWatchpoints[index] = set;
-        }
-    }
-    
     InlineStackEntry inlineStackEntry(
         this, m_codeBlock, m_profiledBlock, 0, 0, VirtualRegister(), VirtualRegister(),
         m_codeBlock->numParameters(), InlineCallFrame::Call);
index 286a533..c2f1635 100644 (file)
@@ -300,7 +300,6 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
         write(SideState);
         return;
 
-    case VariableWatchpoint:
     case TypedArrayWatchpoint:
         read(Watchpoint_fire);
         write(SideState);
@@ -326,10 +325,6 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
         write(Watchpoint_fire);
         return;
 
-    case FunctionReentryWatchpoint:
-        read(Watchpoint_fire);
-        return;
-
     case ToThis:
     case CreateThis:
         read(MiscFields);
@@ -786,7 +781,7 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
         
     case PutClosureVar:
         write(AbstractHeap(Variables, node->varNumber()));
-        def(HeapLocation(ClosureVariableLoc, AbstractHeap(Variables, node->varNumber()), node->child2()), node->child3().node());
+        def(HeapLocation(ClosureVariableLoc, AbstractHeap(Variables, node->varNumber()), node->child1()), node->child3().node());
         return;
         
     case GetGlobalVar:
index c823ec7..96ebe8f 100644 (file)
@@ -102,7 +102,6 @@ bool doesGC(Graph& graph, Node* node)
     case PutClosureVar:
     case GetGlobalVar:
     case PutGlobalVar:
-    case VariableWatchpoint:
     case VarInjectionWatchpoint:
     case CheckCell:
     case AllocationProfileWatchpoint:
@@ -168,7 +167,6 @@ bool doesGC(Graph& graph, Node* node)
     case StoreBarrierWithNullCheck:
     case InvalidationPoint:
     case NotifyWrite:
-    case FunctionReentryWatchpoint:
     case TypedArrayWatchpoint:
     case CheckInBounds:
     case ConstantStoragePointer:
index 625e9ea..17681dd 100644 (file)
@@ -895,6 +895,11 @@ private:
             break;
         }
 
+        case GetClosureVar: {
+            fixEdge<KnownCellUse>(node->child1());
+            break;
+        }
+
         case PutClosureVar: {
             fixEdge<KnownCellUse>(node->child1());
             insertStoreBarrier(m_indexInBlock, node->child1(), node->child3());
@@ -1226,10 +1231,8 @@ private:
         case Flush:
         case PhantomLocal:
         case GetLocalUnlinked:
-        case GetClosureVar:
         case GetGlobalVar:
         case NotifyWrite:
-        case VariableWatchpoint:
         case VarInjectionWatchpoint:
         case AllocationProfileWatchpoint:
         case Call:
@@ -1271,7 +1274,6 @@ private:
         case LoopHint:
         case StoreBarrier:
         case StoreBarrierWithNullCheck:
-        case FunctionReentryWatchpoint:
         case TypedArrayWatchpoint:
         case MovHint:
         case ZombieHint:
index a5e9e5e..4b4ffae 100644 (file)
@@ -1014,14 +1014,54 @@ JSValue Graph::tryGetConstantProperty(const AbstractValue& base, PropertyOffset
     return tryGetConstantProperty(base.m_value, base.m_structure, offset);
 }
 
-JSLexicalEnvironment* Graph::tryGetActivation(Node* node)
+JSValue Graph::tryGetConstantClosureVar(JSValue base, VirtualRegister reg)
 {
-    return node->dynamicCastConstant<JSLexicalEnvironment*>();
+    if (!base)
+        return JSValue();
+    
+    JSLexicalEnvironment* activation = jsDynamicCast<JSLexicalEnvironment*>(base);
+    if (!activation)
+        return JSValue();
+    
+    SymbolTable* symbolTable = activation->symbolTable();
+    ConcurrentJITLocker locker(symbolTable->m_lock);
+    
+    if (symbolTable->m_functionEnteredOnce.hasBeenInvalidated())
+        return JSValue();
+    
+    SymbolTableEntry* entry = symbolTable->entryFor(locker, reg);
+    if (!entry)
+        return JSValue();
+    
+    VariableWatchpointSet* set = entry->watchpointSet();
+    if (!set)
+        return JSValue();
+    
+    JSValue value = set->inferredValue();
+    if (!value)
+        return JSValue();
+    
+    watchpoints().addLazily(symbolTable->m_functionEnteredOnce);
+    watchpoints().addLazily(set);
+    
+    return value;
+}
+
+JSValue Graph::tryGetConstantClosureVar(const AbstractValue& value, VirtualRegister reg)
+{
+    return tryGetConstantClosureVar(value.m_value, reg);
+}
+
+JSValue Graph::tryGetConstantClosureVar(Node* node, VirtualRegister reg)
+{
+    if (!node->hasConstant())
+        return JSValue();
+    return tryGetConstantClosureVar(node->asJSValue(), reg);
 }
 
 WriteBarrierBase<Unknown>* Graph::tryGetRegisters(Node* node)
 {
-    JSLexicalEnvironment* lexicalEnvironment = tryGetActivation(node);
+    JSLexicalEnvironment* lexicalEnvironment = node->dynamicCastConstant<JSLexicalEnvironment*>();
     if (!lexicalEnvironment)
         return 0;
     return lexicalEnvironment->registers();
index aece1d5..a836350 100644 (file)
@@ -741,7 +741,9 @@ public:
     JSValue tryGetConstantProperty(JSValue base, const StructureAbstractValue&, PropertyOffset);
     JSValue tryGetConstantProperty(const AbstractValue&, PropertyOffset);
     
-    JSLexicalEnvironment* tryGetActivation(Node*);
+    JSValue tryGetConstantClosureVar(JSValue base, VirtualRegister);
+    JSValue tryGetConstantClosureVar(const AbstractValue&, VirtualRegister);
+    JSValue tryGetConstantClosureVar(Node*, VirtualRegister);
     WriteBarrierBase<Unknown>* tryGetRegisters(Node*);
     
     JSArrayBufferView* tryGetFoldableView(Node*);
index dafab2c..f2b2a14 100644 (file)
@@ -1203,7 +1203,7 @@ struct Node {
     
     bool hasVariableWatchpointSet()
     {
-        return op() == NotifyWrite || op() == VariableWatchpoint;
+        return op() == NotifyWrite;
     }
     
     VariableWatchpointSet* variableWatchpointSet()
@@ -1364,17 +1364,6 @@ struct Node {
         return m_opInfo;
     }
     
-    bool hasSymbolTable()
-    {
-        return op() == FunctionReentryWatchpoint;
-    }
-    
-    SymbolTable* symbolTable()
-    {
-        ASSERT(hasSymbolTable());
-        return reinterpret_cast<SymbolTable*>(m_opInfo);
-    }
-    
     bool hasArrayMode()
     {
         switch (op()) {
index 2c3cf04..35dcb54 100644 (file)
@@ -188,9 +188,7 @@ namespace JSC { namespace DFG {
     macro(GetGlobalVar, NodeResultJS) \
     macro(PutGlobalVar, NodeMustGenerate) \
     macro(NotifyWrite, NodeMustGenerate) \
-    macro(VariableWatchpoint, NodeMustGenerate) \
     macro(VarInjectionWatchpoint, NodeMustGenerate) \
-    macro(FunctionReentryWatchpoint, NodeMustGenerate) \
     macro(CheckCell, NodeMustGenerate) \
     macro(CheckBadCell, NodeMustGenerate) \
     macro(AllocationProfileWatchpoint, NodeMustGenerate) \
index 68aafea..676c987 100644 (file)
@@ -629,7 +629,6 @@ private:
         case PutStructure:
         case TearOffArguments:
         case CheckArgumentsNotCreated:
-        case VariableWatchpoint:
         case VarInjectionWatchpoint:
         case AllocationProfileWatchpoint:
         case Phantom:
@@ -639,7 +638,6 @@ private:
         case Unreachable:
         case LoopHint:
         case NotifyWrite:
-        case FunctionReentryWatchpoint:
         case TypedArrayWatchpoint:
         case ConstantStoragePointer:
         case MovHint:
index 512ddd7..e9c030d 100644 (file)
@@ -174,7 +174,6 @@ bool safeToExecute(AbstractStateType& state, Graph& graph, Node* node)
     case PutClosureVar:
     case GetGlobalVar:
     case PutGlobalVar:
-    case VariableWatchpoint:
     case VarInjectionWatchpoint:
     case CheckCell:
     case CheckBadCell:
@@ -253,7 +252,6 @@ bool safeToExecute(AbstractStateType& state, Graph& graph, Node* node)
     case StoreBarrierWithNullCheck:
     case InvalidationPoint:
     case NotifyWrite:
-    case FunctionReentryWatchpoint:
     case TypedArrayWatchpoint:
     case CheckInBounds:
     case ConstantStoragePointer:
index 648dd0a..0412b4f 100644 (file)
@@ -3609,7 +3609,9 @@ void SpeculativeJIT::compile(Node* node)
         break;
     }
     case GetClosureVar: {
-        StorageOperand registers(this, node->child1());
+        speculate(node, node->child1());
+
+        StorageOperand registers(this, node->child2());
         GPRTemporary resultTag(this);
         GPRTemporary resultPayload(this);
         GPRReg registersGPR = registers.gpr();
@@ -3621,6 +3623,8 @@ void SpeculativeJIT::compile(Node* node)
         break;
     }
     case PutClosureVar: {
+        speculate(node, node->child1());
+
         StorageOperand registers(this, node->child2());
         JSValueOperand value(this, node->child3());
         GPRTemporary scratchRegister(this);
@@ -3629,8 +3633,6 @@ void SpeculativeJIT::compile(Node* node)
         GPRReg valueTagGPR = value.tagGPR();
         GPRReg valuePayloadGPR = value.payloadGPR();
 
-        speculate(node, node->child1());
-
         m_jit.store32(valueTagGPR, JITCompiler::Address(registersGPR, node->varNumber() * sizeof(Register) + OBJECT_OFFSETOF(EncodedValueDescriptor, asBits.tag)));
         m_jit.store32(valuePayloadGPR, JITCompiler::Address(registersGPR, node->varNumber() * sizeof(Register) + OBJECT_OFFSETOF(EncodedValueDescriptor, asBits.payload)));
         noResult(node);
@@ -4034,8 +4036,7 @@ void SpeculativeJIT::compile(Node* node)
         break;
     }
 
-    case VarInjectionWatchpoint:
-    case VariableWatchpoint: {
+    case VarInjectionWatchpoint: {
         noResult(node);
         break;
     }
@@ -4312,11 +4313,6 @@ void SpeculativeJIT::compile(Node* node)
         break;
     }
         
-    case FunctionReentryWatchpoint: {
-        noResult(node);
-        break;
-    }
-        
     case CreateArguments: {
         JSValueOperand value(this, node->child1());
         GPRTemporary scratch1(this);
index d48fce1..579d3fe 100644 (file)
@@ -3711,7 +3711,9 @@ void SpeculativeJIT::compile(Node* node)
         break;
     }
     case GetClosureVar: {
-        StorageOperand registers(this, node->child1());
+        speculate(node, node->child1());
+
+        StorageOperand registers(this, node->child2());
         GPRTemporary result(this);
         GPRReg registersGPR = registers.gpr();
         GPRReg resultGPR = result.gpr();
@@ -3721,14 +3723,14 @@ void SpeculativeJIT::compile(Node* node)
         break;
     }
     case PutClosureVar: {
+        speculate(node, node->child1());
+
         StorageOperand registers(this, node->child2());
         JSValueOperand value(this, node->child3());
 
         GPRReg registersGPR = registers.gpr();
         GPRReg valueGPR = value.gpr();
 
-        speculate(node, node->child1());
-
         m_jit.store64(valueGPR, JITCompiler::Address(registersGPR, node->varNumber() * sizeof(Register)));
         noResult(node);
         break;
@@ -4087,8 +4089,7 @@ void SpeculativeJIT::compile(Node* node)
         break;
     }
 
-    case VarInjectionWatchpoint:
-    case VariableWatchpoint: {
+    case VarInjectionWatchpoint: {
         noResult(node);
         break;
     }
@@ -4367,11 +4368,6 @@ void SpeculativeJIT::compile(Node* node)
         break;
     }
         
-    case FunctionReentryWatchpoint: {
-        noResult(node);
-        break;
-    }
-        
     case CreateArguments: {
         JSValueOperand value(this, node->child1());
         GPRTemporary scratch1(this);
index 39aea40..00f3643 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013, 2014 Apple Inc. All rights reserved.
+ * Copyright (C) 2013-2015 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -117,18 +117,10 @@ private:
             addLazily(jsCast<JSFunction*>(m_node->cellOperand()->value())->allocationProfileWatchpointSet());
             break;
             
-        case VariableWatchpoint:
-            addLazily(m_node->variableWatchpointSet());
-            break;
-            
         case VarInjectionWatchpoint:
             addLazily(globalObject()->varInjectionWatchpoint());
             break;
             
-        case FunctionReentryWatchpoint:
-            addLazily(m_node->symbolTable()->m_functionEnteredOnce);
-            break;
-            
         case TypedArrayWatchpoint:
             addLazily(m_node->typedArray());
             break;
index e54e396..2f56a74 100644 (file)
@@ -112,10 +112,8 @@ inline CapabilityLevel canCompile(Node* node)
     case StringCharCodeAt:
     case AllocatePropertyStorage:
     case ReallocatePropertyStorage:
-    case FunctionReentryWatchpoint:
     case TypedArrayWatchpoint:
     case GetTypedArrayByteOffset:
-    case VariableWatchpoint:
     case NotifyWrite:
     case StoreBarrier:
     case StoreBarrierWithNullCheck:
index 2d6dadc..0428aec 100644 (file)
@@ -818,8 +818,6 @@ private:
 
         case PhantomLocal:
         case LoopHint:
-        case VariableWatchpoint:
-        case FunctionReentryWatchpoint:
         case TypedArrayWatchpoint:
         case AllocationProfileWatchpoint:
         case MovHint:
@@ -3599,7 +3597,7 @@ private:
     void compileGetClosureVar()
     {
         setJSValue(m_out.load64(
-            addressFor(lowStorage(m_node->child1()), m_node->varNumber())));
+            addressFor(lowStorage(m_node->child2()), m_node->varNumber())));
     }
     
     void compilePutClosureVar()
index 4d1f489..c40f0d5 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012, 2014 Apple Inc. All rights reserved.
+ * Copyright (C) 2012, 2014, 2015 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -117,6 +117,10 @@ void SymbolTable::visitChildren(JSCell* thisCell, SlotVisitor& visitor)
     }
     
     visitor.addUnconditionalFinalizer(thisSymbolTable->m_watchpointCleanup.get());
+    
+    // Save some memory. This is O(n) to rebuild and we do so on the fly.
+    ConcurrentJITLocker locker(thisSymbolTable->m_lock);
+    thisSymbolTable->m_localToEntry = nullptr;
 }
 
 SymbolTable::WatchpointCleanup::WatchpointCleanup(SymbolTable* symbolTable)
@@ -137,6 +141,34 @@ void SymbolTable::WatchpointCleanup::finalizeUnconditionally()
     }
 }
 
+const SymbolTable::LocalToEntryVec& SymbolTable::localToEntry(const ConcurrentJITLocker&)
+{
+    if (UNLIKELY(!m_localToEntry)) {
+        unsigned size = 0;
+        for (auto& entry : m_map) {
+            VirtualRegister reg(entry.value.getIndex());
+            if (reg.isLocal())
+                size = std::max(size, static_cast<unsigned>(reg.toLocal()) + 1);
+        }
+    
+        m_localToEntry = std::make_unique<LocalToEntryVec>(size, nullptr);
+        for (auto& entry : m_map) {
+            VirtualRegister reg(entry.value.getIndex());
+            if (reg.isLocal())
+                m_localToEntry->at(reg.toLocal()) = &entry.value;
+        }
+    }
+    
+    return *m_localToEntry;
+}
+
+SymbolTableEntry* SymbolTable::entryFor(const ConcurrentJITLocker& locker, VirtualRegister reg)
+{
+    if (!reg.isLocal())
+        return nullptr;
+    return localToEntry(locker)[reg.toLocal()];
+}
+
 SymbolTable* SymbolTable::cloneCapturedNames(VM& vm)
 {
     SymbolTable* result = SymbolTable::create(vm);
index 0487790..da3a59a 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2007, 2008, 2012-2014 Apple Inc. All rights reserved.
+ * Copyright (C) 2007, 2008, 2012-2015 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -340,6 +340,7 @@ public:
     typedef HashMap<RefPtr<StringImpl>, GlobalVariableID> UniqueIDMap;
     typedef HashMap<RefPtr<StringImpl>, RefPtr<TypeSet>> UniqueTypeSetMap;
     typedef HashMap<int, RefPtr<StringImpl>, WTF::IntHash<int>, WTF::UnsignedWithZeroKeyHashTraits<int>> RegisterToVariableMap;
+    typedef Vector<SymbolTableEntry*> LocalToEntryVec;
 
     static SymbolTable* create(VM& vm)
     {
@@ -425,6 +426,7 @@ public:
     
     Map::AddResult add(const ConcurrentJITLocker&, StringImpl* key, const SymbolTableEntry& entry)
     {
+        RELEASE_ASSERT(!m_localToEntry);
         return m_map.add(key, entry);
     }
     
@@ -436,6 +438,7 @@ public:
     
     Map::AddResult set(const ConcurrentJITLocker&, StringImpl* key, const SymbolTableEntry& entry)
     {
+        RELEASE_ASSERT(!m_localToEntry);
         return m_map.set(key, entry);
     }
     
@@ -456,6 +459,9 @@ public:
         return contains(locker, key);
     }
     
+    const LocalToEntryVec& localToEntry(const ConcurrentJITLocker&);
+    SymbolTableEntry* entryFor(const ConcurrentJITLocker&, VirtualRegister);
+    
     GlobalVariableID uniqueIDForVariable(const ConcurrentJITLocker&, StringImpl* key, VM& vm);
     GlobalVariableID uniqueIDForRegister(const ConcurrentJITLocker& locker, int registerIndex, VM& vm);
     RefPtr<TypeSet> globalTypeSetForRegister(const ConcurrentJITLocker& locker, int registerIndex, VM& vm);
@@ -529,6 +535,7 @@ private:
     std::unique_ptr<SlowArgument[]> m_slowArguments;
     
     std::unique_ptr<WatchpointCleanup> m_watchpointCleanup;
+    std::unique_ptr<LocalToEntryVec> m_localToEntry;
 
 public:
     InlineWatchpointSet m_functionEnteredOnce;
diff --git a/Source/JavaScriptCore/tests/stress/function-expression-exit.js b/Source/JavaScriptCore/tests/stress/function-expression-exit.js
new file mode 100644 (file)
index 0000000..e22c479
--- /dev/null
@@ -0,0 +1,16 @@
+function foo(x) {
+    var tmp = x + 1;
+    return function() { return 42; }
+}
+
+noInline(foo);
+
+for (var i = 0; i < 10000; ++i) {
+    var result = foo(42)();
+    if (result != 42)
+        throw "Error: bad result in loop: " + result;
+}
+
+var result = foo(42.5)();
+if (result != 42)
+    throw "Error: bad result at end: " + result;
diff --git a/Source/JavaScriptCore/tests/stress/function-reentry-infer-on-self.js b/Source/JavaScriptCore/tests/stress/function-reentry-infer-on-self.js
new file mode 100644 (file)
index 0000000..893ce91
--- /dev/null
@@ -0,0 +1,28 @@
+function thingy(f) {
+    f();
+}
+noInline(thingy);
+
+function foo(a) {
+    var x;
+    if (a)
+        x = a;
+    thingy(function() { return x; });
+    var result = 0;
+    for (var i = 0; i < 100000; ++i)
+        result += x;
+    return result;
+}
+
+noInline(foo);
+
+for (var i = 0; i < 10; ++i) {
+    var result = foo(42);
+    if (result != 4200000)
+        throw "Error: bad first result: " + result;
+}
+
+var result = foo(0);
+if ("" + result != "NaN")
+    throw "Error: bad result at end: " + result;
+
diff --git a/Source/JavaScriptCore/tests/stress/goofy-function-reentry-incorrect-inference.js b/Source/JavaScriptCore/tests/stress/goofy-function-reentry-incorrect-inference.js
new file mode 100644 (file)
index 0000000..a371347
--- /dev/null
@@ -0,0 +1,25 @@
+function foo(a) {
+    var x;
+    if (a)
+        x = a;
+    return [function() {
+        return x;
+    }, function(a) {
+        x = a;
+    }];
+}
+
+var array = foo(false);
+noInline(array[0]);
+noInline(array[1]);
+array[1](42);
+for (var i = 0; i < 10000; ++i) {
+    var result = array[0]();
+    if (result != 42)
+        throw "Error: bad result in loop: " + result;
+}
+
+array[1](43);
+var result = array[0]();
+if (result != 43)
+    throw "Error: bad result at end: " + result;