Bug 1041746 - IonMonkey: GVN: Clear the IsGuard flag when simplifying an instruction too. r=nbp
authorDan Gohman <sunfish@mozilla.com>
Tue, 22 Jul 2014 07:45:07 -0700
changeset 195497 cd2bf43234a313a81feb21f1b442af75cfaa41f5
parent 195496 beb02af9af1c09a574ad73e58556ef9cc7fa56dc
child 195498 a1f9c0d19457100bfeaac4e64e4d7dc0f0998854
push id27184
push userkwierso@gmail.com
push dateWed, 23 Jul 2014 00:39:18 +0000
treeherdermozilla-central@0ad20ad7b70a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnbp
bugs1041746
milestone34.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 1041746 - IonMonkey: GVN: Clear the IsGuard flag when simplifying an instruction too. r=nbp
js/src/jit/MIR.cpp
js/src/jit/ValueNumbering.cpp
--- a/js/src/jit/MIR.cpp
+++ b/js/src/jit/MIR.cpp
@@ -3220,22 +3220,18 @@ MSqrt::trySpecializeFloat32(TempAllocato
 }
 
 MDefinition *
 MBoundsCheck::foldsTo(TempAllocator &alloc)
 {
     if (index()->isConstant() && length()->isConstant()) {
        uint32_t len = length()->toConstant()->value().toInt32();
        uint32_t idx = index()->toConstant()->value().toInt32();
-       if (idx + uint32_t(minimum()) < len && idx + uint32_t(maximum()) < len) {
-           // This bounds check will never fail, so we can clear the Guard flag
-           // and allow it to be deleted.
-           setNotGuard();
+       if (idx + uint32_t(minimum()) < len && idx + uint32_t(maximum()) < len)
            return index();
-       }
     }
 
     return this;
 }
 
 bool
 jit::ElementAccessIsDenseNative(MDefinition *obj, MDefinition *id)
 {
--- a/js/src/jit/ValueNumbering.cpp
+++ b/js/src/jit/ValueNumbering.cpp
@@ -474,36 +474,41 @@ ValueNumberer::visitDefinition(MDefiniti
 
         // If sim doesn't belong to a block, insert it next to def.
         if (sim->block() == nullptr)
             def->block()->insertAfter(def->toInstruction(), sim->toInstruction());
 
         IonSpew(IonSpew_GVN, "    Folded %s%u to %s%u",
                 def->opName(), def->id(), sim->opName(), sim->id());
         ReplaceAllUsesWith(def, sim);
+
+        // The node's foldsTo said |def| can be replaced by |rep|. If |def| is a
+        // guard, then either |rep| is also a guard, or a guard isn't actually
+        // needed, so we can clear |def|'s guard flag and let it be deleted.
+        def->setNotGuardUnchecked();
+
         if (IsDead(def) && !deleteDefsRecursively(def))
             return false;
         def = sim;
     }
 
     // Look for a dominating def which makes this def redundant.
     MDefinition *rep = leader(def);
     if (rep != def) {
         if (rep == nullptr)
             return false;
         if (rep->updateForReplacement(def)) {
             IonSpew(IonSpew_GVN,
                     "    Replacing %s%u with %s%u",
                     def->opName(), def->id(), rep->opName(), rep->id());
             ReplaceAllUsesWith(def, rep);
 
-            // This is effectively what the old GVN did. It allows isGuard()
-            // instructions to be deleted if they are redundant, and the
-            // replacement is not even guaranteed to have isGuard() set.
-            // TODO: Clean this up (bug 1031410).
+            // The node's congruentTo said |def| is congruent to |rep|, and it's
+            // dominated by |rep|. If |def| is a guard, it's covered by |rep|,
+            // so we can clear |def|'s guard flag and let it be deleted.
             def->setNotGuardUnchecked();
 
             if (IsDead(def) && !deleteDefsRecursively(def))
                 return false;
             def = rep;
         }
     }