Bug 1514625 - Clean up and simplify loop entry code in Ion more. r=nbp
authorJan de Mooij <jdemooij@mozilla.com>
Mon, 07 Jan 2019 16:00:51 +0000
changeset 509818 10276b98cd16c4537cbd2f344354f2c38b5b7e04
parent 509817 5c8bcb1cf3e7c6f9bca3e8afb582ce81b8fed361
child 509819 653b89c61e4946a8ec12f33c9b16d4f751e0b956
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnbp
bugs1514625
milestone66.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 1514625 - Clean up and simplify loop entry code in Ion more. r=nbp For a broken loop we used to change CFGLoopEntry to CFGGoto, but that really complicates IonBuilder. It's simpler to keep the CFGLoopEntry and set a flag on it. Differential Revision: https://phabricator.services.mozilla.com/D15575
js/src/jit-test/tests/ion/bug1514625.js
js/src/jit/IonBuilder.cpp
js/src/jit/IonBuilder.h
js/src/jit/IonControlFlow.cpp
js/src/jit/IonControlFlow.h
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ion/bug1514625.js
@@ -0,0 +1,11 @@
+var i = 0;
+evaluate("");
+while (--i >= 0) {
+    if (x > 0) {
+        continue;
+    }
+    switch (i) {
+        default:
+          i(i);
+    }
+}
--- a/js/src/jit/IonBuilder.cpp
+++ b/js/src/jit/IonBuilder.cpp
@@ -1694,55 +1694,25 @@ AbortReasonOr<Ok> IonBuilder::visitBlock
 #endif
 
     pc += CodeSpec[op].length;
     current->updateTrackedSite(bytecodeSite(pc));
   }
   return Ok();
 }
 
