Bug 1395900 part 4 - Add JSOP_ITERNEXT to improve iterator key type information in Ion. r=tcampbell
authorJan de Mooij <jdemooij@mozilla.com>
Wed, 06 Sep 2017 10:03:25 +0200
changeset 428785 57a63c3f76fba9d1bc379eafbc564dc7226bd40e
parent 428784 456b4480fdbfc310c322407e9a4125624530f12e
child 428786 22a0a710fbf9a916318f757a6ff7380281ec0b7a
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcampbell
bugs1395900
milestone57.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 1395900 part 4 - Add JSOP_ITERNEXT to improve iterator key type information in Ion. r=tcampbell
js/src/frontend/BytecodeEmitter.cpp
js/src/jit/BaselineBailouts.cpp
js/src/jit/BaselineCompiler.cpp
js/src/jit/BaselineCompiler.h
js/src/jit/IonBuilder.cpp
js/src/jit/IonBuilder.h
js/src/jit/IonTypes.h
js/src/jsopcode.cpp
js/src/vm/Interpreter.cpp
js/src/vm/Opcodes.h
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -7504,16 +7504,21 @@ BytecodeEmitter::emitForIn(ParseNode* fo
     }
 
     {
 #ifdef DEBUG
         auto loopDepth = this->stackDepth;
 #endif
         MOZ_ASSERT(loopDepth >= 2);
 
+        if (iflags == JSITER_ENUMERATE) {
+            if (!emit1(JSOP_ITERNEXT))                    // ITER ITERVAL
+                return false;
+        }
+
         if (!emitInitializeForInOrOfTarget(forInHead))    // ITER ITERVAL
             return false;
 
         MOZ_ASSERT(this->stackDepth == loopDepth,
                    "iterator and iterval must be left on the stack");
     }
 
     // Perform the loop body.
--- a/js/src/jit/BaselineBailouts.cpp
+++ b/js/src/jit/BaselineBailouts.cpp
@@ -1777,16 +1777,28 @@ HandleLexicalCheckFailure(JSContext* cx,
     if (!innerScript->failedLexicalCheck())
         innerScript->setFailedLexicalCheck();
 
     InvalidateAfterBailout(cx, outerScript, "lexical check failure");
     if (innerScript->hasIonScript())
         Invalidate(cx, innerScript);
 }
 
