Backed out changesets 72e99bad5294 and 2b873082da2f (bug 1138881) for browser_vimemacs.js crashes.
authorRyan VanderMeulen <ryanvm@gmail.com>
Wed, 25 Mar 2015 13:59:38 -0400
changeset 264596 893a0074449111d4fb3f593fcf50edaac0673021
parent 264595 5abf1cce5ebedd4390b040cb63d7fe4051297da6
child 264597 b340b048cd80a589c0d89b603dd0c5f31a1d9ca0
push id4718
push userraliiev@mozilla.com
push dateMon, 11 May 2015 18:39:53 +0000
treeherdermozilla-beta@c20c4ef55f08 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1138881
milestone39.0a1
backs out72e99bad5294dbc0c88daca500eb5f624fc3c046
2b873082da2f30548c894a85d290749134f06080
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
Backed out changesets 72e99bad5294 and 2b873082da2f (bug 1138881) for browser_vimemacs.js crashes. CLOSED TREE
js/src/jit/IonAnalysis.cpp
js/src/jit/IonBuilder.cpp
js/src/jit/IonBuilder.h
js/src/jit/MIR.cpp
js/src/jit/MIR.h
--- a/js/src/jit/IonAnalysis.cpp
+++ b/js/src/jit/IonAnalysis.cpp
@@ -89,69 +89,16 @@ BlockComputesConstant(MBasicBlock *block
         return false;
     for (MInstructionIterator iter = block->begin(); iter != block->end(); ++iter) {
         if (*iter != value || !iter->isGoto())
             return false;
     }
     return true;
 }
 
-// Find phis that are redudant:
-//
-// 1) phi(a, a)
-//     can get replaced by a
-//
-// 2) phi(filtertypeset(a, type1), filtertypeset(a, type1))
-//     equals filtertypeset(a, type1)
-//
-// 3) phi(a, filtertypeset(a, type1))
-//     equals filtertypeset(a, type1 union type(a))
-//     equals filtertypeset(a, type(a))
-//     equals a
-//
-// 4) phi(filtertypeset(a, type1), filtertypeset(a, type2))
-//    equals filtertypeset(a, type1 union type2)
-//
-//    This is the special case. We can only replace this with 'a' iif
-//    type(a) == type1 union type2. Since optimizations could have
-//    happened based on a more specific phi type.
-static bool
-IsPhiRedudantFilter(MPhi *phi)
-{
-    // Handle (1) and (2)
-    if (phi->operandIfRedundant())
-        return true;
-
-    // Handle (3)
-    bool onlyFilters = false;
-    MDefinition *a = phi->getOperand(0);
-    if (a->isFilterTypeSet()) {
-        a = a->toFilterTypeSet()->input();
-        onlyFilters = true;
-    }
-
-    for (size_t i = 1; i < phi->numOperands(); i++) {
-        MDefinition *operand = phi->getOperand(i);
-        if (operand == a) {
-            onlyFilters = false;
-            continue;
-        }
-        if (operand->isFilterTypeSet() && operand->toFilterTypeSet()->input() == a)
-            continue;
-        return false;
-    }
-    if (!onlyFilters)
-        return true;
-
-    // Handle (4)
-    MOZ_ASSERT(onlyFilters);
-    return EqualTypes(a->type(), a->resultTypeSet(),
-                      phi->type(), phi->resultTypeSet());
-}
-
 // Determine whether phiBlock/testBlock simply compute a phi and perform a
 // test on it.
 static bool
 BlockIsSingleTest(MBasicBlock *phiBlock, MBasicBlock *testBlock, MPhi **pphi, MTest **ptest)
 {
     *pphi = nullptr;
     *ptest = nullptr;
 
@@ -179,23 +126,18 @@ BlockIsSingleTest(MBasicBlock *phiBlock,
             MBasicBlock *useBlock = use->consumer()->block();
             if (useBlock == phiBlock || useBlock == testBlock)
                 continue;
         }
         return false;
     }
 
     for (MPhiIterator iter = phiBlock->phisBegin(); iter != phiBlock->phisEnd(); ++iter) {
-        if (*iter == phi)
-            continue;
-
-        if (IsPhiRedudantFilter(*iter))
-            continue;
-
-        return false;
+        if (*iter != phi)
+            return false;
     }
 
     if (phiBlock != testBlock && !testBlock->phisEmpty())
         return false;
 
     *pphi = phi;
     *ptest = test;
 
