Changeset 113136 in webkit

Timestamp:

Apr 3, 2012, 9:25:56 PM (14 years ago)

Author:

fpizlo@apple.com

Message:

jsr/sret should be removed
​https://bugs.webkit.org/show_bug.cgi?id=82986
<rdar://problem/11017015>

Reviewed by Sam Weinig and Geoff Garen.

Replaces jsr/sret with finally block inlining.

• bytecode/CodeBlock.cpp:

(JSC::CodeBlock::dump):

• bytecode/Opcode.h:

(JSC):
(JSC::padOpcodeName):

• bytecompiler/BytecodeGenerator.cpp:

(JSC::BytecodeGenerator::pushFinallyContext):
(JSC::BytecodeGenerator::emitComplexJumpScopes):
(JSC):

• bytecompiler/BytecodeGenerator.h:

(FinallyContext):
(BytecodeGenerator):

• bytecompiler/NodesCodegen.cpp:

(JSC::TryNode::emitBytecode):

• interpreter/Interpreter.cpp:

(JSC::Interpreter::privateExecute):

• jit/JIT.cpp:

(JSC::JIT::privateCompileMainPass):
(JSC::JIT::privateCompile):

• jit/JIT.h:

(JIT):

• jit/JITOpcodes.cpp:

(JSC):

• jit/JITOpcodes32_64.cpp:

(JSC):

• llint/LowLevelInterpreter32_64.asm:
• llint/LowLevelInterpreter64.asm:

Location:

trunk/Source/JavaScriptCore

Files:

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

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2012-04-02  Filip Pizlo  <fpizlo@apple.com>
+
+        jsr/sret should be removed
+        https://bugs.webkit.org/show_bug.cgi?id=82986
+        <rdar://problem/11017015>
+
+        Reviewed by Sam Weinig and Geoff Garen.
+        
+        Replaces jsr/sret with finally block inlining.
+
+        * bytecode/CodeBlock.cpp:
+        (JSC::CodeBlock::dump):
+        * bytecode/Opcode.h:
+        (JSC):
+        (JSC::padOpcodeName):
+        * bytecompiler/BytecodeGenerator.cpp:
+        (JSC::BytecodeGenerator::pushFinallyContext):
+        (JSC::BytecodeGenerator::emitComplexJumpScopes):
+        (JSC):
+        * bytecompiler/BytecodeGenerator.h:
+        (FinallyContext):
+        (BytecodeGenerator):
+        * bytecompiler/NodesCodegen.cpp:
+        (JSC::TryNode::emitBytecode):
+        * interpreter/Interpreter.cpp:
+        (JSC::Interpreter::privateExecute):
+        * jit/JIT.cpp:
+        (JSC::JIT::privateCompileMainPass):
+        (JSC::JIT::privateCompile):
+        * jit/JIT.h:
+        (JIT):
+        * jit/JITOpcodes.cpp:
+        (JSC):
+        * jit/JITOpcodes32_64.cpp:
+        (JSC):
+        * llint/LowLevelInterpreter32_64.asm:
+        * llint/LowLevelInterpreter64.asm:
+
 2012-04-03  Mark Rowe  <mrowe@apple.com>
 

trunk/Source/JavaScriptCore/bytecode/CodeBlock.cpp

@@ -1247 +1247 @@
             int k0 = (++it)->u.operand;
             dataLog("[%4d] throw_reference_error\t %s\n", location, constantName(exec, k0, getConstant(k0)).data());
-            break;
-        }
-        case op_jsr: {
-            int retAddrDst = (++it)->u.operand;
-            int offset = (++it)->u.operand;
-            dataLog("[%4d] jsr\t\t %s, %d(->%d)\n", location, registerName(exec, retAddrDst).data(), offset, location + offset);
-            break;
-        }
-        case op_sret: {
-            int retAddrSrc = (++it)->u.operand;
-            dataLog("[%4d] sret\t\t %s\n", location, registerName(exec, retAddrSrc).data());
             break;
         }

trunk/Source/JavaScriptCore/bytecode/Opcode.h

