Bug 1001346 - IonMonkey MIPS: Updating asm.js stack alignment (followup for bug 860736). r=luke
authorBranislav Rankov <branislav.rankov@imgtec.com>
Thu, 05 Jun 2014 13:02:36 +0200
changeset 186837 b56930ff7e05d909c0b3932b21f7bd2a527237e7
parent 186836 f7c59e556cc6df73c97f537867582daf99c6cb51
child 186838 d09bea16045c03dd1fd05d8630a0be6902a587a1
push idunknown
push userunknown
push dateunknown
reviewersluke
bugs1001346, 860736
milestone32.0a1
Bug 1001346 - IonMonkey MIPS: Updating asm.js stack alignment (followup for bug 860736). r=luke
js/src/jit/arm/MacroAssembler-arm.cpp
js/src/jit/mips/Assembler-mips.h
js/src/jit/mips/CodeGenerator-mips.cpp
js/src/jit/mips/CodeGenerator-mips.h
js/src/jit/mips/MacroAssembler-mips.cpp
js/src/jit/mips/MacroAssembler-mips.h
--- a/js/src/jit/arm/MacroAssembler-arm.cpp
+++ b/js/src/jit/arm/MacroAssembler-arm.cpp
@@ -3871,17 +3871,17 @@ MacroAssemblerARMCompat::callWithABIPre(
 {
     JS_ASSERT(inCall_);
 
     *stackAdjust = ((usedIntSlots_ > NumIntArgRegs) ? usedIntSlots_ - NumIntArgRegs : 0) * sizeof(intptr_t);
 #if defined(JS_CODEGEN_ARM_HARDFP) || defined(JS_ARM_SIMULATOR)
     if (useHardFpABI())
         *stackAdjust += 2*((usedFloatSlots_ > NumFloatArgRegs) ? usedFloatSlots_ - NumFloatArgRegs : 0) * sizeof(intptr_t);
 #endif
-    uint32_t alignmentAtPrologue = (callFromAsmJS) ? AlignmentAtAsmJSPrologue : 0;
+    uint32_t alignmentAtPrologue = callFromAsmJS ? AlignmentAtAsmJSPrologue : 0;
 
     if (!dynamicAlignment_) {
         *stackAdjust += ComputeByteAlignment(framePushed_ + *stackAdjust + alignmentAtPrologue,
                                              StackAlignment);
     } else {
         // sizeof(intptr_t) account for the saved stack pointer pushed by setupUnalignedABICall
         *stackAdjust += ComputeByteAlignment(*stackAdjust + sizeof(intptr_t), StackAlignment);
     }
--- a/js/src/jit/mips/Assembler-mips.h
+++ b/js/src/jit/mips/Assembler-mips.h
@@ -148,18 +148,17 @@ static MOZ_CONSTEXPR_VAR FloatRegister f
 
 // MIPS CPUs can only load multibyte data that is "naturally"
 // four-byte-aligned, sp register should be eight-byte-aligned.
 static const uint32_t StackAlignment = 8;
 static const uint32_t CodeAlignment = 4;
 static const bool StackKeptAligned = true;
 // NativeFrameSize is the size of return adress on stack in AsmJS functions.
 static const uint32_t NativeFrameSize = sizeof(void*);
-static const uint32_t AlignmentAtAsmJSPrologue = 0;
-static const uint32_t AlignmentMidPrologue = NativeFrameSize;
+static const uint32_t AlignmentAtAsmJSPrologue = sizeof(void*);
 
 static const Scale ScalePointer = TimesFour;
 
 // MIPS instruction types
 //                +---------------------------------------------------------------+
 //                |    6      |    5    |    5    |    5    |    5    |    6      |
 //                +---------------------------------------------------------------+
 // Register type  |  Opcode   |    Rs   |    Rt   |    Rd   |    Sa   | Function  |
--- a/js/src/jit/mips/CodeGenerator-mips.cpp
+++ b/js/src/jit/mips/CodeGenerator-mips.cpp
@@ -47,17 +47,17 @@ CodeGeneratorMIPS::generatePrologue()
     return true;
 }
 
 bool
 CodeGeneratorMIPS::generateAsmJSPrologue(Label *stackOverflowLabel)
 {
     JS_ASSERT(gen->compilingAsmJS());
 
-    masm.Push(ra);
+    masm.push(ra);
 
     // The asm.js over-recursed handler wants to be able to assume that SP
     // points to the return address, so perform the check after pushing ra but
     // before pushing frameDepth.
     if (!omitOverRecursedCheck()) {
         masm.branchPtr(Assembler::AboveOrEqual,
                        AsmJSAbsoluteAddress(AsmJSImm_StackLimit),
                        StackPointer,
@@ -79,28 +79,22 @@ CodeGeneratorMIPS::generateEpilogue()
     if (!gen->compilingAsmJS() && gen->info().executionMode() == SequentialExecution) {
         if (!emitTracelogStopEvent(TraceLogger::IonMonkey))
             return false;
         if (!emitTracelogScriptStop())
             return false;
     }
 #endif
 
-    if (gen->compilingAsmJS()) {
-        // Pop the stack we allocated at the start of the function.
+    if (gen->compilingAsmJS())
         masm.freeStack(frameDepth_);
-        masm.Pop(ra);
-        masm.abiret();
-        MOZ_ASSERT(masm.framePushed() == 0);
-    } else {
-        // Pop the stack we allocated at the start of the function.
+    else
         masm.freeStack(frameSize());
-        MOZ_ASSERT(masm.framePushed() == 0);
-        masm.ret();
-    }
+    JS_ASSERT(masm.framePushed() == 0);
+    masm.ret();
     return true;
 }
 
 void
 CodeGeneratorMIPS::branchToBlock(Assembler::FloatFormat fmt, FloatRegister lhs, FloatRegister rhs,
                                  MBasicBlock *mir, Assembler::DoubleCondition cond)
 {
     // Skip past trivial blocks.
@@ -984,24 +978,18 @@ CodeGeneratorMIPS::visitPowHalfD(LPowHal
 MoveOperand
 CodeGeneratorMIPS::toMoveOperand(const LAllocation *a) const
 {
     if (a->isGeneralReg())
         return MoveOperand(ToRegister(a));
     if (a->isFloatReg()) {
         return MoveOperand(ToFloatRegister(a));
     }
-    MOZ_ASSERT((ToStackOffset(a) & 3) == 0);
     int32_t offset = ToStackOffset(a);
-
-    // The way the stack slots work, we assume that everything from
-    // depth == 0 downwards is writable. However, since our frame is included
-    // in this, ensure that the frame gets skipped.
-    if (gen->compilingAsmJS())
-        offset -= AlignmentMidPrologue;
+    MOZ_ASSERT((offset & 3) == 0);
 
     return MoveOperand(StackPointer, offset);
 }
 
 class js::jit::OutOfLineTableSwitch : public OutOfLineCodeBase<CodeGeneratorMIPS>
 {
     MTableSwitch *mir_;
     CodeLabel jumpLabel_;
--- a/js/src/jit/mips/CodeGenerator-mips.h
+++ b/js/src/jit/mips/CodeGenerator-mips.h
@@ -28,44 +28,32 @@ class CodeGeneratorMIPS : public CodeGen
     // Label for the common return path.
     NonAssertingLabel returnLabel_;
     NonAssertingLabel deoptLabel_;
 
     inline Address ToAddress(const LAllocation &a) {
         MOZ_ASSERT(a.isMemory());
         int32_t offset = ToStackOffset(&a);
 
-        // The way the stack slots work, we assume that everything from
-        // depth == 0 downwards is writable however, since our frame is
-        // included in this, ensure that the frame gets skipped.
-        if (gen->compilingAsmJS())
-            offset -= AlignmentMidPrologue;
-
         return Address(StackPointer, offset);
     }
 
     inline Address ToAddress(const LAllocation *a) {
         return ToAddress(*a);
     }
 
     inline Operand ToOperand(const LAllocation &a) {
         if (a.isGeneralReg())
             return Operand(a.toGeneralReg()->reg());
         if (a.isFloatReg())
             return Operand(a.toFloatReg()->reg());
 
         MOZ_ASSERT(a.isMemory());
         int32_t offset = ToStackOffset(&a);
 
-        // The way the stack slots work, we assume that everything from
-        // depth == 0 downwards is writable however, since our frame is
-        // included in this, ensure that the frame gets skipped.
-        if (gen->compilingAsmJS())
-            offset -= AlignmentMidPrologue;
-
         return Operand(StackPointer, offset);
     }
     inline Operand ToOperand(const LAllocation *a) {
         return ToOperand(*a);
     }
     inline Operand ToOperand(const LDefinition *def) {
         return ToOperand(def->output());
     }
--- a/js/src/jit/mips/MacroAssembler-mips.cpp
+++ b/js/src/jit/mips/MacroAssembler-mips.cpp
@@ -3136,31 +3136,34 @@ void
 MacroAssemblerMIPSCompat::alignPointerUp(Register src, Register dest, uint32_t alignment)
 {
     MOZ_ASSERT(alignment > 1);
     ma_addu(dest, src, Imm32(alignment - 1));
     ma_and(dest, dest, Imm32(~(alignment - 1)));
 }
 
 void
-MacroAssemblerMIPSCompat::callWithABIPre(uint32_t *stackAdjust)
+MacroAssemblerMIPSCompat::callWithABIPre(uint32_t *stackAdjust, bool callFromAsmJS)
 {
     MOZ_ASSERT(inCall_);
 
     // Reserve place for $ra.
     *stackAdjust = sizeof(intptr_t);
 
     *stackAdjust += usedArgSlots_ > NumIntArgRegs ?
                     usedArgSlots_ * sizeof(intptr_t) :
                     NumIntArgRegs * sizeof(intptr_t);
 
+    uint32_t alignmentAtPrologue = callFromAsmJS ? AlignmentAtAsmJSPrologue : 0;
+
     if (dynamicAlignment_) {
         *stackAdjust += ComputeByteAlignment(*stackAdjust, StackAlignment);
     } else {
-        *stackAdjust += ComputeByteAlignment(framePushed_ + *stackAdjust, StackAlignment);
+        *stackAdjust += ComputeByteAlignment(framePushed_ + alignmentAtPrologue + *stackAdjust,
+                                             StackAlignment);
     }
 
     reserveStack(*stackAdjust);
 
     // Save $ra because call is going to clobber it. Restore it in
     // callWithABIPost. NOTE: This is needed for calls from BaselineIC.
     // Maybe we can do this differently.
     ma_sw(ra, Address(StackPointer, *stackAdjust - sizeof(intptr_t)));
@@ -3252,17 +3255,17 @@ MacroAssemblerMIPSCompat::callWithABI(vo
     ma_call(ImmPtr(fun));
     callWithABIPost(stackAdjust, result);
 }
 
 void
 MacroAssemblerMIPSCompat::callWithABI(AsmJSImmPtr imm, MoveOp::Type result)
 {
     uint32_t stackAdjust;
-    callWithABIPre(&stackAdjust);
+    callWithABIPre(&stackAdjust, /* callFromAsmJS = */ true);
     call(imm);
     callWithABIPost(stackAdjust, result);
 }
 
 void
 MacroAssemblerMIPSCompat::callWithABI(const Address &fun, MoveOp::Type result)
 {
     // Load the callee in t9, no instruction between the lw and call
--- a/js/src/jit/mips/MacroAssembler-mips.h
+++ b/js/src/jit/mips/MacroAssembler-mips.h
@@ -417,17 +417,19 @@ class MacroAssemblerMIPSCompat : public 
     void call(JitCode *c) {
         BufferOffset bo = m_buffer.nextOffset();
         addPendingJump(bo, ImmPtr(c->raw()), Relocation::JITCODE);
         ma_liPatchable(ScratchRegister, Imm32((uint32_t)c->raw()));
         ma_callIonHalfPush(ScratchRegister);
     }
 
     void appendCallSite(const CallSiteDesc &desc) {
-        enoughMemory_ &= append(CallSite(desc, currentOffset(), framePushed_));
+        // Add an extra sizeof(void*) to include the return address that was
+        // pushed by the call instruction (see CallSite::stackDepth).
+        enoughMemory_ &= append(CallSite(desc, currentOffset(), framePushed_ + sizeof(void*)));
     }
 
     void call(const CallSiteDesc &desc, const Register reg) {
         call(reg);
         appendCallSite(desc);
     }
     void call(const CallSiteDesc &desc, Label *label) {
         call(label);
@@ -1230,17 +1232,17 @@ public:
     void passABIArg(Register reg);
     void passABIArg(FloatRegister reg, MoveOp::Type type);
     void passABIArg(const ValueOperand &regs);
 
   protected:
     bool buildOOLFakeExitFrame(void *fakeReturnAddr);
 
   private:
-    void callWithABIPre(uint32_t *stackAdjust);
+    void callWithABIPre(uint32_t *stackAdjust, bool callFromAsmJS = false);
     void callWithABIPost(uint32_t stackAdjust, MoveOp::Type result);
 
   public:
     // Emits a call to a C/C++ function, resolving all argument moves.
     void callWithABI(void *fun, MoveOp::Type result = MoveOp::GENERAL);
     void callWithABI(AsmJSImmPtr imm, MoveOp::Type result = MoveOp::GENERAL);
     void callWithABI(const Address &fun, MoveOp::Type result = MoveOp::GENERAL);