Bug 1027885 - Don't clobber arg registers in the profiling builtin thunk on ARM (r=dougc)

--HG--
extra : rebase_source : e735dfac5a1e6652049d13145e165dcc98b97b5c

js/src/jit/AsmJS.cpp

@@ -5947,13 +5947,19 @@ static const unsigned FramePushedAfterSave = NonVolatileRegs.gprs().size() * siz
 static bool
 GenerateEntry(ModuleCompiler &m, unsigned exportIndex)
 {
-    PropertyName *funcName = m.module().exportedFunction(exportIndex).name();
-    const ModuleCompiler::Func &func = *m.lookupFunction(funcName);
-
     MacroAssembler &masm = m.masm();
 
     Label begin;
-    GenerateAsmJSEntryPrologue(masm, &begin);
+    masm.align(CodeAlignment);
+    masm.bind(&begin);
+
+#if defined(JS_CODEGEN_ARM)
+    masm.push(lr);
+#elif defined(JS_CODEGEN_MIPS)
+    masm.push(ra);
+#endif
+    masm.subPtr(Imm32(AsmJSFrameBytesAfterReturnAddress), StackPointer);
+    masm.setFramePushed(0);
 
     // In constrast to the system ABI, the Ion convention is that all registers
     // are clobbered by calls. Thus, we must save the caller's non-volatile
@@ -5979,13 +5985,13 @@ GenerateEntry(ModuleCompiler &m, unsigned exportIndex)
     // used by error exit paths to set the stack pointer back to what it was
     // right after the (C++) caller's non-volatile registers were saved so that
     // they can be restored.
-    Register activation = ABIArgGenerator::NonArgReturnVolatileReg0;
+    Register activation = ABIArgGenerator::NonArgReturnReg0;
     masm.loadAsmJSActivation(activation);
     masm.storePtr(StackPointer, Address(activation, AsmJSActivation::offsetOfErrorRejoinSP()));
 
     // Get 'argv' into a non-arg register and save it on the stack.
-    Register argv = ABIArgGenerator::NonArgReturnVolatileReg0;
-    Register scratch = ABIArgGenerator::NonArgReturnVolatileReg1;
+    Register argv = ABIArgGenerator::NonArgReturnReg0;
+    Register scratch = ABIArgGenerator::NonArgReturnReg1;
 #if defined(JS_CODEGEN_X86)
     masm.loadPtr(Address(StackPointer, sizeof(AsmJSFrame) + masm.framePushed()), argv);
 #else
@@ -5994,6 +6000,8 @@ GenerateEntry(ModuleCompiler &m, unsigned exportIndex)
     masm.Push(argv);
 
     // Bump the stack for the call.
+    PropertyName *funcName = m.module().exportedFunction(exportIndex).name();
+    const ModuleCompiler::Func &func = *m.lookupFunction(funcName);
     unsigned stackDec = StackDecrementForCall(masm, func.sig().args());
     masm.reserveStack(stackDec);
 
@@ -6052,8 +6060,9 @@ GenerateEntry(ModuleCompiler &m, unsigned exportIndex)
     JS_ASSERT(masm.framePushed() == 0);
 
     masm.move32(Imm32(true), ReturnReg);
+    masm.addPtr(Imm32(AsmJSFrameBytesAfterReturnAddress), StackPointer);
+    masm.ret();
 
-    GenerateAsmJSEntryEpilogue(masm);
     return m.finishGeneratingEntry(exportIndex, &begin) && !masm.oom();
 }
 
