Bug 1601599 part 4 - Support Ion OSR at all loops. r=tcampbell
authorJan de Mooij <jdemooij@mozilla.com>
Sun, 15 Dec 2019 11:34:34 +0000
changeset 507041 d9c1e44eb0addfc3e44ae299c86381128aa08e7f
parent 507040 93842c3971f8e612537cba5d6d25b482c0d67a66
child 507042 ba6bf1f3c39192724b8e72137ef5403352042453
push id103228
push userjdemooij@mozilla.com
push dateSun, 15 Dec 2019 11:41:22 +0000
treeherderautoland@ba6bf1f3c391 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcampbell
bugs1601599
milestone73.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 1601599 part 4 - Support Ion OSR at all loops. r=tcampbell Depends on D56700 Differential Revision: https://phabricator.services.mozilla.com/D56701
js/src/frontend/BytecodeControlStructures.cpp
js/src/frontend/BytecodeControlStructures.h
js/src/jit/BaselineCodeGen.cpp
js/src/jit/Ion.cpp
js/src/jit/IonBuilder.cpp
js/src/vm/BytecodeUtil.cpp
js/src/vm/BytecodeUtil.h
js/src/vm/Opcodes.h
--- a/js/src/frontend/BytecodeControlStructures.cpp
+++ b/js/src/frontend/BytecodeControlStructures.cpp
@@ -39,42 +39,16 @@ LabelControl::LabelControl(BytecodeEmitt
 LoopControl::LoopControl(BytecodeEmitter* bce, StatementKind loopKind)
     : BreakableControl(bce, loopKind), tdzCache_(bce) {
   MOZ_ASSERT(is<LoopControl>());
 
   LoopControl* enclosingLoop = findNearest<LoopControl>(enclosing());
 
   stackDepth_ = bce->bytecodeSection().stackDepth();
   loopDepth_ = enclosingLoop ? enclosingLoop->loopDepth_ + 1 : 1;
-
-  int loopSlots;
-  if (loopKind == StatementKind::Spread) {
-    // The iterator next method, the iterator, the result array, and
-    // the current array index are on the stack.
-    loopSlots = 4;
-  } else if (loopKind == StatementKind::ForOfLoop) {
-    // The iterator next method, the iterator, and the current value
-    // are on the stack.
-    loopSlots = 3;
-  } else if (loopKind == StatementKind::ForInLoop) {
-    // The iterator is on the stack.
-    loopSlots = 1;
-  } else {
-    // No additional loop values are on the stack.
-    loopSlots = 0;
-  }
-
-  MOZ_ASSERT(loopSlots <= stackDepth_);
-
-  if (enclosingLoop) {
-    canIonOsr_ = (enclosingLoop->canIonOsr_ &&
-                  stackDepth_ == enclosingLoop->stackDepth_ + loopSlots);
-  } else {
-    canIonOsr_ = stackDepth_ == loopSlots;
-  }
 }
 
 bool LoopControl::emitContinueTarget(BytecodeEmitter* bce) {
   // Note: this is always called after emitting the loop body so we must have
   // emitted all 'continues' by now.
   return bce->emitJumpTargetAndPatch(continues);
 }
 
@@ -115,18 +89,17 @@ bool LoopControl::emitLoopHead(BytecodeE
 
   BytecodeOffset off;
   if (!bce->newSrcNote(type)) {
     return false;
   }
   if (!bce->emitJumpTargetOp(JSOP_LOOPHEAD, &off)) {
     return false;
   }
-  SetLoopHeadDepthHintAndFlags(bce->bytecodeSection().code(off), loopDepth_,
-                               canIonOsr_);
+  SetLoopHeadDepthHint(bce->bytecodeSection().code(off), loopDepth_);
 
   return true;
 }
 
 bool LoopControl::emitLoopEnd(BytecodeEmitter* bce, JSOp op,
                               JSTryNoteKind tryNoteKind) {
   JumpList beq;
   if (!bce->emitBackwardJump(op, head_, &beq, &breakTarget_)) {
--- a/js/src/frontend/BytecodeControlStructures.h
+++ b/js/src/frontend/BytecodeControlStructures.h
@@ -113,20 +113,16 @@ class LoopControl : public BreakableCont
   JumpTarget breakTarget_;
 
   // Stack depth when this loop was pushed on the control stack.
   int32_t stackDepth_;
 
   // The loop nesting depth. Used as a hint to Ion.
   uint32_t loopDepth_;
 
-  // Can we OSR into Ion from here? True unless there is non-loop state on the
-  // stack.
-  bool canIonOsr_;
-
  public:
   // Offset of the last continue in the loop.
   JumpList continues;
 
   LoopControl(BytecodeEmitter* bce, StatementKind loopKind);
 
   BytecodeOffset headOffset() const { return head_.offset; }
   BytecodeOffset breakTargetOffset() const { return breakTarget_.offset; }
--- a/js/src/jit/BaselineCodeGen.cpp
+++ b/js/src/jit/BaselineCodeGen.cpp
@@ -1310,21 +1310,16 @@ bool BaselineCompilerCodeGen::emitWarmUp
   masm.store32(countReg, warmUpCounterAddr);
 
   if (JSOp(*pc) == JSOP_LOOPHEAD) {
     // If this is a loop inside a catch or finally block, increment the warmup
     // counter but don't attempt OSR (Ion only compiles the try block).
     if (handler.analysis().info(pc).loopHeadInCatchOrFinally) {
       return true;
     }
-
-    if (!LoopHeadCanIonOsr(pc)) {
-      // OSR into Ion not possible at this loop.
-      return true;
-    }
   }
 
   Label done;
 
   const OptimizationInfo* info =
       IonOptimizations.get(IonOptimizations.firstLevel());
   uint32_t warmUpThreshold = info->compilerWarmUpThreshold(script, pc);
   masm.branch32(Assembler::LessThan, countReg, Imm32(warmUpThreshold), &done);
--- a/js/src/jit/Ion.cpp
+++ b/js/src/jit/Ion.cpp
@@ -2067,17 +2067,17 @@ static OptimizationLevel GetOptimization
   return IonOptimizations.levelForScript(script, pc);
 }
 
 static MethodStatus Compile(JSContext* cx, HandleScript script,
                             BaselineFrame* osrFrame, uint32_t osrFrameSize,
                             jsbytecode* osrPc, bool forceRecompile = false) {
   MOZ_ASSERT(jit::IsIonEnabled());
   MOZ_ASSERT(jit::IsBaselineJitEnabled());
-  MOZ_ASSERT_IF(osrPc != nullptr, LoopHeadCanIonOsr(osrPc));
+
   AutoGeckoProfilerEntry pseudoFrame(
       cx, "Ion script compilation",
       JS::ProfilingCategoryPair::JS_IonCompilation);
 
   if (!script->hasBaselineScript()) {
     return Method_Skipped;
   }
 
@@ -2273,17 +2273,16 @@ static MethodStatus BaselineCanEnterAtEn
 // Decide if a transition from baseline execution to Ion code should occur.
 // May compile or recompile the target JSScript.
 static MethodStatus BaselineCanEnterAtBranch(JSContext* cx, HandleScript script,
                                              BaselineFrame* osrFrame,
                                              uint32_t osrFrameSize,
                                              jsbytecode* pc) {
   MOZ_ASSERT(jit::IsIonEnabled());
   MOZ_ASSERT((JSOp)*pc == JSOP_LOOPHEAD);
-  MOZ_ASSERT(LoopHeadCanIonOsr(pc));
 
   // Skip if the script has been disabled.
   if (!script->canIonCompile()) {
     return Method_Skipped;
   }
 
   // Skip if the script is being compiled off thread.
   if (script->isIonCompilingOffThread()) {
@@ -2351,18 +2350,16 @@ static MethodStatus BaselineCanEnterAtBr
 static bool IonCompileScriptForBaseline(JSContext* cx, BaselineFrame* frame,
                                         uint32_t frameSize, jsbytecode* pc) {
   MOZ_ASSERT(IsIonEnabled());
   MOZ_ASSERT(frame->debugFrameSize() == frameSize);
 
   RootedScript script(cx, frame->script());
   bool isLoopHead = JSOp(*pc) == JSOP_LOOPHEAD;
 
-  MOZ_ASSERT(!isLoopHead || LoopHeadCanIonOsr(pc));
-
   // The Baseline JIT code checks for Ion disabled or compiling off-thread.
   MOZ_ASSERT(script->canIonCompile());
   MOZ_ASSERT(!script->isIonCompilingOffThread());
 
   // If Ion script exists, but PC is not at a loop entry, then Ion will be
   // entered for this script at an appropriate LOOPENTRY or the next time this
   // function is called.
   if (script->hasIonScript() && !isLoopHead) {
@@ -2377,17 +2374,16 @@ static bool IonCompileScriptForBaseline(
   JitSpew(JitSpew_BaselineOSR,
           "WarmUpCounter for %s:%u:%u reached %d at pc %p, trying to switch to "
           "Ion!",
           script->filename(), script->lineno(), script->column(),
           (int)script->getWarmUpCount(), (void*)pc);
 
   MethodStatus stat;
   if (isLoopHead) {
-    MOZ_ASSERT(LoopHeadCanIonOsr(pc));
     JitSpew(JitSpew_BaselineOSR, "  Compile at loop head!");
     stat = BaselineCanEnterAtBranch(cx, script, frame, frameSize, pc);
   } else if (frame->isFunctionFrame()) {
     JitSpew(JitSpew_BaselineOSR,
             "  Compile function from top for later entry!");
     stat = BaselineCanEnterAtEntry(cx, script, frame, frameSize);
   } else {
     return true;
--- a/js/src/jit/IonBuilder.cpp
+++ b/js/src/jit/IonBuilder.cpp
@@ -1808,18 +1808,16 @@ AbortReasonOr<Ok> IonBuilder::jsop_looph
 
   if (hasTerminatedBlock()) {
     // The whole loop is unreachable.
     return Ok();
   }
 
   bool osr = pc == info().osrPc();
   if (osr) {
-    MOZ_ASSERT(LoopHeadCanIonOsr(pc));
-
     MBasicBlock* preheader;
     MOZ_TRY_VAR(preheader, newOsrPreheader(current, pc));
     current->end(MGoto::New(alloc(), preheader));
     MOZ_TRY(setCurrentAndSpecializePhis(preheader));
   }
 
   loopDepth_++;
   MBasicBlock* header;
--- a/js/src/vm/BytecodeUtil.cpp
+++ b/js/src/vm/BytecodeUtil.cpp
@@ -1576,18 +1576,18 @@ static unsigned Disassemble1(JSContext* 
 
     case JOF_ICINDEX:
       if (!sp->jsprintf(" (ic: %u)", GET_ICINDEX(pc))) {
         return 0;
       }
       break;
 
     case JOF_LOOPHEAD:
-      if (!sp->jsprintf(" (ic: %u, data: %u,%u)", GET_ICINDEX(pc),
-                        LoopHeadCanIonOsr(pc), LoopHeadDepthHint(pc))) {
+      if (!sp->jsprintf(" (ic: %u, depthHint: %u)", GET_ICINDEX(pc),
+                        LoopHeadDepthHint(pc))) {
         return 0;
       }
       break;
 
     case JOF_ARGC:
     case JOF_UINT16:
       i = (int)GET_UINT16(pc);
       goto print_int;
--- a/js/src/vm/BytecodeUtil.h
+++ b/js/src/vm/BytecodeUtil.h
@@ -249,29 +249,22 @@ static inline uint32_t GET_ICINDEX(const
 }
 
 static inline void SET_ICINDEX(jsbytecode* pc, uint32_t icIndex) {
   SET_UINT32(pc, icIndex);
 }
 
 static inline unsigned LoopHeadDepthHint(jsbytecode* pc) {
   MOZ_ASSERT(*pc == JSOP_LOOPHEAD);
-  return GET_UINT8(pc + 4) & 0x7f;
+  return GET_UINT8(pc + 4);
 }
 
-static inline bool LoopHeadCanIonOsr(jsbytecode* pc) {
+static inline void SetLoopHeadDepthHint(jsbytecode* pc, unsigned loopDepth) {
   MOZ_ASSERT(*pc == JSOP_LOOPHEAD);
-  return GET_UINT8(pc + 4) & 0x80;
-}
-
-static inline void SetLoopHeadDepthHintAndFlags(jsbytecode* pc,
-                                                unsigned loopDepth,
-                                                bool canIonOsr) {
-  MOZ_ASSERT(*pc == JSOP_LOOPHEAD);
-  uint8_t data = std::min(loopDepth, unsigned(0x7f)) | (canIonOsr ? 0x80 : 0);
+  uint8_t data = std::min(loopDepth, unsigned(UINT8_MAX));
   SET_UINT8(pc + 4, data);
 }
 
 static inline bool IsBackedgePC(jsbytecode* pc) {
   switch (JSOp(*pc)) {
     case JSOP_GOTO:
     case JSOP_IFNE:
       return GET_JUMP_OFFSET(pc) < 0;
--- a/js/src/vm/Opcodes.h
+++ b/js/src/vm/Opcodes.h
@@ -1107,27 +1107,24 @@
      *   Operands: uint16_t argc
      *   Stack: callee, this, args[0], ..., args[argc-1] => rval
      *   nuses: (argc+2)
      */ \
     MACRO(JSOP_FUNCALL, 108, "funcall", NULL, 3, -1, 1, JOF_ARGC|JOF_INVOKE|JOF_TYPESET|JOF_IC) \
     /*
      * This opcode is the target of the backwards jump for some loop.
      *
-     * The uint8 argument is a bitfield. The lower 7 bits of the argument
-     * indicate the loop depth. This value starts at 1 and is just a hint:
-     * deeply nested loops all have the same value. The upper bit is set if Ion
-     * should be able to OSR at this point, which is true unless there is
-     * non-loop state on the stack.
+     * The depthHint value is a loop depth hint for Ion. It starts at 1 and
+     * deeply nested loops all have the same value.
      *
      * See JSOP_JUMPTARGET for the icIndex operand.
      *
      *   Category: Statements
      *   Type: Jumps
-     *   Operands: uint32_t icIndex, uint8_t BITFIELD
+     *   Operands: uint32_t icIndex, uint8_t depthHint
      *   Stack: =>
      */ \
     MACRO(JSOP_LOOPHEAD, 109, "loophead", NULL, 6, 0, 0, JOF_LOOPHEAD) \
     /*
      * Looks up name on the environment chain and pushes the environment which
      * contains the name onto the stack. If not found, pushes global lexical
      * environment onto the stack.
      *