Bug 1362357: wasm: Move exitReason to the wasm::Frame; r=luke
authorBenjamin Bouvier <benj@benj.me>
Fri, 05 May 2017 12:09:48 +0200
changeset 413156 db2227e9cd9b192e334613a1de6df1ddfafebdb0
parent 413155 50f39d5543d8df968c2313e74528c595c3089f2d
child 413157 b30d6d4445b6673ffd481c449a76960b95f24eaa
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs1362357
milestone55.0a1
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
Bug 1362357: wasm: Move exitReason to the wasm::Frame; r=luke MozReview-Commit-ID: 9351D0DQVEf
js/src/vm/Stack.cpp
js/src/vm/Stack.h
js/src/wasm/WasmFrameIterator.cpp
js/src/wasm/WasmFrameIterator.h
js/src/wasm/WasmTypes.h
--- a/js/src/vm/Stack.cpp
+++ b/js/src/vm/Stack.cpp
@@ -1624,39 +1624,36 @@ void
 jit::JitActivation::traceIonRecovery(JSTracer* trc)
 {
     for (RInstructionResults* it = ionRecovery_.begin(); it != ionRecovery_.end(); it++)
         it->trace(trc);
 }
 
 WasmActivation::WasmActivation(JSContext* cx)
   : Activation(cx, Wasm),
