Bug 1448089 - Make MBasicBlock::addPredecessorSameInputsAs fallible. r=tcampbell
authorNicolas B. Pierron <nicolas.b.pierron@gmail.com>
Thu, 19 Apr 2018 07:32:00 -0400
changeset 414740 131166cff47d79cdc09dd89a5a7d5d4ec9af6a75
parent 414739 b876ed208711bc346d7ca95b0599f6e4eb02ff2e
child 414741 6b86063d752831d485d3f61596c653d00b87fb79
push id102416
push userryanvm@gmail.com
push dateFri, 20 Apr 2018 20:17:04 +0000
treeherdermozilla-inbound@131166cff47d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcampbell
bugs1448089
milestone61.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 1448089 - Make MBasicBlock::addPredecessorSameInputsAs fallible. r=tcampbell
js/src/jit/IonAnalysis.cpp
js/src/jit/MIRGraph.cpp
js/src/jit/MIRGraph.h
--- a/js/src/jit/IonAnalysis.cpp
+++ b/js/src/jit/IonAnalysis.cpp
@@ -709,72 +709,77 @@ BlockIsSingleTest(MBasicBlock* phiBlock,
     *pphi = phi;
     *ptest = test;
 
     return true;
 }
 
 // Change block so that it ends in a goto to the specific target block.
 // existingPred is an existing predecessor of the block.
-static void
+static MOZ_MUST_USE bool
 UpdateGotoSuccessor(TempAllocator& alloc, MBasicBlock* block, MBasicBlock* target,
                      MBasicBlock* existingPred)
 {
     MInstruction* ins = block->lastIns();
     MOZ_ASSERT(ins->isGoto());
     ins->toGoto()->target()->removePredecessor(block);
     block->discardLastIns();
 
     MGoto* newGoto = MGoto::New(alloc, target);
     block->end(newGoto);
 
-    target->addPredecessorSameInputsAs(block, existingPred);
+    return target->addPredecessorSameInputsAs(block, existingPred);
 }
 
 // Change block so that it ends in a test of the specified value, going to
 // either ifTrue or ifFalse. existingPred is an existing predecessor of ifTrue
 // or ifFalse with the same values incoming to ifTrue/ifFalse as block.
 // existingPred is not required to be a predecessor of ifTrue/ifFalse if block
 // already ends in a test going to that block on a true/false result.
-static void
+static MOZ_MUST_USE bool
 UpdateTestSuccessors(TempAllocator& alloc, MBasicBlock* block,
                      MDefinition* value, MBasicBlock* ifTrue, MBasicBlock* ifFalse,
                      MBasicBlock* existingPred)
 {
     MInstruction* ins = block->lastIns();
     if (ins->isTest()) {
         MTest* test = ins->toTest();
         MOZ_ASSERT(test->input() == value);
 
         if (ifTrue != test->ifTrue()) {
             test->ifTrue()->removePredecessor(block);
-            ifTrue->addPredecessorSameInputsAs(block, existingPred);
+            if (!ifTrue->addPredecessorSameInputsAs(block, existingPred))
+                return false;
             MOZ_ASSERT(test->ifTrue() == test->getSuccessor(0));
             test->replaceSuccessor(0, ifTrue);
         }
 
         if (ifFalse != test->ifFalse()) {
             test->ifFalse()->removePredecessor(block);
-            ifFalse->addPredecessorSameInputsAs(block, existingPred);
+            if (!ifFalse->addPredecessorSameInputsAs(block, existingPred))
+                return false;
             MOZ_ASSERT(test->ifFalse() == test->getSuccessor(1));
             test->replaceSuccessor(1, ifFalse);
         }
 
-        return;
+        return true;
     }
 
     MOZ_ASSERT(ins->isGoto());
     ins->toGoto()->target()->removePredecessor(block);
     block->discardLastIns();
 
     MTest* test = MTest::New(alloc, value, ifTrue, ifFalse);
     block->end(test);
 
-    ifTrue->addPredecessorSameInputsAs(block, existingPred);
-    ifFalse->addPredecessorSameInputsAs(block, existingPred);
+    if (!ifTrue->addPredecessorSameInputsAs(block, existingPred))
+        return false;
+    if (!ifFalse->addPredecessorSameInputsAs(block, existingPred))
+        return false;
+    return true;
 }
 
 static bool
 MaybeFoldConditionBlock(MIRGraph& graph, MBasicBlock* initialBlock)
 {
     // Optimize the MIR graph to improve the code generated for conditional
     // operations. A test like 'if (a ? b : c)' normally requires four blocks,
     // with a phi for the intermediate value. This can be improved to use three
@@ -869,37 +874,48 @@ MaybeFoldConditionBlock(MIRGraph& graph,
 
     MBasicBlock* trueTarget = trueBranch;
     bool constBool;
     if (BlockComputesConstant(trueBranch, trueResult, &constBool)) {
         trueTarget = constBool ? finalTest->ifTrue() : finalTest->ifFalse();
         phiBlock->removePredecessor(trueBranch);
         graph.removeBlock(trueBranch);
     } else if (initialTest->input() == trueResult) {
-        UpdateGotoSuccessor(graph.alloc(), trueBranch, finalTest->ifTrue(), testBlock);
+        if (!UpdateGotoSuccessor(graph.alloc(), trueBranch, finalTest->ifTrue(), testBlock))
+            return false;
     } else {
-        UpdateTestSuccessors(graph.alloc(), trueBranch, trueResult,
-                             finalTest->ifTrue(), finalTest->ifFalse(), testBlock);
+        if (!UpdateTestSuccessors(graph.alloc(), trueBranch, trueResult,
+                                  finalTest->ifTrue(), finalTest->ifFalse(), testBlock))
+        {
+            return false;
+        }
     }
 
     MBasicBlock* falseTarget = falseBranch;
     if (BlockComputesConstant(falseBranch, falseResult, &constBool)) {
         falseTarget = constBool ? finalTest->ifTrue() : finalTest->ifFalse();
         phiBlock->removePredecessor(falseBranch);
         graph.removeBlock(falseBranch);
     } else if (initialTest->input() == falseResult) {
-        UpdateGotoSuccessor(graph.alloc(), falseBranch, finalTest->ifFalse(), testBlock);
+        if (!UpdateGotoSuccessor(graph.alloc(), falseBranch, finalTest->ifFalse(), testBlock))
+            return false;
     } else {
-        UpdateTestSuccessors(graph.alloc(), falseBranch, falseResult,
-                             finalTest->ifTrue(), finalTest->ifFalse(), testBlock);
+        if (!UpdateTestSuccessors(graph.alloc(), falseBranch, falseResult,
+                                  finalTest->ifTrue(), finalTest->ifFalse(), testBlock))
+        {
+            return false;
+        }
     }
 
     // Short circuit the initial test to skip any constant branch eliminated above.
-    UpdateTestSuccessors(graph.alloc(), initialBlock, initialTest->input(),
-                         trueTarget, falseTarget, testBlock);
+    if (!UpdateTestSuccessors(graph.alloc(), initialBlock, initialTest->input(),
+                              trueTarget, falseTarget, testBlock))
+    {
+        return false;
+    }
 
     // Remove phiBlock, if different from testBlock.
     if (phiBlock != testBlock) {
         testBlock->removePredecessor(phiBlock);
         graph.removeBlock(phiBlock);
     }
 
     // Remove testBlock itself.
@@ -945,17 +961,18 @@ jit::FoldEmptyBlocks(MIRGraph& graph)
         if (succ->numPredecessors() != 1)
             continue;
 
         size_t pos = pred->getSuccessorIndex(block);
         pred->lastIns()->replaceSuccessor(pos, succ);
 
         graph.removeBlock(block);
 
-        succ->addPredecessorSameInputsAs(pred, block);
+        if (!succ->addPredecessorSameInputsAs(pred, block))
+            return false;
         succ->removePredecessor(block);
     }
     return true;
 }
 
 static void
 EliminateTriviallyDeadResumePointOperands(MIRGraph& graph, MResumePoint* rp)
 {
--- a/js/src/jit/MIRGraph.cpp
+++ b/js/src/jit/MIRGraph.cpp
@@ -1141,38 +1141,39 @@ MBasicBlock::addPredecessorPopN(TempAllo
                     entryResumePoint()->replaceOperand(i, phi);
             }
         }
     }
 
     return predecessors_.append(pred);
 }
 
