Changeset 59742 in webkit for trunk/JavaScriptCore/interpreter/Interpreter.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/interpreter/Interpreter.cpp (modified) (9 diffs)

trunk/JavaScriptCore/interpreter/Interpreter.cpp

@@ -468 +468 @@
     printf("[ArgumentCount]            | %10p | %d \n", it, (*it).i()); ++it;
     printf("[Callee]                   | %10p | %p \n", it, (*it).function()); ++it;
-    printf("[OptionalCalleeArguments]  | %10p | %p \n", it, (*it).arguments()); ++it;
     printf("-----------------------------------------------------------------------------\n");
 
@@ -541 +540 @@
         while (!scopeChain->object->inherits(&JSActivation::info))
             scopeChain = scopeChain->pop();
-        static_cast<JSActivation*>(scopeChain->object)->copyRegisters(callFrame->optionalCalleeArguments());
-    } else if (Arguments* arguments = callFrame->optionalCalleeArguments()) {
-        if (!arguments->isTornOff())
-            arguments->copyRegisters();
+        JSActivation* activation = asActivation(scopeChain->object);
+        activation->copyRegisters();
+        if (JSValue arguments = callFrame->r(unmodifiedArgumentsRegister(oldCodeBlock->argumentsRegister())).jsValue())
+            asArguments(arguments)->setActivation(activation);
+    } else if (oldCodeBlock->usesArguments()) {
+        if (JSValue arguments = callFrame->r(unmodifiedArgumentsRegister(oldCodeBlock->argumentsRegister())).jsValue())
+            asArguments(arguments)->copyRegisters();
     }
 
