DFG should inline code blocks that use new_array_buffer
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 11 Oct 2012 19:36:04 +0000 (19:36 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 11 Oct 2012 19:36:04 +0000 (19:36 +0000)
https://bugs.webkit.org/show_bug.cgi?id=98996

Reviewed by Geoffrey Garen.

This adds plumbing to drop in constant buffers from the inlinees to the inliner.
It's smart about not duplicating buffers needlessly but doesn't try to completely
hash-cons them, either.

* bytecode/CodeBlock.h:
(JSC::CodeBlock::numberOfConstantBuffers):
(JSC::CodeBlock::addConstantBuffer):
(JSC::CodeBlock::constantBufferAsVector):
(JSC::CodeBlock::constantBuffer):
* dfg/DFGAbstractState.cpp:
(JSC::DFG::AbstractState::execute):
* dfg/DFGByteCodeParser.cpp:
(ConstantBufferKey):
(JSC::DFG::ConstantBufferKey::ConstantBufferKey):
(JSC::DFG::ConstantBufferKey::operator==):
(JSC::DFG::ConstantBufferKey::hash):
(JSC::DFG::ConstantBufferKey::isHashTableDeletedValue):
(JSC::DFG::ConstantBufferKey::codeBlock):
(JSC::DFG::ConstantBufferKey::index):
(DFG):
(JSC::DFG::ConstantBufferKeyHash::hash):
(JSC::DFG::ConstantBufferKeyHash::equal):
(ConstantBufferKeyHash):
(WTF):
(ByteCodeParser):
(InlineStackEntry):
(JSC::DFG::ByteCodeParser::parseBlock):
(JSC::DFG::ByteCodeParser::InlineStackEntry::InlineStackEntry):
* dfg/DFGCapabilities.h:
(JSC::DFG::canInlineOpcode):
* dfg/DFGOperations.cpp:
* dfg/DFGOperations.h:
* dfg/DFGSpeculativeJIT.h:
(JSC::DFG::SpeculativeJIT::callOperation):
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/CodeBlock.h
Source/JavaScriptCore/dfg/DFGAbstractState.cpp
Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
Source/JavaScriptCore/dfg/DFGCapabilities.h
Source/JavaScriptCore/dfg/DFGOperations.cpp
Source/JavaScriptCore/dfg/DFGOperations.h
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp

index ce6ef66..13237e5 100644 (file)
@@ -1,3 +1,49 @@
+2012-10-11  Filip Pizlo  <fpizlo@apple.com>
+
+        DFG should inline code blocks that use new_array_buffer
+        https://bugs.webkit.org/show_bug.cgi?id=98996
+
+        Reviewed by Geoffrey Garen.
+
+        This adds plumbing to drop in constant buffers from the inlinees to the inliner.
+        It's smart about not duplicating buffers needlessly but doesn't try to completely
+        hash-cons them, either.
+
+        * bytecode/CodeBlock.h:
+        (JSC::CodeBlock::numberOfConstantBuffers):
+        (JSC::CodeBlock::addConstantBuffer):
+        (JSC::CodeBlock::constantBufferAsVector):
+        (JSC::CodeBlock::constantBuffer):
+        * dfg/DFGAbstractState.cpp:
+        (JSC::DFG::AbstractState::execute):
+        * dfg/DFGByteCodeParser.cpp:
+        (ConstantBufferKey):
+        (JSC::DFG::ConstantBufferKey::ConstantBufferKey):
+        (JSC::DFG::ConstantBufferKey::operator==):
+        (JSC::DFG::ConstantBufferKey::hash):
+        (JSC::DFG::ConstantBufferKey::isHashTableDeletedValue):
+        (JSC::DFG::ConstantBufferKey::codeBlock):
+        (JSC::DFG::ConstantBufferKey::index):
+        (DFG):
+        (JSC::DFG::ConstantBufferKeyHash::hash):
+        (JSC::DFG::ConstantBufferKeyHash::equal):
+        (ConstantBufferKeyHash):
+        (WTF):
+        (ByteCodeParser):
+        (InlineStackEntry):
+        (JSC::DFG::ByteCodeParser::parseBlock):
+        (JSC::DFG::ByteCodeParser::InlineStackEntry::InlineStackEntry):
+        * dfg/DFGCapabilities.h:
+        (JSC::DFG::canInlineOpcode):
+        * dfg/DFGOperations.cpp:
+        * dfg/DFGOperations.h:
+        * dfg/DFGSpeculativeJIT.h:
+        (JSC::DFG::SpeculativeJIT::callOperation):
+        * dfg/DFGSpeculativeJIT32_64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+
 2012-10-10  Zoltan Horvath  <zoltan@webkit.org>
 
         Pageload tests should measure memory usage
index dc586bd..01a8ef4 100644 (file)
@@ -925,18 +925,32 @@ namespace JSC {
         }
         RegExp* regexp(int index) const { ASSERT(m_rareData); return m_rareData->m_regexps[index].get(); }
 
-        unsigned addConstantBuffer(unsigned length)
+        unsigned numberOfConstantBuffers() const
+        {
+            if (!m_rareData)
+                return 0;
+            return m_rareData->m_constantBuffers.size();
+        }
+        unsigned addConstantBuffer(const Vector<JSValue>& buffer)
         {
             createRareDataIfNecessary();
             unsigned size = m_rareData->m_constantBuffers.size();
-            m_rareData->m_constantBuffers.append(Vector<JSValue>(length));
+            m_rareData->m_constantBuffers.append(buffer);
             return size;
         }
+        unsigned addConstantBuffer(unsigned length)
+        {
+            return addConstantBuffer(Vector<JSValue>(length));
+        }
 
-        JSValue* constantBuffer(unsigned index)
+        Vector<JSValue>& constantBufferAsVector(unsigned index)
         {
             ASSERT(m_rareData);
-            return m_rareData->m_constantBuffers[index].data();
+            return m_rareData->m_constantBuffers[index];
+        }
+        JSValue* constantBuffer(unsigned index)
+        {
+            return constantBufferAsVector(index).data();
         }
 
         JSGlobalObject* globalObject() { return m_globalObject.get(); }
index 022f33a..9c610db 100644 (file)
@@ -1126,10 +1126,9 @@ bool AbstractState::execute(unsigned indexInBlock)
         break;
         
     case NewArrayBuffer:
-        // Unless we're having a bad time, this node can change its mind about what structure
-        // it uses.
-        node.setCanExit(false);
-        forNode(nodeIndex).set(SpecArray);
+        node.setCanExit(true);
+        forNode(nodeIndex).set(m_graph.globalObjectFor(node.codeOrigin)->arrayStructure());
+        m_haveStructures = true;
         break;
 
     case NewArrayWithSize:
index ac1c711..dc668d9 100644 (file)
 
 namespace JSC { namespace DFG {
 
+class ConstantBufferKey {
+public:
+    ConstantBufferKey()
+        : m_codeBlock(0)
+        , m_index(0)
+    {
+    }
+    
+    ConstantBufferKey(WTF::HashTableDeletedValueType)
+        : m_codeBlock(0)
+        , m_index(1)
+    {
+    }
+    
+    ConstantBufferKey(CodeBlock* codeBlock, unsigned index)
+        : m_codeBlock(codeBlock)
+        , m_index(index)
+    {
+    }
+    
+    bool operator==(const ConstantBufferKey& other) const
+    {
+        return m_codeBlock == other.m_codeBlock
+            && m_index == other.m_index;
+    }
+    
+    unsigned hash() const
+    {
+        return WTF::PtrHash<CodeBlock*>::hash(m_codeBlock) ^ m_index;
+    }
+    
+    bool isHashTableDeletedValue() const
+    {
+        return !m_codeBlock && m_index;
+    }
+    
+    CodeBlock* codeBlock() const { return m_codeBlock; }
+    unsigned index() const { return m_index; }
+    
+private:
+    CodeBlock* m_codeBlock;
+    unsigned m_index;
+};
+
+struct ConstantBufferKeyHash {
+    static unsigned hash(const ConstantBufferKey& key) { return key.hash(); }
+    static bool equal(const ConstantBufferKey& a, const ConstantBufferKey& b)
+    {
+        return a == b;
+    }
+    
+    static const bool safeToCompareToEmptyOrDeleted = true;
+};
+
+} } // namespace JSC::DFG
+
+namespace WTF {
+
+template<typename T> struct DefaultHash;
+template<> struct DefaultHash<JSC::DFG::ConstantBufferKey> {
+    typedef JSC::DFG::ConstantBufferKeyHash Hash;
+};
+
+template<typename T> struct HashTraits;
+template<> struct HashTraits<JSC::DFG::ConstantBufferKey> : SimpleClassHashTraits<JSC::DFG::ConstantBufferKey> { };
+
+} // namespace WTF
+
+namespace JSC { namespace DFG {
+
 // === ByteCodeParser ===
 //
 // This class is used to compile the dataflow graph from a CodeBlock.
@@ -1052,6 +1122,8 @@ private:
     Vector<PhiStackEntry, 16> m_argumentPhiStack;
     Vector<PhiStackEntry, 16> m_localPhiStack;
     
+    HashMap<ConstantBufferKey, unsigned> m_constantBufferCache;
+    
     struct InlineStackEntry {
         ByteCodeParser* m_byteCodeParser;
         
@@ -1070,6 +1142,7 @@ private:
         // direct, caller).
         Vector<unsigned> m_identifierRemap;
         Vector<unsigned> m_constantRemap;
+        Vector<unsigned> m_constantBufferRemap;
         
         // Blocks introduced by this code block, which need successor linking.
         // May include up to one basic block that includes the continuation after
@@ -1896,7 +1969,7 @@ bool ByteCodeParser::parseBlock(unsigned limit)
         case op_new_array_buffer: {
             int startConstant = currentInstruction[2].u.operand;
             int numConstants = currentInstruction[3].u.operand;
-            set(currentInstruction[1].u.operand, addToGraph(NewArrayBuffer, OpInfo(startConstant), OpInfo(numConstants)));
+            set(currentInstruction[1].u.operand, addToGraph(NewArrayBuffer, OpInfo(m_inlineStackTop->m_constantBufferRemap[startConstant]), OpInfo(numConstants)));
             NEXT_OPCODE(op_new_array_buffer);
         }
             
@@ -3251,6 +3324,7 @@ ByteCodeParser::InlineStackEntry::InlineStackEntry(
         
         m_identifierRemap.resize(codeBlock->numberOfIdentifiers());
         m_constantRemap.resize(codeBlock->numberOfConstantRegisters());
+        m_constantBufferRemap.resize(codeBlock->numberOfConstantBuffers());
 
         for (size_t i = 0; i < codeBlock->numberOfIdentifiers(); ++i) {
             StringImpl* rep = codeBlock->identifier(i).impl();
@@ -3279,6 +3353,20 @@ ByteCodeParser::InlineStackEntry::InlineStackEntry(
         }
         for (unsigned i = 0; i < codeBlock->numberOfGlobalResolveInfos(); ++i)
             byteCodeParser->m_codeBlock->addGlobalResolveInfo(std::numeric_limits<unsigned>::max());
+        for (unsigned i = 0; i < codeBlock->numberOfConstantBuffers(); ++i) {
+            // If we inline the same code block multiple times, we don't want to needlessly
+            // duplicate its constant buffers.
+            HashMap<ConstantBufferKey, unsigned>::iterator iter =
+                byteCodeParser->m_constantBufferCache.find(ConstantBufferKey(codeBlock, i));
+            if (iter != byteCodeParser->m_constantBufferCache.end()) {
+                m_constantBufferRemap[i] = iter->value;
+                continue;
+            }
+            Vector<JSValue>& buffer = codeBlock->constantBufferAsVector(i);
+            unsigned newIndex = byteCodeParser->m_codeBlock->addConstantBuffer(buffer);
+            m_constantBufferRemap[i] = newIndex;
+            byteCodeParser->m_constantBufferCache.add(ConstantBufferKey(codeBlock, i), newIndex);
+        }
         
         m_callsiteBlockHeadNeedsLinking = true;
     } else {
@@ -3294,11 +3382,14 @@ ByteCodeParser::InlineStackEntry::InlineStackEntry(
 
         m_identifierRemap.resize(codeBlock->numberOfIdentifiers());
         m_constantRemap.resize(codeBlock->numberOfConstantRegisters());
+        m_constantBufferRemap.resize(codeBlock->numberOfConstantBuffers());
 
         for (size_t i = 0; i < codeBlock->numberOfIdentifiers(); ++i)
             m_identifierRemap[i] = i;
         for (size_t i = 0; i < codeBlock->numberOfConstantRegisters(); ++i)
             m_constantRemap[i] = i + FirstConstantRegisterIndex;
+        for (size_t i = 0; i < codeBlock->numberOfConstantBuffers(); ++i)
+            m_constantBufferRemap[i] = i;
 
         m_callsiteBlockHeadNeedsLinking = false;
     }
index bccde7c..e176069 100644 (file)
@@ -201,10 +201,6 @@ inline bool canInlineOpcode(OpcodeID opcodeID, CodeBlock* codeBlock, Instruction
     case op_resolve:
     case op_resolve_base:
         
-    // Constant buffers aren't copied correctly. This is easy to fix, but for
-    // now we just disable inlining for functions that use them.
-    case op_new_array_buffer:
-        
     // Inlining doesn't correctly remap regular expression operands.
     case op_new_regexp:
         
index d2b972c..0f96759 100644 (file)
@@ -1124,11 +1124,11 @@ EncodedJSValue DFG_OPERATION operationNewArrayWithSize(ExecState* exec, Structur
     return JSValue::encode(JSArray::create(exec->globalData(), arrayStructure, size));
 }
 
-EncodedJSValue DFG_OPERATION operationNewArrayBuffer(ExecState* exec, size_t start, size_t size)
+EncodedJSValue DFG_OPERATION operationNewArrayBuffer(ExecState* exec, Structure* arrayStructure, size_t start, size_t size)
 {
     JSGlobalData& globalData = exec->globalData();
     NativeCallFrameTracer tracer(&globalData, exec);
-    return JSValue::encode(constructArray(exec, exec->codeBlock()->constantBuffer(start), size));
+    return JSValue::encode(constructArray(exec, arrayStructure, exec->codeBlock()->constantBuffer(start), size));
 }
 
 EncodedJSValue DFG_OPERATION operationNewRegexp(ExecState* exec, void* regexpPtr)
index 90eb582..8dcbcdd 100644 (file)
@@ -80,6 +80,7 @@ typedef EncodedJSValue DFG_OPERATION (*J_DFGOperation_ESS)(ExecState*, size_t, s
 typedef EncodedJSValue DFG_OPERATION (*J_DFGOperation_ESt)(ExecState*, Structure*);
 typedef EncodedJSValue DFG_OPERATION (*J_DFGOperation_EStZ)(ExecState*, Structure*, int32_t);
 typedef EncodedJSValue DFG_OPERATION (*J_DFGOperation_EStPS)(ExecState*, Structure*, void*, size_t);
+typedef EncodedJSValue DFG_OPERATION (*J_DFGOperation_EStSS)(ExecState*, Structure*, size_t, size_t);
 typedef EncodedJSValue DFG_OPERATION (*J_DFGOperation_EZ)(ExecState*, int32_t);
 typedef EncodedJSValue DFG_OPERATION (*J_DFGOperation_EZIcfZ)(ExecState*, int32_t, InlineCallFrame*, int32_t);
 typedef EncodedJSValue DFG_OPERATION (*J_DFGOperation_EZZ)(ExecState*, int32_t, int32_t);
@@ -137,7 +138,7 @@ EncodedJSValue DFG_OPERATION operationResolveGlobal(ExecState*, GlobalResolveInf
 EncodedJSValue DFG_OPERATION operationToPrimitive(ExecState*, EncodedJSValue) WTF_INTERNAL;
 EncodedJSValue DFG_OPERATION operationStrCat(ExecState*, void*, size_t) WTF_INTERNAL;
 EncodedJSValue DFG_OPERATION operationNewArray(ExecState*, Structure*, void*, size_t) WTF_INTERNAL;
-EncodedJSValue DFG_OPERATION operationNewArrayBuffer(ExecState*, size_t, size_t) WTF_INTERNAL;
+EncodedJSValue DFG_OPERATION operationNewArrayBuffer(ExecState*, Structure*, size_t, size_t) WTF_INTERNAL;
 EncodedJSValue DFG_OPERATION operationNewEmptyArray(ExecState*, Structure*) WTF_INTERNAL;
 EncodedJSValue DFG_OPERATION operationNewArrayWithSize(ExecState*, Structure*, int32_t) WTF_INTERNAL;
 EncodedJSValue DFG_OPERATION operationNewRegexp(ExecState*, void*) WTF_INTERNAL;
index f2ce6a6..ce33543 100644 (file)
@@ -1239,6 +1239,11 @@ public:
         m_jit.setupArgumentsWithExecState(TrustedImmPtr(structure), TrustedImmPtr(pointer), TrustedImmPtr(size));
         return appendCallWithExceptionCheckSetResult(operation, result);
     }
+    JITCompiler::Call callOperation(J_DFGOperation_EStSS operation, GPRReg result, Structure* structure, size_t index, size_t size)
+    {
+        m_jit.setupArgumentsWithExecState(TrustedImmPtr(structure), TrustedImmPtr(index), TrustedImmPtr(size));
+        return appendCallWithExceptionCheckSetResult(operation, result);
+    }
     JITCompiler::Call callOperation(J_DFGOperation_EPS operation, GPRReg result, void* pointer, size_t size)
     {
         m_jit.setupArgumentsWithExecState(TrustedImmPtr(pointer), TrustedImmPtr(size));
@@ -1562,6 +1567,11 @@ public:
         m_jit.setupArgumentsWithExecState(TrustedImmPtr(structure), TrustedImmPtr(pointer), TrustedImmPtr(size));
         return appendCallWithExceptionCheckSetResult(operation, resultPayload, resultTag);
     }
+    JITCompiler::Call callOperation(J_DFGOperation_EStSS operation, GPRReg resultTag, GPRReg resultPayload, Structure* structure, size_t index, size_t size)
+    {
+        m_jit.setupArgumentsWithExecState(TrustedImmPtr(structure), TrustedImmPtr(index), TrustedImmPtr(size));
+        return appendCallWithExceptionCheckSetResult(operation, resultPayload, resultTag);
+    }
     JITCompiler::Call callOperation(J_DFGOperation_EPS operation, GPRReg resultTag, GPRReg resultPayload, void* pointer, size_t size)
     {
         m_jit.setupArgumentsWithExecState(TrustedImmPtr(pointer), TrustedImmPtr(size));
index 8183f78..bd856ec 100644 (file)
@@ -3585,11 +3585,15 @@ void SpeculativeJIT::compile(Node& node)
     }
         
     case NewArrayBuffer: {
+        JSGlobalObject* globalObject = m_jit.graph().globalObjectFor(node.codeOrigin);
+        if (!globalObject->isHavingABadTime())
+            globalObject->havingABadTimeWatchpoint()->add(speculationWatchpoint());
+        
         flushRegisters();
         GPRResult resultPayload(this);
         GPRResult2 resultTag(this);
         
-        callOperation(operationNewArrayBuffer, resultTag.gpr(), resultPayload.gpr(), node.startConstant(), node.numConstants());
+        callOperation(operationNewArrayBuffer, resultTag.gpr(), resultPayload.gpr(), globalObject->arrayStructure(), node.startConstant(), node.numConstants());
         
         // FIXME: make the callOperation above explicitly return a cell result, or jitAssert the tag is a cell tag.
         cellResult(resultPayload.gpr(), m_compileIndex);
index 5af5066..0b1e14c 100644 (file)
@@ -3591,10 +3591,14 @@ void SpeculativeJIT::compile(Node& node)
     }
         
     case NewArrayBuffer: {
+        JSGlobalObject* globalObject = m_jit.graph().globalObjectFor(node.codeOrigin);
+        if (!globalObject->isHavingABadTime())
+            globalObject->havingABadTimeWatchpoint()->add(speculationWatchpoint());
+        
         flushRegisters();
         GPRResult result(this);
         
-        callOperation(operationNewArrayBuffer, result.gpr(), node.startConstant(), node.numConstants());
+        callOperation(operationNewArrayBuffer, result.gpr(), globalObject->arrayStructure(), node.startConstant(), node.numConstants());
         
         cellResult(result.gpr(), m_compileIndex);
         break;