The DFG should not attempt to guess types in the absence of value
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 24 Sep 2011 22:39:16 +0000 (22:39 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 24 Sep 2011 22:39:16 +0000 (22:39 +0000)
profiles
https://bugs.webkit.org/show_bug.cgi?id=68677

Reviewed by Oliver Hunt.

This adds the ForceOSRExit node, which is ignored by the propagator
and virtual register allocator (and hence ensuring that liveness analysis
works correctly), but forces terminateSpeculativeExecution() in the
back-end. This appears to be a slight speed-up on benchmark averages,
with ~5% swings on individual benchmarks, in both directions. But it's
never a regression on any average, and appears to be a ~1% progression
in the SunSpider average.

This also adds a bit better debugging support in the old JIT and in DFG,
as this was necessary to debug the much more frequent OSR transitions
that occur with this change.

* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::addCall):
(JSC::DFG::ByteCodeParser::getStrongPrediction):
(JSC::DFG::ByteCodeParser::parseBlock):
* dfg/DFGJITCompiler.cpp:
(JSC::DFG::JITCompiler::exitSpeculativeWithOSR):
* dfg/DFGNode.h:
* dfg/DFGPropagator.cpp:
(JSC::DFG::Propagator::propagateNodePredictions):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* jit/JIT.cpp:
(JSC::JIT::privateCompileMainPass):
(JSC::JIT::privateCompileSlowCases):
(JSC::JIT::privateCompile):
* jit/JIT.h:

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
Source/JavaScriptCore/dfg/DFGJITCompiler.cpp
Source/JavaScriptCore/dfg/DFGNode.h
Source/JavaScriptCore/dfg/DFGPropagator.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
Source/JavaScriptCore/jit/JIT.cpp
Source/JavaScriptCore/jit/JIT.h

index 5dc29dc..a355d49 100644 (file)
@@ -1,3 +1,40 @@
+2011-09-24  Filip Pizlo  <fpizlo@apple.com>
+
+        The DFG should not attempt to guess types in the absence of value
+        profiles
+        https://bugs.webkit.org/show_bug.cgi?id=68677
+
+        Reviewed by Oliver Hunt.
+        
+        This adds the ForceOSRExit node, which is ignored by the propagator
+        and virtual register allocator (and hence ensuring that liveness analysis
+        works correctly), but forces terminateSpeculativeExecution() in the
+        back-end. This appears to be a slight speed-up on benchmark averages,
+        with ~5% swings on individual benchmarks, in both directions. But it's
+        never a regression on any average, and appears to be a ~1% progression
+        in the SunSpider average.
+        
+        This also adds a bit better debugging support in the old JIT and in DFG,
+        as this was necessary to debug the much more frequent OSR transitions
+        that occur with this change.
+
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::addCall):
+        (JSC::DFG::ByteCodeParser::getStrongPrediction):
+        (JSC::DFG::ByteCodeParser::parseBlock):
+        * dfg/DFGJITCompiler.cpp:
+        (JSC::DFG::JITCompiler::exitSpeculativeWithOSR):
+        * dfg/DFGNode.h:
+        * dfg/DFGPropagator.cpp:
+        (JSC::DFG::Propagator::propagateNodePredictions):
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * jit/JIT.cpp:
+        (JSC::JIT::privateCompileMainPass):
+        (JSC::JIT::privateCompileSlowCases):
+        (JSC::JIT::privateCompile):
+        * jit/JIT.h:
+
 2011-09-24  Geoffrey Garen  <ggaren@apple.com>
 
         Some Windows build fixage.
