Bug 882565 - IonMonkey: Only skip resumepoints during truncation when no uses where removed. r=jandem, a=akeybl
authorHannes Verschore <hv1989@gmail.com>
Sat, 15 Jun 2013 17:05:02 +0200
changeset 138731 17adfc82ed91
parent 138730 ee9cc4f09fb6
child 138732 6b40d5226c13
push id3880
push userryanvm@gmail.com
push dateMon, 17 Jun 2013 19:47:38 +0000
treeherdermozilla-aurora@17adfc82ed91 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem, akeybl
bugs882565
milestone23.0a2
Bug 882565 - IonMonkey: Only skip resumepoints during truncation when no uses where removed. r=jandem, a=akeybl
js/src/ion/MIR.cpp
js/src/ion/MIR.h
js/src/ion/RangeAnalysis.cpp
js/src/ion/UnreachableCodeElimination.cpp
js/src/jit-test/tests/ion/bug882565-1.js
js/src/jit-test/tests/ion/bug882565.js
--- a/js/src/ion/MIR.cpp
+++ b/js/src/ion/MIR.cpp
@@ -301,16 +301,19 @@ MNode::discardOperand(size_t index)
 
 void
 MDefinition::replaceAllUsesWith(MDefinition *dom)
 {
     JS_ASSERT(dom != NULL);
     if (dom == this)
         return;
 
+    for (size_t i = 0; i < numOperands(); i++)
+        getOperand(i)->setUseRemovedUnchecked();
+
     for (MUseIterator i(usesBegin()); i != usesEnd(); ) {
         JS_ASSERT(i->producer() == this);
         i = i->consumer()->replaceOperand(i, dom);
     }
 }
 
 static inline bool
 IsPowerOfTwo(uint32_t n)
--- a/js/src/ion/MIR.h
+++ b/js/src/ion/MIR.h
@@ -49,17 +49,30 @@ MIRType MIRTypeFromValue(const js::Value
     _(Lowered)       /* (Debug only) has a virtual register */                  \
     _(Guard)         /* Not removable if uses == 0 */                           \
     _(Folded)        /* Has constant folded uses not reflected in SSA */        \
                                                                                 \
     /* The instruction has been marked dead for lazy removal from resume
      * points.
      */                                                                         \
     _(Unused)                                                                   \
-    _(DOMFunction)   /* Contains or uses a common DOM method function */
+    _(DOMFunction)   /* Contains or uses a common DOM method function */        \
+                                                                                \
+    /* Marks if an instruction has fewer uses than the original code.
+     * E.g. UCE can remove code.
+     * Every instruction where an use is/was removed from an instruction and
+     * as a result the number of operands doesn't equal the original code
+     * need to get marked as UseRemoved. This is important for truncation
+     * analysis to know, since if all original uses are still present,
+     * it can ignore resumepoints.
+     * Currently this is done for every pass after IonBuilder and before
+     * Truncate Doubles. So every time removeUse is called, UseRemoved needs
+     * to get set.
+     */                                                                         \
+    _(UseRemoved)
 
 class MDefinition;
 class MInstruction;
 class MBasicBlock;
 class MNode;
 class MUse;
 class MIRGraph;
 class MResumePoint;
--- a/js/src/ion/RangeAnalysis.cpp
+++ b/js/src/ion/RangeAnalysis.cpp
@@ -1392,23 +1392,31 @@ MMul::isOperandTruncated(size_t index) c
 bool
 MToDouble::isOperandTruncated(size_t index) const
 {
     // The return type is used to flag that we are replacing this Double by a
     // Truncate of its operand if needed.
     return type() == MIRType_Int32;
 }
 
-// Ensure that all observables (non-resume point) uses can work with a truncated
+// Ensure that all observables uses can work with a truncated
 // version of the |candidate|'s result.
 static bool
 AllUsesTruncate(MInstruction *candidate)
 {
-    for (MUseDefIterator use(candidate); use; use++) {
-        if (!use.def()->isOperandTruncated(use.index()))
+    for (MUseIterator use(candidate->usesBegin()); use != candidate->usesEnd(); use++) {
+        if (!use->consumer()->isDefinition()) {
+            // We can only skip testing resume points, if all original uses are still present.
+            // Only than testing all uses is enough to guarantee the truncation isn't observerable.
+            if (candidate->isUseRemoved())
+                return false;
+            continue;
+        }
+
+        if (!use->consumer()->toDefinition()->isOperandTruncated(use->index()))
             return false;
     }
 
     return true;
 }
 
 static void
 RemoveTruncatesOnOutput(MInstruction *truncated)
--- a/js/src/ion/UnreachableCodeElimination.cpp
+++ b/js/src/ion/UnreachableCodeElimination.cpp
@@ -138,20 +138,22 @@ void
 UnreachableCodeElimination::checkDependencyAndRemoveUsesFromUnmarkedBlocks(MDefinition *instr)
 {
     // When the instruction depends on removed block,
     // alias analysis needs to get rerun to have the right dependency.
     if (instr->dependency() && !instr->dependency()->block()->isMarked())
         rerunAliasAnalysis_ = true;
 
     for (MUseIterator iter(instr->usesBegin()); iter != instr->usesEnd(); ) {
-        if (!iter->consumer()->block()->isMarked())
+        if (!iter->consumer()->block()->isMarked()) {
+            instr->setUseRemovedUnchecked();
             iter = instr->removeUse(iter);
-        else
+        } else {
             iter++;
+        }
     }
 }
 
 bool
 UnreachableCodeElimination::removeUnmarkedBlocksAndClearDominators()
 {
     // Removes blocks that are not marked from the graph.  For blocks
     // that *are* marked, clears the mark and adjusts the id to its
@@ -216,20 +218,20 @@ UnreachableCodeElimination::removeUnmark
                                 redundantPhis_ = true;
                                 break;
                             }
                         }
                     }
                 }
             }
 