@@ -310,33 +252,16 @@ MaybeFoldConditionBlock(MIRGraph &graph,
     if (!BlockIsSingleTest(phiBlock, testBlock, &phi, &finalTest))
         return;
 
     MDefinition *trueResult = phi->getOperand(phiBlock->indexForPredecessor(trueBranch));
     MDefinition *falseResult = phi->getOperand(phiBlock->indexForPredecessor(falseBranch));
 
     // OK, we found the desired pattern, now transform the graph.
 
-    // Patch up phis that filter their input.
-    for (MPhiIterator iter = phiBlock->phisBegin(); iter != phiBlock->phisEnd(); ++iter) {
-        if (*iter == phi)
-            continue;
-
-        MOZ_ASSERT(IsPhiRedudantFilter(*iter));
-        MDefinition *redundant = (*iter)->operandIfRedundant();
-
-        if (!redundant) {
-            redundant = (*iter)->getOperand(0);
-            if (redundant->isFilterTypeSet())
-                redundant = redundant->toFilterTypeSet()->input();
-        }
-
-        (*iter)->replaceAllUsesWith(redundant);
-    }
-
     // Remove the phi from phiBlock.
     phiBlock->discardPhi(*phiBlock->phisBegin());
 
     // If either trueBranch or falseBranch just computes a constant for the
     // test, determine the block that branch will end up jumping to and eliminate
     // the branch. Otherwise, change the end of the block to a test that jumps
     // directly to successors of testBlock, rather than to testBlock itself.
 
@@ -375,21 +300,122 @@ 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 *lhs)
+IonBuilder::CFGState::AndOr(jsbytecode *join, MBasicBlock *joinStart)
 {
     CFGState state;
     state.state = AND_OR;
     state.stopAt = join;
-    state.branch.ifFalse = lhs;
+    state.branch.ifFalse = joinStart;
     state.branch.test = nullptr;
     return state;
 }
 
 IonBuilder::CFGState
 IonBuilder::CFGState::TableSwitch(jsbytecode *exitpc, MTableSwitch *ins)
 {
     CFGState state;
@@ -2690,35 +2690,26 @@ IonBuilder::processNextTableSwitchCase(C
         return ControlStatus_Error;
     pc = current->pc();
     return ControlStatus_Jumped;
 }
 
 IonBuilder::ControlStatus
 IonBuilder::processAndOrEnd(CFGState &state)
 {
-    MOZ_ASSERT(current);
-    MBasicBlock *lhs = state.branch.ifFalse;
-
-    // Create a new block to represent the join.
-    MBasicBlock *join = newBlock(current, state.stopAt);
-    if (!join)
+    // 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))
         return ControlStatus_Error;
 
-    // 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))
+    if (!setCurrentAndSpecializePhis(state.branch.ifFalse))
         return ControlStatus_Error;
-
-    // Set the join path as current path.
-    if (!setCurrentAndSpecializePhis(join))
-        return ControlStatus_Error;
+    graph().moveBlockToEnd(current);
     pc = current->pc();
     return ControlStatus_Joined;
 }
 
 IonBuilder::ControlStatus
 IonBuilder::processLabelEnd(CFGState &state)
 {
     MOZ_ASSERT(state.state == CFGState::LABEL);
@@ -4121,44 +4112,30 @@ 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);
-    if (!evalLhs || !evalRhs)
+    MBasicBlock *join = newBlock(current, joinStart);
+    if (!evalRhs || !join)
         return false;
 
     MTest *test = (op == JSOP_AND)
-                  ? newTest(lhs, evalRhs, evalLhs)
-                  : newTest(lhs, evalLhs, evalRhs);
+                  ? newTest(lhs, evalRhs, join)
+                  : newTest(lhs, join, evalRhs);
     current->end(test);
 
-    // 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;
+    if (!cfgStack_.append(CFGState::AndOr(joinStart, join)))
+        return false;
+
+    return setCurrentAndSpecializePhis(evalRhs);
 }
 
 bool
 IonBuilder::jsop_dup2()
 {
     uint32_t lhsSlot = current->stackDepth() - 2;
     uint32_t rhsSlot = current->stackDepth() - 1;
     current->pushSlot(lhsSlot);
@@ -7496,16 +7473,46 @@ IonBuilder::getStaticName(JSObject *stat
     MIRType rvalType = types->getKnownMIRType();
     if (barrier != BarrierKind::NoBarrier)
         rvalType = MIRType_Value;
 
     return loadSlot(obj, property.maybeTypes()->definiteSlot(), NumFixedSlots(staticObject),
                     rvalType, barrier, types);
 }
 
+// Whether 'types' includes all possible values represented by input/inputTypes.
+bool
+jit::TypeSetIncludes(TypeSet *types, MIRType input, TypeSet *inputTypes)
+{
+    if (!types)
+        return inputTypes && inputTypes->empty();
+
+    switch (input) {
+      case MIRType_Undefined:
+      case MIRType_Null:
+      case MIRType_Boolean:
+      case MIRType_Int32:
+      case MIRType_Double:
+      case MIRType_Float32:
+      case MIRType_String:
+      case MIRType_Symbol:
+      case MIRType_MagicOptimizedArguments:
+        return types->hasType(TypeSet::PrimitiveType(ValueTypeFromMIRType(input)));
+
+      case MIRType_Object:
+        return types->unknownObject() || (inputTypes && inputTypes->isSubset(types));
+
+      case MIRType_Value:
+        return types->unknown() || (inputTypes && inputTypes->isSubset(types));
+
+      default:
+        MOZ_CRASH("Bad input type");
+    }
+}
+
 // Whether a write of the given value may need a post-write barrier for GC purposes.
 bool
 jit::NeedsPostBarrier(CompileInfo &info, MDefinition *value)
 {
     if (!GetJitContext()->runtime->gcNursery().exists())
         return false;
     return value->mightBeType(MIRType_Object);
 }
--- a/js/src/jit/IonBuilder.h
+++ b/js/src/jit/IonBuilder.h
@@ -1345,14 +1345,16 @@ class CallInfo
     void setImplicitlyUsedUnchecked() {
         fun_->setImplicitlyUsedUnchecked();
         thisArg_->setImplicitlyUsedUnchecked();
         for (uint32_t i = 0; i < argc(); i++)
             getArg(i)->setImplicitlyUsedUnchecked();
     }
 };
 
