Bug 1090037 - Ensure that dominators are defined enough before moving instructions. r=sunfish,h4writer a=lsblakk
authorNicolas B. Pierron <nicolas.b.pierron@mozilla.com>
Wed, 05 Nov 2014 15:34:17 +0100
changeset 233627 1cdbb76b31993b537da5aed49162e491e80b6014
parent 233626 75158d5c8acd8e75eb70759cb20a34748cfe59e6
child 233628 e4a43029e62769ce7953ef8237d23f9095a5b0b5
push id4187
push userbhearsum@mozilla.com
push dateFri, 28 Nov 2014 15:29:12 +0000
treeherdermozilla-beta@f23cc6a30c11 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssunfish, h4writer, lsblakk
bugs1090037
milestone35.0a2
Bug 1090037 - Ensure that dominators are defined enough before moving instructions. r=sunfish,h4writer a=lsblakk
js/src/jit-test/tests/ion/bug1090037.js
js/src/jit/MIR.cpp
js/src/jit/ValueNumbering.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ion/bug1090037.js
@@ -0,0 +1,7 @@
+function f(x) {
+    Math.sin([] | 0 && x)
+}
+for (var j = 0; j < 1; j++) {
+    f(1 && 0)
+}
+
--- a/js/src/jit/MIR.cpp
+++ b/js/src/jit/MIR.cpp
@@ -1272,16 +1272,33 @@ MPhi::foldsTernary()
     if (!trueDef->isConstant() && !falseDef->isConstant())
         return nullptr;
 
     MConstant *c = trueDef->isConstant() ? trueDef->toConstant() : falseDef->toConstant();
     MDefinition *testArg = (trueDef == c) ? falseDef : trueDef;
     if (testArg != test->input())
         return nullptr;
 
+    // This check should be a tautology, except that the constant might be the
+    // result of the removal of a branch.  In such case the domination scope of
+    // the block which is holding the constant might be incomplete. This
+    // condition is used to prevent doing this optimization based on incomplete
+    // information.
+    //
+    // As GVN removed a branch, it will update the dominations rules before
+    // trying to fold this MPhi again. Thus, this condition does not inhibit
+    // this optimization.
+    MBasicBlock *truePred = block()->getPredecessor(firstIsTrueBranch ? 0 : 1);
+    MBasicBlock *falsePred = block()->getPredecessor(firstIsTrueBranch ? 1 : 0);
+    if (!trueDef->block()->dominates(truePred) ||
+        !falseDef->block()->dominates(falsePred))
+    {
+        return nullptr;
+    }
+
     // If testArg is an int32 type we can:
     // - fold testArg ? testArg : 0 to testArg
     // - fold testArg ? 0 : testArg to 0
     if (testArg->type() == MIRType_Int32 && c->vp()->toNumber() == 0) {
         // When folding to the constant we need to hoist it.
         if (trueDef == c && !c->block()->dominates(block()))
             c->block()->moveBefore(pred->lastIns(), c);
         return trueDef;
--- a/js/src/jit/ValueNumbering.cpp
+++ b/js/src/jit/ValueNumbering.cpp
@@ -653,16 +653,20 @@ ValueNumberer::leader(MDefinition *def)
             // The congruent value doesn't dominate. It never will again in this
             // dominator tree, so overwrite it.
             values_.overwrite(p, def);
         } else {
             // No match. Add a new entry.
             if (!values_.add(p, def))
                 return nullptr;
         }
+
+#ifdef DEBUG
+        JitSpew(JitSpew_GVN, "      Recording %s%u", def->opName(), def->id());
+#endif
     }
 
     return def;
 }
 
 // Test whether |phi| is dominated by a congruent phi.
 bool
 ValueNumberer::hasLeader(const MPhi *phi, const MBasicBlock *phiBlock) const