Bug 1566330 follow-up - De-duplicate DebugEpilogue calls in the interpreter. r=iain

The call needs to be part of the bytecode op for the JIT, but the interpreter can emit
a single call at the end. Results in more compact code and we can now assert unique
callVMs in recordCallRetAddr.

Depends on D38477

Differential Revision: https://phabricator.services.mozilla.com/D38633

--HG--
extra : moz-landing-system : lando

js/src/jit/BaselineCodeGen.cpp

@@ -158,12 +158,18 @@ bool BaselineInterpreterHandler::recordCallRetAddr(JSContext* cx,
                                                    uint32_t retOffset) {
   switch (kind) {
     case RetAddrEntry::Kind::DebugPrologue:
+      MOZ_ASSERT(callVMOffsets_.debugPrologueOffset == 0,
+                 "expected single DebugPrologue call");
       callVMOffsets_.debugPrologueOffset = retOffset;
       break;
     case RetAddrEntry::Kind::DebugEpilogue:
+      MOZ_ASSERT(callVMOffsets_.debugEpilogueOffset == 0,
+                 "expected single DebugEpilogue call");
       callVMOffsets_.debugEpilogueOffset = retOffset;
       break;
     case RetAddrEntry::Kind::DebugAfterYield:
+      MOZ_ASSERT(callVMOffsets_.debugAfterYieldOffset == 0,
+                 "expected single DebugAfterYield call");
       callVMOffsets_.debugAfterYieldOffset = retOffset;
       break;
     default:
@@ -1169,7 +1175,7 @@ bool BaselineCodeGen<Handler>::emitDebugPrologue() {
     masm.branchTest32(Assembler::Zero, ReturnReg, ReturnReg, &done);
     {
       masm.loadValue(frame.addressOfReturnValue(), JSReturnOperand);
-      masm.jump(&return_);
+      masm.jump(&returnNoDebugEpilogue_);
     }
     masm.bind(&done);
     return true;
@@ -5180,14 +5186,14 @@ bool BaselineCodeGen<Handler>::emit_JSOP_DEBUGGER() {
   masm.branchTest32(Assembler::Zero, ReturnReg, ReturnReg, &done);
   {
     masm.loadValue(frame.addressOfReturnValue(), JSReturnOperand);
-    masm.jump(&return_);
+    masm.jump(&returnNoDebugEpilogue_);
   }
   masm.bind(&done);
   return true;
 }
 
 template <typename Handler>
-bool BaselineCodeGen<Handler>::emitReturn() {
+bool BaselineCodeGen<Handler>::emitDebugEpilogue() {
   auto ifDebuggee = [this]() {
     // Move return value into the frame's rval slot.
     masm.storeValue(JSReturnOperand, frame.addressOfReturnValue());
@@ -5211,8 +5217,15 @@ bool BaselineCodeGen<Handler>::emitReturn() {
     masm.loadValue(frame.addressOfReturnValue(), JSReturnOperand);
     return true;
   };
-  if (!emitDebugInstrumentation(ifDebuggee)) {
-    return false;
+  return emitDebugInstrumentation(ifDebuggee);
+}
+
+template <typename Handler>
+bool BaselineCodeGen<Handler>::emitReturn() {
+  if (handler.shouldEmitDebugEpilogueAtReturnOp()) {
+    if (!emitDebugEpilogue()) {
+      return false;
+    }
   }
 
   // Only emit the jump if this JSOP_RETRVAL is not the last instruction.
@@ -6012,7 +6025,7 @@ bool BaselineCodeGen<Handler>::emit_JSOP_AFTERYIELD() {
     masm.branchTest32(Assembler::Zero, ReturnReg, ReturnReg, &done);
     {
       masm.loadValue(frame.addressOfReturnValue(), JSReturnOperand);
-      masm.jump(&return_);
+      masm.jump(&returnNoDebugEpilogue_);
     }
     masm.bind(&done);
     return true;
@@ -6790,6 +6803,13 @@ template <typename Handler>
 bool BaselineCodeGen<Handler>::emitEpilogue() {
   masm.bind(&return_);
 
+  if (!handler.shouldEmitDebugEpilogueAtReturnOp()) {
+    if (!emitDebugEpilogue()) {
+      return false;
+    }
+  }
+  masm.bind(&returnNoDebugEpilogue_);
+
 #ifdef JS_TRACE_LOGGING
   if (JS::TraceLoggerSupported() && !emitTraceLoggerExit()) {
     return false;

js/src/jit/BaselineCodeGen.h

@@ -274,7 +274,12 @@ class BaselineCodeGen {
 
   js::Vector<CodeOffset> traceLoggerToggleOffsets_;
 
+  // Shared epilogue code to return to the caller.
   NonAssertingLabel return_;
+
+  // Like return_ but skips the debug epilogue instrumentation.
+  NonAssertingLabel returnNoDebugEpilogue_;
+
   NonAssertingLabel postBarrierSlot_;
 
   CodeOffset profilerEnterFrameToggleOffset_;
@@ -492,6 +497,7 @@ class BaselineCodeGen {
   MOZ_MUST_USE bool emitStackCheck();
   MOZ_MUST_USE bool emitArgumentTypeChecks();
   MOZ_MUST_USE bool emitDebugPrologue();
+  MOZ_MUST_USE bool emitDebugEpilogue();
 
   template <typename F1, typename F2>
   MOZ_MUST_USE bool initEnvironmentChainHelper(const F1& initFunctionEnv,
@@ -551,6 +557,13 @@ class BaselineCompilerHandler {
 
   bool isDefinitelyLastOp() const { return pc_ == script_->lastPC(); }
 
+  bool shouldEmitDebugEpilogueAtReturnOp() const {
+    // The JIT uses the return address -> pc mapping and bakes in the pc
+    // argument so the DebugEpilogue call needs to be part of the returning
+    // bytecode op for this to work.
+    return true;
+  }
+
   JSScript* script() const { return script_; }
   JSScript* maybeScript() const { return script_; }
 
@@ -697,6 +710,13 @@ class BaselineInterpreterHandler {
   JSScript* maybeScript() const { return nullptr; }
   JSFunction* maybeFunction() const { return nullptr; }
 
+  bool shouldEmitDebugEpilogueAtReturnOp() const {
+    // The interpreter doesn't use the return address -> pc mapping and doesn't
+    // bake in bytecode PCs so it can emit a shared DebugEpilogue call instead
+    // of duplicating it for every return op.
+    return false;
+  }
+
   MOZ_MUST_USE bool addDebugInstrumentationOffset(CodeOffset offset) {
     return debugInstrumentationOffsets_.append(offset.offset());
   }