Bug 1443592: Part 7: Use jit::CommonFrameLayout to store the hasCachedSavedFrame flag for Baseline frames. r=jandem

A jit::BaselineFrame is followed in memory by a jit::JitFrameLayout; as the
stack grows downwards, the JitFrameLayout is pushed first, "followed" by the
BaselineFrame at lower addresses, and finally by locals and the operand stack.

A BaselineFrame* points, naturally, at the BaselineFrame structure, but we use
the address of the JitFrameLayout as the address for a physical Ion frame -
actually, a pointer to its base class, jit::CommonFrameLayout. This means that,
if OSR replaces a BaselineFrame with an Ion frame, then walking the stack with a
FrameIter will see an Ion frame where a BaselineFrame used to appear, and the
LiveSavedFrameCache::FramePtr we'll construct from that FrameIter will be
different.

If the LiveSavedFrameCache wants to assert that frames with their bits set
indeed appear in the cache, then we'd better clear the flag whenever we OSR the
frame. But this is a pity; it's still the same frame, representing the same
function activation.

Meanwhile, both BaselineFrame and CommonFrameLayout have their own
hasCachedSavedFrame flags, which is confusing.

This patch changes FramePtr to use a jit::CommonFrameLayout* for both Baseline
frames and physical Ion frames, so OSR does not change the frame's address, and
use CommonFrameLayout's hasCachedSavedFrame flag for both types, so that OSR
need not take any explicit steps to propagate the cached flag to the new frame.

MozReview-Commit-ID: rOMjUXlwIQ

--HG--
extra : rebase_source : 360cca36216e24b5668c28e7f4d02ef76b6a0fb8
extra : source : 000d13ca4b5c8427308753a4a1e23f56245dc19f

js/src/jit/BaselineFrame.h

@@ -72,11 +72,6 @@ class BaselineFrame
         // debug mode OSR sanity checking when it handles corner cases which
         // only arise during exception handling.
         HANDLING_EXCEPTION = 1 << 12,
-
-        // If set, this frame has been on the stack when
-        // |js::SavedStacks::saveCurrentStack| was called, and so there is a
-        // |js::SavedFrame| object cached for this frame.
-        HAS_CACHED_SAVED_FRAME = 1 << 13
     };
 
   protected: // Silence Clang warning about unused private fields.
@@ -312,13 +307,6 @@ class BaselineFrame
         flags_ &= ~HANDLING_EXCEPTION;
     }
 
-    bool hasCachedSavedFrame() const {
-        return flags_ & HAS_CACHED_SAVED_FRAME;
-    }
-    void setHasCachedSavedFrame() {
-        flags_ |= HAS_CACHED_SAVED_FRAME;
-    }
-
     bool overRecursed() const {
         return flags_ & OVER_RECURSED;
     }

js/src/vm/Stack-inl.h

@@ -979,25 +979,22 @@ LiveSavedFrameCache::FramePtr::create(const FrameIter& iter)
     if (iter.done())
         return mozilla::Nothing();
 
-    if (iter.hasUsableAbstractFramePtr()) {
-        auto afp = iter.abstractFramePtr();
-
-        if (afp.isInterpreterFrame())
-            return mozilla::Some(FramePtr(afp.asInterpreterFrame()));
-        if (afp.isBaselineFrame())
-            return mozilla::Some(FramePtr(afp.asBaselineFrame()));
-        if (afp.isWasmDebugFrame())
-            return mozilla::Some(FramePtr(afp.asWasmDebugFrame()));
-        if (afp.isRematerializedFrame())
-            return mozilla::Some(FramePtr(afp.asRematerializedFrame()));
+    if (iter.isPhysicalJitFrame())
+        return mozilla::Some(FramePtr(iter.physicalJitFrame()));
 
+    if (!iter.hasUsableAbstractFramePtr())
         return mozilla::Nothing();
-    }
 
-    if (iter.isPhysicalIonFrame())
-        return mozilla::Some(FramePtr(iter.physicalIonFrame()));
+    auto afp = iter.abstractFramePtr();
 
-    return mozilla::Nothing();
+    if (afp.isInterpreterFrame())
+        return mozilla::Some(FramePtr(afp.asInterpreterFrame()));
+    if (afp.isWasmDebugFrame())
+        return mozilla::Some(FramePtr(afp.asWasmDebugFrame()));
+    if (afp.isRematerializedFrame())
+        return mozilla::Some(FramePtr(afp.asRematerializedFrame()));
+
+    MOZ_CRASH("unexpected frame type");
 }
 
 struct LiveSavedFrameCache::FramePtr::HasCachedMatcher {

js/src/vm/Stack.h

@@ -1172,11 +1172,12 @@ class LiveSavedFrameCache
     // 'hasCachedSavedFrame' bit we can examine and set, and can be converted to
     // a Key to index the cache.
     class FramePtr {
+        // We use jit::CommonFrameLayout for both Baseline frames and Ion
+        // physical frames.
         using Ptr = mozilla::Variant<InterpreterFrame*,
-                                     jit::BaselineFrame*,
+                                     jit::CommonFrameLayout*,
                                      jit::RematerializedFrame*,
-                                     wasm::DebugFrame*,
-                                     jit::CommonFrameLayout*>;
+                                     wasm::DebugFrame*>;
 
         Ptr ptr;
 
@@ -1976,7 +1977,7 @@ class FrameIter
 
     inline bool isIon() const;
     inline bool isBaseline() const;
-    inline bool isPhysicalIonFrame() const;
+    inline bool isPhysicalJitFrame() const;
 
     bool isEvalFrame() const;
     bool isFunctionFrame() const;
@@ -2072,8 +2073,8 @@ class FrameIter
     // This can only be called when isInterp():
     inline InterpreterFrame* interpFrame() const;
 
-    // This can only be called when isPhysicalIonFrame():
-    inline jit::CommonFrameLayout* physicalIonFrame() const;
+    // This can only be called when isPhysicalJitFrame():
+    inline jit::CommonFrameLayout* physicalJitFrame() const;
 
     // This is used to provide a raw interface for debugging.
     void* rawFramePtr() const;
@@ -2306,17 +2307,28 @@ FrameIter::interpFrame() const
 }
 
 inline bool
-FrameIter::isPhysicalIonFrame() const
+FrameIter::isPhysicalJitFrame() const
 {
-    return isJSJit() &&
-           jsJitFrame().isIonScripted() &&
-           ionInlineFrames_.frameNo() == 0;
+    if (!isJSJit())
+        return false;
+
+    auto& jitFrame = jsJitFrame();
+
+    if (jitFrame.isBaselineJS())
+        return true;
+
+    if (jitFrame.isIonScripted()) {
+        // Only the bottom of a group of inlined Ion frames is a physical frame.
+        return ionInlineFrames_.frameNo() == 0;
+    }
+
+    return false;
 }
 
 inline jit::CommonFrameLayout*
-FrameIter::physicalIonFrame() const
+FrameIter::physicalJitFrame() const
 {
-    MOZ_ASSERT(isPhysicalIonFrame());
+    MOZ_ASSERT(isPhysicalJitFrame());
     return jsJitFrame().current();
 }