Bug 867070: IonMonkey: Disable hoisting of some instructions when it doesn't enable hoisting one of their uses, r=dvander
authorHannes Verschore <hv1989@gmail.com>
Tue, 07 May 2013 09:10:52 +0200
changeset 131130 6b1f607fd243d065798adb13bf7d853a74b7af69
parent 131129 ac70913833fdc463785ee26bafd3087498d00975
child 131131 8b62ffcdb0ab83872a6842bc57e48c14bda6b7b4
push id24649
push userryanvm@gmail.com
push dateWed, 08 May 2013 02:10:32 +0000
treeherdermozilla-central@b980d32c366f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdvander
bugs867070
milestone23.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 867070: IonMonkey: Disable hoisting of some instructions when it doesn't enable hoisting one of their uses, r=dvander
js/src/ion/LICM.cpp
--- a/js/src/ion/LICM.cpp
+++ b/js/src/ion/LICM.cpp
@@ -150,54 +150,63 @@ Loop::optimize()
         }
 
         if (isLoopInvariant(ins)) {
             // Flag this instruction as loop invariant.
             ins->setLoopInvariant();
             if (!invariantInstructions.append(ins))
                 return false;
 
-            // Loop through uses of invariant instruction and add back to work list.
-            for (MUseDefIterator iter(ins->toDefinition()); iter; iter++) {
-                MDefinition *consumer = iter.def();
-
-                if (consumer->isInWorklist())
-                    continue;
-
-                // if the consumer of this invariant instruction is in the
-                // loop, and it is also worth hoisting, then process it.
-                if (isInLoop(consumer) && isHoistable(consumer)) {
-                    if (!insertInWorklist(consumer->toInstruction()))
-                        return false;
-                }
-            }
-
             if (IonSpewEnabled(IonSpew_LICM))
                 fprintf(IonSpewFile, " Loop Invariant!\n");
         }
     }
 
     if (!hoistInstructions(invariantInstructions))
         return false;
     return true;
 }
 
 bool
 Loop::hoistInstructions(InstructionQueue &toHoist)
 {
+    // Iterate in post-order (uses before definitions)
+    for (int32_t i = toHoist.length() - 1; i >= 0; i--) {
+        MInstruction *ins = toHoist[i];
+
+        // Don't hoist MConstantElements, MConstant and MBox
+        // if it doesn't enable us to hoist one of its uses.
+        // We want those instructions as close as possible to their use.
+        if (ins->isConstantElements() || ins->isConstant() || ins->isBox()) {
+            bool loopInvariantUse = false;
+            for (MUseDefIterator use(ins); use; use++) {
+                if (use.def()->isLoopInvariant()) {
+                    loopInvariantUse = true;
+                    break;
+                }
+            }
+
+            if (!loopInvariantUse)
+                ins->setNotLoopInvariant();
+        }
+    }
+
     // Move all instructions to the preLoop_ block just before the control instruction.
     for (size_t i = 0; i < toHoist.length(); i++) {
         MInstruction *ins = toHoist[i];
 
         // Loads may have an implicit dependency on either stores (effectful instructions) or
         // control instructions so we should never move these.
         JS_ASSERT(!ins->isControlInstruction());
         JS_ASSERT(!ins->isEffectful());
         JS_ASSERT(ins->isMovable());
 
+        if (!ins->isLoopInvariant())
+            continue;
+
         if (checkHotness(ins->block())) {
             ins->block()->moveBefore(preLoop_->lastIns(), ins);
             ins->setNotLoopInvariant();
         }
     }
 
     return true;
 }
@@ -245,16 +254,17 @@ Loop::isLoopInvariant(MInstruction *ins)
             if (IonSpewEnabled(IonSpew_LICM)) {
                 ins->getOperand(i)->printName(IonSpewFile);
                 fprintf(IonSpewFile, " is in the loop.\n");
             }
 
             return false;
         }
     }
+
     return true;
 }
 
 bool
 Loop::isLoopInvariant(MDefinition *ins)
 {
     if (!isInLoop(ins))
         return true;