Changeset 127393 in webkit

Timestamp:

Sep 2, 2012 2:27:23 PM (12 years ago)

Author:

ggaren@apple.com

Message:

Refactored scope chain opcodes to support optimization for named function expressions
​https://bugs.webkit.org/show_bug.cgi?id=95658

Reviewed by Sam Weinig.

Renamed

push_scope => push_with_scope
push_new_scope => push_name_scope

to clarify the difference between them.

Changed push_with_scope and push_name_scope not to save the new scope in
a temporary register, since doing so made optimization harder.

(The old behavior was a hold-over from when the scope chain wasn't
a GC object, and wouldn't be marked otherwise. Now, the scope chain is
marked because it is a GC object pointed to by the call frame.)

Changed push_name_scope to accept an operand specifying the attributes
for the named property, instead of assuming DontDelete, because a named
function expression needs ReadOnly|DontDelete.

• bytecompiler/BytecodeGenerator.cpp:

(JSC::BytecodeGenerator::highestUsedRegister): Removed this function,
which used to be related to preserving saved scope object temporaries,
because it had no callers.

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• bytecode/CodeBlock.cpp (modified) (2 diffs)
• bytecode/Opcode.h (modified) (1 diff)
• bytecompiler/BytecodeGenerator.cpp (modified) (5 diffs)
• bytecompiler/BytecodeGenerator.h (modified) (2 diffs)
• bytecompiler/NodesCodegen.cpp (modified) (2 diffs)
• interpreter/Interpreter.cpp (modified) (4 diffs)
• jit/JIT.cpp (modified) (1 diff)
• jit/JIT.h (modified) (1 diff)
• jit/JITOpcodes.cpp (modified) (2 diffs)
• jit/JITOpcodes32_64.cpp (modified) (2 diffs)
• jit/JITStubs.cpp (modified) (2 diffs)
• jit/JITStubs.h (modified) (1 diff)
• llint/LLIntSlowPaths.cpp (modified) (2 diffs)
• llint/LLIntSlowPaths.h (modified) (1 diff)
• llint/LowLevelInterpreter.asm (modified) (2 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2012-09-02  Geoffrey Garen  <ggaren@apple.com>
+
+        Refactored scope chain opcodes to support optimization for named function expressions
+        https://bugs.webkit.org/show_bug.cgi?id=95658
+
+        Reviewed by Sam Weinig.
+
+        Renamed
+            push_scope => push_with_scope
+            push_new_scope => push_name_scope
+        to clarify the difference between them.
+
+        Changed push_with_scope and push_name_scope not to save the new scope in
+        a temporary register, since doing so made optimization harder.
+
+        (The old behavior was a hold-over from when the scope chain wasn't
+        a GC object, and wouldn't be marked otherwise. Now, the scope chain is
+        marked because it is a GC object pointed to by the call frame.)
+
+        Changed push_name_scope to accept an operand specifying the attributes
+        for the named property, instead of assuming DontDelete, because a named
+        function expression needs ReadOnly|DontDelete.
+
+        * bytecompiler/BytecodeGenerator.cpp:
+        (JSC::BytecodeGenerator::highestUsedRegister): Removed this function,
+        which used to be related to preserving saved scope object temporaries,
+        because it had no callers.
+
 2012-09-01  Geoffrey Garen  <ggaren@apple.com>
 

trunk/Source/JavaScriptCore/bytecode/CodeBlock.cpp

@@ -1480 +1480 @@
             break;
         }
