Bug 1317675: Check that phis are not guards before removing them; r=h4writer, a=jcristau
authorBenjamin Bouvier <benj@benj.me>
Tue, 15 Nov 2016 18:54:57 +0100
changeset 352679 f50ad6fc76add5cad17a50c50bd9ad50c6fa2915
parent 352678 2b50647ce630beef1640f11487cedfea3460c1d1
child 352680 3c3bd290fee9b2d43e566cb6dbd577194ee63d43
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-esr52@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersh4writer, jcristau
bugs1317675
milestone52.0a2
Bug 1317675: Check that phis are not guards before removing them; r=h4writer, a=jcristau MozReview-Commit-ID: DO4BWhaTPev
js/src/jit-test/tests/ion/gvn-unremovable-phi-bug1317675.js
js/src/jit-test/tests/wasm/regress/gvn-unremovable-phi.js
js/src/jit/ValueNumbering.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ion/gvn-unremovable-phi-bug1317675.js
@@ -0,0 +1,18 @@
+// |jit-test| error: InternalError: too much recursion
+
+function f() {
+    var phi1 = 0;
+    var phi2 = 0;
+    while (true) {
+        if (!phi2) {
+            var add = phi1 + 1;
+            f(add);
+            if (!phi2)
+                return;
+            phi1 = 1;
+            phi2 = 0;
+        }
+    }
+}
+
+f(0);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/wasm/regress/gvn-unremovable-phi.js
@@ -0,0 +1,22 @@
+load(libdir + "wasm.js");
+
+wasmEvalText(`(module
+  (type $type0 (func (param i32)))
+  (func $f (param $p i32)
+    (local $x i32) (local $y i32)
+    loop $top
+      get_local $x
+      get_local $p
+      get_local $x
+      br_if $top
+      i32.const 1
+      tee_local $p
+      get_local $y
+      set_local $x
+      i32.add
+      call $f
+      br_if $top
+      return
+    end
+  )
+)`);
--- a/js/src/jit/ValueNumbering.cpp
+++ b/js/src/jit/ValueNumbering.cpp
@@ -475,27 +475,28 @@ ValueNumberer::removePredecessorAndDoDCE
                "Block marked unreachable should have predecessors removed already");
 
     // Before removing the predecessor edge, scan the phi operands for that edge
     // for dead code before they get removed.
     MOZ_ASSERT(nextDef_ == nullptr);
     for (MPhiIterator iter(block->phisBegin()), end(block->phisEnd()); iter != end; ) {
         MPhi* phi = *iter++;
         MOZ_ASSERT(!values_.has(phi), "Visited phi in block having predecessor removed");
+        MOZ_ASSERT(!phi->isGuard());
 
         MDefinition* op = phi->getOperand(predIndex);
         phi->removeOperand(predIndex);
 
         nextDef_ = iter != end ? *iter : nullptr;
         if (!handleUseReleased(op, DontSetUseRemoved) || !processDeadDefs())
             return false;
 
-        // If |nextDef_| became dead while we had it pinned, advance the iterator
-        // and discard it now.
-        while (nextDef_ && !nextDef_->hasUses()) {
+        // If |nextDef_| became dead while we had it pinned, advance the
+        // iterator and discard it now.
+        while (nextDef_ && !nextDef_->hasUses() && !nextDef_->isGuardRangeBailouts()) {
             phi = nextDef_->toPhi();
             iter++;
             nextDef_ = iter != end ? *iter : nullptr;
             if (!discardDefsRecursively(phi))
                 return false;
         }
     }
     nextDef_ = nullptr;