index 0966221..823fa8d 100644 (file)
@@ -432,18 +432,21 @@ private:
     
     NodeIndex addCall(Interpreter* interpreter, Instruction* currentInstruction, NodeType op)
     {
+        Instruction* putInstruction = currentInstruction + OPCODE_LENGTH(op_call);
+
+        PredictedType prediction = PredictNone;
+        if (interpreter->getOpcodeID(putInstruction->u.opcode) == op_call_put_result)
+            prediction = getStrongPrediction(m_graph.size(), m_currentIndex + OPCODE_LENGTH(op_call));
+        
         addVarArgChild(get(currentInstruction[1].u.operand));
         int argCount = currentInstruction[2].u.operand;
         int registerOffset = currentInstruction[3].u.operand;
         int firstArg = registerOffset - argCount - RegisterFile::CallFrameHeaderSize;
         for (int argIdx = firstArg; argIdx < firstArg + argCount; argIdx++)
             addVarArgChild(get(argIdx));
-        NodeIndex call = addToGraph(Node::VarArg, op, OpInfo(0), OpInfo(PredictNone));
-        Instruction* putInstruction = currentInstruction + OPCODE_LENGTH(op_call);
-        if (interpreter->getOpcodeID(putInstruction->u.opcode) == op_call_put_result) {
+        NodeIndex call = addToGraph(Node::VarArg, op, OpInfo(0), OpInfo(prediction));
+        if (interpreter->getOpcodeID(putInstruction->u.opcode) == op_call_put_result)
             set(putInstruction[1].u.operand, call);
-            stronglyPredict(call, m_currentIndex + OPCODE_LENGTH(op_call));
-        }
         if (RegisterFile::CallFrameHeaderSize + (unsigned)argCount > m_parameterSlots)
             m_parameterSlots = RegisterFile::CallFrameHeaderSize + argCount;
         return call;
@@ -487,7 +490,6 @@ private:
     PredictedType getStrongPrediction(NodeIndex nodeIndex, unsigned bytecodeIndex)
     {
         UNUSED_PARAM(nodeIndex);
-        UNUSED_PARAM(bytecodeIndex);
         
         ValueProfile* profile = m_profiledBlock->valueProfileForBytecodeOffset(bytecodeIndex);
         ASSERT(profile);
@@ -495,19 +497,21 @@ private:
 #if ENABLE(DFG_DEBUG_VERBOSE)
         printf("Dynamic [@%u, bc#%u] prediction: %s\n", nodeIndex, bytecodeIndex, predictionToString(prediction));
 #endif
+        
+        if (prediction == PredictNone) {
+            // We have no information about what values this node generates. Give up
+            // on executing this code, since we're likely to do more damage than good.
+            addToGraph(ForceOSRExit);
+        }
+        
         return prediction;
     }
     
-    void stronglyPredict(NodeIndex nodeIndex, unsigned bytecodeIndex)
-    {
-        m_graph[nodeIndex].predict(getStrongPrediction(nodeIndex, bytecodeIndex) & ~PredictionTagMask, StrongPrediction);
-    }
-    
-    void stronglyPredict(NodeIndex nodeIndex)
+    PredictedType getStrongPrediction()
     {
-        stronglyPredict(nodeIndex, m_currentIndex);
+        return getStrongPrediction(m_graph.size(), m_currentIndex);
     }
-    
+
     NodeIndex makeSafe(NodeIndex nodeIndex)
     {
         if (!m_profiledBlock->likelyToTakeSlowCase(m_currentIndex))
@@ -1058,14 +1062,15 @@ bool ByteCodeParser::parseBlock(unsigned limit)
         // === Property access operations ===
 
         case op_get_by_val: {
+            PredictedType prediction = getStrongPrediction();
+            
             NodeIndex base = get(currentInstruction[2].u.operand);
             NodeIndex property = get(currentInstruction[3].u.operand);
             weaklyPredictArray(base);
             weaklyPredictInt32(property);
 
-            NodeIndex getByVal = addToGraph(GetByVal, OpInfo(0), OpInfo(PredictNone), base, property);
+            NodeIndex getByVal = addToGraph(GetByVal, OpInfo(0), OpInfo(prediction), base, property);
             set(currentInstruction[1].u.operand, getByVal);
-            stronglyPredict(getByVal);
 
             NEXT_OPCODE(op_get_by_val);
         }
@@ -1085,6 +1090,8 @@ bool ByteCodeParser::parseBlock(unsigned limit)
         case op_method_check: {
             Instruction* getInstruction = currentInstruction + OPCODE_LENGTH(op_method_check);
             
+            PredictedType prediction = getStrongPrediction();
+            
             ASSERT(interpreter->getOpcodeID(getInstruction->u.opcode) == op_get_by_id);
             
             NodeIndex base = get(getInstruction[2].u.operand);
@@ -1109,22 +1116,21 @@ bool ByteCodeParser::parseBlock(unsigned limit)
                 methodCheckData.prototype = methodCall.cachedPrototype.get();
                 m_graph.m_methodCheckData.append(methodCheckData);
             } else {
-                NodeIndex getMethod = addToGraph(GetMethod, OpInfo(identifier), OpInfo(PredictNone), base);
+                NodeIndex getMethod = addToGraph(GetMethod, OpInfo(identifier), OpInfo(prediction), base);
                 set(getInstruction[1].u.operand, getMethod);
-                stronglyPredict(getMethod);
             }
             
             m_currentIndex += OPCODE_LENGTH(op_method_check) + OPCODE_LENGTH(op_get_by_id);
             continue;
         }
         case op_get_scoped_var: {
+            PredictedType prediction = getStrongPrediction();
             int dst = currentInstruction[1].u.operand;
             int slot = currentInstruction[2].u.operand;
             int depth = currentInstruction[3].u.operand;
             NodeIndex getScopeChain = addToGraph(GetScopeChain, OpInfo(depth));
-            NodeIndex getScopedVar = addToGraph(GetScopedVar, OpInfo(slot), OpInfo(PredictNone), getScopeChain);
+            NodeIndex getScopedVar = addToGraph(GetScopedVar, OpInfo(slot), OpInfo(prediction), getScopeChain);
             set(dst, getScopedVar);
-            stronglyPredict(getScopedVar);
             NEXT_OPCODE(op_get_scoped_var);
         }
         case op_put_scoped_var: {
@@ -1136,6 +1142,8 @@ bool ByteCodeParser::parseBlock(unsigned limit)
             NEXT_OPCODE(op_put_scoped_var);
         }
         case op_get_by_id: {
+            PredictedType prediction = getStrongPrediction();
+            
             NodeIndex base = get(currentInstruction[2].u.operand);
             unsigned identifierNumber = currentInstruction[3].u.operand;
             
@@ -1148,7 +1156,7 @@ bool ByteCodeParser::parseBlock(unsigned limit)
                 size_t offset = structure->get(*m_globalData, identifier);
                 
                 if (offset != notFound) {
-                    getById = addToGraph(GetByOffset, OpInfo(m_graph.m_storageAccessData.size()), OpInfo(PredictNone), addToGraph(CheckStructure, OpInfo(structure), base));
+                    getById = addToGraph(GetByOffset, OpInfo(m_graph.m_storageAccessData.size()), OpInfo(prediction), addToGraph(CheckStructure, OpInfo(structure), base));
                     
                     StorageAccessData storageAccessData;
                     storageAccessData.offset = offset;
@@ -1158,10 +1166,9 @@ bool ByteCodeParser::parseBlock(unsigned limit)
             }
             
             if (getById == NoNode)
-                getById = addToGraph(GetById, OpInfo(identifierNumber), OpInfo(PredictNone), base);
+                getById = addToGraph(GetById, OpInfo(identifierNumber), OpInfo(prediction), base);
             
             set(currentInstruction[1].u.operand, getById);
-            stronglyPredict(getById);
 
             NEXT_OPCODE(op_get_by_id);
         }
@@ -1181,9 +1188,11 @@ bool ByteCodeParser::parseBlock(unsigned limit)
         }
 
         case op_get_global_var: {
+            PredictedType prediction = getStrongPrediction();
+            
             NodeIndex getGlobalVar = addToGraph(GetGlobalVar, OpInfo(currentInstruction[2].u.operand));
             set(currentInstruction[1].u.operand, getGlobalVar);
-            m_graph.predictGlobalVar(currentInstruction[2].u.operand, getStrongPrediction(getGlobalVar, m_currentIndex) & ~PredictionTagMask, StrongPrediction);
+            m_graph.predictGlobalVar(currentInstruction[2].u.operand, prediction & ~PredictionTagMask, StrongPrediction);
             NEXT_OPCODE(op_get_global_var);
         }
 
