Bug 1073861 - IonMonkey: Don't update types during type policy, r=jandem
authorHannes Verschore <hv1989@gmail.com>
Thu, 02 Oct 2014 17:11:28 +0200
changeset 208475 f4eaa493bf0cb422fd8644cca3329f77df77d3d4
parent 208474 1537d250e6b6e07485188c6a6afad0a15cd77749
child 208476 5beba92a3f8e43ebe6d1abd8fdea72a217a665a4
push id27585
push useremorley@mozilla.com
push dateFri, 03 Oct 2014 13:26:33 +0000
treeherderautoland@e4f5e843a370 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1073861
milestone35.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 1073861 - IonMonkey: Don't update types during type policy, r=jandem
js/src/jit-test/tests/ion/bug1073861.js
js/src/jit/IonBuilder.cpp
js/src/jit/TypePolicy.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ion/bug1073861.js
@@ -0,0 +1,69 @@
+function a(a, b, c, g) {
+    for (;;) {
+        if (0 > c) return a;
+        a: {
+            for (;;) {
+                var k = a.forward[c];
+                if (t(k))
+                    if (k.key < b) a = k;
+                    else break a;
+                else break a
+            }
+            a = void 0
+        }
+        null !=
+            g && (g[c] = a);
+        c -= 1
+    }
+}
+
+function t(a) {
+    return null != a && !1 !== a
+}
+
+
+var d = {forward: [{},null,{}]}
+for (var i=0; i < 1000; i++) {
+    a(d, 0, 1, null);
+    a(d, 0, 0, null);
+}
+
+
+
+
+function test(a) {
+  var t = a[0]
+  if (t) {
+    return t.test;
+  }
+}
+
+function test2(a) {
+  var t = a[0]
+  if (t) {
+    if (t) {
+      return t.test;
+    }
+  }
+}
+
+function test3(a) {
+  var t = a[0]
+  if (t !== null) {
+    if (t !== undefined) {
+      return t.test;
+    }
+  }
+}
+
+var a = [{test:1}]
+var b = [undefined]
+assertEq(test(b), undefined)
+assertEq(test(a), 1)
+assertEq(test(a), 1)
+assertEq(test2(b), undefined)
+assertEq(test2(a), 1)
+assertEq(test2(a), 1)
+assertEq(test3(b), undefined)
+assertEq(test3(a), 1)
+assertEq(test3(a), 1)
--- a/js/src/jit/IonBuilder.cpp
+++ b/js/src/jit/IonBuilder.cpp
@@ -3139,54 +3139,59 @@ IonBuilder::tableSwitch(JSOp op, jssrcno
 }
 
 bool
 IonBuilder::replaceTypeSet(MDefinition *subject, types::TemporaryTypeSet *type, MTest *test)
 {
     if (type->unknown())
         return true;
 
-    MFilterTypeSet *replace = nullptr;
+    MInstruction *replace = nullptr;
     MDefinition *ins;
 
     for (uint32_t i = 0; i < current->stackDepth(); i++) {
         ins = current->getSlot(i);
 
         // Instead of creating a new MFilterTypeSet, try to update the old one.
         if (ins->isFilterTypeSet() && ins->getOperand(0) == subject &&
             ins->dependency() == test)
         {
             types::TemporaryTypeSet *intersect =
                 types::TypeSet::intersectSets(ins->resultTypeSet(), type, alloc_->lifoAlloc());
             if (!intersect)
                 return false;
 
             ins->toFilterTypeSet()->setResultType(intersect->getKnownMIRType());
             ins->toFilterTypeSet()->setResultTypeSet(intersect);
+
+            if (ins->type() == MIRType_Undefined)
+                current->setSlot(i, constant(UndefinedValue()));
+            if (ins->type() == MIRType_Null)
+                current->setSlot(i, constant(NullValue()));
             continue;
         }
 
         if (ins == subject) {
             if (!replace) {
                 replace = MFilterTypeSet::New(alloc(), subject, type);
-
                 if (!replace)
                     return false;
-                if (replace == subject)
-                    break;
 
                 current->add(replace);
 
-                if (replace != subject) {
-                    // Make sure we don't hoist it above the MTest, we can use the
-                    // 'dependency' of an MInstruction. This is normally used by
-                    // Alias Analysis, but won't get overwritten, since this
-                    // instruction doesn't have an AliasSet.
-                    replace->setDependency(test);
-                }
+                // Make sure we don't hoist it above the MTest, we can use the
+                // 'dependency' of an MInstruction. This is normally used by
+                // Alias Analysis, but won't get overwritten, since this
+                // instruction doesn't have an AliasSet.
+                replace->setDependency(test);
+
+                if (replace->type() == MIRType_Undefined)
+                    replace = constant(UndefinedValue());
+                if (replace->type() == MIRType_Null)
+                    replace = constant(NullValue());
             }
             current->setSlot(i, replace);
         }
     }
     return true;
 }
 
 bool
