Bug 1543592 part 2 - Make JSOP_AFTERYIELD a jump target op. r=tcampbell
authorJan de Mooij <jdemooij@mozilla.com>
Thu, 11 Apr 2019 16:50:47 +0000
changeset 469255 c35e1a0a6cfb68df8bb1a5370faf6c918aa86fcb
parent 469254 4584d95fcaeaad9e6808bc34906807fd7ed6c1d2
child 469256 b1b702b25f5387d419eea9d6c0351d478090d5d0
push id112776
push usershindli@mozilla.com
push dateFri, 12 Apr 2019 16:20:17 +0000
treeherdermozilla-inbound@b4501ced5619 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcampbell
bugs1543592
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 1543592 part 2 - Make JSOP_AFTERYIELD a jump target op. r=tcampbell This will help the Baseline interpreter restore its interpreterICEntry field without calling into C++. Depends on D27032 Differential Revision: https://phabricator.services.mozilla.com/D27033
js/src/frontend/BytecodeEmitter.cpp
js/src/jit-test/tests/debug/bug1368736.js
js/src/jit/BaselineCompiler.cpp
js/src/vm/BytecodeUtil.h
js/src/vm/Interpreter.cpp
js/src/vm/Opcodes.h
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -2302,17 +2302,18 @@ bool BytecodeEmitter::emitYieldOp(JSOp o
 
   uint32_t resumeIndex;
   if (!allocateResumeIndex(bytecodeSection().offset(), &resumeIndex)) {
     return false;
   }
 
   SET_RESUMEINDEX(bytecodeSection().code(off), resumeIndex);
 
-  return emit1(JSOP_AFTERYIELD);
+  ptrdiff_t unusedOffset;
+  return emitJumpTargetOp(JSOP_AFTERYIELD, &unusedOffset);
 }
 
 bool BytecodeEmitter::emitSetThis(BinaryNode* setThisNode) {
   // ParseNodeKind::SetThis is used to update |this| after a super() call
   // in a derived class constructor.
 
   MOZ_ASSERT(setThisNode->isKind(ParseNodeKind::SetThis));
   MOZ_ASSERT(setThisNode->left()->isKind(ParseNodeKind::Name));
--- a/js/src/jit-test/tests/debug/bug1368736.js
+++ b/js/src/jit-test/tests/debug/bug1368736.js
@@ -1,13 +1,13 @@
 g = newGlobal({newCompartment: true});
 hits = 0;
 Debugger(g).onDebuggerStatement = function(frame) {
     // Set a breakpoint at the JSOP_AFTERYIELD op.
-    frame.script.setBreakpoint(75, {hit: function() { hits++; }});
+    frame.script.setBreakpoint(79, {hit: function() { hits++; }});
 }
 g.eval(`
 function* range() {
     debugger;
     for (var i = 0; i < 3; i++) {
         yield i;
     }
 }
--- a/js/src/jit/BaselineCompiler.cpp
+++ b/js/src/jit/BaselineCompiler.cpp
@@ -5438,16 +5438,20 @@ bool BaselineCodeGen<Handler>::emit_JSOP
 
 template <typename Handler>
 bool BaselineCodeGen<Handler>::emit_JSOP_AWAIT() {
   return emit_JSOP_YIELD();
 }
 
 template <typename Handler>
 bool BaselineCodeGen<Handler>::emit_JSOP_AFTERYIELD() {
+  if (!emit_JSOP_JUMPTARGET()) {
+    return false;
+  }
+
   auto ifDebuggee = [this]() {
     frame.assertSyncedStack();
     masm.loadBaselineFramePtr(BaselineFrameReg, R0.scratchReg());
     prepareVMCall();
     pushBytecodePCArg();
     pushArg(R0.scratchReg());
 
     using Fn = bool (*)(JSContext*, BaselineFrame*, jsbytecode*, bool*);
--- a/js/src/vm/BytecodeUtil.h
+++ b/js/src/vm/BytecodeUtil.h
@@ -355,16 +355,17 @@ static inline bool BytecodeFallsThrough(
   }
 }
 
 static inline bool BytecodeIsJumpTarget(JSOp op) {
   switch (op) {
     case JSOP_JUMPTARGET:
     case JSOP_LOOPHEAD:
     case JSOP_LOOPENTRY:
+    case JSOP_AFTERYIELD:
       return true;
     default:
       return false;
   }
 }
 
 MOZ_ALWAYS_INLINE unsigned StackUses(jsbytecode* pc) {
   JSOp op = JSOp(*pc);
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -4118,19 +4118,20 @@ static MOZ_NEVER_INLINE JS_HAZ_JSNATIVE_
           default:
             MOZ_CRASH("bad resumeKind");
         }
       }
       ADVANCE_AND_DISPATCH(0);
     }
 
     CASE(JSOP_AFTERYIELD) {
-      // No-op in the interpreter, as AbstractGeneratorObject::resume takes care
-      // of fixing up InterpreterFrames.
+      // AbstractGeneratorObject::resume takes care of setting the frame's
+      // debuggee flag.
       MOZ_ASSERT_IF(REGS.fp()->script()->isDebuggee(), REGS.fp()->isDebuggee());
+      COUNT_COVERAGE();
     }
     END_CASE(JSOP_AFTERYIELD)
 
     CASE(JSOP_FINALYIELDRVAL) {
       ReservedRooted<JSObject*> gen(&rootObject0, &REGS.sp[-1].toObject());
       REGS.sp--;
       AbstractGeneratorObject::finalSuspend(gen);
       goto successful_return_continuation;
--- a/js/src/vm/Opcodes.h
+++ b/js/src/vm/Opcodes.h
@@ -2240,25 +2240,27 @@
      * by the JITs so the script always runs in the interpreter.
      *
      *   Category: Other
      *   Operands:
      *   Stack: =>
      */ \
     MACRO(JSOP_FORCEINTERPRETER, 207, "forceinterpreter", NULL, 1, 0, 0, JOF_BYTE) \
     /*
-     * Bytecode emitted after 'yield' expressions to help the Debugger fix up
-     * the frame in the JITs. No-op in the interpreter.
+     * Bytecode emitted after 'yield' expressions. This is useful for the
+     * Debugger and AbstractGeneratorObject::isAfterYieldOrAwait. It's treated
+     * as jump target op so that the Baseline Interpreter can efficiently
+     * restore the frame's interpreterICEntry when resuming a generator.
      *
-     *   Category: Operators
-     *   Type: Debugger
-     *   Operands:
+     *   Category: Statements
+     *   Type: Generator
+     *   Operands: uint32_t icIndex
      *   Stack: =>
      */ \
-    MACRO(JSOP_AFTERYIELD, 208, "afteryield", NULL, 1, 0, 0, JOF_BYTE) \
+    MACRO(JSOP_AFTERYIELD, 208, "afteryield", NULL, 5, 0, 0, JOF_ICINDEX) \
     /*
      * Pops the generator and the return value 'promise', stops interpretation
      * and returns 'promise'. Pushes resolved value onto the stack.
      *
      *   Category: Statements
      *   Type: Generator
      *   Operands: uint24_t resumeIndex
      *   Stack: promise, gen => resolved