DFG IR should refer to FunctionExecutables directly and not via the CodeBlock
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Mar 2015 06:55:52 +0000 (06:55 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Mar 2015 06:55:52 +0000 (06:55 +0000)
https://bugs.webkit.org/show_bug.cgi?id=142229

Reviewed by Mark Lam and Benjamin Poulain.

Anytime a DFG IR node refers to something in CodeBlock, it has three effects:

- Cumbersome API for accessing the thing that the node refers to.

- Not obvious how to create a new such node after bytecode parsing, especially if the
  thing it refers to isn't already in the CodeBlock. We have done this in the past, but
  it usually involves subtle changes to CodeBlock.

- Not obvious how to inline code that ends up using such nodes. Again, when we have done
  this, it involved subtle changes to CodeBlock.

Prior to this change, the NewFunction* node types used an index into tables in CodeBlock.
For this reason, those operations were not inlineable. But the functin tables in CodeBlock
just point to FunctionExecutables, which are cells; this means that we can just abstract
these operands in DFG IR as cellOperands. cellOperands use DFG::FrozenValue, which means
that GC registration happens automagically. Even better, our dumping for cellOperand
already did FunctionExecutable dumping - so that functionality gets to be deduplicated.

Because this change increases the number of users of cellOperand, it also adds some
convenience methods for using it. For example, whereas before you'd say things like:

    jsCast<Foo*>(node->cellOperand()->value())

you can now just say:

    node->castOperand<Foo*>()

This change also changes existing cellOperand users to use the new conveniance API when
applicable.

* bytecode/CodeBlock.cpp:
(JSC::CodeBlock::jettisonFunctionDeclsAndExprs):
* bytecode/CodeBlock.h:
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::parseBlock):
* dfg/DFGCapabilities.cpp:
(JSC::DFG::capabilityLevel):
* dfg/DFGFrozenValue.h:
(JSC::DFG::FrozenValue::cell):
(JSC::DFG::FrozenValue::dynamicCast):
(JSC::DFG::FrozenValue::cast):
* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::dump):
(JSC::DFG::Graph::registerFrozenValues):
* dfg/DFGNode.h:
(JSC::DFG::Node::hasCellOperand):
(JSC::DFG::Node::castOperand):
(JSC::DFG::Node::hasFunctionDeclIndex): Deleted.
(JSC::DFG::Node::functionDeclIndex): Deleted.
(JSC::DFG::Node::hasFunctionExprIndex): Deleted.
(JSC::DFG::Node::functionExprIndex): Deleted.
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileNewFunctionNoCheck):
(JSC::DFG::SpeculativeJIT::compileNewFunctionExpression):
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGWatchpointCollectionPhase.cpp:
(JSC::DFG::WatchpointCollectionPhase::handle):
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::LowerDFGToLLVM::compileCheckCell):
(JSC::FTL::LowerDFGToLLVM::compileNativeCallOrConstruct):

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

13 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/CodeBlock.cpp
Source/JavaScriptCore/bytecode/CodeBlock.h
Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
Source/JavaScriptCore/dfg/DFGCapabilities.cpp
Source/JavaScriptCore/dfg/DFGFrozenValue.h
Source/JavaScriptCore/dfg/DFGGraph.cpp
Source/JavaScriptCore/dfg/DFGNode.h
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
Source/JavaScriptCore/dfg/DFGWatchpointCollectionPhase.cpp
Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp

index 2450ceb..3ad7462 100644 (file)
@@ -1,3 +1,74 @@
+2015-03-03  Filip Pizlo  <fpizlo@apple.com>
+
+        DFG IR should refer to FunctionExecutables directly and not via the CodeBlock
+        https://bugs.webkit.org/show_bug.cgi?id=142229
+
+        Reviewed by Mark Lam and Benjamin Poulain.
+        
+        Anytime a DFG IR node refers to something in CodeBlock, it has three effects:
+
+        - Cumbersome API for accessing the thing that the node refers to.
+        
+        - Not obvious how to create a new such node after bytecode parsing, especially if the
+          thing it refers to isn't already in the CodeBlock. We have done this in the past, but
+          it usually involves subtle changes to CodeBlock.
+        
+        - Not obvious how to inline code that ends up using such nodes. Again, when we have done
+          this, it involved subtle changes to CodeBlock.
+        
+        Prior to this change, the NewFunction* node types used an index into tables in CodeBlock.
+        For this reason, those operations were not inlineable. But the functin tables in CodeBlock
+        just point to FunctionExecutables, which are cells; this means that we can just abstract
+        these operands in DFG IR as cellOperands. cellOperands use DFG::FrozenValue, which means
+        that GC registration happens automagically. Even better, our dumping for cellOperand
+        already did FunctionExecutable dumping - so that functionality gets to be deduplicated.
+        
+        Because this change increases the number of users of cellOperand, it also adds some
+        convenience methods for using it. For example, whereas before you'd say things like:
+        
+            jsCast<Foo*>(node->cellOperand()->value())
+        
+        you can now just say:
+        
+            node->castOperand<Foo*>()
+        
+        This change also changes existing cellOperand users to use the new conveniance API when
+        applicable.
+
+        * bytecode/CodeBlock.cpp:
+        (JSC::CodeBlock::jettisonFunctionDeclsAndExprs):
+        * bytecode/CodeBlock.h:
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::parseBlock):
+        * dfg/DFGCapabilities.cpp:
+        (JSC::DFG::capabilityLevel):
+        * dfg/DFGFrozenValue.h:
+        (JSC::DFG::FrozenValue::cell):
+        (JSC::DFG::FrozenValue::dynamicCast):
+        (JSC::DFG::FrozenValue::cast):
+        * dfg/DFGGraph.cpp:
+        (JSC::DFG::Graph::dump):
+        (JSC::DFG::Graph::registerFrozenValues):
+        * dfg/DFGNode.h:
+        (JSC::DFG::Node::hasCellOperand):
+        (JSC::DFG::Node::castOperand):
+        (JSC::DFG::Node::hasFunctionDeclIndex): Deleted.
+        (JSC::DFG::Node::functionDeclIndex): Deleted.
+        (JSC::DFG::Node::hasFunctionExprIndex): Deleted.
+        (JSC::DFG::Node::functionExprIndex): Deleted.
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileNewFunctionNoCheck):
+        (JSC::DFG::SpeculativeJIT::compileNewFunctionExpression):
+        * dfg/DFGSpeculativeJIT32_64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGWatchpointCollectionPhase.cpp:
+        (JSC::DFG::WatchpointCollectionPhase::handle):
+        * ftl/FTLLowerDFGToLLVM.cpp:
+        (JSC::FTL::LowerDFGToLLVM::compileCheckCell):
+        (JSC::FTL::LowerDFGToLLVM::compileNativeCallOrConstruct):
+
 2015-03-03  Michael Saboff  <msaboff@apple.com>
 
         DelayedReleaseScope drops locks during GC which can cause a thread switch and code reentry
index 7c220b1..ff70b72 100644 (file)
@@ -3008,6 +3008,12 @@ bool CodeBlock::findConstant(JSValue v, unsigned& index)
     return false;
 }
 
+void CodeBlock::jettisonFunctionDeclsAndExprs()
+{
+    m_functionDecls.clear();
+    m_functionExprs.clear();
+}
+
 #if ENABLE(JIT)
 void CodeBlock::unlinkCalls()
 {
index 61ab3bd..7763bb9 100644 (file)
@@ -657,6 +657,8 @@ public:
     FunctionExecutable* functionDecl(int index) { return m_functionDecls[index].get(); }
     int numberOfFunctionDecls() { return m_functionDecls.size(); }
     FunctionExecutable* functionExpr(int index) { return m_functionExprs[index].get(); }
+    
+    void jettisonFunctionDeclsAndExprs();
 
     RegExp* regexp(int index) const { return m_unlinkedCode->regexp(index); }
 
index 04e66fa..443eb13 100644 (file)
@@ -3665,22 +3665,26 @@ bool ByteCodeParser::parseBlock(unsigned limit)
         }
             
         case op_new_func: {
+            FunctionExecutable* decl = m_inlineStackTop->m_profiledBlock->functionDecl(currentInstruction[3].u.operand);
+            FrozenValue* frozen = m_graph.freezeStrong(decl);
             if (!currentInstruction[4].u.operand) {
                 set(VirtualRegister(currentInstruction[1].u.operand),
-                    addToGraph(NewFunctionNoCheck, OpInfo(currentInstruction[3].u.operand), get(VirtualRegister(currentInstruction[2].u.operand))));
+                    addToGraph(NewFunctionNoCheck, OpInfo(frozen), get(VirtualRegister(currentInstruction[2].u.operand))));
             } else {
                 set(VirtualRegister(currentInstruction[1].u.operand),
                     addToGraph(
                         NewFunction,
-                        OpInfo(currentInstruction[3].u.operand),
+                        OpInfo(frozen),
                         get(VirtualRegister(currentInstruction[1].u.operand)), get(VirtualRegister(currentInstruction[2].u.operand))));
             }
             NEXT_OPCODE(op_new_func);
         }
 
         case op_new_func_exp: {
+            FunctionExecutable* expr = m_inlineStackTop->m_profiledBlock->functionExpr(currentInstruction[3].u.operand);
+            FrozenValue* frozen = m_graph.freezeStrong(expr);
             set(VirtualRegister(currentInstruction[1].u.operand),
-                addToGraph(NewFunctionExpression, OpInfo(currentInstruction[3].u.operand), get(VirtualRegister(currentInstruction[2].u.operand))));
+                addToGraph(NewFunctionExpression, OpInfo(frozen), get(VirtualRegister(currentInstruction[2].u.operand))));
             NEXT_OPCODE(op_new_func_exp);
         }
 
