Bug 1541404 part 26 - Some JSOP_FORCEINTERPRETER changes. r=tcampbell
authorJan de Mooij <jdemooij@mozilla.com>
Fri, 10 May 2019 09:55:47 +0000
changeset 532192 da74be741b81bb930c64a223ae36f63359b3367c
parent 532191 8c4c1145857040627723cf4913e4d30702bd171c
child 532193 757f1c83278b03859f4f23e85e22e732c2bc313f
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [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 26 - Some JSOP_FORCEINTERPRETER changes. r=tcampbell Initially the plan was for the Baseline Interpreter to "support" JSOP_FORCEINTERPRETER like the C++ interpreter. However this complicated many things: * We needed to do a VM call to allocate a TypeScript when we resumed generators (the only place where it was needed). * This meant our Baseline Interpreter warm-up heuristics didn't apply to generators. * It complicates the profiler work because it assumes all Baseline Interpreter frames have a TypeScript. We've been moving towards making the Baseline Interpreter more like the JITs for now (requiring a TypeScript instead of a BaselineScript) so I think this is the right move until we change that. This also improves things for the Baseline Compiler: we now have a MOZ_CRASH instead of an abort. Differential Revision: https://phabricator.services.mozilla.com/D30331
js/src/frontend/BytecodeEmitter.cpp
js/src/jit/BaselineCompiler.cpp
js/src/jit/BaselineCompiler.h
js/src/jit/BaselineJIT.cpp
js/src/vm/JSScript.h
js/src/vm/Opcodes.h
js/src/vm/TypeInference.cpp
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -7018,22 +7018,27 @@ bool BytecodeEmitter::emitSelfHostedResu
   if (!emit2(JSOP_RESUME, operand)) {
     return false;
   }
 
   return true;
 }
 
 bool BytecodeEmitter::emitSelfHostedForceInterpreter() {
+  // JSScript::hasForceInterpreterOp() relies on JSOP_FORCEINTERPRETER being the
+  // first bytecode op in the script.
+  MOZ_ASSERT(bytecodeSection().code().empty());
+
   if (!emit1(JSOP_FORCEINTERPRETER)) {
     return false;
   }
   if (!emit1(JSOP_UNDEFINED)) {
     return false;
   }
+
   return true;
 }
 
 bool BytecodeEmitter::emitSelfHostedAllowContentIter(BinaryNode* callNode) {
   ListNode* argsList = &callNode->right()->as<ListNode>();
 
   if (argsList->count() != 1) {
     reportNeedMoreArgsError(callNode, "allowContentIter", "1", "", argsList);
--- a/js/src/jit/BaselineCompiler.cpp
+++ b/js/src/jit/BaselineCompiler.cpp
@@ -1530,26 +1530,16 @@ void BaselineCodeGen<Handler>::emitProfi
   profilerExitFrameToggleOffset_ = toggleOffset;
 }
 
 template <typename Handler>
 bool BaselineCodeGen<Handler>::emit_JSOP_NOP() {
   return true;
 }
 
-template <>
-bool BaselineCompilerCodeGen::emit_JSOP_FORCEINTERPRETER() {
-  MOZ_CRASH("Unexpected JSOP_FORCEINTERPRETER in compiler");
-}
-
-template <>
-bool BaselineInterpreterCodeGen::emit_JSOP_FORCEINTERPRETER() {
-  return true;
-}
-
 template <typename Handler>
 bool BaselineCodeGen<Handler>::emit_JSOP_ITERNEXT() {
   return true;
 }
 
 template <typename Handler>
 bool BaselineCodeGen<Handler>::emit_JSOP_NOP_DESTRUCTURING() {
   return true;
@@ -5777,23 +5767,27 @@ bool BaselineCodeGen<Handler>::emitGener
   Register callee = regs.takeAny();
   masm.unboxObject(
       Address(genObj, AbstractGeneratorObject::offsetOfCalleeSlot()), callee);
 
   // Load the return value.
   ValueOperand retVal = regs.takeAnyValue();
   masm.loadValue(frame.addressOfStackValue(-1), retVal);
 
-  // Branch to interpret if the script does not have a BaselineScript (if the
-  // Baseline Interpreter is not enabled). Note that we don't relazify generator
-  // scripts, so the function is guaranteed to be non-lazy.
+  // Branch to interpret if the script does not have a TypeScript or
+  // BaselineScript (depending on whether the Baseline Interpreter is enabled).
+  // Note that we don't relazify generator scripts, so the function is
+  // guaranteed to be non-lazy.
   Label interpret;
   Register scratch1 = regs.takeAny();
   masm.loadPtr(Address(callee, JSFunction::offsetOfScript()), scratch1);
-  if (!JitOptions.baselineInterpreter) {
+  if (JitOptions.baselineInterpreter) {
+    Address typesAddr(scratch1, JSScript::offsetOfTypes());
+    masm.branchPtr(Assembler::Equal, typesAddr, ImmPtr(nullptr), &interpret);
+  } else {
     Address baselineAddr(scratch1, JSScript::offsetOfBaselineScript());
     masm.branchPtr(Assembler::BelowOrEqual, baselineAddr,
                    ImmPtr(BASELINE_DISABLED_SCRIPT), &interpret);
   }
 
 #ifdef JS_TRACE_LOGGING
   if (JS::TraceLoggerSupported() && !emitTraceLoggerResume(scratch1, regs)) {
     return false;
@@ -5999,39 +5993,38 @@ bool BaselineCodeGen<Handler>::emitGener
 #endif
     masm.jump(code);
 
     // Pop arguments and frame pointer (pushed by prepareVMCall) from
     // framePushed.
     masm.implicitPop((fun.explicitStackSlots() + 1) * sizeof(void*));
   }
 
-  // If the generator script has no JIT code, call into the VM.
-  if (interpret.used()) {
-    masm.bind(&interpret);
-
-    prepareVMCall();
-    if (resumeKind == GeneratorResumeKind::Next) {
-      pushArg(ImmGCPtr(cx->names().next));
-    } else if (resumeKind == GeneratorResumeKind::Throw) {
-      pushArg(ImmGCPtr(cx->names().throw_));
-    } else {
-      MOZ_ASSERT(resumeKind == GeneratorResumeKind::Return);
-      pushArg(ImmGCPtr(cx->names().return_));
-    }
-
-    masm.loadValue(frame.addressOfStackValue(-1), retVal);
-    pushArg(retVal);
-    pushArg(genObj);
-
-    using Fn = bool (*)(JSContext*, HandleObject, HandleValue,
-                        HandlePropertyName, MutableHandleValue);
-    if (!callVM<Fn, jit::InterpretResume>()) {
-      return false;
-    }
+  // Call into the VM to run in the C++ interpreter if there's no TypeScript or
+  // BaselineScript.
+  masm.bind(&interpret);
+
+  prepareVMCall();
+  if (resumeKind == GeneratorResumeKind::Next) {
+    pushArg(ImmGCPtr(cx->names().next));
+  } else if (resumeKind == GeneratorResumeKind::Throw) {
+    pushArg(ImmGCPtr(cx->names().throw_));
+  } else {
+    MOZ_ASSERT(resumeKind == GeneratorResumeKind::Return);
+    pushArg(ImmGCPtr(cx->names().return_));
+  }
+
+  masm.loadValue(frame.addressOfStackValue(-1), retVal);
+  pushArg(retVal);
+  pushArg(genObj);
+
+  using Fn = bool (*)(JSContext*, HandleObject, HandleValue, HandlePropertyName,
+                      MutableHandleValue);
+  if (!callVM<Fn, jit::InterpretResume>()) {
+    return false;
   }
 
   // After the generator returns, we restore the stack pointer, switch back to
   // the current realm, push the return value, and we're done.
   masm.bind(&returnTarget);
   masm.computeEffectiveAddress(frame.addressOfStackValue(-1),
                                masm.getStackPointer());
   if (JSScript* script = handler.maybeScript()) {
@@ -6568,27 +6561,22 @@ MethodStatus BaselineCompiler::emitBody(
     }
 
     // Emit traps for breakpoints and step mode.
     if (MOZ_UNLIKELY(compileDebugInstrumentation()) && !emitDebugTrap()) {
       return Method_Error;
     }
 
     switch (op) {
-      // ===== NOT Yet Implemented =====
       case JSOP_FORCEINTERPRETER:
-        // Intentionally not implemented.
+        // Caller must have checked script->hasForceInterpreterOp().
       case JSOP_UNUSED71:
       case JSOP_UNUSED149:
       case JSOP_LIMIT:
-        // === !! WARNING WARNING WARNING !! ===
-        // DO NOT add new ops to this list! All bytecode ops MUST have Baseline
-        // support. Follow-up bugs are not acceptable.
-        JitSpew(JitSpew_BaselineAbort, "Unhandled op: %s", CodeName[op]);
-        return Method_CantCompile;
+        MOZ_CRASH("Unexpected op");
 
 #define EMIT_OP(OP)                                            \
   case OP:                                                     \
     if (MOZ_UNLIKELY(!this->emit_##OP())) return Method_Error; \
     break;
         OPCODE_LIST(EMIT_OP)
 #undef EMIT_OP
     }
--- a/js/src/jit/BaselineCompiler.h
+++ b/js/src/jit/BaselineCompiler.h
@@ -413,17 +413,16 @@ class BaselineCodeGen {
   MOZ_MUST_USE bool emitNextIC();
   MOZ_MUST_USE bool emitInterruptCheck();
   MOZ_MUST_USE bool emitWarmUpCounterIncrement();
   MOZ_MUST_USE bool emitTraceLoggerResume(Register script,
                                           AllocatableGeneralRegisterSet& regs);
 
 #define EMIT_OP(op) bool emit_##op();
   OPCODE_LIST(EMIT_OP)
-  EMIT_OP(JSOP_FORCEINTERPRETER)
 #undef EMIT_OP
 
   // JSOP_NEG, JSOP_BITNOT, JSOP_INC, JSOP_DEC
   MOZ_MUST_USE bool emitUnaryArith();
 
   // JSOP_BITXOR, JSOP_LSH, JSOP_ADD etc.
   MOZ_MUST_USE bool emitBinaryArith();
 
--- a/js/src/jit/BaselineJIT.cpp
+++ b/js/src/jit/BaselineJIT.cpp
@@ -294,16 +294,20 @@ static MethodStatus CanEnterBaselineJIT(
   if (!CanLikelyAllocateMoreExecutableMemory()) {
     return Method_Skipped;
   }
 
   if (!cx->realm()->ensureJitRealmExists(cx)) {
     return Method_Error;
   }
 
+  if (script->hasForceInterpreterOp()) {
+    return Method_CantCompile;
+  }
+
   // Frames can be marked as debuggee frames independently of its underlying
   // script being a debuggee script, e.g., when performing
   // Debugger.Frame.prototype.eval.
   bool forceDebugInstrumentation =
       osrSourceFrame && osrSourceFrame.isDebuggee();
   return BaselineCompile(cx, script, forceDebugInstrumentation);
 }
 
@@ -311,16 +315,20 @@ static MethodStatus CanEnterBaselineInte
                                                 HandleScript script) {
   MOZ_ASSERT(jit::IsBaselineEnabled(cx));
   MOZ_ASSERT(JitOptions.baselineInterpreter);
 
   if (script->types()) {
     return Method_Compiled;
   }
 
+  if (script->hasForceInterpreterOp()) {
+    return Method_CantCompile;
+  }
+
   // Check script warm-up counter.
   if (script->getWarmUpCount() <=
       JitOptions.baselineInterpreterWarmUpThreshold) {
     return Method_Skipped;
   }
 
   if (!cx->realm()->ensureJitRealmExists(cx)) {
     return Method_Error;
--- a/js/src/vm/JSScript.h
+++ b/js/src/vm/JSScript.h
@@ -2096,16 +2096,22 @@ class JSScript : public js::gc::TenuredC
   // Script bytecode is immutable after creation.
   jsbytecode* code() const {
     if (!scriptData_) {
       return nullptr;
     }
     return scriptData_->code();
   }
 
+  bool hasForceInterpreterOp() const {
+    // JSOP_FORCEINTERPRETER, if present, must be the first op.
+    MOZ_ASSERT(length() >= 1);
+    return JSOp(*code()) == JSOP_FORCEINTERPRETER;
+  }
+
   js::AllBytecodesIterable allLocations() {
     return js::AllBytecodesIterable(this);
   }
 
   js::BytecodeLocation location() { return js::BytecodeLocation(this, code()); }
 
   bool isUncompleted() const {
     // code() becomes non-null only if this script is complete.
--- a/js/src/vm/Opcodes.h
+++ b/js/src/vm/Opcodes.h
@@ -2232,17 +2232,18 @@
      *   Category: Variables and Scopes
      *   Type: Arguments
      *   Operands: uint8_t numHops
      *   Stack: => callee
      */ \
     MACRO(JSOP_ENVCALLEE, 206, "envcallee", NULL, 2, 0, 1, JOF_UINT8) \
     /*
      * No-op bytecode only emitted in some self-hosted functions. Not handled
-     * by the JITs so the script always runs in the interpreter.
+     * by the JITs or Baseline Interpreter so the script always runs in the C++
+     * interpreter.
      *
      *   Category: Other
      *   Operands:
      *   Stack: =>
      */ \
     MACRO(JSOP_FORCEINTERPRETER, 207, "forceinterpreter", NULL, 1, 0, 0, JOF_BYTE) \
     /*
      * Bytecode emitted after 'yield' expressions. This is useful for the
--- a/js/src/vm/TypeInference.cpp
+++ b/js/src/vm/TypeInference.cpp
@@ -3486,16 +3486,20 @@ TypeScript::TypeScript(JSScript* script,
 
   FillBytecodeTypeMap(script, bytecodeTypeMap());
 }
 
 bool JSScript::makeTypes(JSContext* cx) {
   MOZ_ASSERT(!types_);
   cx->check(this);
 
+  // Scripts that will never run in the Baseline Interpreter or the JITs don't
+  // need a TypeScript.
+  MOZ_ASSERT(!hasForceInterpreterOp());
+
   AutoEnterAnalysis enter(cx);
 
   UniquePtr<jit::ICScript> icScript(jit::ICScript::create(cx, this));
   if (!icScript) {
     return false;
   }
 
   // We need to call prepareForDestruction on ICScript before we |delete| it.