-void
+bool
 MBasicBlock::addPredecessorSameInputsAs(MBasicBlock* pred, MBasicBlock* existingPred)
 {
     MOZ_ASSERT(pred);
     MOZ_ASSERT(predecessors_.length() > 0);
 
     // Predecessors must be finished, and at the correct stack depth.
     MOZ_ASSERT(pred->hasLastIns());
     MOZ_ASSERT(!pred->successorWithPhis());
 
     AutoEnterOOMUnsafeRegion oomUnsafe;
 
     if (!phisEmpty()) {
         size_t existingPosition = indexForPredecessor(existingPred);
         for (MPhiIterator iter = phisBegin(); iter != phisEnd(); iter++) {
             if (!iter->addInputSlow(iter->getOperand(existingPosition)))
-                oomUnsafe.crash("MBasicBlock::addPredecessorAdjustPhis");
+                return false;
         }
     }
 
     if (!predecessors_.append(pred))
-        oomUnsafe.crash("MBasicBlock::addPredecessorAdjustPhis");
+        return false;
+    return true;
 }
 
 bool
 MBasicBlock::addPredecessorWithoutPhis(MBasicBlock* pred)
 {
     // Predecessors must be finished.
     MOZ_ASSERT(pred && pred->hasLastIns());
     return predecessors_.append(pred);
--- a/js/src/jit/MIRGraph.h
+++ b/js/src/jit/MIRGraph.h
@@ -267,17 +267,17 @@ class MBasicBlock : public TempObject, p
     // Adds a predecessor. Every predecessor must have the same exit stack
     // depth as the entry state to this block. Adding a predecessor
     // automatically creates phi nodes and rewrites uses as needed.
     MOZ_MUST_USE bool addPredecessor(TempAllocator& alloc, MBasicBlock* pred);
     MOZ_MUST_USE bool addPredecessorPopN(TempAllocator& alloc, MBasicBlock* pred, uint32_t popped);
 
     // Add a predecessor which won't introduce any new phis to this block.
     // This may be called after the contents of this block have been built.
-    void addPredecessorSameInputsAs(MBasicBlock* pred, MBasicBlock* existingPred);
+    MOZ_MUST_USE bool addPredecessorSameInputsAs(MBasicBlock* pred, MBasicBlock* existingPred);
 
     // Stranger utilities used for inlining.
     MOZ_MUST_USE bool addPredecessorWithoutPhis(MBasicBlock* pred);
     void inheritSlots(MBasicBlock* parent);
     MOZ_MUST_USE bool initEntrySlots(TempAllocator& alloc);
 
     // Replaces an edge for a given block with a new block. This is
     // used for critical edge splitting.