-        case op_push_scope: {
-            int r0 = (++it)->u.operand;
-            dataLog("[%4d] push_scope\t %s", location, registerName(exec, r0).data());
+        case op_push_with_scope: {
+            int r0 = (++it)->u.operand;
+            dataLog("[%4d] push_with_scope\t %s", location, registerName(exec, r0).data());
             dumpBytecodeCommentAndNewLine(location);
             break;
@@ -1491 +1491 @@
             break;
         }
-        case op_push_new_scope: {
-            int r0 = (++it)->u.operand;
+        case op_push_name_scope: {
             int id0 = (++it)->u.operand;
             int r1 = (++it)->u.operand;
-            dataLog("[%4d] push_new_scope \t%s, %s, %s", location, registerName(exec, r0).data(), idName(id0, m_identifiers[id0]).data(), registerName(exec, r1).data());
+            unsigned attributes = (++it)->u.operand;
+            dataLog("[%4d] push_name_scope \t%s, %s, %u", location, idName(id0, m_identifiers[id0]).data(), registerName(exec, r1).data(), attributes);
             dumpBytecodeCommentAndNewLine(location);
             break;

trunk/Source/JavaScriptCore/bytecode/Opcode.h

@@ -189 +189 @@
         macro(op_next_pname, 7) \
         \
-        macro(op_push_scope, 2) \
+        macro(op_push_with_scope, 2) \
         macro(op_pop_scope, 1) \
-        macro(op_push_new_scope, 4) \
+        macro(op_push_name_scope, 4) \
         \
         macro(op_catch, 2) \

trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp

@@ -641 +641 @@
     result->setTemporary();
     return result;
-}
-
-RegisterID* BytecodeGenerator::highestUsedRegister()
-{
-    size_t count = m_codeBlock->m_numCalleeRegisters;
-    while (m_calleeRegisters.size() < count)
-        newRegister();
-    return &m_calleeRegisters.last();
 }
 
@@ -2094 +2086 @@
 }
 
-RegisterID* BytecodeGenerator::emitPushScope(RegisterID* scope)
-{
-    ASSERT(scope->isTemporary());
+RegisterID* BytecodeGenerator::emitPushWithScope(RegisterID* scope)
+{
     ControlFlowContext context;
     context.isFinallyBlock = false;
@@ -2102 +2093 @@
     m_dynamicScopeDepth++;
 
-    return emitUnaryNoDstOp(op_push_scope, scope);
+    return emitUnaryNoDstOp(op_push_with_scope, scope);
 }
 
@@ -2453 +2444 @@
 }
 
-void BytecodeGenerator::emitPushNewScope(RegisterID* dst, const Identifier& property, RegisterID* value)
+void BytecodeGenerator::emitPushNameScope(const Identifier& property, RegisterID* value, unsigned attributes)
 {
     ControlFlowContext context;
@@ -2460 +2451 @@
     m_dynamicScopeDepth++;
 
-    emitOpcode(op_push_new_scope);
-    instructions().append(dst->index());
+    emitOpcode(op_push_name_scope);
     instructions().append(addConstant(property));
     instructions().append(value->index());
+    instructions().append(attributes);
 }
 

trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h

@@ -297 +297 @@
         RegisterID* newTemporary();
 
-        RegisterID* highestUsedRegister();
-
         // The same as newTemporary(), but this function returns "suggestion" if
         // "suggestion" is a temporary. This function is helpful in situations
@@ -523 +521 @@
         void emitThrowReferenceError(const String& message);
 
-        void emitPushNewScope(RegisterID* dst, const Identifier& property, RegisterID* value);
-
-        RegisterID* emitPushScope(RegisterID* scope);
+        void emitPushNameScope(const Identifier& property, RegisterID* value, unsigned attributes);
+
+        RegisterID* emitPushWithScope(RegisterID* scope);
         void emitPopScope();
 

trunk/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp

@@ -1757 +1757 @@
 {
     generator.emitDebugHook(WillExecuteStatement, firstLine(), lastLine(), column());
-    
-    RefPtr<RegisterID> scope = generator.newTemporary();
-    generator.emitNode(scope.get(), m_expr); // scope must be protected until popped
+
+    RefPtr<RegisterID> scope = generator.emitNode(m_expr);
     generator.emitExpressionInfo(m_divot, m_expressionLength, 0);
-    generator.emitPushScope(scope.get());
+    generator.emitPushWithScope(scope.get());
     RegisterID* result = generator.emitNode(dst, m_statement);
     generator.emitPopScope();
@@ -1997 +1996 @@
         }
         
-        generator.emitPushNewScope(exceptionRegister.get(), m_exceptionIdent, exceptionRegister.get());
+        generator.emitPushNameScope(m_exceptionIdent, exceptionRegister.get(), DontDelete);
         generator.emitNode(dst, m_catchBlock);
         generator.emitPopScope();

trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp

@@ -1427 +1427 @@
     
 #if ENABLE(CLASSIC_INTERPRETER)
-NEVER_INLINE JSScope* Interpreter::createExceptionScope(CallFrame* callFrame, const Instruction* vPC)
+NEVER_INLINE JSScope* Interpreter::createNameScope(CallFrame* callFrame, const Instruction* vPC)
 {
-    int dst = vPC[1].u.operand;
     CodeBlock* codeBlock = callFrame->codeBlock();
-    Identifier& property = codeBlock->identifier(vPC[2].u.operand);
-    JSValue value = callFrame->r(vPC[3].u.operand).jsValue();
-    JSNameScope* scope = JSNameScope::create(callFrame, property, value, DontDelete);
-    callFrame->uncheckedR(dst) = JSValue(scope);
+    Identifier& property = codeBlock->identifier(vPC[1].u.operand);
+    JSValue value = callFrame->r(vPC[2].u.operand).jsValue();
+    unsigned attributes = vPC[3].u.operand;
+    JSNameScope* scope = JSNameScope::create(callFrame, property, value, attributes);
     return scope;
 }
@@ -4815 +4814 @@
         NEXT_INSTRUCTION();
     }
