Bug 1298139 - Make LoopUnroller code handle OOM correctly. r=nbp
authorJan de Mooij <jdemooij@mozilla.com>
Sat, 27 May 2017 23:40:51 +0200
changeset 409133 3f347989ea45c9ab7e1a3498b6bebb268f34362e
parent 409132 a5bee800882e91f9609c934707bd48187819a987
child 409134 4000d93da08918e8651e8680e32dcdf09c9c9f33
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnbp
bugs1298139
milestone55.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 1298139 - Make LoopUnroller code handle OOM correctly. r=nbp
js/src/jit/LoopUnroller.cpp
--- a/js/src/jit/LoopUnroller.cpp
+++ b/js/src/jit/LoopUnroller.cpp
@@ -44,19 +44,19 @@ struct LoopUnroller
     MBasicBlock* oldPreheader;
     MBasicBlock* newPreheader;
 
     // Map terms in the original loop to terms in the current unrolled iteration.
     DefinitionMap unrolledDefinitions;
 
     MDefinition* getReplacementDefinition(MDefinition* def);
     MResumePoint* makeReplacementResumePoint(MBasicBlock* block, MResumePoint* rp);
-    bool makeReplacementInstruction(MInstruction* ins);
+    MOZ_MUST_USE bool makeReplacementInstruction(MInstruction* ins);
 
-    void go(LoopIterationBound* bound);
+    MOZ_MUST_USE bool go(LoopIterationBound* bound);
 };
 
 } // namespace
 
 MDefinition*
 LoopUnroller::getReplacementDefinition(MDefinition* def)
 {
     if (def->block()->id() < header->id()) {
@@ -94,16 +94,18 @@ LoopUnroller::makeReplacementInstruction
 
     unrolledBackedge->add(clone);
 
     if (!unrolledDefinitions.putNew(ins, clone))
         return false;
 
     if (MResumePoint* old = ins->resumePoint()) {
         MResumePoint* rp = makeReplacementResumePoint(unrolledBackedge, old);
+        if (!rp)
+            return false;
         clone->setResumePoint(rp);
     }
 
     return true;
 }
 
 MResumePoint*
 LoopUnroller::makeReplacementResumePoint(MBasicBlock* block, MResumePoint* rp)
@@ -118,168 +120,176 @@ LoopUnroller::makeReplacementResumePoint
 
     MResumePoint* clone = MResumePoint::New(alloc, block, rp, inputs);
     if (!clone)
         return nullptr;
 
     return clone;
 }
 