@@ -6122,7 +6131,7 @@ GenerateFFIInterpExit(ModuleCompiler &m, const ModuleCompiler::ExitDescriptor &e
 
     // Fill the argument array.
     unsigned offsetToCallerStackArgs = sizeof(AsmJSFrame) + masm.framePushed();
-    Register scratch = ABIArgGenerator::NonArgReturnVolatileReg0;
+    Register scratch = ABIArgGenerator::NonArgReturnReg0;
     FillArgumentArray(m, exit.sig().args(), offsetToArgv, offsetToCallerStackArgs, scratch);
 
     // Prepare the arguments for the call to InvokeFromAsmJS_*.
@@ -6241,8 +6250,8 @@ GenerateFFIIonExit(ModuleCompiler &m, const ModuleCompiler::ExitDescriptor &exit
     argOffset += sizeof(size_t);
 
     // 2. Callee
-    Register callee = ABIArgGenerator::NonArgReturnVolatileReg0;   // live until call
-    Register scratch = ABIArgGenerator::NonArgReturnVolatileReg1;  // clobbered
+    Register callee = ABIArgGenerator::NonArgReturnReg0;   // live until call
+    Register scratch = ABIArgGenerator::NonArgReturnReg1;  // repeatedly clobbered
 
     // 2.1. Get ExitDatum
     unsigned globalDataOffset = m.module().exitIndexToGlobalDataOffset(exitIndex);
@@ -6461,6 +6470,10 @@ GenerateFFIExits(ModuleCompiler &m, const ModuleCompiler::ExitDescriptor &exit,
 // pushes an AsmJSFrame on the stack, that means we must rebuild the stack
 // frame. Fortunately, these are low arity functions and everything is passed in
 // regs on everything but x86 anyhow.
+//
+// NB: Since this thunk is being injected at system ABI callsites, it must
+//     preserve the argument registers (going in) and the return register
+//     (coming out) and preserve non-volatile registers.
 static bool
 GenerateBuiltinThunk(ModuleCompiler &m, AsmJSExit::BuiltinKind builtin)
 {
@@ -6510,20 +6523,24 @@ GenerateBuiltinThunk(ModuleCompiler &m, AsmJSExit::BuiltinKind builtin)
     Label begin;
     GenerateAsmJSExitPrologue(masm, framePushed, AsmJSExit::Builtin(builtin), &begin);
 
-    unsigned offsetToCallerStackArgs = sizeof(AsmJSFrame) + masm.framePushed();
     for (ABIArgMIRTypeIter i(argTypes); !i.done(); i++) {
         if (i->kind() != ABIArg::Stack)
             continue;
+#if !defined(JS_CODEGEN_ARM)
+        unsigned offsetToCallerStackArgs = sizeof(AsmJSFrame) + masm.framePushed();
         Address srcAddr(StackPointer, offsetToCallerStackArgs + i->offsetFromArgBase());
         Address dstAddr(StackPointer, i->offsetFromArgBase());
         if (i.mirType() == MIRType_Int32 || i.mirType() == MIRType_Float32) {
-            masm.load32(srcAddr, ABIArgGenerator::NonArgReturnVolatileReg0);
-            masm.store32(ABIArgGenerator::NonArgReturnVolatileReg0, dstAddr);
+            masm.load32(srcAddr, ABIArgGenerator::NonArg_VolatileReg);
+            masm.store32(ABIArgGenerator::NonArg_VolatileReg, dstAddr);
         } else {
             JS_ASSERT(i.mirType() == MIRType_Double);
             masm.loadDouble(srcAddr, ScratchDoubleReg);
             masm.storeDouble(ScratchDoubleReg, dstAddr);
         }
+#else
+        MOZ_CRASH("Architecture should have enough registers for all builtin calls");
+#endif
     }
 
     AssertStackAlignment(masm);
@@ -6571,7 +6588,7 @@ GenerateAsyncInterruptExit(ModuleCompiler &m, Label *throwLabel)
     masm.setFramePushed(0);         // set to zero so we can use masm.framePushed() below
     masm.PushRegsInMask(AllRegsExceptSP); // save all GP/FP registers (except SP)
 
-    Register scratch = ABIArgGenerator::NonArgReturnVolatileReg0;
+    Register scratch = ABIArgGenerator::NonArgReturnReg0;
 
     // Store resumePC into the reserved space.
     masm.loadAsmJSActivation(scratch);
@@ -6726,7 +6743,7 @@ GenerateThrowStub(ModuleCompiler &m, Label *throwLabel)
     // We are about to pop all frames in this AsmJSActivation. Set fp to null to
     // maintain the invariant that fp is either null or pointing to a valid
     // frame.
-    Register activation = ABIArgGenerator::NonArgReturnVolatileReg0;
+    Register activation = ABIArgGenerator::NonArgReturnReg0;
     masm.loadAsmJSActivation(activation);
     masm.storePtr(ImmWord(0), Address(activation, AsmJSActivation::offsetOfFP()));
 

js/src/jit/AsmJSFrameIterator.cpp

@@ -99,12 +99,12 @@ AsmJSFrameIterator::computeLine(uint32_t *column) const
 // generation.
 #if defined(JS_CODEGEN_X64)
 static const unsigned PushedRetAddr = 0;
-static const unsigned PushedFP = 11;
-static const unsigned StoredFP = 15;
+static const unsigned PushedFP = 10;
+static const unsigned StoredFP = 14;
 #elif defined(JS_CODEGEN_X86)
 static const unsigned PushedRetAddr = 0;
-static const unsigned PushedFP = 9;
-static const unsigned StoredFP = 12;
+static const unsigned PushedFP = 8;
+static const unsigned StoredFP = 11;
 #elif defined(JS_CODEGEN_ARM)
 static const unsigned PushedRetAddr = 4;
 static const unsigned PushedFP = 16;
@@ -136,7 +136,15 @@ static void
 GenerateProfilingPrologue(MacroAssembler &masm, unsigned framePushed, AsmJSExit::Reason reason,
                           Label *begin)
 {
-    Register act = ABIArgGenerator::NonArgReturnVolatileReg0;
+#if !defined (JS_CODEGEN_ARM)
+    Register scratch = ABIArgGenerator::NonArg_VolatileReg;
+#else
+    // Unfortunately, there are no unused non-arg volatile registers on ARM --
+    // the MacroAssembler claims both lr and ip -- so we use the second scratch
+    // register (lr) and be very careful not to call any methods that use it.
+    Register scratch = lr;
+    masm.setSecondScratchReg(InvalidReg);
+#endif
 
     // AsmJSProfilingFrameIterator needs to know the offsets of several key
     // instructions from 'begin'. To save space, we make these offsets static
@@ -153,16 +161,20 @@ GenerateProfilingPrologue(MacroAssembler &masm, unsigned framePushed, AsmJSExit:
         PushRetAddr(masm);
         JS_ASSERT(PushedRetAddr == masm.currentOffset() - offsetAtBegin);
 
-        masm.loadAsmJSActivation(act);
-        masm.push(Address(act, AsmJSActivation::offsetOfFP()));
+        masm.loadAsmJSActivation(scratch);
+        masm.push(Address(scratch, AsmJSActivation::offsetOfFP()));
         JS_ASSERT(PushedFP == masm.currentOffset() - offsetAtBegin);
 
-        masm.storePtr(StackPointer, Address(act, AsmJSActivation::offsetOfFP()));
+        masm.storePtr(StackPointer, Address(scratch, AsmJSActivation::offsetOfFP()));
         JS_ASSERT(StoredFP == masm.currentOffset() - offsetAtBegin);
     }
 
     if (reason != AsmJSExit::None)
-        masm.store32(Imm32(reason), Address(act, AsmJSActivation::offsetOfExitReason()));
+        masm.store32_NoSecondScratch(Imm32(reason), Address(scratch, AsmJSActivation::offsetOfExitReason()));
+
+#if defined(JS_CODEGEN_ARM)
+    masm.setSecondScratchReg(lr);
+#endif
 
     if (framePushed)
         masm.subPtr(Imm32(framePushed), StackPointer);
@@ -173,15 +185,18 @@ static void
 GenerateProfilingEpilogue(MacroAssembler &masm, unsigned framePushed, AsmJSExit::Reason reason,
                           Label *profilingReturn)
 {
-    Register act = ABIArgGenerator::NonArgReturnVolatileReg0;
+    Register scratch = ABIArgGenerator::NonReturn_VolatileReg0;
+#if defined(JS_CODEGEN_ARM) || defined(JS_CODEGEN_MIPS)
+    Register scratch2 = ABIArgGenerator::NonReturn_VolatileReg1;
+#endif
 
     if (framePushed)
         masm.addPtr(Imm32(framePushed), StackPointer);
 
-    masm.loadAsmJSActivation(act);
+    masm.loadAsmJSActivation(scratch);
 
     if (reason != AsmJSExit::None)
-        masm.store32(Imm32(AsmJSExit::None), Address(act, AsmJSActivation::offsetOfExitReason()));
+        masm.store32(Imm32(AsmJSExit::None), Address(scratch, AsmJSActivation::offsetOfExitReason()));
 
     // AsmJSProfilingFrameIterator assumes that there is only a single 'ret'
     // instruction (whose offset is recorded by profilingReturn) after the store
@@ -192,12 +207,11 @@ GenerateProfilingEpilogue(MacroAssembler &masm, unsigned framePushed, AsmJSExit:
 #if defined(JS_CODEGEN_ARM)
         AutoForbidPools afp(&masm, /* number of instructions in scope = */ 3);
 #endif
-#if defined(JS_CODEGEN_X86) || defined(JS_CODEGEN_X64)
-        masm.pop(Operand(act, AsmJSActivation::offsetOfFP()));
+#if defined(JS_CODEGEN_ARM) || defined(JS_CODEGEN_MIPS)
+        masm.pop(scratch2);
+        masm.storePtr(scratch2, Address(scratch, AsmJSActivation::offsetOfFP()));
 #else
-        Register fp = ABIArgGenerator::NonArgReturnVolatileReg1;
-        masm.pop(fp);
-        masm.storePtr(fp, Address(act, AsmJSActivation::offsetOfFP()));
+        masm.pop(Operand(scratch, AsmJSActivation::offsetOfFP()));
 #endif
         masm.bind(profilingReturn);
         masm.ret();
@@ -315,7 +329,7 @@ js::GenerateAsmJSStackOverflowExit(MacroAssembler &masm, Label *overflowExit, La
     // value again. Do not update AsmJSFrame::callerFP as it is not necessary in
     // the non-profiling case (there is no return path from this point) and, in
     // the profiling case, it is already correct.
-    Register activation = ABIArgGenerator::NonArgReturnVolatileReg0;
+    Register activation = ABIArgGenerator::NonArgReturnReg0;
     masm.loadAsmJSActivation(activation);
     masm.storePtr(StackPointer, Address(activation, AsmJSActivation::offsetOfFP()));
 
@@ -329,29 +343,6 @@ js::GenerateAsmJSStackOverflowExit(MacroAssembler &masm, Label *overflowExit, La
     masm.jump(throwLabel);
 }
 
-void
-js::GenerateAsmJSEntryPrologue(MacroAssembler &masm, Label *begin)
-{
-    // Stack-unwinding stops at the entry prologue, so there is no need to
-    // update AsmJSActivation::fp. Furthermore, on ARM/MIPS, GlobalReg is not
-    // yet initialized, so we can't even if we wanted to.
-    masm.align(CodeAlignment);
-    masm.bind(begin);
-    PushRetAddr(masm);
-    masm.subPtr(Imm32(AsmJSFrameBytesAfterReturnAddress), StackPointer);
-    masm.setFramePushed(0);
-}
-
-void
-js::GenerateAsmJSEntryEpilogue(MacroAssembler &masm)
-{
-    // Inverse of GenerateAsmJSEntryPrologue:
-    JS_ASSERT(masm.framePushed() == 0);
-    masm.addPtr(Imm32(AsmJSFrameBytesAfterReturnAddress), StackPointer);
-    masm.ret();
-    masm.setFramePushed(0);
-}
-
 void
 js::GenerateAsmJSExitPrologue(MacroAssembler &masm, unsigned framePushed, AsmJSExit::Reason reason,
                               Label *begin)

js/src/jit/AsmJSFrameIterator.h

@@ -154,11 +154,6 @@ void
 GenerateAsmJSStackOverflowExit(jit::MacroAssembler &masm, jit::Label *overflowExit,
                                jit::Label *throwLabel);
 
-void
-GenerateAsmJSEntryPrologue(jit::MacroAssembler &masm, jit::Label *begin);
-void
-GenerateAsmJSEntryEpilogue(jit::MacroAssembler &masm);
-
 void
 GenerateAsmJSExitPrologue(jit::MacroAssembler &masm, unsigned framePushed, AsmJSExit::Reason reason,
                           jit::Label *begin);

js/src/jit/arm/Assembler-arm.cpp

@@ -77,8 +77,11 @@ ABIArgGenerator::next(MIRType type)
 
     return current_;
 }
-const Register ABIArgGenerator::NonArgReturnVolatileReg0 = r4;
-const Register ABIArgGenerator::NonArgReturnVolatileReg1 = r5;
+
+const Register ABIArgGenerator::NonArgReturnReg0 = r4;
+const Register ABIArgGenerator::NonArgReturnReg1 = r5;
+const Register ABIArgGenerator::NonReturn_VolatileReg0 = r2;
+const Register ABIArgGenerator::NonReturn_VolatileReg1 = r3;
 
 // Encode a standard register when it is being used as src1, the dest, and an
 // extra register. These should never be called with an InvalidReg.

js/src/jit/arm/Assembler-arm.h

@@ -63,6 +63,7 @@ static MOZ_CONSTEXPR_VAR Register HeapReg = r11;
 static MOZ_CONSTEXPR_VAR Register CallTempNonArgRegs[] = { r5, r6, r7, r8 };
 static const uint32_t NumCallTempNonArgRegs =
     mozilla::ArrayLength(CallTempNonArgRegs);
+
 class ABIArgGenerator
 {
     unsigned intRegIndex_;
@@ -75,8 +76,10 @@ class ABIArgGenerator
     ABIArg next(MIRType argType);
     ABIArg &current() { return current_; }
     uint32_t stackBytesConsumedSoFar() const { return stackOffset_; }
-    static const Register NonArgReturnVolatileReg0;
-    static const Register NonArgReturnVolatileReg1;
+    static const Register NonArgReturnReg0;
+    static const Register NonArgReturnReg1;
+    static const Register NonReturn_VolatileReg0;
+    static const Register NonReturn_VolatileReg1;
 };
 
 static MOZ_CONSTEXPR_VAR Register PreBarrierReg = r1;

js/src/jit/arm/MacroAssembler-arm.cpp

@@ -2457,6 +2457,13 @@ MacroAssemblerARMCompat::store32(Register src, const BaseIndex &dest)
     ma_str(src, DTRAddr(base, DtrRegImmShift(dest.index, LSL, scale)));
 }
 
+void
+MacroAssemblerARMCompat::store32_NoSecondScratch(Imm32 src, const Address &address)
+{
+    move32(src, ScratchRegister);
+    storePtr(ScratchRegister, address);
+}
+
 void
 MacroAssemblerARMCompat::storePtr(ImmWord imm, const Address &address)
 {

js/src/jit/arm/MacroAssembler-arm.h

@@ -1358,6 +1358,8 @@ class MacroAssemblerARMCompat : public MacroAssemblerARM
     void store32(Imm32 src, const Address &address);
     void store32(Imm32 src, const BaseIndex &address);
 
+    void store32_NoSecondScratch(Imm32 src, const Address &address);
+
     void storePtr(ImmWord imm, const Address &address);
     void storePtr(ImmPtr imm, const Address &address);
     void storePtr(ImmGCPtr imm, const Address &address);

js/src/jit/mips/Assembler-mips.cpp

@@ -64,8 +64,11 @@ ABIArgGenerator::next(MIRType type)
     return current_;
 
 }
-const Register ABIArgGenerator::NonArgReturnVolatileReg0 = t0;
-const Register ABIArgGenerator::NonArgReturnVolatileReg1 = t1;
+const Register ABIArgGenerator::NonArgReturnReg0 = t0;
+const Register ABIArgGenerator::NonArgReturnReg1 = t1;
+const Register ABIArgGenerator::NonArg_VolatileReg = v0;
+const Register ABIArgGenerator::NonReturn_VolatileReg0 = a0;
+const Register ABIArgGenerator::NonReturn_VolatileReg1 = a1;
 
 // Encode a standard register when it is being used as rd, the rs, and
 // an extra register(rt). These should never be called with an InvalidReg.

js/src/jit/mips/Assembler-mips.h

@@ -94,8 +94,11 @@ class ABIArgGenerator
         return usedArgSlots_ * sizeof(intptr_t);
     }
 
-    static const Register NonArgReturnVolatileReg0;
-    static const Register NonArgReturnVolatileReg1;
+    static const Register NonArgReturnReg0;
+    static const Register NonArgReturnReg1;
+    static const Register NonArg_VolatileReg;
+    static const Register NonReturn_VolatileReg0;
+    static const Register NonReturn_VolatileReg1;
 };
 
 static MOZ_CONSTEXPR_VAR Register PreBarrierReg = a1;

js/src/jit/shared/MacroAssembler-x86-shared.h

@@ -383,6 +383,10 @@ class MacroAssemblerX86Shared : public Assembler
     void store32(const S &src, const T &dest) {
         movl(src, Operand(dest));
     }
+    template <typename S, typename T>
+    void store32_NoSecondScratch(const S &src, const T &dest) {
+        store32(src, dest);
+    }
     void loadDouble(const Address &src, FloatRegister dest) {
         movsd(src, dest);
     }

js/src/jit/x64/Assembler-x64.cpp

@@ -75,9 +75,11 @@ ABIArgGenerator::next(MIRType type)
 }
 
 // Avoid r11, which is the MacroAssembler's ScratchReg.
-const Register ABIArgGenerator::NonArgReturnVolatileReg0 = r10;
-const Register ABIArgGenerator::NonArgReturnVolatileReg1 = r12;
+const Register ABIArgGenerator::NonArgReturnReg0 = r10;
+const Register ABIArgGenerator::NonArgReturnReg1 = r12;
 const Register ABIArgGenerator::NonVolatileReg = r13;
+const Register ABIArgGenerator::NonArg_VolatileReg = rax;
+const Register ABIArgGenerator::NonReturn_VolatileReg0 = rcx;
 
 void
 Assembler::writeRelocation(JmpSrc src, Relocation::Kind reloc)

js/src/jit/x64/Assembler-x64.h

@@ -170,9 +170,11 @@ class ABIArgGenerator
     uint32_t stackBytesConsumedSoFar() const { return stackOffset_; }
 
     // Note: these registers are all guaranteed to be different
-    static const Register NonArgReturnVolatileReg0;
-    static const Register NonArgReturnVolatileReg1;
+    static const Register NonArgReturnReg0;
+    static const Register NonArgReturnReg1;
     static const Register NonVolatileReg;
+    static const Register NonArg_VolatileReg;
+    static const Register NonReturn_VolatileReg0;
 };
 
 static MOZ_CONSTEXPR_VAR Register OsrFrameReg = IntArgReg3;

js/src/jit/x86/Assembler-x86.cpp

@@ -35,9 +35,11 @@ ABIArgGenerator::next(MIRType type)
     return current_;
 }
 
-const Register ABIArgGenerator::NonArgReturnVolatileReg0 = ecx;
-const Register ABIArgGenerator::NonArgReturnVolatileReg1 = edx;
+const Register ABIArgGenerator::NonArgReturnReg0 = ecx;
+const Register ABIArgGenerator::NonArgReturnReg1 = edx;
 const Register ABIArgGenerator::NonVolatileReg = ebx;
+const Register ABIArgGenerator::NonArg_VolatileReg = eax;
+const Register ABIArgGenerator::NonReturn_VolatileReg0 = ecx;
 
 void
 Assembler::executableCopy(uint8_t *buffer)

js/src/jit/x86/Assembler-x86.h

@@ -81,9 +81,11 @@ class ABIArgGenerator
     uint32_t stackBytesConsumedSoFar() const { return stackOffset_; }
 
     // Note: these registers are all guaranteed to be different
-    static const Register NonArgReturnVolatileReg0;
-    static const Register NonArgReturnVolatileReg1;
+    static const Register NonArgReturnReg0;
+    static const Register NonArgReturnReg1;
     static const Register NonVolatileReg;
+    static const Register NonArg_VolatileReg;
+    static const Register NonReturn_VolatileReg0;
 };
 
 static MOZ_CONSTEXPR_VAR Register OsrFrameReg = edx;