@@ -190 +190 @@
         macro(op_throw_reference_error, 2) \
         \
-        macro(op_jsr, 3) \
-        macro(op_sret, 2) \
-        \
         macro(op_debug, 4) \
         macro(op_profile_will_call, 2) \

trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp

@@ -1 +1 @@
 /*
- * Copyright (C) 2008, 2009 Apple Inc. All rights reserved.
+ * Copyright (C) 2008, 2009, 2012 Apple Inc. All rights reserved.
  * Copyright (C) 2008 Cameron Zwarich <cwzwarich@uwaterloo.ca>
  * Copyright (C) 2012 Igalia, S.L.
@@ -2005 +2005 @@
 }
 
-void BytecodeGenerator::pushFinallyContext(Label* target, RegisterID* retAddrDst)
+void BytecodeGenerator::pushFinallyContext(StatementNode* finallyBlock)
 {
     ControlFlowContext scope;
     scope.isFinallyBlock = true;
-    FinallyContext context = { target, retAddrDst };
+    FinallyContext context = {
+        finallyBlock,
+        m_scopeContextStack.size(),
+        m_switchContextStack.size(),
+        m_forInContextStack.size(),
+        m_labelScopes.size(),
+        m_finallyDepth,
+        m_dynamicScopeDepth
+    };
     scope.finallyContext = context;
     m_scopeContextStack.append(scope);
@@ -2135 +2143 @@
             emitLabel(nextInsn.get());
         }
-
+        
+        Vector<ControlFlowContext> savedScopeContextStack;
+        Vector<SwitchInfo> savedSwitchContextStack;
+        Vector<ForInContext> savedForInContextStack;
+        SegmentedVector<LabelScope, 8> savedLabelScopes;
         while (topScope > bottomScope && topScope->isFinallyBlock) {
-            emitJumpSubroutine(topScope->finallyContext.retAddrDst, topScope->finallyContext.finallyAddr);
+            // Save the current state of the world while instating the state of the world
+            // for the finally block.
+            FinallyContext finallyContext = topScope->finallyContext;
+            bool flipScopes = finallyContext.scopeContextStackSize != m_scopeContextStack.size();
+            bool flipSwitches = finallyContext.switchContextStackSize != m_switchContextStack.size();
+            bool flipForIns = finallyContext.forInContextStackSize != m_forInContextStack.size();
+            bool flipLabelScopes = finallyContext.labelScopesSize != m_labelScopes.size();
+            int topScopeIndex = -1;
+            int bottomScopeIndex = -1;
+            if (flipScopes) {
+                topScopeIndex = topScope - m_scopeContextStack.begin();
+                bottomScopeIndex = bottomScope - m_scopeContextStack.begin();
+                savedScopeContextStack = m_scopeContextStack;
+                m_scopeContextStack.shrink(finallyContext.scopeContextStackSize);
+            }
+            if (flipSwitches) {
+                savedSwitchContextStack = m_switchContextStack;
+                m_switchContextStack.shrink(finallyContext.switchContextStackSize);
+            }
+            if (flipForIns) {
+                savedForInContextStack = m_forInContextStack;
+                m_forInContextStack.shrink(finallyContext.forInContextStackSize);
+            }
+            if (flipLabelScopes) {
+                savedLabelScopes = m_labelScopes;
+                while (m_labelScopes.size() > finallyContext.labelScopesSize)
+                    m_labelScopes.removeLast();
+            }
+            int savedFinallyDepth = m_finallyDepth;
+            m_finallyDepth = finallyContext.finallyDepth;
+            int savedDynamicScopeDepth = m_dynamicScopeDepth;
+            m_dynamicScopeDepth = finallyContext.dynamicScopeDepth;
+            
+            // Emit the finally block.
+            emitNode(finallyContext.finallyBlock);
+            
+            // Restore the state of the world.
+            if (flipScopes) {
+                m_scopeContextStack = savedScopeContextStack;
+                topScope = &m_scopeContextStack[topScopeIndex]; // assert it's within bounds
+                bottomScope = m_scopeContextStack.begin() + bottomScopeIndex; // don't assert, since it the index might be -1.
+            }
+            if (flipSwitches)
+                m_switchContextStack = savedSwitchContextStack;
+            if (flipForIns)
+                m_forInContextStack = savedForInContextStack;
+            if (flipLabelScopes)
+                m_labelScopes = savedLabelScopes;
+            m_finallyDepth = savedFinallyDepth;
+            m_dynamicScopeDepth = savedDynamicScopeDepth;
+            
             --topScope;
         }
@@ -2215 +2277 @@
     emitOpcode(op_throw_reference_error);
     instructions().append(addConstantValue(jsString(globalData(), message))->index());
-}
-
-PassRefPtr<Label> BytecodeGenerator::emitJumpSubroutine(RegisterID* retAddrDst, Label* finally)
-{
-    size_t begin = instructions().size();
-
-    emitOpcode(op_jsr);
-    instructions().append(retAddrDst->index());
-    instructions().append(finally->bind(begin, instructions().size()));
-    emitLabel(newLabel().get()); // Record the fact that the next instruction is implicitly labeled, because op_sret will return to it.
-    return finally;
-}
-
-void BytecodeGenerator::emitSubroutineReturn(RegisterID* retAddrSrc)
-{
-    emitOpcode(op_sret);
-    instructions().append(retAddrSrc->index());
 }
 

trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h

@@ -1 +1 @@
 /*
- * Copyright (C) 2008, 2009 Apple Inc. All rights reserved.
+ * Copyright (C) 2008, 2009, 2012 Apple Inc. All rights reserved.
  * Copyright (C) 2008 Cameron Zwarich <cwzwarich@uwaterloo.ca>
  * Copyright (C) 2012 Igalia, S.L.
@@ -72 +72 @@
 
     struct FinallyContext {
-        Label* finallyAddr;
-        RegisterID* retAddrDst;
+        StatementNode* finallyBlock;
+        unsigned scopeContextStackSize;
+        unsigned switchContextStackSize;
+        unsigned forInContextStackSize;
+        unsigned labelScopesSize;
+        int finallyDepth;
+        int dynamicScopeDepth;
     };
 
@@ -480 +485 @@
         PassRefPtr<Label> emitJumpScopes(Label* target, int targetScopeDepth);
 
-        PassRefPtr<Label> emitJumpSubroutine(RegisterID* retAddrDst, Label*);
-        void emitSubroutineReturn(RegisterID* retAddrSrc);
-
         RegisterID* emitGetPropertyNames(RegisterID* dst, RegisterID* base, RegisterID* i, RegisterID* size, Label* breakTarget);
         RegisterID* emitNextPropertyName(RegisterID* dst, RegisterID* base, RegisterID* i, RegisterID* size, RegisterID* iter, Label* target);
@@ -505 +507 @@
         bool hasFinaliser() { return m_finallyDepth != 0; }
 
-        void pushFinallyContext(Label* target, RegisterID* returnAddrDst);
+        void pushFinallyContext(StatementNode* finallyBlock);
         void popFinallyContext();
 

trunk/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp

@@ -2 +2 @@
 *  Copyright (C) 1999-2002 Harri Porten (porten@kde.org)
 *  Copyright (C) 2001 Peter Kelly (pmk@post.com)
-*  Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
+*  Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2012 Apple Inc. All rights reserved.
 *  Copyright (C) 2007 Cameron Zwarich (cwzwarich@uwaterloo.ca)
 *  Copyright (C) 2007 Maks Orlovich
@@ -1728 +1728 @@
     if (generator.scopeDepth()) {
         RefPtr<Label> l0 = generator.newLabel();
-        if (generator.hasFinaliser() && !r0->isTemporary()) {
+        if (generator.hasFinaliser()) {
             returnRegister = generator.emitMove(generator.newTemporary(), r0);
             r0 = returnRegister.get();
@@ -1958 +1958 @@
 
     RefPtr<Label> tryStartLabel = generator.newLabel();
-    RefPtr<Label> finallyStart;
-    RefPtr<RegisterID> finallyReturnAddr;
-    if (m_finallyBlock) {
-        finallyStart = generator.newLabel();
-        finallyReturnAddr = generator.newTemporary();
-        generator.pushFinallyContext(finallyStart.get(), finallyReturnAddr.get());
-    }
+    if (m_finallyBlock)
+        generator.pushFinallyContext(m_finallyBlock);
 
     generator.emitLabel(tryStartLabel.get());
@@ -1986 +1981 @@
     if (m_finallyBlock) {
         generator.popFinallyContext();
-        // there may be important registers live at the time we jump
-        // to a finally block (such as for a return or throw) so we
-        // ref the highest register ever used as a conservative
-        // approach to not clobbering anything important
-        RefPtr<RegisterID> highestUsedRegister = generator.highestUsedRegister();
+
         RefPtr<Label> finallyEndLabel = generator.newLabel();
 
-        // Normal path: invoke the finally block, then jump over it.
-        generator.emitJumpSubroutine(finallyReturnAddr.get(), finallyStart.get());
+        // Normal path: run the finally code, and jump to the end.
+        generator.emitNode(dst, m_finallyBlock);
         generator.emitJump(finallyEndLabel.get());
 
@@ -2000 +1991 @@
         RefPtr<Label> here = generator.emitLabel(generator.newLabel().get());
         RefPtr<RegisterID> tempExceptionRegister = generator.emitCatch(generator.newTemporary(), tryStartLabel.get(), here.get());
-        generator.emitJumpSubroutine(finallyReturnAddr.get(), finallyStart.get());
+        generator.emitNode(dst, m_finallyBlock);
         generator.emitThrow(tempExceptionRegister.get());
-
-        // The finally block.
-        generator.emitLabel(finallyStart.get());
-        generator.emitNode(dst, m_finallyBlock);
-        generator.emitSubroutineReturn(finallyReturnAddr.get());
 
         generator.emitLabel(finallyEndLabel.get());

trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp

@@ -5187 +5187 @@
         NEXT_INSTRUCTION();
     }
-    DEFINE_OPCODE(op_jsr) {
-        /* jsr retAddrDst(r) target(offset)
-
-           Places the address of the next instruction into the retAddrDst
-           register and jumps to offset target from the current instruction.
-        */
-        int retAddrDst = vPC[1].u.operand;
-        int target = vPC[2].u.operand;
-        callFrame->r(retAddrDst) = vPC + OPCODE_LENGTH(op_jsr);
-
-        vPC += target;
-        NEXT_INSTRUCTION();
-    }
-    DEFINE_OPCODE(op_sret) {
-        /* sret retAddrSrc(r)
-
-         Jumps to the address stored in the retAddrSrc register. This
-         differs from op_jmp because the target address is stored in a
-         register, not as an immediate.
-        */
-        int retAddrSrc = vPC[1].u.operand;
-        vPC = callFrame->r(retAddrSrc).vPC();
-        NEXT_INSTRUCTION();
-    }
     DEFINE_OPCODE(op_debug) {
         /* debug debugHookID(n) firstLine(n) lastLine(n)

trunk/Source/JavaScriptCore/jit/JIT.cpp

@@ -284 +284 @@
         DEFINE_OP(op_jngreater)
         DEFINE_OP(op_jngreatereq)
-        DEFINE_OP(op_jsr)
         DEFINE_OP(op_jtrue)
         DEFINE_OP(op_loop)
@@ -341 +340 @@
         DEFINE_OP(op_rshift)
         DEFINE_OP(op_urshift)
-        DEFINE_OP(op_sret)
         DEFINE_OP(op_strcat)
         DEFINE_OP(op_stricteq)
@@ -666 +664 @@
     }
 
-    // Link absolute addresses for jsr
-    for (Vector<JSRInfo>::iterator iter = m_jsrSites.begin(); iter != m_jsrSites.end(); ++iter)
-        patchBuffer.patch(iter->storeLocation, patchBuffer.locationOf(iter->target).executableAddress());
-
     m_codeBlock->setNumberOfStructureStubInfos(m_propertyAccessCompilationInfo.size());
     for (unsigned i = 0; i < m_propertyAccessCompilationInfo.size(); ++i) {

trunk/Source/JavaScriptCore/jit/JIT.h

@@ -281 +281 @@
 
     private:
-        struct JSRInfo {
-            DataLabelPtr storeLocation;
-            Label target;
-
-            JSRInfo(DataLabelPtr storeLocation, Label targetLocation)
-                : storeLocation(storeLocation)
-                , target(targetLocation)
-            {
-            }
-        };
-
         JIT(JSGlobalData*, CodeBlock* = 0);
 
@@ -830 +819 @@
         void emit_op_jngreater(Instruction*);
         void emit_op_jngreatereq(Instruction*);
-        void emit_op_jsr(Instruction*);
         void emit_op_jtrue(Instruction*);
         void emit_op_loop(Instruction*);
@@ -884 +872 @@
         void emit_op_ret_object_or_this(Instruction*);
         void emit_op_rshift(Instruction*);
-        void emit_op_sret(Instruction*);
         void emit_op_strcat(Instruction*);
         void emit_op_stricteq(Instruction*);
@@ -1067 +1054 @@
 
         unsigned m_bytecodeOffset;
-        Vector<JSRInfo> m_jsrSites;
         Vector<SlowCaseEntry> m_slowCases;
         Vector<SwitchRecord> m_switches;

trunk/Source/JavaScriptCore/jit/JITOpcodes.cpp

@@ -743 +743 @@
 }
 
-void JIT::emit_op_jsr(Instruction* currentInstruction)
-{
-    int retAddrDst = currentInstruction[1].u.operand;
-    int target = currentInstruction[2].u.operand;
-    DataLabelPtr storeLocation = storePtrWithPatch(TrustedImmPtr(0), Address(callFrameRegister, sizeof(Register) * retAddrDst));
-    addJump(jump(), target);
-    m_jsrSites.append(JSRInfo(storeLocation, label()));
-    killLastResultRegister();
-}
-
-void JIT::emit_op_sret(Instruction* currentInstruction)
-{
-    jump(Address(callFrameRegister, sizeof(Register) * currentInstruction[1].u.operand));
-    killLastResultRegister();
-}
-
 void JIT::emit_op_eq(Instruction* currentInstruction)
 {

trunk/Source/JavaScriptCore/jit/JITOpcodes32_64.cpp

@@ -922 +922 @@
 }
 
-void JIT::emit_op_jsr(Instruction* currentInstruction)
-{
-    int retAddrDst = currentInstruction[1].u.operand;
-    int target = currentInstruction[2].u.operand;
-    DataLabelPtr storeLocation = storePtrWithPatch(TrustedImmPtr(0), Address(callFrameRegister, sizeof(Register) * retAddrDst));
-    addJump(jump(), target);
-    m_jsrSites.append(JSRInfo(storeLocation, label()));
-}
-
-void JIT::emit_op_sret(Instruction* currentInstruction)
-{
-    jump(Address(callFrameRegister, sizeof(Register) * currentInstruction[1].u.operand));
-}
-
 void JIT::emit_op_eq(Instruction* currentInstruction)
 {

trunk/Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm

@@ -1572 +1572 @@
 
 
-_llint_op_jsr:
-    traceExecution()
-    loadi 4[PC], t0
-    addi 3 * 4, PC, t1
-    storei t1, [cfr, t0, 8]
-    dispatchBranch(8[PC])
-
-
-_llint_op_sret:
-    traceExecution()
-    loadi 4[PC], t0
-    loadp [cfr, t0, 8], PC
-    dispatch(0)
-
-
 _llint_op_end:
     traceExecution()

trunk/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm

@@ -1430 +1430 @@
 
 
-_llint_op_jsr:
-    traceExecution()
-    loadis 8[PB, PC, 8], t0
-    addi 3, PC, t1
-    storei t1, [cfr, t0, 8]
-    dispatchInt(16[PB, PC, 8])
-
-
-_llint_op_sret:
-    traceExecution()
-    loadis 8[PB, PC, 8], t0
-    loadi [cfr, t0, 8], PC
-    dispatch(0)
-
-
 _llint_op_end:
     traceExecution()