-void
+bool
 LoopUnroller::go(LoopIterationBound* bound)
 {
     // For now we always unroll loops the same number of times.
     static const size_t UnrollCount = 10;
 
     JitSpew(JitSpew_Unrolling, "Attempting to unroll loop");
 
     header = bound->header;
 
     // UCE might have determined this isn't actually a loop.
     if (!header->isLoopHeader())
-        return;
+        return true;
 
     backedge = header->backedge();
     oldPreheader = header->loopPredecessor();
 
     MOZ_ASSERT(oldPreheader->numSuccessors() == 1);
 
     // Only unroll loops with two blocks: an initial one ending with the
     // bound's test, and the body ending with the backedge.
     MTest* test = bound->test;
     if (header->lastIns() != test)
-        return;
+        return true;
     if (test->ifTrue() == backedge) {
         if (test->ifFalse()->id() <= backedge->id())
-            return;
+            return true;
     } else if (test->ifFalse() == backedge) {
         if (test->ifTrue()->id() <= backedge->id())
-            return;
+            return true;
     } else {
-        return;
+        return true;
     }
     if (backedge->numPredecessors() != 1 || backedge->numSuccessors() != 1)
-        return;
+        return true;
     MOZ_ASSERT(backedge->phisEmpty());
 
     MBasicBlock* bodyBlocks[] = { header, backedge };
 
     // All instructions in the header and body must be clonable.
     for (size_t i = 0; i < ArrayLength(bodyBlocks); i++) {
         MBasicBlock* block = bodyBlocks[i];
         for (MInstructionIterator iter(block->begin()); iter != block->end(); iter++) {
             MInstruction* ins = *iter;
             if (ins->canClone())
                 continue;
             if (ins->isTest() || ins->isGoto() || ins->isInterruptCheck())
                 continue;
 #ifdef JS_JITSPEW
             JitSpew(JitSpew_Unrolling, "Aborting: can't clone instruction %s", ins->opName());
 #endif
-            return;
+            return true;
         }
     }
 
     // Compute the linear inequality we will use for exiting the unrolled loop:
     //
     // iterationBound - iterationCount - UnrollCount >= 0
     //
     LinearSum remainingIterationsInequality(bound->boundSum);
     if (!remainingIterationsInequality.add(bound->currentSum, -1))
-        return;
+        return true;
     if (!remainingIterationsInequality.add(-int32_t(UnrollCount)))
-        return;
+        return true;
 
     // Terms in the inequality need to be either loop invariant or phis from
     // the original header.
     for (size_t i = 0; i < remainingIterationsInequality.numTerms(); i++) {
         MDefinition* def = remainingIterationsInequality.term(i).term;
         if (def->isDiscarded())
-            return;
+            return true;
         if (def->block()->id() < header->id())
             continue;
         if (def->block() == header && def->isPhi())
             continue;
-        return;
+        return true;
     }
 
     // OK, we've checked everything, now unroll the loop.
 
     JitSpew(JitSpew_Unrolling, "Unrolling loop");
 
     // The old preheader will go before the unrolled loop, and the old loop
     // will need a new empty preheader.
     const CompileInfo& info = oldPreheader->info();
     if (header->trackedPc()) {
         unrolledHeader =
             MBasicBlock::New(graph, nullptr, info,
                              oldPreheader, header->trackedSite(), MBasicBlock::LOOP_HEADER);
+        if (!unrolledHeader)
+            return false;
         unrolledBackedge =
             MBasicBlock::New(graph, nullptr, info,
                              unrolledHeader, backedge->trackedSite(), MBasicBlock::NORMAL);
+        if (!unrolledBackedge)
+            return false;
         newPreheader =
             MBasicBlock::New(graph, nullptr, info,
                              unrolledHeader, oldPreheader->trackedSite(), MBasicBlock::NORMAL);
+        if (!newPreheader)
+            return false;
     } else {
         unrolledHeader = MBasicBlock::New(graph, info, oldPreheader, MBasicBlock::LOOP_HEADER);
+        if (!unrolledHeader)
+            return false;
         unrolledBackedge = MBasicBlock::New(graph, info, unrolledHeader, MBasicBlock::NORMAL);
+        if (!unrolledBackedge)
+            return false;
         newPreheader = MBasicBlock::New(graph, info, unrolledHeader, MBasicBlock::NORMAL);
+        if (!newPreheader)
+            return false;
     }
 
     unrolledHeader->discardAllResumePoints();
     unrolledBackedge->discardAllResumePoints();
     newPreheader->discardAllResumePoints();
 
     // Insert new blocks at their RPO position, and update block ids.
     graph.insertBlockAfter(oldPreheader, unrolledHeader);
     graph.insertBlockAfter(unrolledHeader, unrolledBackedge);
     graph.insertBlockAfter(unrolledBackedge, newPreheader);
     graph.renumberBlocksAfter(oldPreheader);
 
-    // We don't tolerate allocation failure after this point.
-    // TODO: This is a bit drastic, is it possible to improve this?
-    AutoEnterOOMUnsafeRegion oomUnsafe;
-
     if (!unrolledDefinitions.init())
-        oomUnsafe.crash("LoopUnroller::go");
+        return false;
 
     // Add phis to the unrolled loop header which correspond to the phis in the
     // original loop header.
     MOZ_ASSERT(header->getPredecessor(0) == oldPreheader);
     for (MPhiIterator iter(header->phisBegin()); iter != header->phisEnd(); iter++) {
         MPhi* old = *iter;
         MOZ_ASSERT(old->numOperands() == 2);
         MPhi* phi = MPhi::New(alloc);
         phi->setResultType(old->type());
         phi->setResultTypeSet(old->resultTypeSet());
         phi->setRange(old->range());
 
         unrolledHeader->addPhi(phi);
 
         if (!phi->reserveLength(2))
-            oomUnsafe.crash("LoopUnroller::go");
+            return false;
 
         // Set the first input for the phi for now. We'll set the second after
         // finishing the unroll.
         phi->addInput(old->getOperand(0));
 
         // The old phi will now take the value produced by the unrolled loop.
         old->replaceOperand(0, phi);
 
         if (!unrolledDefinitions.putNew(old, phi))
-            oomUnsafe.crash("LoopUnroller::go");
+            return false;
     }
 
     // The loop condition can bail out on e.g. integer overflow, so make a
     // resume point based on the initial resume point of the original header.
     MResumePoint* headerResumePoint = header->entryResumePoint();
     if (headerResumePoint) {
         MResumePoint* rp = makeReplacementResumePoint(unrolledHeader, headerResumePoint);
         if (!rp)
-            oomUnsafe.crash("LoopUnroller::makeReplacementResumePoint");
+            return false;
         unrolledHeader->setEntryResumePoint(rp);
 
         // Perform an interrupt check at the start of the unrolled loop.
         unrolledHeader->add(MInterruptCheck::New(alloc));
     }
 
     // Generate code for the test in the unrolled loop.
     for (size_t i = 0; i < remainingIterationsInequality.numTerms(); i++) {
@@ -292,57 +302,57 @@ LoopUnroller::go(LoopIterationBound* bou
     unrolledHeader->end(unrolledTest);
 
     // Make an entry resume point for the unrolled body. The unrolled header
     // does not have side effects on stack values, even if the original loop
     // header does, so use the same resume point as for the unrolled header.
     if (headerResumePoint) {
         MResumePoint* rp = makeReplacementResumePoint(unrolledBackedge, headerResumePoint);
         if (!rp)
-            oomUnsafe.crash("LoopUnroller::makeReplacementResumePoint");
+            return false;
         unrolledBackedge->setEntryResumePoint(rp);
     }
 
     // Make an entry resume point for the new preheader. There are no
     // instructions which use this but some other stuff wants one to be here.
     if (headerResumePoint) {
         MResumePoint* rp = makeReplacementResumePoint(newPreheader, headerResumePoint);
         if (!rp)
-            oomUnsafe.crash("LoopUnroller::makeReplacementResumePoint");
+            return false;
         newPreheader->setEntryResumePoint(rp);
     }
 
     // Generate the unrolled code.
     MOZ_ASSERT(UnrollCount > 1);
     size_t unrollIndex = 0;
     while (true) {
         // Clone the contents of the original loop into the unrolled loop body.
         for (size_t i = 0; i < ArrayLength(bodyBlocks); i++) {
             MBasicBlock* block = bodyBlocks[i];
             for (MInstructionIterator iter(block->begin()); iter != block->end(); iter++) {
                 MInstruction* ins = *iter;
                 if (ins->canClone()) {
                     if (!makeReplacementInstruction(*iter))
-                        oomUnsafe.crash("LoopUnroller::makeReplacementDefinition");
+                        return false;
                 } else {
                     // Control instructions are handled separately.
                     MOZ_ASSERT(ins->isTest() || ins->isGoto() || ins->isInterruptCheck());
                 }
             }
         }
 
         // Compute the value of each loop header phi after the execution of
         // this unrolled iteration.
         MDefinitionVector phiValues(alloc);
         MOZ_ASSERT(header->getPredecessor(1) == backedge);
         for (MPhiIterator iter(header->phisBegin()); iter != header->phisEnd(); iter++) {
             MPhi* old = *iter;
             MDefinition* oldInput = old->getOperand(1);
             if (!phiValues.append(getReplacementDefinition(oldInput)))
-                oomUnsafe.crash("LoopUnroller::go");
+                return false;
         }
 
         unrolledDefinitions.clear();
 
         if (unrollIndex == UnrollCount - 1) {
             // We're at the end of the last unrolled iteration, set the
             // backedge input for the unrolled loop phis.
             size_t phiIndex = 0;
@@ -354,17 +364,17 @@ LoopUnroller::go(LoopIterationBound* bou
             break;
         }
 
         // Update the map for the phis in the next iteration.
         size_t phiIndex = 0;
         for (MPhiIterator iter(header->phisBegin()); iter != header->phisEnd(); iter++) {
             MPhi* old = *iter;
             if (!unrolledDefinitions.putNew(old, phiValues[phiIndex++]))
-                oomUnsafe.crash("LoopUnroller::go");
+                return false;
         }
         MOZ_ASSERT(phiIndex == phiValues.length());
 
         unrollIndex++;
     }
 
     MGoto* backedgeJump = MGoto::New(alloc, unrolledHeader);
     unrolledBackedge->end(backedgeJump);
@@ -374,32 +384,34 @@ LoopUnroller::go(LoopIterationBound* bou
     oldPreheader->discardLastIns();
     oldPreheader->end(MGoto::New(alloc, unrolledHeader));
 
     // Place the new preheader before the original loop.
     newPreheader->end(MGoto::New(alloc, header));
 
     // Cleanup the MIR graph.
     if (!unrolledHeader->addPredecessorWithoutPhis(unrolledBackedge))
-        oomUnsafe.crash("LoopUnroller::go");
+        return false;
     header->replacePredecessor(oldPreheader, newPreheader);
     oldPreheader->setSuccessorWithPhis(unrolledHeader, 0);
     newPreheader->setSuccessorWithPhis(header, 0);
     unrolledBackedge->setSuccessorWithPhis(unrolledHeader, 1);
+    return true;
 }
 
 bool
 jit::UnrollLoops(MIRGraph& graph, const LoopIterationBoundVector& bounds)
 {
     if (bounds.empty())
         return true;
 
     for (size_t i = 0; i < bounds.length(); i++) {
         LoopUnroller unroller(graph);
-        unroller.go(bounds[i]);
+        if (!unroller.go(bounds[i]))
+            return false;
     }
 
     // The MIR graph is valid, but now has several new blocks. Update or
     // recompute previous analysis information for the remaining optimization
     // passes.
     ClearDominatorTree(graph);
     if (!BuildDominatorTree(graph))
         return false;