@@ -1415,33 +1424,36 @@ bool ByteCodeParser::parseBlock(unsigned limit)
             NEXT_OPCODE(op_call_put_result);
 
         case op_resolve: {
+            PredictedType prediction = getStrongPrediction();
+            
             unsigned identifier = currentInstruction[2].u.operand;
 
-            NodeIndex resolve = addToGraph(Resolve, OpInfo(identifier), OpInfo(PredictNone));
+            NodeIndex resolve = addToGraph(Resolve, OpInfo(identifier), OpInfo(prediction));
             set(currentInstruction[1].u.operand, resolve);
-            stronglyPredict(resolve);
 
             NEXT_OPCODE(op_resolve);
         }
 
         case op_resolve_base: {
+            PredictedType prediction = getStrongPrediction();
+            
             unsigned identifier = currentInstruction[2].u.operand;
 
-            NodeIndex resolve = addToGraph(currentInstruction[3].u.operand ? ResolveBaseStrictPut : ResolveBase, OpInfo(identifier), OpInfo(PredictNone));
+            NodeIndex resolve = addToGraph(currentInstruction[3].u.operand ? ResolveBaseStrictPut : ResolveBase, OpInfo(identifier), OpInfo(prediction));
             set(currentInstruction[1].u.operand, resolve);
-            stronglyPredict(resolve);
 
             NEXT_OPCODE(op_resolve_base);
         }
             
         case op_resolve_global: {
-            NodeIndex resolve = addToGraph(ResolveGlobal, OpInfo(m_graph.m_resolveGlobalData.size()), OpInfo(PredictNone));
+            PredictedType prediction = getStrongPrediction();
+            
+            NodeIndex resolve = addToGraph(ResolveGlobal, OpInfo(m_graph.m_resolveGlobalData.size()), OpInfo(prediction));
             m_graph.m_resolveGlobalData.append(ResolveGlobalData());
             ResolveGlobalData& data = m_graph.m_resolveGlobalData.last();
             data.identifierNumber = currentInstruction[2].u.operand;
             data.resolveInfoIndex = m_globalResolveNumber++;
             set(currentInstruction[1].u.operand, resolve);
-            stronglyPredict(resolve);
 
             NEXT_OPCODE(op_resolve_global);
         }
