Changeset 34838 in webkit for trunk/JavaScriptCore/VM/CodeGenerator.cpp

Timestamp:

Jun 27, 2008, 3:35:33 PM (17 years ago)

Author:

[email protected]

Message:

JavaScriptCore:

2008-06-27 Geoffrey Garen <​[email protected]>

Reviewed by Oliver Hunt.

One RegisterFile to rule them all!

SunSpider reports a 0.2% speedup.

This patch removes the RegisterFileStack abstraction and replaces it with
a single register file that

(a) allocates a fixed storage area, including a fixed area for global
vars, so that no operation may cause the register file to reallocate

and

(b) swaps between global storage areas when executing code in different
global objects.

This patch also changes the layout of the register file so that all call
frames, including call frames for global code, get a header. This is
required to support re-entrant global code. It also just makes things simpler.

• VM/CodeGenerator.cpp: (KJS::CodeGenerator::addGlobalVar): New function. Differs from addVar in that

(a) global vars don't contribute to a CodeBlock's numLocals count, since
global storage is fixed and allocated at startup

and

(b) references to global vars get shifted to elide intermediate stack
between "r" and the global storage area.

• VM/Machine.cpp: (KJS::Machine::dumpRegisters): Updated this function to match the new register file layout, and added the ability to dump exact identifiers for the different parts of a call frame.

(KJS::Machine::unwindCallFrame): Updated this function to match the new
register file layout.

(KJS::Machine::execute): Updated this function to initialize a call frame
header for global code, and to swap global storage areas when switching
to execution in a new global object.

(KJS::Machine::privateExecute): Got rid of "safeForReentry" and re-reading
of registerBase because the register file is always safe for reentry now,
and registerBase never changes.

• VM/Machine.h: Moved the call frame header enum from Machine to RegisterFile, to resolve a header dependency problem (a good sign that the enum belonged in RegisterFile all along!)

• VM/RegisterFile.cpp:
• VM/RegisterFile.h: Changed RegisterFile to mmap a fixed size register area. This allows us to avoid re-allocting the register file later on. Instead, we rely on the OS to allocate physical pages to the register file as necessary.

• VM/RegisterFileStack.cpp: Removed. Tada!
• VM/RegisterFileStack.h: Removed. Tada!

• kjs/DebuggerCallFrame.cpp: Updated this class to match the new register file layout, greatly simplifying it in the process.

• kjs/JSActivation.h:
• kjs/JSActivation.cpp: Moved some of this logic up to JSVariableObject, since the global object now needs to be able to tear off its registers just like the activation object.

• kjs/JSFunction.cpp: No need to fiddle with the register file anymore.

• kjs/JSGlobalObject.h:
• kjs/JSGlobalObject.cpp: Updated JSGlobalObject to support moving its global storage area into and out of the register file.

• kjs/PropertySlot.cpp: No need to fiddle with the register file anymore.

• kjs/collector.cpp: Renamed markStackObjectConservatively to markConservatively, since we don't just mark stack objects this way.

Also, added code to mark the machine's register file.

• kjs/config.h: Moved some platforms #defines from here...
• wtf/Platform.h: ...to here, to support mmap/VirtualAlloc detection in RegisterFile.h.

LayoutTests:

2008-06-26 Geoffrey Garen <​[email protected]>

Reviewed by Oliver Hunt.

Added a test for what happens when a script exceeds the limit on declared
global variables.

• fast/js/global-var-limit-expected.txt: Added.
• fast/js/global-var-limit.html: Added.

• fast/js/global-recursion-on-full-stack-expected.txt: Updated for new (slightly more correct) behavior. Since the stack overflow happens in the middle of a try/catch block, it should be caught, instead of logged to the console.

File:

• trunk/JavaScriptCore/VM/CodeGenerator.cpp (modified) (8 diffs)

trunk/JavaScriptCore/VM/CodeGenerator.cpp

@@ -149 +149 @@
 }
 
-bool CodeGenerator::addVar(const Identifier& ident, RegisterID*& r0, bool isConstant)
+bool CodeGenerator::addVar(const Identifier& ident, bool isConstant, RegisterID*& r0)
 {
     int index = m_nextVar;
@@ -168 +168 @@
 }
 
-CodeGenerator::CodeGenerator(ProgramNode* programNode, const Debugger* debugger, const ScopeChain& scopeChain, SymbolTable* symbolTable, CodeBlock* codeBlock, VarStack& varStack, FunctionStack& functionStack, bool canCreateVariables)
+bool CodeGenerator::addGlobalVar(const Identifier& ident, bool isConstant, RegisterID*& r0)
+{
+    int index = m_nextVar;
+    SymbolTableEntry newEntry(index, isConstant ? ReadOnly : 0);
+    pair<SymbolTable::iterator, bool> result = symbolTable().add(ident.ustring().rep(), newEntry);
+
+    if (!result.second)
+        index = result.first->second.getIndex();
+    else {
+        --m_nextVar;
+        m_locals.append(index + m_globalVarStorageOffset);
+    }
+
+    r0 = &m_locals[localsIndex(index)];
+    return result.second;
+}
+
+CodeGenerator::CodeGenerator(ProgramNode* programNode, const Debugger* debugger, const ScopeChain& scopeChain, SymbolTable* symbolTable, CodeBlock* codeBlock, VarStack& varStack, FunctionStack& functionStack)
     : m_shouldEmitDebugHooks(!!debugger)
     , m_scopeChain(&scopeChain)
