[JAEGER] Bug 587833 reland part 2: remove VMFrame::scriptedReturn ARM fixes
authorJacob Bramley <jacob.bramley@arm.com>
Fri, 20 Aug 2010 13:21:46 -0700
changeset 53472 9d32c04f0ef8251b80aa6684ca2612d3f62a58d0
parent 53471 ccf68d4e76fea7fa25af20c60c00cd2d83196dd0
child 53473 3067044ca8b25c58676a7f15e63c5748285fe1e2
push id1
push userroot
push dateTue, 26 Apr 2011 22:38:44 +0000
treeherdermozilla-beta@bfdb6e623a36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs587833
milestone2.0b5pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
[JAEGER] Bug 587833 reland part 2: remove VMFrame::scriptedReturn ARM fixes
js/src/methodjit/BaseAssembler.h
js/src/methodjit/Compiler.cpp
js/src/methodjit/MethodJIT.cpp
js/src/methodjit/TrampolineCompiler.cpp
--- a/js/src/methodjit/BaseAssembler.h
+++ b/js/src/methodjit/BaseAssembler.h
@@ -308,27 +308,27 @@ static const JSC::MacroAssembler::Regist
         callPatches.append(CallPatch(differenceBetween(startLabel, cl), fun));
         return cl;
     }
 
     Call call(RegisterID reg) {
         return MacroAssembler::call(reg);
     }
 