index 0d36ea6..a19c306 100644 (file)
@@ -205,6 +205,8 @@ CapabilityLevel capabilityLevel(OpcodeID opcodeID, CodeBlock* codeBlock, Instruc
     case op_get_generic_property_enumerator:
     case op_next_enumerator_pname:
     case op_to_index_string:
+    case op_new_func:
+    case op_new_func_exp:
         return CanCompileAndInline;
 
     case op_put_to_scope: {
@@ -226,8 +228,6 @@ CapabilityLevel capabilityLevel(OpcodeID opcodeID, CodeBlock* codeBlock, Instruc
 
     case op_new_regexp: 
     case op_create_lexical_environment:
-    case op_new_func:
-    case op_new_func_exp:
     case op_switch_string: // Don't inline because we don't want to copy string tables in the concurrent JIT.
         return CanCompile;
 
index d08586f..ce7f0af 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2014 Apple Inc. All rights reserved.
+ * Copyright (C) 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
@@ -68,6 +68,19 @@ public:
     bool operator!() const { return !m_value; }
     
     JSValue value() const { return m_value; }
+    JSCell* cell() const { return m_value.asCell(); }
+    
+    template<typename T>
+    T dynamicCast()
+    {
+        return jsDynamicCast<T>(value());
+    }
+    template<typename T>
+    T cast()
+    {
+        return jsCast<T>(value());
+    }
+    
     Structure* structure() const { return m_structure; }
     
     void strengthenTo(ValueStrength strength)
index 4b4ffae..c299d0f 100644 (file)
@@ -246,14 +246,6 @@ void Graph::dump(PrintStream& out, const char* prefix, Node* node, DumpContext*
             }
         }
     }
-    if (node->hasFunctionDeclIndex()) {
-        FunctionExecutable* executable = m_codeBlock->functionDecl(node->functionDeclIndex());
-        out.print(comma, FunctionExecutableDump(executable));
-    }
-    if (node->hasFunctionExprIndex()) {
-        FunctionExecutable* executable = m_codeBlock->functionExpr(node->functionExprIndex());
-        out.print(comma, FunctionExecutableDump(executable));
-    }
     if (node->hasStorageAccessData()) {
         StorageAccessData& storageAccessData = node->storageAccessData();
         out.print(comma, "id", storageAccessData.identifierNumber, "{", identifiers()[storageAccessData.identifierNumber], "}");
@@ -1120,6 +1112,10 @@ void Graph::registerFrozenValues()
     }
     m_codeBlock->constants().shrinkToFit();
     m_codeBlock->constantsSourceCodeRepresentation().shrinkToFit();
+    
+    // We have no use DFG IR have no need for FunctionExecutable*'s in the CodeBlock, since we
+    // use frozen values to refer to them.
+    m_codeBlock->jettisonFunctionDeclsAndExprs();
 }
 
 void Graph::visitChildren(SlotVisitor& visitor)
index f2b2a14..a721e6d 100644 (file)
@@ -1183,6 +1183,9 @@ struct Node {
         case CheckCell:
         case NativeConstruct:
         case NativeCall:
+        case NewFunctionNoCheck:
+        case NewFunction:
+        case NewFunctionExpression:
             return true;
         default:
             return false;
@@ -1195,6 +1198,12 @@ struct Node {
         return reinterpret_cast<FrozenValue*>(m_opInfo);
     }
     
+    template<typename T>
+    T castOperand()
+    {
+        return cellOperand()->cast<T>();
+    }
+    
     void setCellOperand(FrozenValue* value)
     {
         ASSERT(hasCellOperand());
@@ -1341,29 +1350,6 @@ struct Node {
         }
     }
     
-    bool hasFunctionDeclIndex()
-    {
-        return op() == NewFunction
-            || op() == NewFunctionNoCheck;
-    }
-    
-    unsigned functionDeclIndex()
-    {
-        ASSERT(hasFunctionDeclIndex());
-        return m_opInfo;
-    }
-    
-    bool hasFunctionExprIndex()
-    {
-        return op() == NewFunctionExpression;
-    }
-    
-    unsigned functionExprIndex()
-    {
-        ASSERT(hasFunctionExprIndex());
-        return m_opInfo;
-    }
-    
     bool hasArrayMode()
     {
         switch (op()) {
index 1e64ca9..6f5a6ea 100644 (file)
@@ -4256,7 +4256,8 @@ void SpeculativeJIT::compileNewFunctionNoCheck(Node* node)
     GPRReg scopeGPR = scope.gpr();
     flushRegisters();
     callOperation(
-        operationNewFunctionNoCheck, resultGPR, scopeGPR, m_jit.codeBlock()->functionDecl(node->functionDeclIndex()));
+        operationNewFunctionNoCheck, resultGPR, scopeGPR,
+        node->castOperand<FunctionExecutable*>());
     cellResult(resultGPR, node);
 }
 
@@ -4269,8 +4270,7 @@ void SpeculativeJIT::compileNewFunctionExpression(Node* node)
     flushRegisters();
     callOperation(
         operationNewFunctionNoCheck,
-        resultGPR, scopeGPR, 
-        m_jit.codeBlock()->functionExpr(node->functionExprIndex()));
+        resultGPR, scopeGPR,  node->castOperand<FunctionExecutable*>());
     cellResult(resultGPR, node);
 }
 