@@ -174 +191 @@
     , m_scopeNode(programNode)
     , m_codeBlock(codeBlock)
-    , m_thisRegister(Machine::ProgramCodeThisRegister)
+    , m_thisRegister(RegisterFile::ProgramCodeThisRegister)
     , m_finallyDepth(0)
     , m_dynamicScopeDepth(0)
@@ -183 +200 @@
     , m_lastOpcodeID(op_end)
 {
-    // Global code can inherit previously defined symbols.
-    int size = symbolTable->size() + 1; // + 1 slot for  "this"
+    // FIXME: Move code that modifies the global object to Machine::execute.
+    
+    m_codeBlock->numVars = 1; // Allocate space for "this"
+
+    JSGlobalObject* globalObject = scopeChain.globalObject();
+    ExecState* exec = globalObject->globalExec();
+    RegisterFile* registerFile = &exec->globalData().machine->registerFile();
+    
+    // Shift register indexes in generated code to elide registers allocated by intermediate stack frames.
+    m_globalVarStorageOffset =  -1 - RegisterFile::CallFrameHeaderSize - registerFile->size();
 
     // Add previously defined symbols to bookkeeping.
-    m_locals.resize(size);
+    m_locals.resize(symbolTable->size());
     SymbolTable::iterator end = symbolTable->end();
     for (SymbolTable::iterator it = symbolTable->begin(); it != end; ++it)
-        m_locals[localsIndex(it->second.getIndex())].setIndex(it->second.getIndex());
-
-    // Shift new symbols so they get stored prior to previously defined symbols.
-    m_nextVar -= size;
-
-    JSGlobalObject* globalObject = scopeChain.globalObject();
-
-    ExecState* exec = globalObject->globalExec();
-    
-    // FIXME: Move the execution-related parts of this code to Machine::execute.
-
-    if (canCreateVariables) {
+        m_locals[localsIndex(it->second.getIndex())].setIndex(it->second.getIndex() + m_globalVarStorageOffset);
+
+    bool canOptimizeNewGlobals = symbolTable->size() + functionStack.size() + varStack.size() < registerFile->maxGlobals();
+    if (canOptimizeNewGlobals) {
+        // Shift new symbols so they get stored prior to existing symbols.
+        m_nextVar -= symbolTable->size();
+
         for (size_t i = 0; i < functionStack.size(); ++i) {
             FuncDeclNode* funcDecl = functionStack[i].get();
             globalObject->removeDirect(funcDecl->m_ident); // Make sure our new function is not shadowed by an old property.
-            emitNewFunction(addVar(funcDecl->m_ident, false), funcDecl);
+            emitNewFunction(addGlobalVar(funcDecl->m_ident, false), funcDecl);
         }
 
         for (size_t i = 0; i < varStack.size(); ++i) {
             if (!globalObject->hasProperty(exec, varStack[i].first))
-                emitLoad(addVar(varStack[i].first, varStack[i].second & DeclarationStacks::IsConstant), jsUndefined());
+                emitLoad(addGlobalVar(varStack[i].first, varStack[i].second & DeclarationStacks::IsConstant), jsUndefined());
         }
     } else {
@@ -256 +276 @@
         if (ident == propertyNames().arguments)
             continue;
-
-        RegisterID* r0;
-        addVar(ident, r0, varStack[i].second & DeclarationStacks::IsConstant);
+        addVar(ident, varStack[i].second & DeclarationStacks::IsConstant);
     }
 
@@ -280 +298 @@
     , m_scopeNode(evalNode)
     , m_codeBlock(codeBlock)
-    , m_thisRegister(Machine::ProgramCodeThisRegister)
+    , m_thisRegister(RegisterFile::ProgramCodeThisRegister)
     , m_finallyDepth(0)
     , m_dynamicScopeDepth(0)
     , m_codeType(EvalCode)
     , m_continueDepth(0)
-    , m_nextVar(-1)
     , m_globalData(&scopeChain.globalObject()->globalExec()->globalData())
     , m_lastOpcodeID(op_end)
@@ -817 +834 @@
 
     // Reserve space for call frame.
-    Vector<RefPtr<RegisterID>, Machine::CallFrameHeaderSize> callFrame;
-    for (int i = 0; i < Machine::CallFrameHeaderSize; ++i)
+    Vector<RefPtr<RegisterID>, RegisterFile::CallFrameHeaderSize> callFrame;
+    for (int i = 0; i < RegisterFile::CallFrameHeaderSize; ++i)
         callFrame.append(newTemporary());
 
@@ -848 +865 @@
 {
     // Reserve space for call frame.
-    Vector<RefPtr<RegisterID>, Machine::CallFrameHeaderSize> callFrame;
-    for (int i = 0; i < Machine::CallFrameHeaderSize; ++i)
+    Vector<RefPtr<RegisterID>, RegisterFile::CallFrameHeaderSize> callFrame;
+    for (int i = 0; i < RegisterFile::CallFrameHeaderSize; ++i)
         callFrame.append(newTemporary());