Changeset 59742 in webkit for trunk/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp

Timestamp:

May 18, 2010, 10:10:11 PM (15 years ago)

Author:

[email protected]

Message:

JavaScriptCore: Simplified handling of 'arguments' -- 1.2% SunSpider speedup
​https://bugs.webkit.org/show_bug.cgi?id=39200

Reviewed by Darin Adler.

Removed the reserved OptionalCalleeArguments slot from the CallFrame.
Now, slots for 'arguments' are allocated and initialized only by
functions that might need them.

• bytecode/CodeBlock.cpp:

(JSC::CodeBlock::dump): Updated for new bytecode operands.

(JSC::CodeBlock::CodeBlock):

• bytecode/CodeBlock.h:

(JSC::unmodifiedArgumentsRegister): Added a helper function for mapping
from the arguments register to its unmodified counterpart.

(JSC::CodeBlock::setArgumentsRegister):
(JSC::CodeBlock::argumentsRegister):
(JSC::CodeBlock::usesArguments): Changed from a "usesArguments" bool to
an optional int index representing the arguments register.

• bytecode/Opcode.h: Updated for new bytecode operands.

• bytecompiler/BytecodeGenerator.cpp:

(JSC::BytecodeGenerator::addVar): Factored out a helper function for
allocating an anonymous var.

(JSC::BytecodeGenerator::BytecodeGenerator): Merged / simplified some
arguments vs activation logic, and added code to allocate the arguments
registers when needed.

(JSC::BytecodeGenerator::createArgumentsIfNecessary): Updated for new bytecode operands.

(JSC::BytecodeGenerator::emitCallEval): No need to create the arguments
object before calling eval; the activation object will lazily create the
arguments object if eval resolves it.

(JSC::BytecodeGenerator::emitReturn): Updated for new bytecode operands.

(JSC::BytecodeGenerator::emitPushScope):
(JSC::BytecodeGenerator::emitPushNewScope): Ditto emitCallEval.

• bytecompiler/BytecodeGenerator.h:

(JSC::BytecodeGenerator::addVar): Factored out a helper function for
allocating an anonymous var.

(JSC::BytecodeGenerator::registerFor): No more need for special handling
of the arguments registers; they're allocated just like normal registers
now.

• interpreter/CallFrame.h:

(JSC::ExecState::callerFrame):
(JSC::ExecState::init):

• interpreter/CallFrameClosure.h:

(JSC::CallFrameClosure::resetCallFrame): Nixed optionalCalleeArguments.

• interpreter/Interpreter.cpp:

(JSC::Interpreter::dumpRegisters):
(JSC::Interpreter::unwindCallFrame):
(JSC::Interpreter::privateExecute):
(JSC::Interpreter::retrieveArguments): Opcodes accessing 'arguments' now
take operands specifying registers, just like all other opcodes.
JSActivation::copyRegisters is no longer responsible for tearing off the
arguments object; instead, the VM is responsible for both.

Also, a behavior change: Each access to f.arguments creates a new object,
unless f itself uses 'arguments'. This matches Chrome, and is necessary
for the optimization. f.arguments is a nonstandard, deprecated feature,
so high fidelity to a given implementation is not necessarily a goal.
Also, as illustrated by the new test case, the identity of f.arguments
has been broken since 2008, except in the case where f itself accesses
f.arguments -- but nobody seemed to notice. So, hopefully this change won't
break the web.

• interpreter/Register.h: Nixed the special arguments accessor. It's no

longer needed.

• interpreter/RegisterFile.h:

(JSC::RegisterFile::):

• jit/JITCall.cpp:

(JSC::JIT::compileOpCallInitializeCallFrame):
(JSC::JIT::compileOpCall):

• jit/JITOpcodes.cpp:

(JSC::JIT::emit_op_tear_off_activation):
(JSC::JIT::emit_op_tear_off_arguments):
(JSC::JIT::emit_op_create_arguments):
(JSC::JIT::emit_op_init_arguments):

• jit/JITOpcodes32_64.cpp:

(JSC::JIT::emit_op_tear_off_activation):
(JSC::JIT::emit_op_tear_off_arguments):
(JSC::JIT::emit_op_create_arguments):
(JSC::JIT::emit_op_init_arguments): The actual optimization: Removed
OptionalCalleeArguments from the callframe slot. Now, it doesn't need
to be initialized for most calls.

• jit/JITStubs.cpp:

(JSC::DEFINE_STUB_FUNCTION):

• jit/JITStubs.h:

(JSC::): Updated stubs to support arbitrary 'arguments' registers,
instead of hard-coding something in the call frame.

• runtime/Arguments.h:

(JSC::JSActivation::copyRegisters): Removed some obfuscatory abstraction.

• runtime/Executable.h:

(JSC::FunctionExecutable::generatedByteCode): Added a helper for accessing
the 'arguments' register. In a future patch, that kind of data should
probably move out of CodeBlock and into Executable.

• runtime/JSActivation.cpp:

(JSC::JSActivation::getOwnPropertySlot):
(JSC::JSActivation::argumentsGetter):

• runtime/JSActivation.h: Simplified / fixed access to 'arguments' via

the activation object. It now implements the same behavior implemented
by optimized variable access in the VM. This simplifies some other
things, too -- like eval code generation.

LayoutTests: Simplified handling of 'arguments' -- 1.2% SunSpider speedup
​https://bugs.webkit.org/show_bug.cgi?id=39200

Reviewed by Darin Adler.

• fast/js/function-dot-arguments-expected.txt:
• fast/js/script-tests/function-dot-arguments.js:

