Bug 1041746 - IonMonkey: GVN: Clear the IsGuard flag when simplifying an instruction too. r=nbp, a=sledru
authorDan Gohman <sunfish@mozilla.com>
Tue, 22 Jul 2014 07:45:07 -0700
changeset 217269 a751d6d7affa52cfaeff3dd749d957b328edab8d
parent 217268 1e0b3a5dcde78c99801b44cf776a93d3cb0a6e72
child 217270 fde6340c0cbbf79d45f453e775dd35f7e128999c
push id515
push userraliiev@mozilla.com
push dateMon, 06 Oct 2014 12:51:51 +0000
treeherdermozilla-release@267c7a481bef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnbp, sledru
bugs1041746
milestone33.0a2
Bug 1041746 - IonMonkey: GVN: Clear the IsGuard flag when simplifying an instruction too. r=nbp, a=sledru
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;
         }
     }