-    exitFP_(nullptr),
-    exitReason_(wasm::ExitReason::Fixed::None)
+    exitFP_(nullptr)
 {
     // Now that the WasmActivation is fully initialized, make it visible to
     // asynchronous profiling.
     registerProfiling();
 }
 
 WasmActivation::~WasmActivation()
 {
     // Hide this activation from the profiler before is is destroyed.
     unregisterProfiling();
 
     MOZ_ASSERT(!interrupted());
     MOZ_ASSERT(exitFP_ == nullptr);
-    MOZ_ASSERT(exitReason_.isNone());
 }
 
 void
 WasmActivation::unwindExitFP(wasm::Frame* exitFP)
 {
     exitFP_ = exitFP;
-    exitReason_ = wasm::ExitReason::Fixed::None;
 }
 
 void
 WasmActivation::startInterrupt(void* pc, uint8_t* fp)
 {
     MOZ_ASSERT(pc);
     MOZ_ASSERT(fp);
 
--- a/js/src/vm/Stack.h
+++ b/js/src/vm/Stack.h
@@ -1736,36 +1736,31 @@ class InterpreterFrameIterator
 
 // An eventual goal is to remove WasmActivation and to run asm code in a
 // JitActivation interleaved with Ion/Baseline jit code. This would allow
 // efficient calls back and forth but requires that we can walk the stack for
 // all kinds of jit code.
 class WasmActivation : public Activation
 {
     wasm::Frame* exitFP_;
-    wasm::ExitReason exitReason_;
 
   public:
     explicit WasmActivation(JSContext* cx);
     ~WasmActivation();
 
     bool isProfiling() const {
         return true;
     }
 
     // Returns null or the final wasm::Frame* when wasm exited this
     // WasmActivation.
     wasm::Frame* exitFP() const { return exitFP_; }
 
-    // Returns the reason why wasm code called out of wasm code.
-    wasm::ExitReason exitReason() const { return exitReason_; }
-
     // Written by JIT code:
     static unsigned offsetOfExitFP() { return offsetof(WasmActivation, exitFP_); }
-    static unsigned offsetOfExitReason() { return offsetof(WasmActivation, exitReason_); }
 
     // Interrupts are started from the interrupt signal handler (or the ARM
     // simulator) and cleared by WasmHandleExecutionInterrupt or WasmHandleThrow
     // when the interrupt is handled.
     void startInterrupt(void* pc, uint8_t* fp);
     void finishInterrupt();
     bool interrupted() const;
     void* resumePC() const;
--- a/js/src/wasm/WasmFrameIterator.cpp
+++ b/js/src/wasm/WasmFrameIterator.cpp
@@ -246,52 +246,64 @@ FrameIterator::debugTrapCallsite() const
 // Prologue/epilogue code generation
 
 // These constants reflect statically-determined offsets in the
 // prologue/epilogue. The offsets are dynamically asserted during code
 // generation.
 #if defined(JS_CODEGEN_X64)
 static const unsigned PushedRetAddr = 0;
 static const unsigned PushedTLS = 2;
-static const unsigned PushedFP = 3;
-static const unsigned SetFP = 6;
-static const unsigned PoppedFP = 2;
+static const unsigned PushedExitReason = 4;
+static const unsigned PushedFP = 5;
+static const unsigned SetFP = 8;
+static const unsigned PoppedFP = 4;
+static const unsigned PoppedExitReason = 2;
 #elif defined(JS_CODEGEN_X86)
 static const unsigned PushedRetAddr = 0;
 static const unsigned PushedTLS = 1;
-static const unsigned PushedFP = 2;
-static const unsigned SetFP = 4;
-static const unsigned PoppedFP = 1;
+static const unsigned PushedExitReason = 3;
+static const unsigned PushedFP = 4;
+static const unsigned SetFP = 6;
+static const unsigned PoppedFP = 2;
+static const unsigned PoppedExitReason = 1;
 #elif defined(JS_CODEGEN_ARM)
 static const unsigned BeforePushRetAddr = 0;
 static const unsigned PushedRetAddr = 4;
 static const unsigned PushedTLS = 8;
-static const unsigned PushedFP = 12;
-static const unsigned SetFP = 16;
-static const unsigned PoppedFP = 4;
+static const unsigned PushedExitReason = 16;
+static const unsigned PushedFP = 20;
+static const unsigned SetFP = 24;
+static const unsigned PoppedFP = 8;
+static const unsigned PoppedExitReason = 4;
 #elif defined(JS_CODEGEN_ARM64)
 static const unsigned BeforePushRetAddr = 0;
 static const unsigned PushedRetAddr = 0;
 static const unsigned PushedTLS = 0;
+static const unsigned PushedExitReason = 0;
 static const unsigned PushedFP = 0;
 static const unsigned SetFP = 0;
 static const unsigned PoppedFP = 0;
+static const unsigned PoppedExitReason = 0;
 #elif defined(JS_CODEGEN_MIPS32) || defined(JS_CODEGEN_MIPS64)
 static const unsigned BeforePushRetAddr = 0;
 static const unsigned PushedRetAddr = 4;
 static const unsigned PushedTLS = 8;
-static const unsigned PushedFP = 12;
-static const unsigned SetFP = 16;
-static const unsigned PoppedFP = 4;
+static const unsigned PushedExitReason = 12;
+static const unsigned PushedFP = 16;
+static const unsigned SetFP = 20;
+static const unsigned PoppedFP = 8;
+static const unsigned PoppedExitReason = 4;
 #elif defined(JS_CODEGEN_NONE)
 static const unsigned PushedRetAddr = 0;
 static const unsigned PushedTLS = 0;
+static const unsigned PushedExitReason = 0;
 static const unsigned PushedFP = 0;
 static const unsigned SetFP = 0;
 static const unsigned PoppedFP = 0;
+static const unsigned PoppedExitReason = 0;
 #else
 # error "Unknown architecture!"
 #endif
 
 static void
 PushRetAddr(MacroAssembler& masm, unsigned entry)
 {
 #if defined(JS_CODEGEN_ARM)
@@ -322,104 +334,113 @@ GenerateCallablePrologue(MacroAssembler&
 {
     // ProfilingFrameIterator needs to know the offsets of several key
     // instructions from entry. To save space, we make these offsets static
     // constants and assert that they match the actual codegen below. On ARM,
     // this requires AutoForbidPools to prevent a constant pool from being
     // randomly inserted between two instructions.
     {
 #if defined(JS_CODEGEN_ARM)
-        AutoForbidPools afp(&masm, /* number of instructions in scope = */ 8);
+        AutoForbidPools afp(&masm, /* number of instructions in scope = */ 7);
 #endif
         *entry = masm.currentOffset();
 
         PushRetAddr(masm, *entry);
         MOZ_ASSERT_IF(!masm.oom(), PushedRetAddr == masm.currentOffset() - *entry);
         masm.push(WasmTlsReg);
         MOZ_ASSERT_IF(!masm.oom(), PushedTLS == masm.currentOffset() - *entry);
+        masm.push(Imm32(reason.encode()));
+        MOZ_ASSERT_IF(!masm.oom(), PushedExitReason == masm.currentOffset() - *entry);
         masm.push(FramePointer);
         MOZ_ASSERT_IF(!masm.oom(), PushedFP == masm.currentOffset() - *entry);
         masm.moveStackPtrTo(FramePointer);
         MOZ_ASSERT_IF(!masm.oom(), SetFP == masm.currentOffset() - *entry);
     }
 
     if (!reason.isNone()) {
-        // Native callers expect the native ABI, which assume that nonvolatile
-        // registers are preserved.
-        Register scratch = ABINonArgReg0;
-        if (reason.isNative() && !scratch.volatile_())
-            masm.Push(scratch);
+        Register act = ABINonArgReg0;
 
-        LoadActivation(masm, scratch);
-        masm.wasmAssertNonExitInvariants(scratch);
-        Address exitReason(scratch, WasmActivation::offsetOfExitReason());
-        masm.store32(Imm32(reason.raw()), exitReason);
-        Address exitFP(scratch, WasmActivation::offsetOfExitFP());
-        masm.storePtr(FramePointer, exitFP);
+        // Native callers expect the native ABI, which assume that non-saved
+        // registers are preserved. Explicitly preserve the act register
+        // in that case.
+        if (reason.isNative() && !act.volatile_())
+            masm.Push(act);
 
-        if (reason.isNative() && !scratch.volatile_())
-            masm.Pop(scratch);
+        LoadActivation(masm, act);
+        masm.wasmAssertNonExitInvariants(act);
+        masm.storePtr(FramePointer, Address(act, WasmActivation::offsetOfExitFP()));
+
+        if (reason.isNative() && !act.volatile_())
+            masm.Pop(act);
     }
 
     if (framePushed)
         masm.subFromStackPtr(Imm32(framePushed));
 }
 
 static void
 GenerateCallableEpilogue(MacroAssembler& masm, unsigned framePushed, ExitReason reason,
                          uint32_t* ret)
 {
     if (framePushed)
         masm.addToStackPtr(Imm32(framePushed));
 
     if (!reason.isNone()) {
-        // See comment in GenerateCallablePrologue.
-        Register scratch = ABINonArgReturnReg0;
-        if (reason.isNative() && !scratch.volatile_())
-            masm.Push(scratch);
+        Register act = ABINonArgReturnReg0;
 
-        LoadActivation(masm, scratch);
-        Address exitFP(scratch, WasmActivation::offsetOfExitFP());
-        masm.storePtr(ImmWord(0), exitFP);
-        Address exitReason(scratch, WasmActivation::offsetOfExitReason());
+        // See comment in GenerateCallablePrologue.
+        if (reason.isNative() && !act.volatile_())
+            masm.Push(act);
+
+        LoadActivation(masm, act);
+        masm.storePtr(ImmWord(0), Address(act, WasmActivation::offsetOfExitFP()));
+
 #ifdef DEBUG
         // Check the passed exitReason is the same as the one on entry.
-        masm.push(scratch);
-        masm.loadPtr(exitReason, ABINonArgReturnReg0);
+        // Do it here rather than in the pop sequence to not perturbate the
+        // static stack structure in debug vs optimized mode.
+        Register scratch = act;
+        size_t exitReasonSlot = 1 + (reason.isNative() && !scratch.volatile_() ? 1 : 0);
+        masm.load32(Address(masm.getStackPointer(), exitReasonSlot * sizeof(void*)), scratch);
         Label ok;
-        masm.branch32(Assembler::Condition::Equal, ABINonArgReturnReg0, Imm32(reason.raw()), &ok);
+        masm.branch32(Assembler::Condition::Equal, scratch, Imm32(reason.encode()), &ok);
         masm.breakpoint();
         masm.bind(&ok);
-        masm.pop(scratch);
 #endif
-        masm.store32(Imm32(ExitReason::None().raw()), exitReason);
 
-        if (reason.isNative() && !scratch.volatile_())
-            masm.Pop(scratch);
+        if (reason.isNative() && !act.volatile_())
+            masm.Pop(act);
     }
 
     // Forbid pools for the same reason as described in GenerateCallablePrologue.
 #if defined(JS_CODEGEN_ARM)
-    AutoForbidPools afp(&masm, /* number of instructions in scope = */ 3);
+    AutoForbidPools afp(&masm, /* number of instructions in scope = */ 7);
 #endif
 
     // There is an important ordering constraint here: fp must be repointed to
     // the caller's frame before any field of the frame currently pointed to by
     // fp is popped: asynchronous signal handlers (which use stack space
     // starting at sp) could otherwise clobber these fields while they are still
     // accessible via fp (fp fields are read during frame iteration which is
     // *also* done asynchronously).
 
     masm.pop(FramePointer);
     DebugOnly<uint32_t> poppedFP = masm.currentOffset();
+
+    // Pop the exit reason to WasmTlsReg; it's going to be clobbered just
+    // thereafter to store the real value of WasmTlsReg.
+    masm.pop(WasmTlsReg);
+    DebugOnly<uint32_t> poppedExitReason = masm.currentOffset();
+
     masm.pop(WasmTlsReg);
     *ret = masm.currentOffset();
     masm.ret();
 
     MOZ_ASSERT_IF(!masm.oom(), PoppedFP == *ret - poppedFP);
+    MOZ_ASSERT_IF(!masm.oom(), PoppedExitReason == *ret - poppedExitReason);
 }
 
 void
 wasm::GenerateFunctionPrologue(MacroAssembler& masm, unsigned framePushed, const SigIdDesc& sigId,
                                FuncOffsets* offsets)
 {
     // Flush pending pools so they do not get dumped between the 'begin' and
     // 'normalEntry' offsets since the difference must be less than UINT8_MAX
@@ -532,16 +553,20 @@ AssertMatchesCallSite(const WasmActivati
 }
 
 void
 ProfilingFrameIterator::initFromExitFP()
 {
     Frame* fp = activation_->exitFP();
     void* pc = fp->returnAddress;
 
+    // The iterator inserts a pretend innermost frame for ExitReasons.
+    // This allows the variety of exit reasons to show up in the callstack.
+    exitReason_ = ExitReason::Decode(fp->encodedExitReason);
+
     stackAddress_ = fp;
 
     code_ = activation_->compartment()->wasm.lookupCode(pc);
     MOZ_ASSERT(code_);
 
     codeRange_ = code_->lookupRange(pc);
     MOZ_ASSERT(codeRange_);
 
@@ -570,20 +595,16 @@ ProfilingFrameIterator::initFromExitFP()
       case CodeRange::DebugTrap:
       case CodeRange::Inline:
       case CodeRange::Throw:
       case CodeRange::Interrupt:
       case CodeRange::FarJumpIsland:
         MOZ_CRASH("Unexpected CodeRange kind");
     }
 
-    // The iterator inserts a pretend innermost frame for non-None ExitReasons.
-    // This allows the variety of exit reasons to show up in the callstack.
-    exitReason_ = activation_->exitReason();
-
     MOZ_ASSERT(!done());
 }
 
 typedef JS::ProfilingFrameIterator::RegisterState RegisterState;
 
 ProfilingFrameIterator::ProfilingFrameIterator(const WasmActivation& activation,
                                                const RegisterState& state)
   : activation_(&activation),
@@ -662,32 +683,46 @@ ProfilingFrameIterator::ProfilingFrameIt
         } else
 #endif
         if (offsetFromEntry == PushedRetAddr || codeRange->isThunk()) {
             // The return address has been pushed on the stack but fp still
             // points to the caller's fp.
             callerPC_ = sp[0];
             callerFP_ = fp;
             AssertMatchesCallSite(*activation_, callerPC_, callerFP_);
-        } else if (offsetFromEntry == PushedTLS) {
-            // The return address and caller's TLS have been pushed on the stack; fp
-            // is still the caller's fp.
+        } else if (offsetFromEntry >= PushedTLS && offsetFromEntry < PushedExitReason) {
+            // The return address and caller's TLS have been pushed on the
+            // stack; fp is still the caller's fp.
             callerPC_ = sp[1];
             callerFP_ = fp;
             AssertMatchesCallSite(*activation_, callerPC_, callerFP_);
+        } else if (offsetFromEntry == PushedExitReason) {
+            // The return address, caller's TLS and exit reason have been
+            // pushed on the stack; fp is still the caller's fp.
+            callerPC_ = sp[2];
+            callerFP_ = fp;
+            AssertMatchesCallSite(*activation_, callerPC_, callerFP_);
         } else if (offsetFromEntry == PushedFP) {
             // The full Frame has been pushed; fp is still the caller's fp.
             MOZ_ASSERT(fp == reinterpret_cast<Frame*>(sp)->callerFP);
             callerPC_ = reinterpret_cast<Frame*>(sp)->returnAddress;
             callerFP_ = fp;
             AssertMatchesCallSite(*activation_, callerPC_, callerFP_);
         } else if (offsetInCode == codeRange->ret() - PoppedFP) {
-            // The callerFP field of the Frame has been popped into fp.
+            // The callerFP field of the Frame has been popped into fp, but the
+            // exit reason hasn't been popped yet.
+            callerPC_ = sp[2];
+            callerFP_ = fp;
+            AssertMatchesCallSite(*activation_, callerPC_, callerFP_);
+        } else if (offsetInCode == codeRange->ret() - PoppedExitReason) {
+            // The callerFP field of the Frame has been popped into fp, and the
+            // exit reason has been popped.
             callerPC_ = sp[1];
             callerFP_ = fp;
+            AssertMatchesCallSite(*activation_, callerPC_, callerFP_);
         } else if (offsetInCode == codeRange->ret()) {
             // Both the TLS and callerFP fields have been popped and fp now
             // points to the caller's frame.
             callerPC_ = sp[0];
             callerFP_ = fp;
             AssertMatchesCallSite(*activation_, callerPC_, callerFP_);
         } else {
             // Not in the prologue/epilogue.
--- a/js/src/wasm/WasmFrameIterator.h
+++ b/js/src/wasm/WasmFrameIterator.h
@@ -88,16 +88,18 @@ enum class SymbolicAddress;
 // An ExitReason describes the possible reasons for leaving compiled wasm
 // code or the state of not having left compiled wasm code
 // (ExitReason::None). It is either a known reason, or a enumeration to a native
 // function that is used for better display in the profiler.
 class ExitReason
 {
     uint32_t payload_;
 
+    ExitReason() {}
+
   public:
     enum class Fixed : uint32_t
     {
         None,          // default state, the pc is in wasm code
         ImportJit,     // fast-path call directly into JIT code
         ImportInterp,  // slow-path call into C++ Invoke()
         BuiltinNative, // fast-path call directly into native C++ code
         Trap,          // call to trap handler for the trap in WasmActivation::trap
@@ -112,23 +114,29 @@ class ExitReason
 
     explicit ExitReason(SymbolicAddress sym)
       : payload_(0x1 | (uint32_t(sym) << 1))
     {
         MOZ_ASSERT(uint32_t(sym) <= (UINT32_MAX << 1), "packing constraints");
         MOZ_ASSERT(!isFixed());
     }
 
+    static ExitReason Decode(uint32_t payload) {
+        ExitReason reason;
+        reason.payload_ = payload;
+        return reason;
+    }
+
     static ExitReason None() { return ExitReason(ExitReason::Fixed::None); }
 
     bool isFixed() const { return (payload_ & 0x1) == 0; }
     bool isNone() const { return isFixed() && fixed() == Fixed::None; }
     bool isNative() const { return !isFixed() || fixed() == Fixed::BuiltinNative; }
 
-    uint32_t raw() const {
+    uint32_t encode() const {
         return payload_;
     }
     Fixed fixed() const {
         MOZ_ASSERT(isFixed());
         return Fixed(payload_ >> 1);
     }
     SymbolicAddress symbolic() const {
         MOZ_ASSERT(!isFixed());
--- a/js/src/wasm/WasmTypes.h
+++ b/js/src/wasm/WasmTypes.h
@@ -1622,16 +1622,20 @@ WASM_DECLARE_POD_VECTOR(MemoryAccess, Me
 // time is (sizeof(wasm::Frame) + masm.framePushed) % WasmStackAlignment.
 
 struct Frame
 {
     // The caller's Frame*. See GenerateCallableEpilogue for why this must be
     // the first field of wasm::Frame (in a downward-growing stack).
     Frame* callerFP;
 
+    // The raw payload of an ExitReason describing why we've left wasm. It is
+    // non null if and only if a call exited wasm code.
+    uint32_t encodedExitReason;
+
     // The saved value of WasmTlsReg on entry to the function. This is
     // effectively the callee's instance.
     TlsData* tls;
 
     // The return address pushed by the call (in the case of ARM/MIPS the return
     // address is pushed by the first instruction of the prologue).
     void* returnAddress;
 
@@ -1675,23 +1679,16 @@ class DebugFrame
             bool isDebuggee_ : 1;
             bool prevUpToDate_ : 1;
             bool hasCachedSavedFrame_ : 1;
             bool hasCachedReturnJSValue_ : 1;
         };
         void* flagsWord_;
     };
 
-    // Padding so that DebugFrame has Alignment.
-#if JS_BITS_PER_WORD == 32
-  protected: // suppress clang's -Wunused-private-field warning-as-error
-    void* padding_;
-  private:
-#endif
-
     // The Frame goes at the end since the stack grows down.
     Frame frame_;
 
   public:
     Frame& frame() { return frame_; }
     uint32_t funcIndex() const { return funcIndex_; }
     Instance* instance() const { return frame_.instance(); }
     GlobalObject* global() const;