(argumentsIdentity): Updated to match new behavior.

• fast/js/function-dot-arguments2-expected.txt:
• fast/js/function-dot-arguments2.html: New tests for some things that

weren't covered before.

• fast/js/global-recursion-on-full-stack.html: Rejiggered the stack

usage in this test. Since stack usage is more efficient now, you
need a slightly different usage pattern to hit the exact thing this
test wanted to test.

• fast/js/kde/script-tests/function_arguments.js:

(f): Updated to more specifically test what this was trying to test,
to avoid just testing the identity of f.arguments.

File:

• trunk/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp (modified) (8 diffs)

trunk/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp

@@ -175 +175 @@
     }
 
-    ++m_codeBlock->m_numVars;
-    r0 = newRegister();
+    r0 = addVar();
     return true;
 }
@@ -296 +295 @@
     , m_scopeNode(functionBody)
     , m_codeBlock(codeBlock)
+    , m_activationRegister(0)
     , m_finallyDepth(0)
     , m_dynamicScopeDepth(0)
@@ -313 +313 @@
     codeBlock->setGlobalData(m_globalData);
 
-    bool usesArguments = functionBody->usesArguments();
-    codeBlock->setUsesArguments(usesArguments);
-    if (usesArguments) {
-        m_argumentsRegister.setIndex(RegisterFile::OptionalCalleeArguments);
-        addVar(propertyNames().arguments, false);
-    }
-
     if (m_codeBlock->needsFullScopeChain()) {
-        ++m_codeBlock->m_numVars;
-        m_activationRegisterIndex = newRegister()->index();
+        m_activationRegister = addVar();
         emitOpcode(op_enter_with_activation);
-        instructions().append(m_activationRegisterIndex);
+        instructions().append(m_activationRegister->index());
     } else
         emitOpcode(op_enter);
 
-    if (usesArguments) {
+    // Both op_tear_off_activation and op_tear_off_arguments tear off the 'arguments'
+    // object, if created.
+    if (m_codeBlock->needsFullScopeChain() || functionBody->usesArguments()) {
+        RegisterID* unmodifiedArgumentsRegister = addVar(); // Anonymous, so it can't be modified by user code.
+        RegisterID* argumentsRegister = addVar(propertyNames().arguments, false); // Can be changed by assigning to 'arguments'.
+
+        // We can save a little space by hard-coding the knowledge that the two
+        // 'arguments' values are stored in consecutive registers, and storing
+        // only the index of the assignable one.
+        codeBlock->setArgumentsRegister(argumentsRegister->index());
+        ASSERT_UNUSED(unmodifiedArgumentsRegister, unmodifiedArgumentsRegister->index() == JSC::unmodifiedArgumentsRegister(codeBlock->argumentsRegister()));
+
         emitOpcode(op_init_arguments);
+        instructions().append(argumentsRegister->index());
 
         // The debugger currently retrieves the arguments object from an activation rather than pulling
         // it from a call frame.  In the long-term it should stop doing that (<rdar://problem/6911886>),
         // but for now we force eager creation of the arguments object when debugging.
-        if (m_shouldEmitDebugHooks)
+        if (m_shouldEmitDebugHooks) {
             emitOpcode(op_create_arguments);
+            instructions().append(argumentsRegister->index());
+        }
     }
 
@@ -1386 +1392 @@
 }
 
-
 RegisterID* BytecodeGenerator::emitNewFunctionExpression(RegisterID* r0, FuncExprNode* n)
 {
@@ -1405 +1410 @@
 void BytecodeGenerator::createArgumentsIfNecessary()
 {
-    if (m_codeBlock->usesArguments() && m_codeType == FunctionCode)
-        emitOpcode(op_create_arguments);
+    if (m_codeType != FunctionCode)
+        return;
+    ASSERT(m_codeBlock->usesArguments());
+
+    emitOpcode(op_create_arguments);
+    instructions().append(m_codeBlock->argumentsRegister());
 }
 
 RegisterID* BytecodeGenerator::emitCallEval(RegisterID* dst, RegisterID* func, RegisterID* thisRegister, ArgumentsNode* argumentsNode, unsigned divot, unsigned startOffset, unsigned endOffset)
 {
-    createArgumentsIfNecessary();
     return emitCall(op_call_eval, dst, func, thisRegister, argumentsNode, divot, startOffset, endOffset);
 }
@@ -1527 +1535 @@
     if (m_codeBlock->needsFullScopeChain()) {
         emitOpcode(op_tear_off_activation);
-        instructions().append(m_activationRegisterIndex);
-    } else if (m_codeBlock->usesArguments() && m_codeBlock->m_numParameters > 1)
+        instructions().append(m_activationRegister->index());
+        instructions().append(m_codeBlock->argumentsRegister());
+    } else if (m_codeBlock->usesArguments() && m_codeBlock->m_numParameters > 1) { // If there are no named parameters, there's nothing to tear off, since extra / unnamed parameters get copied to the arguments object at construct time.
         emitOpcode(op_tear_off_arguments);
+        instructions().append(m_codeBlock->argumentsRegister());
+    }
 
     return emitUnaryNoDstOp(op_ret, src);
@@ -1636 +1647 @@
     m_scopeContextStack.append(context);
     m_dynamicScopeDepth++;
-    createArgumentsIfNecessary();
 
     return emitUnaryNoDstOp(op_push_scope, scope);
@@ -1900 +1910 @@
     m_scopeContextStack.append(context);
     m_dynamicScopeDepth++;
-    
-    createArgumentsIfNecessary();
 
     emitOpcode(op_push_new_scope);