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

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

js/src/frontend/BytecodeEmitter.cpp

@@ -7023,12 +7023,17 @@ bool BytecodeEmitter::emitSelfHostedResumeGenerator(BinaryNode* callNode) {
 }
 
 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;
 }
 

js/src/jit/BaselineCompiler.cpp

@@ -1535,16 +1535,6 @@ 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;
@@ -5782,13 +5772,17 @@ bool BaselineCodeGen<Handler>::emitGeneratorResume(
   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);
@@ -6004,29 +5998,28 @@ bool BaselineCodeGen<Handler>::emitGeneratorResume(
     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);
+  // 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_));
-    }
+  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);
+  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;
-    }
+  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
@@ -6573,17 +6566,12 @@ MethodStatus BaselineCompiler::emitBody() {
     }
 
     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:                                                     \

js/src/jit/BaselineCompiler.h

@@ -418,7 +418,6 @@ class BaselineCodeGen {
 
 #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

js/src/jit/BaselineJIT.cpp

@@ -299,6 +299,10 @@ static MethodStatus CanEnterBaselineJIT(JSContext* cx, HandleScript script,
     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.
@@ -316,6 +320,10 @@ static MethodStatus CanEnterBaselineInterpreter(JSContext* cx,
     return Method_Compiled;
   }
 
+  if (script->hasForceInterpreterOp()) {
+    return Method_CantCompile;
+  }
+
   // Check script warm-up counter.
   if (script->getWarmUpCount() <=
       JitOptions.baselineInterpreterWarmUpThreshold) {

js/src/vm/JSScript.h

@@ -2101,6 +2101,12 @@ class JSScript : public js::gc::TenuredCell {
     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/src/vm/Opcodes.h

@@ -2237,7 +2237,8 @@
     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:

js/src/vm/TypeInference.cpp

@@ -3491,6 +3491,10 @@ 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));