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 bd2eaf89c1e6a4d647c9000f795c7a2bf1cb8957..afd3899133fee0c484894a850490bd2eab17de3f 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 1495c97b2fe2b71b307ad45533a8861f3950194b..90f98aab91d32df467670c67c32fdc29d7906de0 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 fce954fcd91c7c31da11214cc231da039e3122b4..04e66fafaf15c1bb569597113e97b0ccee30c856 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 286a533b31027fe780e2719eb1e13f902963580e..c2f163504ba8796c81027a1510cfc831d555b9f7 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 c823ec7758d28fa94de8082dd79fe884f2a87df7..96ebe8f63b8a1d8b17c6f74f7060b6e49daa3a33 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 625e9ea8c5aaf74d4cfdef8ef6adbbec861b1d4c..17681ddb6f96292ad6266bd6b427753a98d48182 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 a5e9e5e5b1c41a8b50d4e3805790e7221700fd11..4b4ffae633066555b4e2558f519b2a515456c3ac 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 aece1d520eff14eb942b822f5f2eeff99141ac9d..a83635039ecd64112832d073c2f86a7194caf849 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 dafab2cb3c65c6f44ffaa5f1e2090f7eeed454aa..f2b2a14c8b237624c832111274e8a6f0bacc9636 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 2c3cf04766311bc39dd26b68283fbe053a6cef12..35dcb54789cb860616e32d19c1cfc2699404a944 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 68aafeac9976816a14f24a07e107abc11a009c43..676c987eb8d5e832a593352b6883d7602a630e63 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 512ddd7e4c688497f4e0128bceaade806496408b..e9c030d4e772753ef13a3036f6678e3c29bbf737 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 648dd0a85815f48396f1d65348895a540850d49a..0412b4f9581570669fabd8ac738d1ecd95ce92d4 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 d48fce18a7a39707247540507c9e9a4407f273a9..579d3fe0a50ce4eb8264205f14b99bd088867f77 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 39aea40f5c37f6d7e0f97786987fab12867245b8..00f3643f62cf78d0461ccec5ab25860ada00206e 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 e54e396752d95adbc36c94251ece49b7fcfbf0af..2f56a7400b65b20eeb71c5e3516fbe025304de26 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 2d6dadcbcf2fb8731e87a24b8867653d68611918..0428aec3a596f6156d1d7b52f432c7c0592cecf3 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 4d1f489df425fb9bdb7ad9c6f842e8e3ffa44bb3..c40f0d5ad8adfd20ec8fc2f57b19b2bf090c0f83 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 0487790094b0206dcb64bb244a55a330d217aea4..da3a59a04e3015c14a976e25ed88eb606a0dacd1 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;