Bug 1541404 part 34 - Fix BaselineDebugModeOSR to also recompile interpreter frames. r=tcampbell
authorJan de Mooij <jdemooij@mozilla.com>
Thu, 16 May 2019 08:56:53 +0000
changeset 532888 9faf24ffeaaf5f1e38fca1c6630716aeca7982bf
parent 532887 02a438fa11a8b1be6b0db28aa3f5c39b7ff6f8e8
child 532889 5765e6384ff13d09dce8580df9abb4ed63178a31
push id11276
push userrgurzau@mozilla.com
push dateMon, 20 May 2019 13:11:24 +0000
treeherdermozilla-beta@847755a7c325 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcampbell
bugs1541404
milestone68.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 1541404 part 34 - Fix BaselineDebugModeOSR to also recompile interpreter frames. r=tcampbell This matches what we do for C++-interpreter frames in CollectInterpreterStackScripts and SkipInterpreterFrameEntries. It's necessary for Interpreter => JIT OSR to work correctly. This fixes remaining jit-test failures with --blinterp-eager Differential Revision: https://phabricator.services.mozilla.com/D31050
js/src/jit/BaselineDebugModeOSR.cpp
js/src/vm/Debugger.cpp
--- a/js/src/jit/BaselineDebugModeOSR.cpp
+++ b/js/src/jit/BaselineDebugModeOSR.cpp
@@ -161,24 +161,28 @@ static bool CollectJitStackScripts(JSCon
     switch (frame.type()) {
       case FrameType::BaselineJS: {
         JSScript* script = frame.script();
 
         if (!obs.shouldRecompileOrInvalidate(script)) {
           break;
         }
 
-        // Baseline Interpreter frames don't need recompilation.
         BaselineFrame* baselineFrame = frame.baselineFrame();
+
         if (baselineFrame->runningInInterpreter()) {
-          break;
-        }
-
-        if (BaselineDebugModeOSRInfo* info =
-                baselineFrame->getDebugModeOSRInfo()) {
+          // Baseline Interpreter frames for scripts that have a BaselineScript
+          // or IonScript don't need to be patched but they do need to be
+          // invalidated and recompiled. See also CollectInterpreterStackScripts
+          // for C++ interpreter frames.
+          if (!entries.append(DebugModeOSREntry(script))) {
+            return false;
+          }
+        } else if (BaselineDebugModeOSRInfo* info =
+                       baselineFrame->getDebugModeOSRInfo()) {
           // If patching a previously patched yet unpopped frame, we can
           // use the BaselineDebugModeOSRInfo on the frame directly to
           // patch. Indeed, we cannot use frame.resumePCinCurrentFrame(), as
           // it points into the debug mode OSR handler and cannot be
           // used to look up a corresponding RetAddrEntry.
           //
           // See case F in PatchBaselineFramesForDebugMode.
           if (!entries.append(DebugModeOSREntry(script, info))) {
@@ -357,25 +361,27 @@ static void PatchBaselineFramesForDebugM
     switch (frame.type()) {
       case FrameType::BaselineJS: {
         // If the script wasn't recompiled or is not observed, there's
         // nothing to patch.
         if (!obs.shouldRecompileOrInvalidate(frame.script())) {
           break;
         }
 
-        // Baseline Interpreter frames don't need recompilation.
-        BaselineFrame* baselineFrame = frame.baselineFrame();
-        if (baselineFrame->runningInInterpreter()) {
+        DebugModeOSREntry& entry = entries[entryIndex];
+
+        if (!entry.recompiled()) {
+          entryIndex++;
           break;
         }
 
-        DebugModeOSREntry& entry = entries[entryIndex];
-
-        if (!entry.recompiled()) {
+        BaselineFrame* baselineFrame = frame.baselineFrame();
+        if (baselineFrame->runningInInterpreter()) {
+          // We recompiled the script's BaselineScript but Baseline Interpreter
+          // frames don't need to be patched.
           entryIndex++;
           break;
         }
 
         JSScript* script = entry.script;
         uint32_t pcOffset = entry.pcOffset;
         jsbytecode* pc = script->offsetToPC(pcOffset);
 
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -2983,25 +2983,17 @@ static bool UpdateExecutionObservability
     if (actIter->compartment()->zone() != zone) {
       continue;
     }
 
     for (OnlyJSJitFrameIter iter(actIter); !iter.done(); ++iter) {
       const JSJitFrameIter& frame = iter.frame();
       switch (frame.type()) {
         case FrameType::BaselineJS:
-          // BaselineScripts that are active on the stack get recompiled and
-          // other (affected) BaselineScripts are discarded. If we're running in
-          // the Baseline Interpreter don't mark the script as active here to
-          // prevent BaselineScripts from falling through the cracks: when we
-          // don't dicard them here (because active) and also don't recompile
-          // them (because recompilation skips interpreter frames).
-          if (!frame.baselineFrame()->runningInInterpreter()) {
-            MarkTypeScriptActiveIfObservable(frame.script(), obs);
-          }
+          MarkTypeScriptActiveIfObservable(frame.script(), obs);
           break;
         case FrameType::IonJS:
           MarkTypeScriptActiveIfObservable(frame.script(), obs);
           for (InlineFrameIterator inlineIter(cx, &frame); inlineIter.more();
                ++inlineIter) {
             MarkTypeScriptActiveIfObservable(inlineIter.script(), obs);
           }
           break;