index fa541eb..519893d 100644 (file)
@@ -109,10 +109,6 @@ void JITCompiler::exitSpeculativeWithOSR(const OSRExit& exit, SpeculationRecover
     fprintf(stderr, "OSR exit for Node @%d (bc#%u) at JIT offset 0x%x   ", (int)exit.m_nodeIndex, exit.m_bytecodeIndex, debugOffset());
     exit.dump(stderr);
 #endif
-#if ENABLE(DFG_JIT_BREAK_ON_SPECULATION_FAILURE)
-    breakpoint();
-#endif
-    
 #if ENABLE(DFG_VERBOSE_SPECULATION_FAILURE)
     SpeculationFailureDebugInfo* debugInfo = new SpeculationFailureDebugInfo;
     debugInfo->codeBlock = m_codeBlock;
@@ -121,6 +117,10 @@ void JITCompiler::exitSpeculativeWithOSR(const OSRExit& exit, SpeculationRecover
     debugCall(debugOperationPrintSpeculationFailure, debugInfo);
 #endif
     
+#if ENABLE(DFG_JIT_BREAK_ON_SPECULATION_FAILURE)
+    breakpoint();
+#endif
+    
 #if ENABLE(DFG_SUCCESS_STATS)
     static SamplingCounter counter("SpeculationFailure");
     emitCount(counter);
index 6de0dfe..1bc57b8 100644 (file)
@@ -305,7 +305,12 @@ static inline const char* arithNodeFlagsAsString(ArithNodeFlags flags)
     macro(Branch, NodeMustGenerate | NodeIsTerminal | NodeIsBranch) \
     macro(Return, NodeMustGenerate | NodeIsTerminal) \
     macro(Throw, NodeMustGenerate | NodeIsTerminal) \
-    macro(ThrowReferenceError, NodeMustGenerate | NodeIsTerminal)
+    macro(ThrowReferenceError, NodeMustGenerate | NodeIsTerminal) \
+    \
+    /* This is a pseudo-terminal. It means that execution should fall out of DFG at */\
+    /* this point, but execution does continue in the basic block - just in a */\
+    /* different compiler. */\
+    macro(ForceOSRExit, NodeMustGenerate)
 
 // This enum generates a monotonically increasing id for all Node types,
 // and is used by the subsequent enum to fill out the id (as accessed via the NodeIdMask).
index 48c5a85..1a4963d 100644 (file)
@@ -606,6 +606,7 @@ private:
         case Phi:
         case Throw:
         case ThrowReferenceError:
+        case ForceOSRExit:
             break;
             
         // This gets ignored because it doesn't do anything.
index 4dbc74a..6158ef2 100644 (file)
@@ -2089,6 +2089,11 @@ void SpeculativeJIT::compile(Node& node)
         jsValueResult(resultGPR, m_compileIndex);
         break;
     }
+        
+    case ForceOSRExit: {
+        terminateSpeculativeExecution();
+        break;
+    }
 
     case Phantom:
         // This is a no-op.
index 309a0c6..aa2effb 100644 (file)
@@ -211,6 +211,10 @@ void JIT::privateCompileMainPass()
         if (m_canBeOptimized)
             m_jitCodeMapEncoder.append(m_bytecodeOffset, differenceBetween(m_startOfCode, label()));
 #endif
+        
+#if ENABLE(JIT_VERBOSE)
+        printf("Old JIT emitting code for bc#%u at offset 0x%lx.\n", m_bytecodeOffset, differenceBetween(m_startOfCode, label()));
+#endif
 
         switch (m_interpreter->getOpcodeID(currentInstruction->u.opcode)) {
         DEFINE_BINARY_OP(op_del_by_val)
@@ -420,6 +424,10 @@ void JIT::privateCompileSlowCases()
             rareCaseProfile = m_codeBlock->addRareCaseProfile(m_bytecodeOffset);
 #endif
 
+#if ENABLE(JIT_VERBOSE)
+        printf("Old JIT emitting slow code for bc#%u at offset 0x%lx.\n", m_bytecodeOffset, differenceBetween(m_startOfCode, label()));
+#endif
+
         switch (m_interpreter->getOpcodeID(currentInstruction->u.opcode)) {
         DEFINE_SLOWCASE_OP(op_add)
         DEFINE_SLOWCASE_OP(op_bitand)
@@ -671,7 +679,13 @@ JITCode JIT::privateCompile(CodePtr* functionEntryArityCheck)
     if (m_codeBlock->codeType() == FunctionCode && functionEntryArityCheck)
         *functionEntryArityCheck = patchBuffer.locationOf(arityCheck);
     
-    return JITCode(patchBuffer.finalizeCode(), JITCode::BaselineJIT);
+    CodeRef result = patchBuffer.finalizeCode();
+    
+#if ENABLE(JIT_VERBOSE)
+    printf("JIT generated code for %p at [%p, %p).\n", m_codeBlock, result.executableMemory()->start(), result.executableMemory()->end());
+#endif
+    
+    return JITCode(result, JITCode::BaselineJIT);
 }
 
 void JIT::linkFor(JSFunction* callee, CodeBlock* callerCodeBlock, CodeBlock* calleeCodeBlock, JIT::CodePtr code, CallLinkInfo* callLinkInfo, int callerArgCount, JSGlobalData* globalData, CodeSpecializationKind kind)
index a824f3e..97c3de3 100644 (file)
@@ -28,6 +28,8 @@
 
 #if ENABLE(JIT)
 
+// Verbose logging of code generation
+#define ENABLE_JIT_VERBOSE 0
 // Verbose logging for OSR-related code.
 #define ENABLE_JIT_VERBOSE_OSR 0