Attachment 119611 Details for Bug 74718 – Patch

[patch] Patch

bug-74718-20111216162535.patch (text/plain), 10.51 KB, created by Andy Wingo on 2011-12-16 07:25:37 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Andy Wingo

Created: 2011-12-16 07:25:37 PST

Size: 10.51 KB

patch

obsolete

>Subversion Revision: 102931
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 2e124d6009aa07db6d1fa4de1d970a7ad832e606..c67f796d1a8b8676f1db784e1141849c7c872106 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,5 +1,38 @@
> 2011-12-16  Andy Wingo  <wingo@igalia.com>
> 
>+        Remove the `value' argument to op_push_new_scope
>+        https://bugs.webkit.org/show_bug.cgi?id=74718
>+
>+        For exception scopes, instead of pushing a scope with a value
>+        already set in it, push a fresh scope and use op_put_scoped_var to
>+        bind the var.  This will allow op_push_new_scope to be used for
>+        multi-variable scopes, like ES6 block scopes.
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * bytecode/CodeBlock.cpp (JSC::CodeBlock::dump): Update dump.
>+        * bytecode/Opcode.h:
>+        * bytecompiler/BytecodeGenerator.cpp:
>+        (JSC::BytecodeGenerator::emitPushBlockScope): Use the variant of
>+        JSStaticScope::clone that doesn't bind a variable.  (The other one
>+        is used by named function expressions.)
>+        * bytecompiler/BytecodeGenerator.h: Update declarations.
>+        * bytecompiler/NodesCodegen.cpp:
>+        (JSC::TryNode::emitBytecode): Fat-finger op_put_scoped_var with
>+        known indices.  We could call findScopedProperty, but it might
>+        fail if there is a `with' in scope.
>+        * interpreter/Interpreter.cpp:
>+        (JSC::Interpreter::createExceptionScope): Simplify.
>+        (JSC::Interpreter::privateExecute):
>+        * jit/JITOpcodes.cpp:
>+        (JSC::JIT::emit_op_push_new_scope):
>+        * jit/JITOpcodes32_64.cpp:
>+        (JSC::JIT::emit_op_push_new_scope):
>+        * jit/JITStubs.cpp:
>+        (JSC::DEFINE_STUB_FUNCTION): Adapt to op_push_new_scope change.
>+
>+2011-12-16  Andy Wingo  <wingo@igalia.com>
>+
>         Optimize access to block-scoped local variables
>         https://bugs.webkit.org/show_bug.cgi?id=74708
> 
>diff --git a/Source/JavaScriptCore/bytecode/CodeBlock.cpp b/Source/JavaScriptCore/bytecode/CodeBlock.cpp
>index ca2e5c7d21de72895567de479dadfa6732ffa0ef..66d3855306586ec412444f4df7ea54dafb0ba93d 100644
>--- a/Source/JavaScriptCore/bytecode/CodeBlock.cpp
>+++ b/Source/JavaScriptCore/bytecode/CodeBlock.cpp
>@@ -1220,8 +1220,7 @@ void CodeBlock::dump(ExecState* exec, const Vector<Instruction>::const_iterator&
>         case op_push_new_scope: {
>             int r0 = (++it)->u.operand;
>             int r1 = (++it)->u.operand;
>-            int r2 = (++it)->u.operand;
>-            printf("[%4d] push_new_scope \t%s, %s, %s\n", location, registerName(exec, r0).data(), registerName(exec, r1).data(), registerName(exec, r2).data());
>+            printf("[%4d] push_new_scope \t%s, %s\n", location, registerName(exec, r0).data(), registerName(exec, r1).data());
>             break;
>         }
>         case op_jmp_scopes: {
>diff --git a/Source/JavaScriptCore/bytecode/Opcode.h b/Source/JavaScriptCore/bytecode/Opcode.h
>index 949d69fd8ac72470bf65fe51349bd3961ee58238..52a2f7043e27daf460da422b3a640340f0cd7366 100644
>--- a/Source/JavaScriptCore/bytecode/Opcode.h
>+++ b/Source/JavaScriptCore/bytecode/Opcode.h
>@@ -183,7 +183,7 @@ namespace JSC {
>         \
>         macro(op_push_scope, 2) \
>         macro(op_pop_scope, 1) \
>-        macro(op_push_new_scope, 4) \
>+        macro(op_push_new_scope, 3) \
>         \
>         macro(op_catch, 2) \
>         macro(op_throw, 2) \
>diff --git a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
>index 25cdb58d2430f1f39f012c8f4dc75226a91a20bd..689e2f7e0bfcbb3720d865719112f9b233b3a677 100644
>--- a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
>+++ b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
>@@ -2226,7 +2226,7 @@ void BytecodeGenerator::emitSubroutineReturn(RegisterID* retAddrSrc)
>     instructions().append(retAddrSrc->index());
> }
> 
>-void BytecodeGenerator::emitPushBlockScope(RegisterID* dst, JSStaticScopeObject *scopeTemplate, RegisterID* value)
>+void BytecodeGenerator::emitPushBlockScope(RegisterID* dst, JSStaticScopeObject *scopeTemplate)
> {
>     ControlFlowContext context;
>     context.blockType = ScopeBlock;
>@@ -2238,7 +2238,6 @@ void BytecodeGenerator::emitPushBlockScope(RegisterID* dst, JSStaticScopeObject
>     emitOpcode(op_push_new_scope);
>     instructions().append(dst->index());
>     instructions().append(templ->index());
>-    instructions().append(value->index());
> }
> 
> void BytecodeGenerator::emitPopBlockScope()
>diff --git a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h
>index 3ef885b36b7980b277298398ffd9c0a69ff11ba1..cb042be3a9716b3f98a1dd4eb24fc0feb8310bc8 100644
>--- a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h
>+++ b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h
>@@ -372,7 +372,7 @@ namespace JSC {
> 
>         void emitThrowReferenceError(const UString& message);
> 
>-        void emitPushBlockScope(RegisterID* dst, JSStaticScopeObject *scopeTemplate, RegisterID* value);
>+        void emitPushBlockScope(RegisterID* dst, JSStaticScopeObject *scopeTemplate);
>         void emitPopBlockScope();
> 
>         RegisterID* emitPushScope(RegisterID* scope);
>diff --git a/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp b/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
>index bed72d34077f8bd6b4f38a270bebb75583e7199b..2b38e6c89667d790d638714adb10e53092550ae0 100644
>--- a/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
>+++ b/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
>@@ -1941,7 +1941,14 @@ RegisterID* TryNode::emitBytecode(BytecodeGenerator& generator, RegisterID* dst)
>         } else {
>             JSStaticScopeObject* scopeTemplate = JSStaticScopeObject::create(generator.scopeChain()->globalObject->globalExec());
>             scopeTemplate->addVariable(m_exceptionIdent, DontDelete);
>-            generator.emitPushBlockScope(exceptionRegister.get(), scopeTemplate, exceptionRegister.get());
>+            generator.emitPushBlockScope(generator.newTemporary(), scopeTemplate);
>+            JSObject* globalObject = 0;
>+            int index = 0;
>+            int depth = -1;
>+            // We could call findScopedProperty, but we already where to store
>+            // the var: at the top of the scope chain, in the 0th slot, even if
>+            // there is a `with' around or other such crazy things.
>+            generator.emitPutScopedVar(depth, index, exceptionRegister.get(), globalObject);
>         }
>         generator.emitNode(dst, m_catchBlock);
>         if (m_catchHasEval)
>diff --git a/Source/JavaScriptCore/interpreter/Interpreter.cpp b/Source/JavaScriptCore/interpreter/Interpreter.cpp
>index 05364d7ced5af6d25678567862c13782ce476321..5a76ba08126b2fa653990665d57dccbe875438e1 100644
>--- a/Source/JavaScriptCore/interpreter/Interpreter.cpp
>+++ b/Source/JavaScriptCore/interpreter/Interpreter.cpp
>@@ -1409,12 +1409,11 @@ NEVER_INLINE ScopeChainNode* Interpreter::createExceptionScope(CallFrame* callFr
> {
>     int dst = vPC[1].u.operand;
>     int templ = vPC[2].u.operand;
>-    int value = vPC[3].u.operand;
> 
>     ASSERT(callFrame->r(templ).jsValue().isObject());
>     JSObject* scopeTemplate = asObject(callFrame->r(templ).jsValue());
>     ASSERT(scopeTemplate->isStaticScopeObject());
>-    JSStaticScopeObject* newScope = static_cast<JSStaticScopeObject*>(scopeTemplate)->clone(callFrame, callFrame->r(value).jsValue());
>+    JSStaticScopeObject* newScope = static_cast<JSStaticScopeObject*>(scopeTemplate)->clone(callFrame);
>     callFrame->uncheckedR(dst) = newScope;
> 
>     return callFrame->scopeChain()->push(newScope);
>@@ -4869,11 +4868,11 @@ skip_id_custom_self:
>     goto *(&&skip_new_scope);
> #endif
>     DEFINE_OPCODE(op_push_new_scope) {
>-        /* new_scope dst(r) templ(r) value(r)
>+        /* new_scope dst(r) templ(r)
>          
>-           Constructs a new StaticScopeObject whose one property is set to
>-           value.  That scope object is then pushed onto the ScopeChain.  The
>-           scope object is then stored in dst for GC.
>+           Constructs a new StaticScopeObject, using templ as a template. That
>+           scope object is then pushed onto the ScopeChain. The scope object is
>+           then stored in dst for GC.
>          */
>         callFrame->setScopeChain(createExceptionScope(callFrame, vPC));
> 
>diff --git a/Source/JavaScriptCore/jit/JITOpcodes.cpp b/Source/JavaScriptCore/jit/JITOpcodes.cpp
>index 3bb600ac1d1ba99f5e78a07008435eacf4ed30b9..ffcd8ee8fba63154f9176dd890ec7483a82281cd 100644
>--- a/Source/JavaScriptCore/jit/JITOpcodes.cpp
>+++ b/Source/JavaScriptCore/jit/JITOpcodes.cpp
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2009 Apple Inc. All rights reserved.
>+ * Copyright (C) 2009, 2011 Apple Inc. All rights reserved.
>  * Copyright (C) 2010 Patrick Gansterer <paroga@paroga.com>
>  *
>  * Redistribution and use in source and binary forms, with or without
>@@ -1013,7 +1013,6 @@ void JIT::emit_op_push_new_scope(Instruction* currentInstruction)
> {
>     JITStubCall stubCall(this, cti_op_push_new_scope);
>     stubCall.addArgument(currentInstruction[2].u.operand, regT1);
>-    stubCall.addArgument(currentInstruction[3].u.operand, regT2);
>     stubCall.call(currentInstruction[1].u.operand);
> }
> 
>diff --git a/Source/JavaScriptCore/jit/JITOpcodes32_64.cpp b/Source/JavaScriptCore/jit/JITOpcodes32_64.cpp
>index 5d5865f9e5fc04d6dd62183d12584d0ebfbafbb3..69e3fdfe230a9452b290258987182b3f5401cb18 100644
>--- a/Source/JavaScriptCore/jit/JITOpcodes32_64.cpp
>+++ b/Source/JavaScriptCore/jit/JITOpcodes32_64.cpp
>@@ -1332,7 +1332,6 @@ void JIT::emit_op_push_new_scope(Instruction* currentInstruction)
> {
>     JITStubCall stubCall(this, cti_op_push_new_scope);
>     stubCall.addArgument(currentInstruction[2].u.operand);
>-    stubCall.addArgument(currentInstruction[3].u.operand);
>     stubCall.call(currentInstruction[1].u.operand);
> }
> 
>diff --git a/Source/JavaScriptCore/jit/JITStubs.cpp b/Source/JavaScriptCore/jit/JITStubs.cpp
>index a732fe672b0b5e1b1e9e872d6d21681390ba336e..a1ddf7c85584a98d55a93782f0be25f39e04ac94 100644
>--- a/Source/JavaScriptCore/jit/JITStubs.cpp
>+++ b/Source/JavaScriptCore/jit/JITStubs.cpp
>@@ -3535,8 +3535,7 @@ DEFINE_STUB_FUNCTION(JSObject*, op_push_new_scope)
>     ASSERT(scopeTemplate->isStaticScopeObject());
> 
>     CallFrame* callFrame = stackFrame.callFrame;
>-    JSValue value = stackFrame.args[1].jsValue();
>-    JSStaticScopeObject* newScope = static_cast<JSStaticScopeObject*>(scopeTemplate)->clone(callFrame, value);
>+    JSStaticScopeObject* newScope = static_cast<JSStaticScopeObject*>(scopeTemplate)->clone(callFrame);
>     callFrame->setScopeChain(callFrame->scopeChain()->push(newScope));
>     return newScope;
> }

Actions: View | Formatted Diff | Diff

Attachments on bug 74718: 119611 | 119620 | 122961