-    DEFINE_OPCODE(op_push_scope) {
-        /* push_scope scope(r)
+    DEFINE_OPCODE(op_push_with_scope) {
+        /* push_with_scope scope(r)
 
            Converts register scope to object, and pushes it onto the top
-           of the current scope chain.  The contents of the register scope
-           are replaced by the result of toObject conversion of the scope.
+           of the scope chain.
         */
         int scope = vPC[1].u.operand;
@@ -4827 +4825 @@
         CHECK_FOR_EXCEPTION();
 
-        callFrame->uncheckedR(scope) = JSValue(o);
         callFrame->setScope(JSWithScope::create(callFrame, o));
 
-        vPC += OPCODE_LENGTH(op_push_scope);
+        vPC += OPCODE_LENGTH(op_push_with_scope);
         NEXT_INSTRUCTION();
     }
@@ -4928 +4925 @@
     goto *(&&skip_new_scope);
 #endif
-    DEFINE_OPCODE(op_push_new_scope) {
-        /* new_scope dst(r) property(id) value(r)
+    DEFINE_OPCODE(op_push_name_scope) {
+        /* new_scope property(id) value(r) attributes(unsigned)
          
-           Constructs a new NameScopeObject with property set to value.  That scope
-           object is then pushed onto the ScopeChain.  The scope object is then stored
-           in dst for GC.
+           Constructs a name scope of the form { property<attributes>: value },
+           and pushes it onto the scope chain.
          */
-        callFrame->setScope(createExceptionScope(callFrame, vPC));
-
-        vPC += OPCODE_LENGTH(op_push_new_scope);
+        callFrame->setScope(createNameScope(callFrame, vPC));
+
+        vPC += OPCODE_LENGTH(op_push_name_scope);
         NEXT_INSTRUCTION();
     }

trunk/Source/JavaScriptCore/jit/JIT.cpp

@@ -319 +319 @@
         DEFINE_OP(op_profile_did_call)
         DEFINE_OP(op_profile_will_call)
-        DEFINE_OP(op_push_new_scope)
-        DEFINE_OP(op_push_scope)
+        DEFINE_OP(op_push_name_scope)
+        DEFINE_OP(op_push_with_scope)
         case op_put_by_id_out_of_line:
         case op_put_by_id_transition_direct:

trunk/Source/JavaScriptCore/jit/JIT.h

@@ -672 +672 @@
         void emit_op_profile_did_call(Instruction*);
         void emit_op_profile_will_call(Instruction*);
-        void emit_op_push_new_scope(Instruction*);
-        void emit_op_push_scope(Instruction*);
+        void emit_op_push_name_scope(Instruction*);
+        void emit_op_push_with_scope(Instruction*);
         void emit_op_put_by_id(Instruction*);
         void emit_op_put_by_index(Instruction*);

trunk/Source/JavaScriptCore/jit/JITOpcodes.cpp

@@ -1004 +1004 @@
 }
 
