Bug 1021251 - OdinMonkey: simplify AsmJSFrameIter a bit (r=bbouvier)

--HG--
extra : rebase_source : 0ee29c3ace29a1f93d3fc56d0ec07b44108ee995

js/src/jit/AsmJSLink.cpp

@@ -36,48 +36,61 @@ using mozilla::BinarySearch;
 using mozilla::IsNaN;
 using mozilla::PodZero;
 
-AsmJSFrameIterator::AsmJSFrameIterator(const AsmJSActivation *activation)
+static uint8_t *
+ReturnAddressForExitCall(uint8_t **psp)
 {
-    if (!activation || activation->isInterruptedSP()) {
-        PodZero(this);
-        JS_ASSERT(done());
-        return;
-    }
-
-    module_ = &activation->module();
-    sp_ = activation->exitSP();
-
+    uint8_t *sp = *psp;
 #if defined(JS_CODEGEN_X86) || defined(JS_CODEGEN_X64)
     // For calls to Ion/C++ on x86/x64, the exitSP is the SP right before the call
     // to C++. Since the call instruction pushes the return address, we know
     // that the return address is 1 word below exitSP.
-    returnAddress_ = *(uint8_t**)(sp_ - sizeof(void*));
+    return *(uint8_t**)(sp - sizeof(void*));
 #elif defined(JS_CODEGEN_ARM)
     // For calls to Ion/C++ on ARM, the *caller* pushes the return address on
     // the stack. For Ion, this is just part of the ABI. For C++, the return
     // address is explicitly pushed before the call since we cannot expect the
     // callee to immediately push lr. This means that exitSP points to the
     // return address.
-    returnAddress_ = *(uint8_t**)sp_;
+    return *(uint8_t**)sp;
 #elif defined(JS_CODEGEN_MIPS)
-    // On MIPS we have two cases. Exit to C++ will store return addres at
-    // sp + 16, While on exits to Ion, the return address will be stored at
-    // sp + 0. We indicate exits to ion by setting the lowest bit of stored sp.
-
-    // Check if this is the exit to Ion.
-    if (uint32_t(sp_) & 0x1) {
-        // Clear the low bit.
-        sp_ -= 0x1;
-        returnAddress_ = *(uint8_t**)sp_;
-    } else {
-        // This is exit to C++
-        returnAddress_ = *(uint8_t**)(sp_ + ShadowStackSpace);
+    // On MIPS we have two cases: an exit to C++ will store the return address
+    // at ShadowStackSpace above sp; an exit to Ion will store the return
+    // address at sp. To distinguish the two cases, the low bit of sp (which is
+    // aligned and therefore zero) is set for Ion exits.
+    if (uintptr_t(sp) & 0x1) {
+        sp = *psp -= 0x1;  // Clear the low bit
+        return *(uint8_t**)sp;
     }
+    return *(uint8_t**)(sp + ShadowStackSpace);
 #else
 # error "Unknown architecture!"
 #endif
+}
 
-    settle();
+static uint8_t *
+ReturnAddressForJitCall(uint8_t *sp)
+{
+    // Once inside JIT code, sp always points to the word before the return
+    // address.
+    return *(uint8_t**)(sp - sizeof(void*));
+}
+
+AsmJSFrameIterator::AsmJSFrameIterator(const AsmJSActivation *activation)
+  : module_(nullptr)
+{
+    if (!activation || activation->isInterruptedSP())
+        return;
+
+    module_ = &activation->module();
+    sp_ = activation->exitSP();
+
+    settle(ReturnAddressForExitCall(&sp_));
+}
+
+void
+AsmJSFrameIterator::operator++()
+{
+    settle(ReturnAddressForJitCall(sp_));
 }
 
 struct GetCallSite
@@ -90,43 +103,31 @@ struct GetCallSite
 };
 
 void
-AsmJSFrameIterator::popFrame()
+AsmJSFrameIterator::settle(uint8_t *returnAddress)
 {
-    // After adding stackDepth, sp points to the word before the return address,
-    // on both ARM and x86/x64.
-    sp_ += callsite_->stackDepth();
-    returnAddress_ = *(uint8_t**)(sp_ - sizeof(void*));
-}
+    uint32_t target = returnAddress - module_->codeBase();
+    size_t lowerBound = 0;
+    size_t upperBound = module_->numCallSites();
 
-void
-AsmJSFrameIterator::settle()
-{
-    while (true) {
-        uint32_t target = returnAddress_ - module_->codeBase();
-        size_t lowerBound = 0;
-        size_t upperBound = module_->numCallSites();
-
-        size_t match;
-        if (!BinarySearch(GetCallSite(*module_), lowerBound, upperBound, target, &match)) {
-            callsite_ = nullptr;
-            return;
-        }
-
-        callsite_ = &module_->callSite(match);
-
-        if (callsite_->isExit()) {
-            popFrame();
-            continue;
-        }
-
-        if (callsite_->isEntry()) {
-            callsite_ = nullptr;
-            return;
-        }
-
-        JS_ASSERT(callsite_->isNormal());
+    size_t match;
+    if (!BinarySearch(GetCallSite(*module_), lowerBound, upperBound, target, &match)) {
+        module_ = nullptr;
         return;
     }
+
+    callsite_ = &module_->callSite(match);
+
+    if (callsite_->isEntry()) {
+        module_ = nullptr;
+        return;
+    }
+
+    sp_ += callsite_->stackDepth();
+
+    if (callsite_->isExit())
+        return settle(ReturnAddressForJitCall(sp_));
+
+    JS_ASSERT(callsite_->isNormal());
 }
 
 JSAtom *

js/src/jit/AsmJSLink.h

@@ -23,15 +23,13 @@ class AsmJSFrameIterator
     const AsmJSModule *module_;
     const jit::CallSite *callsite_;
     uint8_t *sp_;
-    uint8_t *returnAddress_;
 
-    void popFrame();
-    void settle();
+    void settle(uint8_t *returnAddress);
 
   public:
     explicit AsmJSFrameIterator(const AsmJSActivation *activation);
-    void operator++() { popFrame(); settle(); }
-    bool done() const { return !callsite_; }
+    void operator++();
+    bool done() const { return !module_; }
     JSAtom *functionDisplayAtom() const;
     unsigned computeLine(uint32_t *column) const;
 };