-#if defined(JS_CPU_ARM)
-    void ret() {
-        /* The return sequence emitted by the ARM macro-assembler assumes that the return address
-         * is in LR. We could load it into LR before calling it, but it's probably better to simply
-         * pop into the PC. (Note that JaegerTrampoline expects the return sequence to pop this
-         * single word from the stack, so the stack will be unaligned on return from JIT code.
-         * JaegerTrampoline fixes this up.) */
-        MacroAssembler::pop(JSC::ARMRegisters::pc);
+    void restoreReturnAddress()
+    {
+#ifndef JS_CPU_ARM
+        /* X86 and X64's "ret" instruction expects a return address on the stack. */
+        push(Address(JSFrameReg, offsetof(JSStackFrame, ncode)));
+#else
+        /* ARM returns either using its link register (LR) or directly from the stack, but masm.ret()
+         * always emits a return to LR. */
+        load32(Address(JSFrameReg, offsetof(JSStackFrame, ncode)), JSC::ARMRegisters::lr);
+#endif
     }
-    // #else fall back to the inherited implementation in MacroAssembler::ret().
-#endif
 
     void finalize(uint8 *ncode) {
         JSC::JITCode jc(ncode, size());
         JSC::CodeBlock cb(jc);
         JSC::RepatchBuffer repatchBuffer(&cb);
 
         for (size_t i = 0; i < callPatches.length(); i++) {
             JSC::MacroAssemblerCodePtr cp(ncode + callPatches[i].distance);
--- a/js/src/methodjit/Compiler.cpp
+++ b/js/src/methodjit/Compiler.cpp
@@ -1557,37 +1557,27 @@ mjit::Compiler::jsop_getglobal(uint32 in
 
     RegisterID reg = frame.allocReg();
     Address address = masm.objSlotRef(globalObj, reg, slot);
     frame.freeReg(reg);
     frame.push(address);
 }
 
 void
-mjit::Compiler::restoreReturnAddress(Assembler &masm)
-{
-#ifndef JS_CPU_ARM
-    masm.push(Address(JSFrameReg, offsetof(JSStackFrame, ncode)));
-#else
-    masm.move(Address(JSFrameReg, offsetof(JSStackFrame, ncode)), JSC::ARMRegisters::lr);
-#endif
-}
-
-void
 mjit::Compiler::emitReturn()
 {
     /*
      * if (!f.inlineCallCount)
      *     return;
      */
     Jump noInlineCalls = masm.branchPtr(Assembler::Equal,
                                         FrameAddress(offsetof(VMFrame, entryFp)),
                                         JSFrameReg);
     stubcc.linkExit(noInlineCalls, Uses(frame.frameDepth()));
-    restoreReturnAddress(stubcc.masm);
+    stubcc.masm.restoreReturnAddress();
     stubcc.masm.ret();
 
     JS_ASSERT_IF(!fun, JSOp(*PC) == JSOP_STOP);
 
     /*
      * If there's a function object, deal with the fact that it can escape.
      * Note that after we've placed the call object, all tracked state can
      * be thrown away. This will happen anyway because the next live opcode
@@ -1638,17 +1628,17 @@ mjit::Compiler::emitReturn()
     masm.storePtr(Registers::ReturnReg, Address(Registers::ArgReg1, offsetof(JSContext, fp)));
 
     JS_STATIC_ASSERT(Registers::ReturnReg != JSReturnReg_Data);
     JS_STATIC_ASSERT(Registers::ReturnReg != JSReturnReg_Type);
 
     Address rval(JSFrameReg, JSStackFrame::offsetReturnValue());
     masm.loadPayload(rval, JSReturnReg_Data);
     masm.loadTypeTag(rval, JSReturnReg_Type);
-    restoreReturnAddress(masm);
+    masm.restoreReturnAddress();
     masm.move(Registers::ReturnReg, JSFrameReg);
 #ifdef DEBUG
     masm.storePtr(ImmPtr(JSStackFrame::sInvalidPC),
                   Address(JSFrameReg, offsetof(JSStackFrame, savedPC)));
 #endif
     masm.ret();
 }
 
--- a/js/src/methodjit/MethodJIT.cpp
+++ b/js/src/methodjit/MethodJIT.cpp
@@ -51,48 +51,47 @@ using namespace js::mjit;
 
 /*
  * Explanation of VMFrame activation and various helper thunks below.
  *
  * JaegerTrampoline  - Executes a method JIT-compiled JSFunction. This function
  *    creates a VMFrame on the machine stack and calls into JIT'd code. The JIT'd
  *    code will eventually return to the VMFrame.
  *
- *  - Called from C++ functions JaegerShot and JaegerBomb.
+ *  - Called from C++ function EnterMethodJIT.
  *  - Parameters: cx, fp, code, stackLimit, safePoint
  *  - Notes: safePoint is used in combination with SafePointTrampoline,
  *           explained further down.
  *
  * JaegerThrowpoline - Calls into an exception handler from JIT'd code, and if a
  *    scripted exception handler is not found, unwinds the VMFrame and returns
  *    to C++.
  *
  *  - To start exception handling, we return from a stub call to the throwpoline.
  *  - On entry to the throwpoline, the normal conditions of the jit-code ABI
  *    are satisfied.
  *  - To do the unwinding and find out where to continue executing, we call
  *    js_InternalThrow.
  *  - js_InternalThrow may return 0, which means the place to continue, if any,
  *    is above this JaegerShot activation, so we just return, in the same way
  *    the trampoline does.
- *  - Otherwise, js_InternalThrow returns a jit-code address to continue
- *    execution
- *    at. Because the jit-code ABI conditions are satisfied, we can just jump
- *    to that point.
+ *  - Otherwise, js_InternalThrow returns a jit-code address to continue execution
+ *    at. Because the jit-code ABI conditions are satisfied, we can just jump to
+ *    that point.
  *
  *
  * SafePointTrampoline  - Inline script calls link their return addresses through
  *    JSStackFrame::ncode. This includes the return address that unwinds back
  *    to JaegerTrampoline. However, the tracer integration code often wants to
  *    enter a method JIT'd function at an arbitrary safe point. Safe points
  *    do not have the return address linking code that the method prologue has.
  *    SafePointTrampoline is a thunk which correctly links the initial return
- *    address. It is used in JaegerBomb, and passed as the "script code"
- *    parameter. Using the "safePoint" parameter to JaegerTrampoline, it correctly
- *    jumps to the intended point in the method.
+ *    address. It is used in JaegerShotAtSafePoint, and passed as the "script
+ *    code" parameter. Using the "safePoint" parameter to JaegerTrampoline, it
+ *    correctly jumps to the intended point in the method.
  *
  *  - Used by JaegerTrampoline()
  *
  * InjectJaegerReturn - Implements the tail of InlineReturn. This is needed for
  *    tracer integration, where a "return" opcode might not be a safe-point,
  *    and thus the return path must be injected by hijacking the stub return
  *    address.
  *
@@ -211,18 +210,17 @@ SYMBOL_STRING(JaegerTrampoline) ":"     
     /* Set cx->regs and set the active frame. Save rdx and align frame in one. */
     "pushq %rdx"                         "\n"
     "movq  %rsp, %rdi"                   "\n"
     "call " SYMBOL_STRING_RELOC(SetVMFrameRegs) "\n"
     "movq  %rsp, %rdi"                   "\n"
     "call " SYMBOL_STRING_RELOC(PushActiveVMFrame) "\n"
 
     /*
-     * Jump into into the JIT'd code. The call implicitly fills in
-     * the precious f.scriptedReturn member of VMFrame.
+     * Jump into into the JIT'd code.
      */
     "call *0(%rsp)"                      "\n"
     "movq %rsp, %rdi"                    "\n"
     "call " SYMBOL_STRING_RELOC(PopActiveVMFrame) "\n"
     "movq %rsp, %rdi"                    "\n"
     "call " SYMBOL_STRING_RELOC(UnsetVMFrameRegs) "\n"
 
     "addq $0x58, %rsp"                   "\n"
@@ -408,19 +406,17 @@ SYMBOL_STRING(SafePointTrampoline) ":"  
 JS_STATIC_ASSERT(sizeof(VMFrame) == 80);
 JS_STATIC_ASSERT(offsetof(VMFrame, savedLR) ==          (4*19));
 JS_STATIC_ASSERT(offsetof(VMFrame, entryFp) ==          (4*10));
 JS_STATIC_ASSERT(offsetof(VMFrame, stackLimit) ==       (4*9));
 JS_STATIC_ASSERT(offsetof(VMFrame, cx) ==               (4*8));
 JS_STATIC_ASSERT(offsetof(VMFrame, fp) ==               (4*7));
 JS_STATIC_ASSERT(offsetof(VMFrame, oldRegs) ==          (4*4));
 JS_STATIC_ASSERT(offsetof(VMFrame, previous) ==         (4*3));
-JS_STATIC_ASSERT(offsetof(VMFrame, scriptedReturn) ==   (4*0));
 JS_STATIC_ASSERT(offsetof(JSStackFrame, ncode) == 60);
-JS_STATIC_ASSERT(offsetof(JSStackFrame, rval) == 40);
 
 asm volatile (
 ".text\n"
 ".globl " SYMBOL_STRING(InjectJaegerReturn)   "\n"
 SYMBOL_STRING(InjectJaegerReturn) ":"         "\n"
     /* Restore frame regs. */
     "ldr r1, [r11, #40]"                    "\n" /* fp->rval data */
     "ldr r2, [r11, #44]"                    "\n" /* fp->rval type */
@@ -428,31 +424,40 @@ SYMBOL_STRING(InjectJaegerReturn) ":"   
     "ldr r11, [sp, #28]"                    "\n" /* load f.fp */
     "bx  r0"                                "\n"
 );
 
 asm volatile (
 ".text\n"
 ".globl " SYMBOL_STRING(SafePointTrampoline)  "\n"
 SYMBOL_STRING(SafePointTrampoline) ":"
-    "str lr, [r11, #60]"                    "\n"
-    /* This should load the fifth parameter from JaegerTrampoline and jump to it. */
-    ""                                 "\n"
+    /*
+     * On entry to SafePointTrampoline:
+     *         r11 = fp
+     *      sp[80] = safePoint
+     */
+    "ldr    ip, [sp, #80]"                  "\n"
+    /* Save the return address (in JaegerTrampoline) to fp->ncode. */
+    "str    lr, [r11, #60]"                 "\n"
+    /* Jump to 'safePoint' via 'ip' because a load into the PC from an address on
+     * the stack looks like a return, and may upset return stack prediction. */
+    "bx     ip"                             "\n"
 );
 
 asm volatile (
 ".text\n"
 ".globl " SYMBOL_STRING(JaegerTrampoline)   "\n"
 SYMBOL_STRING(JaegerTrampoline) ":"         "\n"
     /*
      * On entry to JaegerTrampoline:
-     *      r0 = cx
-     *      r1 = fp
-     *      r2 = code
-     *      r3 = inlineCallCount
+     *         r0 = cx
+     *         r1 = fp
+     *         r2 = code
+     *         r3 = stackLimit
+     *      sp[0] = safePoint
      *
      * The VMFrame for ARM looks like this:
      *  [ lr        ]   \
      *  [ r11       ]   |
      *  [ r10       ]   |
      *  [ r9        ]   | Callee-saved registers.                             
      *  [ r8        ]   | VFP registers d8-d15 may be required here too, but  
      *  [ r7        ]   | unconditionally preserving them might be expensive
@@ -462,35 +467,37 @@ SYMBOL_STRING(JaegerTrampoline) ":"     
      *  [ entryFp   ]
      *  [ stkLimit  ]
      *  [ cx        ]
      *  [ fp        ]
      *  [ regs.sp   ]
      *  [ regs.pc   ]
      *  [ oldRegs   ]
      *  [ previous  ]
-     *  [ args.ptr  ]
+     *  [ args.ptr3 ]
      *  [ args.ptr2 ]
-     *  [ srpt. ret ]   } Scripted return.
+     *  [ args.ptr  ]
      */
     
-    /* Push callee-saved registers. TODO: Do we actually need to push all of them? If the
-     * compiled JavaScript function is EABI-compliant, we only need to push what we use in
-     * JaegerTrampoline. */
+    /* Push callee-saved registers. */
 "   push    {r4-r11,lr}"                        "\n"
     /* Push interesting VMFrame content. */
 "   push    {r1}"                               "\n"    /* entryFp */
 "   push    {r3}"                               "\n"    /* stackLimit */
 "   push    {r0}"                               "\n"    /* cx */
 "   push    {r1}"                               "\n"    /* fp */
     /* Remaining fields are set elsewhere, but we need to leave space for them. */
 "   sub     sp, sp, #(4*7)"                     "\n"
 
+    /* Preserve 'code' (r2) in an arbitrary callee-saved register. */
+"   mov     r4, r2"                             "\n"
+    /* Preserve 'fp' (r1) in r11 (JSFrameReg) for SafePointTrampoline. */
+"   mov     r11, r1"                            "\n"
+
 "   mov     r0, sp"                             "\n"
-"   mov     r4, r2"                             "\n"    /* Preserve r2 ('code') in a callee-saved register. */
 "   bl  " SYMBOL_STRING_RELOC(SetVMFrameRegs)   "\n"
 "   mov     r0, sp"                             "\n"
 "   bl  " SYMBOL_STRING_RELOC(PushActiveVMFrame)"\n"
 
     /* Call the compiled JavaScript function. */
 "   blx     r4"                                 "\n"
 
     /* Tidy up. */
@@ -506,30 +513,32 @@ SYMBOL_STRING(JaegerTrampoline) ":"     
 "   mov     r0, #1"                         "\n"
 "   pop     {r4-r11,pc}"                    "\n"
 );
 
 asm volatile (
 ".text\n"
 ".globl " SYMBOL_STRING(JaegerThrowpoline)  "\n"
 SYMBOL_STRING(JaegerThrowpoline) ":"        "\n"
-    /* Restore 'f', as it will have been clobbered. */
+    /* Find the VMFrame pointer for js_InternalThrow. */
 "   mov     r0, sp"                         "\n"
 
     /* Call the utility function that sets up the internal throw routine. */
 "   bl  " SYMBOL_STRING_RELOC(js_InternalThrow) "\n"
     
-    /* If 0 was returned, just bail out as normal. Otherwise, we have a 'catch' or 'finally' clause
-     * to execute. */
+    /* If js_InternalThrow found a scripted handler, jump to it. Otherwise, tidy
+     * up and return. */
 "   cmp     r0, #0"                         "\n"
 "   bxne    r0"                             "\n"
 
-    /* Skip past the parameters we pushed (such as cx and the like). */
-"   add     sp, sp, #(4*7 + 4*4)"           "\n"
-
+    /* Tidy up, then return '0' to represent an unhandled exception. */
+"   mov     r0, sp"                             "\n"
+"   bl  " SYMBOL_STRING_RELOC(PopActiveVMFrame) "\n"
+"   add     sp, sp, #(4*7 + 4*4)"               "\n"
+"   mov     r0, #0"                         "\n"
 "   pop     {r4-r11,pc}"                    "\n"
 );
 
 asm volatile (
 ".text\n"
 ".globl " SYMBOL_STRING(JaegerStubVeneer)   "\n"
 SYMBOL_STRING(JaegerStubVeneer) ":"         "\n"
     /* We enter this function as a veneer between a compiled method and one of the js_ stubs. We
--- a/js/src/methodjit/TrampolineCompiler.cpp
+++ b/js/src/methodjit/TrampolineCompiler.cpp
@@ -134,21 +134,17 @@ TrampolineCompiler::generateForceReturn(
     masm.loadPtr(FrameAddress(offsetof(VMFrame, cx)), Registers::ArgReg1);
     masm.storePtr(Registers::ReturnReg, FrameAddress(offsetof(VMFrame, fp)));
     masm.storePtr(Registers::ReturnReg, Address(Registers::ArgReg1, offsetof(JSContext, fp)));
 
     Address rval(JSFrameReg, JSStackFrame::offsetReturnValue());
     masm.loadPayload(rval, JSReturnReg_Data);
     masm.loadTypeTag(rval, JSReturnReg_Type);
 
-#ifndef JS_CPU_ARM
-    masm.push(Address(JSFrameReg, offsetof(JSStackFrame, ncode)));
-#else
-    masm.move(Address(JSFrameReg, offsetof(JSStackFrame, ncode)), JSC::ARMRegisters::lr);
-#endif
+    masm.restoreReturnAddress();
 
     masm.move(Registers::ReturnReg, JSFrameReg);
 #ifdef DEBUG
     masm.storePtr(ImmPtr(JSStackFrame::sInvalidPC),
                   Address(JSFrameReg, offsetof(JSStackFrame, savedPC)));
 #endif
 
     masm.ret();