-void JIT::emit_op_push_scope(Instruction* currentInstruction)
-{
-    JITStubCall stubCall(this, cti_op_push_scope);
+void JIT::emit_op_push_with_scope(Instruction* currentInstruction)
+{
+    JITStubCall stubCall(this, cti_op_push_with_scope);
     stubCall.addArgument(currentInstruction[1].u.operand, regT2);
     stubCall.call(currentInstruction[1].u.operand);
@@ -1073 +1073 @@
 }
 
-void JIT::emit_op_push_new_scope(Instruction* currentInstruction)
-{
-    JITStubCall stubCall(this, cti_op_push_new_scope);
-    stubCall.addArgument(TrustedImmPtr(&m_codeBlock->identifier(currentInstruction[2].u.operand)));
-    stubCall.addArgument(currentInstruction[3].u.operand, regT2);
-    stubCall.call(currentInstruction[1].u.operand);
+void JIT::emit_op_push_name_scope(Instruction* currentInstruction)
+{
+    JITStubCall stubCall(this, cti_op_push_name_scope);
+    stubCall.addArgument(TrustedImmPtr(&m_codeBlock->identifier(currentInstruction[1].u.operand)));
+    stubCall.addArgument(currentInstruction[2].u.operand, regT2);
+    stubCall.addArgument(TrustedImm32(currentInstruction[3].u.operand));
+    stubCall.call();
 }
 

trunk/Source/JavaScriptCore/jit/JITOpcodes32_64.cpp

@@ -1359 +1359 @@
 }
 
-void JIT::emit_op_push_scope(Instruction* currentInstruction)
-{
-    JITStubCall stubCall(this, cti_op_push_scope);
+void JIT::emit_op_push_with_scope(Instruction* currentInstruction)
+{
+    JITStubCall stubCall(this, cti_op_push_with_scope);
     stubCall.addArgument(currentInstruction[1].u.operand);
     stubCall.call(currentInstruction[1].u.operand);
@@ -1398 +1398 @@
 }
 
-void JIT::emit_op_push_new_scope(Instruction* currentInstruction)
-{
-    JITStubCall stubCall(this, cti_op_push_new_scope);
-    stubCall.addArgument(TrustedImmPtr(&m_codeBlock->identifier(currentInstruction[2].u.operand)));
-    stubCall.addArgument(currentInstruction[3].u.operand);
-    stubCall.call(currentInstruction[1].u.operand);
+void JIT::emit_op_push_name_scope(Instruction* currentInstruction)
+{
+    JITStubCall stubCall(this, cti_op_push_name_scope);
+    stubCall.addArgument(TrustedImmPtr(&m_codeBlock->identifier(currentInstruction[1].u.operand)));
+    stubCall.addArgument(currentInstruction[2].u.operand);
+    stubCall.addArgument(TrustedImm32(currentInstruction[3].u.operand));
+    stubCall.call();
 }
 

trunk/Source/JavaScriptCore/jit/JITStubs.cpp

@@ -3142 +3142 @@
 }
 
-DEFINE_STUB_FUNCTION(JSObject*, op_push_scope)
+DEFINE_STUB_FUNCTION(JSObject*, op_push_with_scope)
 {
     STUB_INIT_STACK_FRAME(stackFrame);
@@ -3259 +3259 @@
 }
 
-DEFINE_STUB_FUNCTION(JSObject*, op_push_new_scope)
-{
-    STUB_INIT_STACK_FRAME(stackFrame);
-
-    JSNameScope* scope = JSNameScope::create(stackFrame.callFrame, stackFrame.args[0].identifier(), stackFrame.args[1].jsValue(), DontDelete);
+DEFINE_STUB_FUNCTION(void, op_push_name_scope)
+{
+    STUB_INIT_STACK_FRAME(stackFrame);
+
+    JSNameScope* scope = JSNameScope::create(stackFrame.callFrame, stackFrame.args[0].identifier(), stackFrame.args[1].jsValue(), stackFrame.args[2].int32());
 
     CallFrame* callFrame = stackFrame.callFrame;
     callFrame->setScope(scope);
-    return scope;
 }
 

trunk/Source/JavaScriptCore/jit/JITStubs.h

@@ -418 +418 @@
     JSObject* JIT_STUB cti_op_new_regexp(STUB_ARGS_DECLARATION) WTF_INTERNAL;
     JSObject* JIT_STUB cti_op_push_activation(STUB_ARGS_DECLARATION) WTF_INTERNAL;
-    JSObject* JIT_STUB cti_op_push_new_scope(STUB_ARGS_DECLARATION) WTF_INTERNAL;
-    JSObject* JIT_STUB cti_op_push_scope(STUB_ARGS_DECLARATION) WTF_INTERNAL;
+    void JIT_STUB cti_op_push_name_scope(STUB_ARGS_DECLARATION) WTF_INTERNAL;
+    JSObject* JIT_STUB cti_op_push_with_scope(STUB_ARGS_DECLARATION) WTF_INTERNAL;
     JSObject* JIT_STUB cti_op_put_by_id_transition_realloc(STUB_ARGS_DECLARATION) WTF_INTERNAL;
     JSPropertyNameIterator* JIT_STUB cti_op_get_pnames(STUB_ARGS_DECLARATION) WTF_INTERNAL;

trunk/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp

@@ -1531 +1531 @@
 }
 
-LLINT_SLOW_PATH_DECL(slow_path_push_scope)
-{
-    LLINT_BEGIN();
-    JSValue v = LLINT_OP(1).jsValue();
+LLINT_SLOW_PATH_DECL(slow_path_push_with_scope)
+{
+    LLINT_BEGIN();
+    JSValue v = LLINT_OP_C(1).jsValue();
     JSObject* o = v.toObject(exec);
     LLINT_CHECK_EXCEPTION();
     
-    LLINT_OP(1) = o;
     exec->setScope(JSWithScope::create(exec, o));
     
@@ -1551 +1550 @@
 }
 
-LLINT_SLOW_PATH_DECL(slow_path_push_new_scope)
+LLINT_SLOW_PATH_DECL(slow_path_push_name_scope)
 {
     LLINT_BEGIN();
     CodeBlock* codeBlock = exec->codeBlock();
-    JSNameScope* scope = JSNameScope::create(exec, codeBlock->identifier(pc[2].u.operand), LLINT_OP(3).jsValue(), DontDelete);
+    JSNameScope* scope = JSNameScope::create(exec, codeBlock->identifier(pc[1].u.operand), LLINT_OP(2).jsValue(), pc[3].u.operand);
     exec->setScope(scope);
-    LLINT_RETURN(scope);
+    LLINT_END();
 }
 

trunk/Source/JavaScriptCore/llint/LLIntSlowPaths.h

@@ -203 +203 @@
 LLINT_SLOW_PATH_HIDDEN_DECL(slow_path_get_pnames);
 LLINT_SLOW_PATH_HIDDEN_DECL(slow_path_next_pname);
-LLINT_SLOW_PATH_HIDDEN_DECL(slow_path_push_scope);
+LLINT_SLOW_PATH_HIDDEN_DECL(slow_path_push_with_scope);
 LLINT_SLOW_PATH_HIDDEN_DECL(slow_path_pop_scope);
-LLINT_SLOW_PATH_HIDDEN_DECL(slow_path_push_new_scope);
+LLINT_SLOW_PATH_HIDDEN_DECL(slow_path_push_name_scope);
 LLINT_SLOW_PATH_HIDDEN_DECL(slow_path_throw);
 LLINT_SLOW_PATH_HIDDEN_DECL(slow_path_throw_reference_error);

trunk/Source/JavaScriptCore/llint/LowLevelInterpreter.asm

@@ -774 +774 @@
 
 
-_llint_op_push_scope:
-    traceExecution()
-    callSlowPath(_llint_slow_path_push_scope)
+_llint_op_push_with_scope:
+    traceExecution()
+    callSlowPath(_llint_slow_path_push_with_scope)
     dispatch(2)
 
@@ -786 +786 @@
 
 
-_llint_op_push_new_scope:
-    traceExecution()
-    callSlowPath(_llint_slow_path_push_new_scope)
+_llint_op_push_name_scope:
+    traceExecution()
+    callSlowPath(_llint_slow_path_push_name_scope)
     dispatch(4)