Bug 1007213 - Capture implicit dead branches caused by type barriers. r=sunfish

js/src/jit-test/tests/ion/bug1007213.js

@@ -0,0 +1,21 @@
+
+function getval(o) {
+    return obj.val
+}
+function f(x, o) {
+    var lhs = -(~x >>> 0)
+    var rhs = getval(o)
+    return (lhs - rhs >> 0)
+}
+function getObj(v) {
+    return {
+        val: v
+    }
+}
+
+var obj = getObj(1)
+assertEq(f(0, obj), 0)
+assertEq(f(0, obj), 0)
+obj = getObj('can has bug?')
+obj = getObj(.5)
+assertEq(f(0, obj), 1)

js/src/jit/MIR.h

@@ -76,16 +76,19 @@ MIRType MIRTypeFromValue(const js::Value &vp)
      * points.
      */                                                                         \
     _(Unused)                                                                   \
-    /* 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.
+                                                                                \
+    /* When a branch is removed, the uses of multiple instructions are removed.
+     * The removal of branches is based on hypotheses.  These hypotheses might
+     * fail, in which case we need to bailout from the current code.
+     *
+     * When we implement a destructive optimization, we need to consider the
+     * failing cases, and consider the fact that we might resume the execution
+     * into a branch which was removed from the compiler.  As such, a
+     * destructive optimization need to take into acount removed branches.
+     *
+     * In order to let destructive optimizations know about removed branches, we
+     * have to annotate instructions with the UseRemoved flag.  This flag
+     * annotates instruction which were used in removed branches.
      */                                                                         \
     _(UseRemoved)                                                               \
                                                                                 \

js/src/jit/MIRGraph.cpp

@@ -997,6 +997,36 @@ MBasicBlock::discardPhiAt(MPhiIterator &at)
     return result;
 }
 
+void
+MBasicBlock::flagOperandsOfPrunedBranches(MInstruction *ins)
+{
+    // Find the previous resume point which would be used for bailing out.
+    MResumePoint *rp = nullptr;
+    for (MInstructionReverseIterator iter = rbegin(ins); iter != rend(); iter++) {
+        rp = iter->resumePoint();
+        if (rp)
+            break;
+    }
+
+    // If none, take the entry resume point.
+    if (!rp)
+        rp = entryResumePoint();
+
+    // The only blocks which do not have any entryResumePoint in Ion, are the
+    // SplitEdge blocks.  SplitEdge blocks only have a Goto instruction before
+    // Range Analysis phase.  In adjustInputs, we are manipulating instructions
+    // which have a TypePolicy.  So, as a Goto has no operand and no type
+    // policy, the entry resume point should exists.
+    MOZ_ASSERT(rp);
+
+    // Flag all operand as being potentially used.
+    while (rp) {
+        for (size_t i = 0, end = rp->numOperands(); i < end; i++)
+            rp->getOperand(i)->setUseRemovedUnchecked();
+        rp = rp->caller();
+    }
+}
+
 bool
 MBasicBlock::addPredecessor(TempAllocator &alloc, MBasicBlock *pred)
 {

js/src/jit/MIRGraph.h

@@ -301,6 +301,13 @@ class MBasicBlock : public TempObject, public InlineListNode<MBasicBlock>
     // Discards a phi instruction and updates predecessor successorWithPhis.
     MPhiIterator discardPhiAt(MPhiIterator &at);
 
+    // Some instruction which are guarding against some MIRType value, or
+    // against a type expectation should be considered as removing a potenatial
+    // branch where the guard does not hold.  We need to register such
+    // instructions in order to do destructive optimizations correctly, such as
+    // Range Analysis.
+    void flagOperandsOfPrunedBranches(MInstruction *ins);
+
     // Mark this block as having been removed from the graph.
     void markAsDead() {
         MOZ_ASSERT(kind_ != DEAD);

js/src/jit/RangeAnalysis.cpp

@@ -2499,11 +2499,10 @@ ComputeRequestedTruncateKind(MDefinition *candidate)
     MDefinition::TruncateKind kind = MDefinition::Truncate;
     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, or if the value does not need conversion.
-            // Otherwise a branch removed by UCE might rely on the non-truncated
-            // value, and any bailout with a truncated value might lead an
-            // incorrect value.
+            // Truncation is a destructive optimization, as such, we need to pay
+            // attention to removed branches and prevent optimization
+            // destructive optimizations if we have no alternative. (see
+            // UseRemoved flag)
             if (candidate->isUseRemoved() && needsConversion)
                 kind = Min(kind, MDefinition::TruncateAfterBailouts);
             continue;

js/src/jit/TypePolicy.cpp

@@ -273,6 +273,12 @@ TypeBarrierPolicy::adjustInputs(TempAllocator &alloc, MInstruction *def)
 
         MUnbox *unbox = MUnbox::New(alloc, ins->getOperand(0), outputType, MUnbox::TypeBarrier);
         ins->block()->insertBefore(ins, unbox);
+
+        // The TypeBarrier is equivalent to removing branches with unexpected
+        // types.  The unexpected types would have changed Range Analysis
+        // predictions.  As such, we need to prevent destructive optimizations.
+        ins->block()->flagOperandsOfPrunedBranches(unbox);
+
         ins->replaceOperand(0, unbox);
         return true;
     }