index 0412b4f..700b194 100644 (file)
@@ -3749,7 +3749,7 @@ void SpeculativeJIT::compile(Node* node)
         
     case CheckCell: {
         SpeculateCellOperand cell(this, node->child1());
-        speculationCheck(BadCell, JSValueSource::unboxedCell(cell.gpr()), node->child1(), m_jit.branchWeakPtr(JITCompiler::NotEqual, cell.gpr(), node->cellOperand()->value().asCell()));
+        speculationCheck(BadCell, JSValueSource::unboxedCell(cell.gpr()), node->child1(), m_jit.branchWeakPtr(JITCompiler::NotEqual, cell.gpr(), node->cellOperand()->cell()));
         noResult(node);
         break;
     }
@@ -4682,7 +4682,7 @@ void SpeculativeJIT::compile(Node* node)
         addSlowPathGenerator(
             slowPathCall(
                 notCreated, this, operationNewFunction, JSValueRegs(resultTagGPR, resultPayloadGPR), scopeGPR,
-                m_jit.codeBlock()->functionDecl(node->functionDeclIndex())));
+                node->castOperand<FunctionExecutable*>()));
         
         jsValueResult(resultTagGPR, resultPayloadGPR, node);
         break;
index 579d3fe..07d4635 100644 (file)
@@ -3834,7 +3834,7 @@ void SpeculativeJIT::compile(Node* node)
         
     case CheckCell: {
         SpeculateCellOperand cell(this, node->child1());
-        speculationCheck(BadCell, JSValueSource::unboxedCell(cell.gpr()), node->child1(), m_jit.branchWeakPtr(JITCompiler::NotEqual, cell.gpr(), node->cellOperand()->value().asCell()));
+        speculationCheck(BadCell, JSValueSource::unboxedCell(cell.gpr()), node->child1(), m_jit.branchWeakPtr(JITCompiler::NotEqual, cell.gpr(), node->cellOperand()->cell()));
         noResult(node);
         break;
     }
@@ -4697,7 +4697,7 @@ void SpeculativeJIT::compile(Node* node)
         addSlowPathGenerator(
             slowPathCall(
                 notCreated, this, operationNewFunction,
-                resultGPR, scopeGPR, m_jit.codeBlock()->functionDecl(node->functionDeclIndex())));
+                resultGPR, scopeGPR, node->castOperand<FunctionExecutable*>()));
         
         jsValueResult(resultGPR, node);
         break;
index 00f3643..1e4000f 100644 (file)
@@ -114,7 +114,7 @@ private:
             break;
             
         case AllocationProfileWatchpoint:
-            addLazily(jsCast<JSFunction*>(m_node->cellOperand()->value())->allocationProfileWatchpointSet());
+            addLazily(m_node->castOperand<JSFunction*>()->allocationProfileWatchpointSet());
             break;
             
         case VarInjectionWatchpoint:
index 0428aec..ec67ad3 100644 (file)
@@ -1850,7 +1850,7 @@ private:
         
         speculate(
             BadCell, jsValueValue(cell), m_node->child1().node(),
-            m_out.notEqual(cell, weakPointer(m_node->cellOperand()->value().asCell())));
+            m_out.notEqual(cell, weakPointer(m_node->cellOperand()->cell())));
     }
     
     void compileCheckBadCell()
@@ -3770,7 +3770,7 @@ private:
         int numPassedArgs = m_node->numChildren() - 1;
         int numArgs = numPassedArgs;
 
-        JSFunction* knownFunction = jsCast<JSFunction*>(m_node->cellOperand()->value().asCell());
+        JSFunction* knownFunction = m_node->castOperand<JSFunction*>();
         NativeFunction function = knownFunction->nativeFunction();
 
         Dl_info info;