+static void
+HandleIterNextNonStringBailout(JSContext* cx, HandleScript outerScript, HandleScript innerScript)
+{
+    JitSpew(JitSpew_IonBailouts, "Non-string iterator value %s:%zu, inlined into %s:%zu",
+            innerScript->filename(), innerScript->lineno(),
+            outerScript->filename(), outerScript->lineno());
+
+    // This should only happen when legacy generators are used.
+    ForbidCompilation(cx, innerScript);
+    InvalidateAfterBailout(cx, outerScript, "non-string iterator value");
+}
+
 static bool
 CopyFromRematerializedFrame(JSContext* cx, JitActivation* act, uint8_t* fp, size_t inlineDepth,
                             BaselineFrame* frame)
 {
     RematerializedFrame* rematFrame = act->lookupRematerializedFrame(fp, inlineDepth);
 
     // We might not have rematerialized a frame if the user never requested a
     // Debugger.Frame for it.
@@ -2012,22 +2024,25 @@ jit::FinishBailoutToBaseline(BaselineBai
         // thus rely on the check for frequent bailouts to recompile the current
         // script.
         break;
 
       // Invalid assumption based on baseline code.
       case Bailout_OverflowInvalidate:
         outerScript->setHadOverflowBailout();
         MOZ_FALLTHROUGH;
-      case Bailout_NonStringInputInvalidate:
       case Bailout_DoubleOutput:
       case Bailout_ObjectIdentityOrTypeGuard:
         HandleBaselineInfoBailout(cx, outerScript, innerScript);
         break;
 
+      case Bailout_IterNextNonString:
+        HandleIterNextNonStringBailout(cx, outerScript, innerScript);
+        break;
+
       case Bailout_ArgumentCheck:
         // Do nothing, bailout will resume before the argument monitor ICs.
         break;
       case Bailout_BoundsCheck:
       case Bailout_Detached:
         HandleBoundsCheckFailure(cx, outerScript, innerScript);
         break;
       case Bailout_ShapeGuard:
--- a/js/src/jit/BaselineCompiler.cpp
+++ b/js/src/jit/BaselineCompiler.cpp
@@ -1017,17 +1017,16 @@ BaselineCompiler::emitBody()
             return Method_Error;
 
         switch (op) {
           // ===== NOT Yet Implemented =====
           case JSOP_FORCEINTERPRETER:
             // Intentionally not implemented.
           case JSOP_SETINTRINSIC:
             // Run-once opcode during self-hosting initialization.
-          case JSOP_UNUSED222:
           case JSOP_UNUSED223:
           case JSOP_LIMIT:
             // === !! WARNING WARNING WARNING !! ===
             // Do you really want to sacrifice performance by not implementing
             // this operation in the BaselineCompiler?
             JitSpew(JitSpew_BaselineAbort, "Unhandled op: %s", CodeName[op]);
             return Method_CantCompile;
 
@@ -1065,16 +1064,22 @@ OPCODE_LIST(EMIT_OP)
 
 bool
 BaselineCompiler::emit_JSOP_NOP()
 {
     return true;
 }
 
 bool
+BaselineCompiler::emit_JSOP_ITERNEXT()
+{
+    return true;
+}
+
+bool
 BaselineCompiler::emit_JSOP_NOP_DESTRUCTURING()
 {
     return true;
 }
 
 bool
 BaselineCompiler::emit_JSOP_TRY_DESTRUCTURING_ITERCLOSE()
 {
--- a/js/src/jit/BaselineCompiler.h
+++ b/js/src/jit/BaselineCompiler.h
@@ -28,16 +28,17 @@
 
 namespace js {
 namespace jit {
 
 #define OPCODE_LIST(_)         \
     _(JSOP_NOP)                \
     _(JSOP_NOP_DESTRUCTURING)  \
     _(JSOP_LABEL)              \
+    _(JSOP_ITERNEXT)           \
     _(JSOP_POP)                \
     _(JSOP_POPN)               \
     _(JSOP_DUPAT)              \
     _(JSOP_ENTERWITH)          \
     _(JSOP_LEAVEWITH)          \
     _(JSOP_DUP)                \
     _(JSOP_DUP2)               \
     _(JSOP_SWAP)               \
--- a/js/src/jit/IonBuilder.cpp
+++ b/js/src/jit/IonBuilder.cpp
@@ -668,16 +668,17 @@ IonBuilder::analyzeNewLoopTypes(const CF
               case JSOP_IN:
               case JSOP_INSTANCEOF:
               case JSOP_HASOWN:
                 type = MIRType::Boolean;
                 break;
               case JSOP_DOUBLE:
                 type = MIRType::Double;
                 break;
+              case JSOP_ITERNEXT:
               case JSOP_STRING:
               case JSOP_TOSTRING:
               case JSOP_TYPEOF:
               case JSOP_TYPEOFEXPR:
                 type = MIRType::String;
                 break;
               case JSOP_SYMBOL:
                 type = MIRType::Symbol;
@@ -2261,16 +2262,19 @@ IonBuilder::inspectOpcode(JSOp op)
         return jsop_toasyncgen();
 
       case JSOP_TOASYNCITER:
         return jsop_toasynciter();
 
       case JSOP_TOID:
         return jsop_toid();
 
+      case JSOP_ITERNEXT:
+        return jsop_iternext();
+
       case JSOP_LAMBDA:
         return jsop_lambda(info().getFunction(pc));
 
       case JSOP_LAMBDA_ARROW:
         return jsop_lambda_arrow(info().getFunction(pc));
 
       case JSOP_SETFUNNAME:
         return jsop_setfunname(GET_UINT8(pc));
@@ -2439,17 +2443,16 @@ IonBuilder::inspectOpcode(JSOp op)
         // Do you really want to sacrifice performance by not implementing this
         // operation in the optimizing compiler?
         break;
 
       case JSOP_FORCEINTERPRETER:
         // Intentionally not implemented.
         break;
 
-      case JSOP_UNUSED222:
       case JSOP_UNUSED223:
       case JSOP_LIMIT:
         break;
     }
 
     // Track a simpler message, since the actionable abort message is a
     // static string, and the internal opcode name isn't an actionable
     // thing anyways.
@@ -12617,16 +12620,33 @@ IonBuilder::jsop_iterend()
     MDefinition* iter = current->pop();
     MInstruction* ins = MIteratorEnd::New(alloc(), iter);
 
     current->add(ins);
 
     return resumeAfter(ins);
 }
 
+AbortReasonOr<Ok>
+IonBuilder::jsop_iternext()
+{
+    MDefinition* def = current->pop();
+    MOZ_ASSERT(def->type() == MIRType::Value);
+
+    // The value should be a string in most cases. Legacy generators can return
+    // non-string values, so in that case bailout and give up Ion compilation
+    // of the script.
+    MInstruction* unbox = MUnbox::New(alloc(), def, MIRType::String, MUnbox::Fallible,
+                                      Bailout_IterNextNonString);
+    current->add(unbox);
+    current->push(unbox);
+
+    return Ok();
+}
+
 MDefinition*
 IonBuilder::walkEnvironmentChain(unsigned hops)
 {
     MDefinition* env = current->getSlot(info().environmentChainSlot());
 
     for (unsigned i = 0; i < hops; i++) {
         MInstruction* ins = MEnclosingEnvironment::New(alloc(), env);
         current->add(ins);
--- a/js/src/jit/IonBuilder.h
+++ b/js/src/jit/IonBuilder.h
@@ -573,16 +573,17 @@ class IonBuilder
     AbortReasonOr<Ok> jsop_toasync();
     AbortReasonOr<Ok> jsop_toasyncgen();
     AbortReasonOr<Ok> jsop_toasynciter();
     AbortReasonOr<Ok> jsop_toid();
     AbortReasonOr<Ok> jsop_iter(uint8_t flags);
     AbortReasonOr<Ok> jsop_itermore();
     AbortReasonOr<Ok> jsop_isnoiter();
     AbortReasonOr<Ok> jsop_iterend();
+    AbortReasonOr<Ok> jsop_iternext();
     AbortReasonOr<Ok> jsop_in();
     AbortReasonOr<Ok> jsop_hasown();
     AbortReasonOr<Ok> jsop_instanceof();
     AbortReasonOr<Ok> jsop_getaliasedvar(EnvironmentCoordinate ec);
     AbortReasonOr<Ok> jsop_setaliasedvar(EnvironmentCoordinate ec);
     AbortReasonOr<Ok> jsop_debugger();
     AbortReasonOr<Ok> jsop_newtarget();
     AbortReasonOr<Ok> jsop_checkisobj(uint8_t kind);
--- a/js/src/jit/IonTypes.h
+++ b/js/src/jit/IonTypes.h
@@ -129,17 +129,17 @@ enum BailoutKind
     // Bailouts caused by invalid assumptions based on Baseline code.
     // Causes immediate invalidation.
 
     // Like Bailout_Overflow, but causes immediate invalidation.
     Bailout_OverflowInvalidate,
 
     // Like NonStringInput, but should cause immediate invalidation.
     // Used for jsop_iternext.
-    Bailout_NonStringInputInvalidate,
+    Bailout_IterNextNonString,
 
     // Used for integer division, multiplication and modulo.
     // If there's a remainder, bails to return a double.
     // Can also signal overflow or result of -0.
     // Can also signal division by 0 (returns inf, a double).
     Bailout_DoubleOutput,
 
     // END Invalid assumptions bailouts
@@ -224,18 +224,18 @@ BailoutKindString(BailoutKind kind)
       case Bailout_BadDerivedConstructorReturn:
         return "Bailout_BadDerivedConstructorReturn";
       case Bailout_FirstExecution:
         return "Bailout_FirstExecution";
 
       // Bailouts caused by invalid assumptions.
       case Bailout_OverflowInvalidate:
         return "Bailout_OverflowInvalidate";
-      case Bailout_NonStringInputInvalidate:
-        return "Bailout_NonStringInputInvalidate";
+      case Bailout_IterNextNonString:
+        return "Bailout_IterNextNonString";
       case Bailout_DoubleOutput:
         return "Bailout_DoubleOutput";
 
       // Other bailouts.
       case Bailout_ArgumentCheck:
         return "Bailout_ArgumentCheck";
       case Bailout_BoundsCheck:
         return "Bailout_BoundsCheck";
--- a/js/src/jsopcode.cpp
+++ b/js/src/jsopcode.cpp
@@ -690,16 +690,17 @@ BytecodeParser::simulateOp(JSOp op, uint
       case JSOP_SETARG:
       case JSOP_SETINTRINSIC:
       case JSOP_SETLOCAL:
       case JSOP_THROWSETALIASEDCONST:
       case JSOP_THROWSETCALLEE:
       case JSOP_THROWSETCONST:
       case JSOP_INITALIASEDLEXICAL:
       case JSOP_INITIALYIELD:
+      case JSOP_ITERNEXT:
         // Keep the top value.
         MOZ_ASSERT(nuses == 1);
         MOZ_ASSERT(ndefs == 1);
         break;
 
       case JSOP_INITHOMEOBJECT:
         // Keep the top 2 values.
         MOZ_ASSERT(nuses == 2);
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -2008,17 +2008,17 @@ CASE(EnableInterruptsPseudoOpcode)
     SANITY_CHECKS();
     DISPATCH_TO(op);
 }
 
 /* Various 1-byte no-ops. */
 CASE(JSOP_NOP)
 CASE(JSOP_NOP_DESTRUCTURING)
 CASE(JSOP_TRY_DESTRUCTURING_ITERCLOSE)
-CASE(JSOP_UNUSED222)
+CASE(JSOP_ITERNEXT)
 CASE(JSOP_UNUSED223)
 CASE(JSOP_CONDSWITCH)
 {
     MOZ_ASSERT(CodeSpec[*REGS.pc].length == 1);
     ADVANCE_AND_DISPATCH(1);
 }
 
 CASE(JSOP_TRY)
--- a/js/src/vm/Opcodes.h
+++ b/js/src/vm/Opcodes.h
@@ -2264,18 +2264,26 @@ 1234567890123456789012345678901234567890
     /*
      * Pushes the current global's builtin prototype for a given proto key
      *   Category: Literals
      *   Type: Constants
      *   Operands: uint8_t kind
      *   Stack: => %BuiltinPrototype%
      */ \
     macro(JSOP_BUILTINPROTO, 221, "builtinproto", NULL, 2,  0,  1,  JOF_UINT8) \
-    macro(JSOP_UNUSED222,     222,"unused222",     NULL,  1,  0,  0,  JOF_BYTE) \
-    macro(JSOP_UNUSED223,     223,"unused223",     NULL,  1,  0,  0,  JOF_BYTE) \
+    \
+    /*
+     * NOP opcode to hint to IonBuilder that the value on top of the stack is
+     * the (likely string) key in a for-in loop.
+     *   Category: Other
+     *   Operands:
+     *   Stack: val => val
+     */ \
+    macro(JSOP_ITERNEXT,      222, "iternext",   NULL,  1,  1,  1,  JOF_BYTE) \
+    macro(JSOP_UNUSED223,     223, "unused223",  NULL,  1,  0,  0,  JOF_BYTE) \
     \
     /*
      * Creates rest parameter array for current function call, and pushes it
      * onto the stack.
      *   Category: Variables and Scopes
      *   Type: Arguments
      *   Operands:
      *   Stack: => rest