Bug 1138881 - IonMonkey: Improve types at AndOr, r=bhackett
☠☠ backed out by 893a00744491 ☠ ☠
authorHannes Verschore <hv1989@gmail.com>
Wed, 25 Mar 2015 16:15:27 +0100
changeset 266046 2b873082da2f30548c894a85d290749134f06080
parent 266045 c2e1dd95909181f470c80d71985bf8aca9679568
child 266047 72e99bad5294dbc0c88daca500eb5f624fc3c046
push id830
push userraliiev@mozilla.com
push dateFri, 19 Jun 2015 19:24:37 +0000
treeherdermozilla-release@932614382a68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbhackett
bugs1138881
milestone39.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 1138881 - IonMonkey: Improve types at AndOr, r=bhackett
js/src/jit/IonAnalysis.cpp
js/src/jit/IonBuilder.cpp
--- a/js/src/jit/IonAnalysis.cpp
+++ b/js/src/jit/IonAnalysis.cpp
@@ -300,122 +300,21 @@ MaybeFoldConditionBlock(MIRGraph &graph,
     }
 
     // Remove testBlock itself.
     finalTest->ifTrue()->removePredecessor(testBlock);
     finalTest->ifFalse()->removePredecessor(testBlock);
     graph.removeBlock(testBlock);
 }
 