-            // When we remove a call, we can't leave the corresponding MPassArg in the graph.
-            // Since lowering will fail. Replace it with the argument for the exceptional
-            // case when it is kept alive in a ResumePoint.
-            // DCE will remove the unused MPassArg instruction.
+            // When we remove a call, we can't leave the corresponding MPassArg
+            // in the graph. Since lowering will fail. Replace it with the
+            // argument for the exceptional case when it is kept alive in a
+            // ResumePoint. DCE will remove the unused MPassArg instruction.
             for (MInstructionIterator iter(block->begin()); iter != block->end(); iter++) {
                 if (iter->isCall()) {
                     MCall *call = iter->toCall();
                     for (size_t i = 0; i < call->numStackArgs(); i++) {
                         JS_ASSERT(call->getArg(i)->isPassArg());
                         MPassArg *arg = call->getArg(i)->toPassArg();
                         arg->replaceAllUsesWith(arg->getArgument());
                     }
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ion/bug882565-1.js
@@ -0,0 +1,21 @@
+function zero() { return 0; }
+function f(x, a) {
+  var test = 0x7fffffff;
+
+  for (var i=0; i<100; i++)
+  {
+    if (i == 0) {
+      test += 1;
+      var t = (test > zero()) * (0xffffffff >>> x);
+    }
+    var test2 = test | 0;
+    return [test2,t];
+  }
+}
+var t = f(0, "");
+assertEq(t[0], 0x80000000 | 0);
+assertEq(t[1], 0xffffffff >>> 0);
+
+var t = f(0);
+assertEq(t[0], 0x80000000 | 0);
+assertEq(t[1], 0xffffffff >>> 0);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ion/bug882565.js
@@ -0,0 +1,4 @@
+function zero() { return 0; }
+function f(x) { return (0xffffffff > zero()) * (0xffffffff >>> x); }
+assertEq(f(0), 4294967295);
+assertEq(f(0), 4294967295);