-bool IonBuilder::blockIsOSREntry(const CFGBlock* block,
-                                 const CFGBlock* predecessor) {
-  jsbytecode* entryPc = block->startPc();
-
-  if (!info().osrPc()) {
-    return false;
-  }
-
-  if (entryPc == predecessor->startPc()) {
-    // The predecessor is the actual osr entry block. Since it is empty
-    // the current block also starts a the osr pc. But it isn't the osr entry.
-    MOZ_ASSERT(predecessor->stopPc() == predecessor->startPc());
-    return false;
-  }
-
-  if (block->stopPc() == block->startPc() && block->stopIns()->isBackEdge()) {
-    // An empty block with only a backedge can never be a loop entry.
-    return false;
-  }
-
-  MOZ_ASSERT(*info().osrPc() == JSOP_LOOPENTRY);
-  return info().osrPc() == entryPc;
-}
-
 AbortReasonOr<Ok> IonBuilder::visitGoto(CFGGoto* ins) {
-  // Test if this potentially was a fake loop and create OSR entry if that is
-  // the case.
-  const CFGBlock* successor = ins->getSuccessor(0);
-  if (blockIsOSREntry(successor, cfgCurrent)) {
-    MBasicBlock* preheader;
-    MOZ_TRY_VAR(preheader, newOsrPreheader(current, successor->startPc(), pc));
-    current->end(MGoto::New(alloc(), preheader));
-    MOZ_TRY(setCurrentAndSpecializePhis(preheader));
-  }
-
+  return emitGoto(ins->successor(), ins->popAmount());
+}
+
+AbortReasonOr<Ok> IonBuilder::emitGoto(CFGBlock* successor, size_t popAmount) {
   size_t id = successor->id();
   bool create = !blockWorklist[id] || blockWorklist[id]->isDead();
 
-  current->popn(ins->popAmount());
+  current->popn(popAmount);
 
   if (create) {
     MOZ_TRY_VAR(blockWorklist[id], newBlock(current, successor->startPc()));
   }
 
   MBasicBlock* succ = blockWorklist[id];
   current->end(MGoto::New(alloc(), succ));
 
@@ -1787,25 +1757,37 @@ AbortReasonOr<Ok> IonBuilder::visitBackE
     default:
       return abort(r);
   }
 }
 
 AbortReasonOr<Ok> IonBuilder::visitLoopEntry(CFGLoopEntry* loopEntry) {
   unsigned stackPhiCount = loopEntry->stackPhiCount();
   const CFGBlock* successor = loopEntry->getSuccessor(0);
-  bool osr = blockIsOSREntry(successor, cfgCurrent);
+  bool osr = successor->startPc() == info().osrPc();
   if (osr) {
     MOZ_ASSERT(loopEntry->canOsr());
     MBasicBlock* preheader;
     MOZ_TRY_VAR(preheader, newOsrPreheader(current, successor->startPc(), pc));
     current->end(MGoto::New(alloc(), preheader));
     MOZ_TRY(setCurrentAndSpecializePhis(preheader));
   }
 
+  if (loopEntry->isBrokenLoop()) {
+    // A "broken loop" is a loop that does not actually loop, for example:
+    //
+    //   while (x) {
+    //      return true;
+    //   }
+    //
+    // A broken loop has no backedge so we don't need a loop header and loop
+    // phis. Just emit a Goto to the loop entry.
+    return emitGoto(loopEntry->successor(), /* popAmount = */ 0);
+  }
+
   loopDepth_++;
   MBasicBlock* header;
   MOZ_TRY_VAR(header, newPendingLoopHeader(current, successor->startPc(), osr,
                                            loopEntry->canOsr(), stackPhiCount));
   blockWorklist[successor->id()] = header;
 
   current->end(MGoto::New(alloc(), header));
 
--- a/js/src/jit/IonBuilder.h
+++ b/js/src/jit/IonBuilder.h
@@ -138,17 +138,17 @@ class IonBuilder : public MIRGenerator,
   // Please see the Big Honkin' Comment about how resume points work in
   // IonBuilder.cpp, near the definition for this function.
   AbortReasonOr<Ok> resume(MInstruction* ins, jsbytecode* pc,
                            MResumePoint::Mode mode);
   AbortReasonOr<Ok> resumeAt(MInstruction* ins, jsbytecode* pc);
   AbortReasonOr<Ok> resumeAfter(MInstruction* ins);
   AbortReasonOr<Ok> maybeInsertResume();
 
-  bool blockIsOSREntry(const CFGBlock* block, const CFGBlock* predecessor);
+  AbortReasonOr<Ok> emitGoto(CFGBlock* successor, size_t popAmount);
 
   void insertRecompileCheck();
 
   bool usesEnvironmentChain();
 
   AbortReasonOr<Ok> initParameters();
   void initLocals();
   void rewriteParameter(uint32_t slotIdx, MDefinition* param);
--- a/js/src/jit/IonControlFlow.cpp
+++ b/js/src/jit/IonControlFlow.cpp
@@ -987,20 +987,17 @@ ControlFlowGenerator::processWhileOrForI
 
   return ControlStatus::Jumped;
 }
 
 ControlFlowGenerator::ControlStatus ControlFlowGenerator::processBrokenLoop(
     CFGState& state) {
   MOZ_ASSERT(!current);
 
-  {
-    state.loop.entry->setStopIns(CFGGoto::New(
-        alloc(), state.loop.entry->stopIns()->toLoopEntry()->successor()));
-  }
+  state.loop.entry->stopIns()->toLoopEntry()->setIsBrokenLoop();
 
   // If the loop started with a condition (while/for) then even if the
   // structure never actually loops, the condition itself can still fail and
   // thus we must resume at the successor, if one exists.
   current = state.loop.successor;
   if (current) {
     if (!addBlock(current)) {
       return ControlStatus::Error;
--- a/js/src/jit/IonControlFlow.h
+++ b/js/src/jit/IonControlFlow.h
@@ -446,54 +446,63 @@ class CFGBackEdge : public CFGUnaryContr
  * That block is the only block allowed to have a backedge.
  *
  * JMP block
  *
  */
 class CFGLoopEntry : public CFGUnaryControlInstruction {
   bool canOsr_;
   bool isForIn_;
+  bool isBrokenLoop_;
   size_t stackPhiCount_;
   jsbytecode* loopStopPc_;
 
   CFGLoopEntry(CFGBlock* block, size_t stackPhiCount)
       : CFGUnaryControlInstruction(block),
         canOsr_(false),
         isForIn_(false),
+        isBrokenLoop_(false),
         stackPhiCount_(stackPhiCount),
         loopStopPc_(nullptr) {}
 
-  CFGLoopEntry(CFGBlock* block, bool canOsr, bool isForIn, size_t stackPhiCount,
-               jsbytecode* loopStopPc)
+  CFGLoopEntry(CFGBlock* block, bool canOsr, bool isForIn, bool isBrokenLoop,
+               size_t stackPhiCount, jsbytecode* loopStopPc)
       : CFGUnaryControlInstruction(block),
         canOsr_(canOsr),
         isForIn_(isForIn),
+        isBrokenLoop_(isBrokenLoop),
         stackPhiCount_(stackPhiCount),
         loopStopPc_(loopStopPc) {}
 
  public:
   CFG_CONTROL_HEADER(LoopEntry);
   TRIVIAL_CFG_NEW_WRAPPERS
 
   static CFGLoopEntry* CopyWithNewTargets(TempAllocator& alloc,
                                           CFGLoopEntry* old,
                                           CFGBlock* loopEntry) {
     return new (alloc) CFGLoopEntry(loopEntry, old->canOsr(), old->isForIn(),
-                                    old->stackPhiCount(), old->loopStopPc());
+                                    old->isBrokenLoop(), old->stackPhiCount(),
+                                    old->maybeLoopStopPc());
   }
 
   void setCanOsr() { canOsr_ = true; }
 
+  bool isBrokenLoop() const { return isBrokenLoop_; }
+  void setIsBrokenLoop() { isBrokenLoop_ = true; }
+
   bool canOsr() const { return canOsr_; }
 
   void setIsForIn() { isForIn_ = true; }
   bool isForIn() const { return isForIn_; }
 
   size_t stackPhiCount() const { return stackPhiCount_; }
 
+  jsbytecode* maybeLoopStopPc() const { return loopStopPc_; }
+
   jsbytecode* loopStopPc() const {
     MOZ_ASSERT(loopStopPc_);
     return loopStopPc_;
   }
 
   void setLoopStopPc(jsbytecode* loopStopPc) { loopStopPc_ = loopStopPc; }
 };