-static void
-MaybeFoldAndOrBlock(MIRGraph &graph, MBasicBlock *initialBlock)
-{
-    // Optimize the MIR graph to improve the code generated for && and ||
-    // operations when they are used in tests. This is very similar to the
-    // above method for folding condition blocks, though the two are
-    // separated (with as much common code as possible) for clarity. This
-    // normally requires three blocks. The final test can always be eliminated,
-    // though we don't try to constant fold away the branch block as well.
-
-    // Look for a triangle pattern:
-    //
-    //       initialBlock
-    //         /     |
-    // branchBlock   |
-    //         \     |
-    //         phiBlock
-    //            |
-    //        testBlock
-    //
-    // Where phiBlock contains a single phi combining values pushed onto the
-    // stack by initialBlock and testBlock, and testBlock contains a test on
-    // that phi. phiBlock and testBlock may be the same block; generated code
-    // will use different blocks if the &&/|| is in an inlined function.
-
-    MInstruction *ins = initialBlock->lastIns();
-    if (!ins->isTest())
-        return;
-    MTest *initialTest = ins->toTest();
-
-    bool branchIsTrue = true;
-    MBasicBlock *branchBlock = initialTest->ifTrue();
-    MBasicBlock *phiBlock = initialTest->ifFalse();
-    if (branchBlock->numSuccessors() != 1 || branchBlock->getSuccessor(0) != phiBlock) {
-        branchIsTrue = false;
-        branchBlock = initialTest->ifFalse();
-        phiBlock = initialTest->ifTrue();
-    }
-
-    if (branchBlock->numSuccessors() != 1 || branchBlock->getSuccessor(0) != phiBlock)
-        return;
-    if (branchBlock->numPredecessors() != 1 || phiBlock->numPredecessors() != 2)
-        return;
-
-    if (initialBlock->isLoopBackedge() || branchBlock->isLoopBackedge())
-        return;
-
-    MBasicBlock *testBlock = phiBlock;
-    if (testBlock->numSuccessors() == 1) {
-        if (testBlock->isLoopBackedge())
-            return;
-        testBlock = testBlock->getSuccessor(0);
-        if (testBlock->numPredecessors() != 1)
-            return;
-    }
-
-    // Make sure the test block does not have any outgoing loop backedges.
-    if (!SplitCriticalEdgesForBlock(graph, testBlock))
-        CrashAtUnhandlableOOM("MaybeFoldAndOrBlock");
-
-    MPhi *phi;
-    MTest *finalTest;
-    if (!BlockIsSingleTest(phiBlock, testBlock, &phi, &finalTest))
-        return;
-
-    MDefinition *branchResult = phi->getOperand(phiBlock->indexForPredecessor(branchBlock));
-    MDefinition *initialResult = phi->getOperand(phiBlock->indexForPredecessor(initialBlock));
-
-    if (initialResult != initialTest->input())
-        return;
-
-    // OK, we found the desired pattern, now transform the graph.
-
-    // Remove the phi from phiBlock.
-    phiBlock->discardPhi(*phiBlock->phisBegin());
-
-    // Change the end of the initial and branch blocks to a test that jumps
-    // directly to successors of testBlock, rather than to testBlock itself.
-
-    UpdateTestSuccessors(graph.alloc(), initialBlock, initialResult,
-                         branchIsTrue ? branchBlock : finalTest->ifTrue(),
-                         branchIsTrue ? finalTest->ifFalse() : branchBlock,
-                         testBlock);
-
-    UpdateTestSuccessors(graph.alloc(), branchBlock, branchResult,
-                         finalTest->ifTrue(), finalTest->ifFalse(), testBlock);
-
-    // Remove phiBlock, if different from testBlock.
-    if (phiBlock != testBlock) {
-        testBlock->removePredecessor(phiBlock);
-        graph.removeBlock(phiBlock);
-    }
-
-    // Remove testBlock itself.
-    finalTest->ifTrue()->removePredecessor(testBlock);
-    finalTest->ifFalse()->removePredecessor(testBlock);
-    graph.removeBlock(testBlock);
-}
-
 void
 jit::FoldTests(MIRGraph &graph)
 {
-    for (MBasicBlockIterator block(graph.begin()); block != graph.end(); block++) {
+    for (MBasicBlockIterator block(graph.begin()); block != graph.end(); block++)
         MaybeFoldConditionBlock(graph, *block);
-        MaybeFoldAndOrBlock(graph, *block);
-    }
 }
 
 static void
 EliminateTriviallyDeadResumePointOperands(MIRGraph &graph, MResumePoint *rp)
 {
     // If we will pop the top of the stack immediately after resuming,
     // then don't preserve the top value in the resume point.
     if (rp->mode() != MResumePoint::ResumeAt || *rp->pc() != JSOP_POP)
--- a/js/src/jit/IonBuilder.cpp
+++ b/js/src/jit/IonBuilder.cpp
@@ -297,22 +297,22 @@ IonBuilder::CFGState::IfElse(jsbytecode 
     state.stopAt = trueEnd;
     state.branch.falseEnd = falseEnd;
     state.branch.ifFalse = ifFalse;
     state.branch.test = test;
     return state;
 }
 
 IonBuilder::CFGState
-IonBuilder::CFGState::AndOr(jsbytecode *join, MBasicBlock *joinStart)
+IonBuilder::CFGState::AndOr(jsbytecode *join, MBasicBlock *lhs)
 {
     CFGState state;
     state.state = AND_OR;
     state.stopAt = join;
-    state.branch.ifFalse = joinStart;
+    state.branch.ifFalse = lhs;
     state.branch.test = nullptr;
     return state;
 }
 
 IonBuilder::CFGState
 IonBuilder::CFGState::TableSwitch(jsbytecode *exitpc, MTableSwitch *ins)
 {
     CFGState state;
@@ -2690,26 +2690,35 @@ IonBuilder::processNextTableSwitchCase(C
         return ControlStatus_Error;
     pc = current->pc();
     return ControlStatus_Jumped;
 }
 
 IonBuilder::ControlStatus
 IonBuilder::processAndOrEnd(CFGState &state)
 {
-    // We just processed the RHS of an && or || expression.
-    // Now jump to the join point (the false block).
-    current->end(MGoto::New(alloc(), state.branch.ifFalse));
-
-    if (!state.branch.ifFalse->addPredecessor(alloc(), current))
+    MOZ_ASSERT(current);
+    MBasicBlock *lhs = state.branch.ifFalse;
+
+    // Create a new block to represent the join.
+    MBasicBlock *join = newBlock(current, state.stopAt);
+    if (!join)
         return ControlStatus_Error;
 
-    if (!setCurrentAndSpecializePhis(state.branch.ifFalse))
+    // End the rhs.
+    current->end(MGoto::New(alloc(), join));
+
+    // End the lhs.
+    lhs->end(MGoto::New(alloc(), join));
+    if (!join->addPredecessor(alloc(), state.branch.ifFalse))
         return ControlStatus_Error;
-    graph().moveBlockToEnd(current);
+
+    // Set the join path as current path.
+    if (!setCurrentAndSpecializePhis(join))
+        return ControlStatus_Error;
     pc = current->pc();
     return ControlStatus_Joined;
 }
 
 IonBuilder::ControlStatus
 IonBuilder::processLabelEnd(CFGState &state)
 {
     MOZ_ASSERT(state.state == CFGState::LABEL);
@@ -4112,30 +4121,44 @@ IonBuilder::jsop_andor(JSOp op)
 
     jsbytecode *rhsStart = pc + js_CodeSpec[op].length;
     jsbytecode *joinStart = pc + GetJumpOffset(pc);
     MOZ_ASSERT(joinStart > pc);
 
     // We have to leave the LHS on the stack.
     MDefinition *lhs = current->peek(-1);
 
+    MBasicBlock *evalLhs = newBlock(current, joinStart);
     MBasicBlock *evalRhs = newBlock(current, rhsStart);
-    MBasicBlock *join = newBlock(current, joinStart);
-    if (!evalRhs || !join)
+    if (!evalLhs || !evalRhs)
         return false;
 
     MTest *test = (op == JSOP_AND)
-                  ? newTest(lhs, evalRhs, join)
-                  : newTest(lhs, join, evalRhs);
+                  ? newTest(lhs, evalRhs, evalLhs)
+                  : newTest(lhs, evalLhs, evalRhs);
     current->end(test);
 
-    if (!cfgStack_.append(CFGState::AndOr(joinStart, join)))
-        return false;
-
-    return setCurrentAndSpecializePhis(evalRhs);
+    // Create the lhs block and specialize.
+    if (!setCurrentAndSpecializePhis(evalLhs))
+        return false;
+
+    if (!improveTypesAtTest(test->getOperand(0), test->ifTrue() == current, test))
+        return false;
+
+    // Create the rhs block.
+    if (!cfgStack_.append(CFGState::AndOr(joinStart, evalLhs)))
+        return false;
+
+    if (!setCurrentAndSpecializePhis(evalRhs))
+        return false;
+
+    if (!improveTypesAtTest(test->getOperand(0), test->ifTrue() == current, test))
+        return false;
+
+    return true;
 }
 
 bool
 IonBuilder::jsop_dup2()
 {
     uint32_t lhsSlot = current->stackDepth() - 2;
     uint32_t rhsSlot = current->stackDepth() - 1;
     current->pushSlot(lhsSlot);