@@ -3777 +3779 @@
     }
     DEFINE_OPCODE(op_tear_off_activation) {
-        /* tear_off_activation activation(r)
-
-           Copy all locals and parameters to new memory allocated on
-           the heap, and make the passed activation use this memory
-           in the future when looking up entries in the symbol table.
-           If there is an 'arguments' object, then it will also use
-           this memory for storing the named parameters, but not any
-           extra arguments.
-
-           This opcode should only be used immediately before op_ret.
-        */
-
-        int src = vPC[1].u.operand;
+        /* tear_off_activation activation(r) arguments(r)
+
+           Copy locals and named parameters from the register file to the heap.
+           Point the bindings in 'activation' and 'arguments' to this new backing
+           store. (Note that 'arguments' may not have been created. If created,
+           'arguments' already holds a copy of any extra / unnamed parameters.)
+
+           This opcode appears before op_ret in functions that require full scope chains.
+        */
+
+        int src1 = vPC[1].u.operand;
+        int src2 = vPC[2].u.operand;
         ASSERT(callFrame->codeBlock()->needsFullScopeChain());
 
-        asActivation(callFrame->r(src).jsValue())->copyRegisters(callFrame->optionalCalleeArguments());
+        JSActivation* activation = asActivation(callFrame->r(src1).jsValue());
+        activation->copyRegisters();
+
+        if (JSValue arguments = callFrame->r(unmodifiedArgumentsRegister(src2)).jsValue())
+            asArguments(arguments)->setActivation(activation);
 
         vPC += OPCODE_LENGTH(op_tear_off_activation);
@@ -3798 +3803 @@
     }
     DEFINE_OPCODE(op_tear_off_arguments) {
-        /* tear_off_arguments
-
-           Copy all arguments to new memory allocated on the heap,
-           and make the 'arguments' object use this memory in the
-           future when looking up named parameters, but not any
-           extra arguments. If an activation object exists for the
-           current function context, then the tear_off_activation
-           opcode should be used instead.
-
-           This opcode should only be used immediately before op_ret.
-        */
-
-        ASSERT(callFrame->codeBlock()->usesArguments() && !callFrame->codeBlock()->needsFullScopeChain());
-
-        if (callFrame->optionalCalleeArguments())
-            callFrame->optionalCalleeArguments()->copyRegisters();
+        /* tear_off_arguments arguments(r)
+
+           Copy named parameters from the register file to the heap. Point the
+           bindings in 'arguments' to this new backing store. (Note that
+           'arguments' may not have been created. If created, 'arguments' already
+           holds a copy of any extra / unnamed parameters.)
+
+           This opcode appears before op_ret in functions that don't require full
+           scope chains, but do use 'arguments'.
+        */
+
+        int src1 = vPC[1].u.operand;
+        ASSERT(!callFrame->codeBlock()->needsFullScopeChain() && callFrame->codeBlock()->usesArguments());
+
+        if (JSValue arguments = callFrame->r(unmodifiedArgumentsRegister(src1)).jsValue())
+            asArguments(arguments)->copyRegisters();
 
         vPC += OPCODE_LENGTH(op_tear_off_arguments);
@@ -3849 +3854 @@
         /* enter
 
-           Initializes local variables to undefined and fills constant
-           registers with their values. If the code block requires an
-           activation, enter_with_activation should be used instead.
-
-           This opcode should only be used at the beginning of a code
-           block.
+           Initializes local variables to undefined. If the code block requires
+           an activation, enter_with_activation is used instead.
+
+           This opcode appears only at the beginning of a code block.
         */
 
@@ -3869 +3872 @@
         /* enter_with_activation dst(r)
 
-           Initializes local variables to undefined, fills constant
-           registers with their values, creates an activation object,
-           and places the new activation both in dst and at the top
-           of the scope chain. If the code block does not require an
-           activation, enter should be used instead.
-
-           This opcode should only be used at the beginning of a code
-           block.
+           Initializes local variables to undefined, creates an activation object,
+           places it in dst, and pushes it onto the scope chain.
+
+           This opcode appears only at the beginning of a code block.
         */
 
@@ -3914 +3913 @@
     }
     DEFINE_OPCODE(op_init_arguments) {
-        /* create_arguments
-
-           Initialises the arguments object reference to null to ensure
-           we can correctly detect that we need to create it later (or
-           avoid creating it altogether).
-
-           This opcode should only be used at the beginning of a code
-           block.
+        /* create_arguments dst(r)
+
+           Initialises 'arguments' to JSValue().
+
+           This opcode appears only at the beginning of a code block.
          */
-        callFrame->r(RegisterFile::ArgumentsRegister) = JSValue();
+        int dst = vPC[1].u.operand;
+
+        callFrame->r(dst) = JSValue();
+        callFrame->r(unmodifiedArgumentsRegister(dst)) = JSValue();
         vPC += OPCODE_LENGTH(op_init_arguments);
         NEXT_INSTRUCTION();
     }
     DEFINE_OPCODE(op_create_arguments) {
-        /* create_arguments
+        /* create_arguments dst(r)
 
            Creates the 'arguments' object and places it in both the
@@ -3935 +3934 @@
          */
         
-         if (!callFrame->r(RegisterFile::ArgumentsRegister).jsValue()) {
-             Arguments* arguments = new (globalData) Arguments(callFrame);
-             callFrame->setCalleeArguments(arguments);
-             callFrame->r(RegisterFile::ArgumentsRegister) = JSValue(arguments);
-         }
+        int dst = vPC[1].u.operand;
+
+        if (!callFrame->r(dst).jsValue()) {
+            Arguments* arguments = new (globalData) Arguments(callFrame);
+            callFrame->r(dst) = JSValue(arguments);
+            callFrame->r(unmodifiedArgumentsRegister(dst)) = JSValue(arguments);
+        }
         vPC += OPCODE_LENGTH(op_create_arguments);
         NEXT_INSTRUCTION();
@@ -4420 +4421 @@
     if (codeBlock->usesArguments()) {
         ASSERT(codeBlock->codeType() == FunctionCode);
-        SymbolTable& symbolTable = *codeBlock->symbolTable();
-        int argumentsIndex = symbolTable.get(functionCallFrame->propertyNames().arguments.ustring().rep()).getIndex();
-        if (!functionCallFrame->r(argumentsIndex).jsValue()) {
-            Arguments* arguments = new (callFrame) Arguments(functionCallFrame);
-            functionCallFrame->setCalleeArguments(arguments);
-            functionCallFrame->r(RegisterFile::ArgumentsRegister) = JSValue(arguments);
-        }
-        return functionCallFrame->r(argumentsIndex).jsValue();
-    }
-
-    Arguments* arguments = functionCallFrame->optionalCalleeArguments();
-    if (!arguments) {
-        arguments = new (functionCallFrame) Arguments(functionCallFrame);
-        arguments->copyRegisters();
-        callFrame->setCalleeArguments(arguments);
-    }
-
+        int argumentsRegister = codeBlock->argumentsRegister();
+        if (!functionCallFrame->r(argumentsRegister).jsValue()) {
+            JSValue arguments = JSValue(new (callFrame) Arguments(functionCallFrame));
+            functionCallFrame->r(argumentsRegister) = arguments;
+            functionCallFrame->r(unmodifiedArgumentsRegister(argumentsRegister)) = arguments;
+        }
+        return functionCallFrame->r(argumentsRegister).jsValue();
+    }
+
+    Arguments* arguments = new (functionCallFrame) Arguments(functionCallFrame);
+    arguments->copyRegisters();
     return arguments;
 }