@@ -3307,17 +3312,17 @@ IonBuilder::improveTypesAtCompare(MCompa
 bool
 IonBuilder::improveTypesAtTest(MDefinition *ins, bool trueBranch, MTest *test)
 {
     // We explore the test condition to try and deduce
     // as much type information as possible.
     if (!ins)
         return true;
 
-    switch(ins->op()) {
+    switch (ins->op()) {
       case MDefinition::Op_Not:
         return improveTypesAtTest(ins->toNot()->getOperand(0), !trueBranch, test);
       case MDefinition::Op_IsObject: {
         types::TemporaryTypeSet *oldType = ins->getOperand(0)->resultTypeSet();
         if (!oldType)
             return true;
         if (oldType->unknown() || !oldType->mightBeMIRType(MIRType_Object))
             return true;
@@ -4240,17 +4245,22 @@ IonBuilder::inlineScriptedCall(CallInfo 
     {
         types::StackTypeSet *types = types::TypeScript::ThisTypes(calleeScript);
         if (!types->unknown()) {
             types::TemporaryTypeSet *clonedTypes = types->clone(alloc_->lifoAlloc());
             if (!clonedTypes)
                 return oom();
             MTypeBarrier *barrier = MTypeBarrier::New(alloc(), callInfo.thisArg(), clonedTypes);
             current->add(barrier);
-            callInfo.setThis(barrier);
+            if (barrier->type() == MIRType_Undefined)
+                callInfo.setThis(constant(UndefinedValue()));
+            else if (barrier->type() == MIRType_Null)
+                callInfo.setThis(constant(NullValue()));
+            else
+                callInfo.setThis(barrier);
         }
     }
 
     // Start inlining.
     LifoAlloc *lifoAlloc = alloc_->lifoAlloc();
     InlineScriptTree *inlineScriptTree =
         info().inlineScriptTree()->addCallee(alloc_, pc, calleeScript);
     if (!inlineScriptTree)
--- a/js/src/jit/TypePolicy.cpp
+++ b/js/src/jit/TypePolicy.cpp
@@ -254,44 +254,45 @@ TypeBarrierPolicy::adjustInputs(TempAllo
     if (outputType == MIRType_Value) {
         // XXX: Possible optimization: decrease resultTypeSet to only include
         // the inputType. This will remove the need for boxing.
         MOZ_ASSERT(inputType != MIRType_Value);
         ins->replaceOperand(0, boxAt(alloc, ins, ins->getOperand(0)));
         return true;
     }
 
-    // Input is a value. Unbox the input to the requested type.
-    if (inputType == MIRType_Value) {
-        MOZ_ASSERT(outputType != MIRType_Value);
+    // Box input if needed.
+    if (inputType != MIRType_Value) {
+        MOZ_ASSERT(ins->alwaysBails());
+        ins->replaceOperand(0, boxAt(alloc, ins, ins->getOperand(0)));
+    }
 
-        // We can't unbox a value to null/undefined/lazyargs. So keep output
-        // also a value.
-        if (IsNullOrUndefined(outputType) || outputType == MIRType_MagicOptimizedArguments) {
-            MOZ_ASSERT(!ins->hasDefUses());
-            ins->setResultType(MIRType_Value);
-            return true;
-        }
-
-        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);
+    // We can't unbox a value to null/undefined/lazyargs. So keep output
+    // also a value.
+    // Note: Using setResultType shouldn't be done in TypePolicies,
+    //       Here it is fine, since the type barrier has no uses.
+    if (IsNullOrUndefined(outputType) || outputType == MIRType_MagicOptimizedArguments) {
+        MOZ_ASSERT(!ins->hasDefUses());
+        ins->setResultType(MIRType_Value);
         return true;
     }
 
-    // In the remaining cases we will alway bail. OutputType doesn't matter.
-    // Take inputType so we can use redefine during lowering.
-    MOZ_ASSERT(ins->alwaysBails());
-    ins->setResultType(inputType);
+    // Unbox / propagate the right type.
+    MUnbox::Mode mode = MUnbox::TypeBarrier;
+    MInstruction *replace = MUnbox::New(alloc, ins->getOperand(0), ins->type(), mode);
+
+    ins->block()->insertBefore(ins, replace);
+    ins->replaceOperand(0, replace);
+    if (!replace->typePolicy()->adjustInputs(alloc, replace))
+        return false;
+
+    // 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(replace);
 
     return true;
 }
 
 bool
 TestPolicy::adjustInputs(TempAllocator &alloc, MInstruction *ins)
 {
     MDefinition *op = ins->getOperand(0);
@@ -846,39 +847,56 @@ ClampPolicy::adjustInputs(TempAllocator 
 
     return true;
 }
 
 bool
 FilterTypeSetPolicy::adjustInputs(TempAllocator &alloc, MInstruction *ins)
 {
     MOZ_ASSERT(ins->numOperands() == 1);
+    MIRType inputType = ins->getOperand(0)->type();
+    MIRType outputType = ins->type();
 
-    // Do nothing if already same type.
-    if (ins->type() == ins->getOperand(0)->type())
+    // Input and output type are already in accordance.
+    if (inputType == outputType)
         return true;
 
-    // Box input if ouput type is MIRType_Value
-    if (ins->type() == MIRType_Value) {
+    // Output is a value, box the input.
+    if (outputType == MIRType_Value) {
+        MOZ_ASSERT(inputType != MIRType_Value);
         ins->replaceOperand(0, boxAt(alloc, ins, ins->getOperand(0)));
         return true;
     }
 
-    // For simplicity just mark output type as MIRType_Value if input type
-    // is MIRType_Value. It should be possible to unbox, but we need to
-    // add extra code for Undefined/Null.
-    if (ins->getOperand(0)->type() == MIRType_Value) {
+    // The outputType should always be a subset of the inputType.
+    // So if types don't equal, the input type is definitely a MIRType_Value.
+    if (inputType != MIRType_Value)
+        MOZ_CRASH("Types should be in accordance.");
+
+    // We can't unbox a value to null/undefined/lazyargs. So keep output
+    // also a value.
+    // Note: Using setResultType shouldn't be done in TypePolicies,
+    //       Here it is fine, since the type barrier has no uses.
+    if (IsNullOrUndefined(outputType) || outputType == MIRType_MagicOptimizedArguments) {
+        MOZ_ASSERT(!ins->hasDefUses());
         ins->setResultType(MIRType_Value);
         return true;
     }
 
-    // In all other cases we will definitely bail, since types don't
-    // correspond. Just box and mark output as MIRType_Value.
-    ins->replaceOperand(0, boxAt(alloc, ins, ins->getOperand(0)));
-    ins->setResultType(MIRType_Value);
+    // Unbox / propagate the right type.
+    MUnbox::Mode mode = MUnbox::Infallible;
+    MInstruction *replace = MUnbox::New(alloc, ins->getOperand(0), ins->type(), mode);
+
+    ins->block()->insertBefore(ins, replace);
+    ins->replaceOperand(0, replace);
+    if (!replace->typePolicy()->adjustInputs(alloc, replace))
+        return false;
+
+    // Carry over the dependency the MFilterTypeSet had.
+    replace->setDependency(ins->dependency());
 
     return true;
 }
 
 // Lists of all TypePolicy specializations which are used by MIR Instructions.
 #define TYPE_POLICY_LIST(_)                     \
     _(ArithPolicy)                              \
     _(BitwisePolicy)                            \