+bool TypeSetIncludes(TypeSet *types, MIRType input, TypeSet *inputTypes);
+
 bool NeedsPostBarrier(CompileInfo &info, MDefinition *value);
 
 } // namespace jit
 } // namespace js
 
 #endif /* jit_IonBuilder_h */
--- a/js/src/jit/MIR.cpp
+++ b/js/src/jit/MIR.cpp
@@ -1764,71 +1764,16 @@ jit::MergeTypes(MIRType *ptype, Temporar
                 *ptypeSet = TypeSet::unionSets(*ptypeSet, newTypeSet, alloc);
         } else {
             *ptypeSet = nullptr;
         }
     }
     return true;
 }
 
-// Tests whether 'types' includes all possible values represented by
-// input/inputTypes.
-bool
-jit::TypeSetIncludes(TypeSet *types, MIRType input, TypeSet *inputTypes)
-{
-    if (!types)
-        return inputTypes && inputTypes->empty();
-
-    switch (input) {
-      case MIRType_Undefined:
-      case MIRType_Null:
-      case MIRType_Boolean:
-      case MIRType_Int32:
-      case MIRType_Double:
-      case MIRType_Float32:
-      case MIRType_String:
-      case MIRType_Symbol:
-      case MIRType_MagicOptimizedArguments:
-        return types->hasType(TypeSet::PrimitiveType(ValueTypeFromMIRType(input)));
-
-      case MIRType_Object:
-        return types->unknownObject() || (inputTypes && inputTypes->isSubset(types));
-
-      case MIRType_Value:
-        return types->unknown() || (inputTypes && inputTypes->isSubset(types));
-
-      default:
-        MOZ_CRASH("Bad input type");
-    }
-}
-
-// Tests if two type combos (type/typeset) are equal.
-bool
-jit::EqualTypes(MIRType type1, TemporaryTypeSet *typeset1,
-                MIRType type2, TemporaryTypeSet *typeset2)
-{
-    // Types should equal.
-    if (type1 != type2)
-        return false;
-
-    // Both have equal type and no typeset.
-    if (!typeset1 && !typeset2)
-        return true;
-
-    // If only one instructions has a typeset.
-    // Test if the typset contains the same information as the MIRType.
-    if (typeset1 && !typeset2)
-        return TypeSetIncludes(typeset1, type2, nullptr);
-    if (!typeset1 && typeset2)
-        return TypeSetIncludes(typeset2, type1, nullptr);
-
-    // Typesets should equal.
-    return typeset1->equals(typeset2);
-}
-
 bool
 MPhi::specializeType()
 {
 #ifdef DEBUG
     MOZ_ASSERT(!specialized_);
     specialized_ = true;
 #endif
 
--- a/js/src/jit/MIR.h
+++ b/js/src/jit/MIR.h
@@ -2821,22 +2821,16 @@ class MThrow
 
 // Fabricate a type set containing only the type of the specified object.
 TemporaryTypeSet *
 MakeSingletonTypeSet(CompilerConstraintList *constraints, JSObject *obj);
 
 bool
 MergeTypes(MIRType *ptype, TemporaryTypeSet **ptypeSet,
            MIRType newType, TemporaryTypeSet *newTypeSet);
-bool
-TypeSetIncludes(TypeSet *types, MIRType input, TypeSet *inputTypes);
-
-bool
-EqualTypes(MIRType type1, TemporaryTypeSet *typeset1,
-           MIRType type2, TemporaryTypeSet *typeset2);
 
 // Helper class to assert all GC pointers embedded in MIR instructions are
 // tenured. Off-thread Ion compilation and nursery GCs can happen in parallel,
 // so it's invalid to store pointers to nursery things. There's no need to root
 // these pointers, as GC is suppressed during compilation and off-thread
 // compilations are canceled on every major GC.
 template <typename T>
 class AlwaysTenured