Bug 1365518 - Fix polymorphic inlining to work correctly for functions that had their group/proto changed. r=shu
authorJan de Mooij <jdemooij@mozilla.com>
Sat, 27 May 2017 16:29:46 +0200
changeset 409131 3bfd7a30226705e59fad14d1708a973d72e2a668
parent 409130 2ba8ded0fe690cc55a55e574effff0d41daefaad
child 409132 a5bee800882e91f9609c934707bd48187819a987
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersshu
bugs1365518
milestone55.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 1365518 - Fix polymorphic inlining to work correctly for functions that had their group/proto changed. r=shu
js/src/jit-test/tests/ion/bug1365518.js
js/src/jit/IonBuilder.cpp
js/src/jit/IonBuilder.h
js/src/jit/MIR.cpp
js/src/jit/MIR.h
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ion/bug1365518.js
@@ -0,0 +1,13 @@
+function init() {
+    foo = () => 1;
+    bar = () => 2;
+    foo.__proto__ = function() {};
+}
+function test() {
+    var arr = [foo, bar];
+    for (var i = 0; i < 1300; i++) {
+        assertEq(arr[i % 2](), i % 2 + 1);
+    }
+}
+init();
+test();
--- a/js/src/jit/IonBuilder.cpp
+++ b/js/src/jit/IonBuilder.cpp
@@ -268,17 +268,17 @@ IonBuilder::getSingleCallTarget(Temporar
     if (JSFunction* fun = key->group()->maybeInterpretedFunction())
         return fun;
 
     return nullptr;
 }
 
 AbortReasonOr<Ok>
 IonBuilder::getPolyCallTargets(TemporaryTypeSet* calleeTypes, bool constructing,
-                               ObjectVector& targets, uint32_t maxTargets)
+                               InliningTargets& targets, uint32_t maxTargets)
 {
     MOZ_ASSERT(targets.empty());
 
     if (!calleeTypes)
         return Ok();
 
     if (calleeTypes->baseFlags() != 0)
         return Ok();
@@ -287,20 +287,21 @@ IonBuilder::getPolyCallTargets(Temporary
 
     if (objCount == 0 || objCount > maxTargets)
         return Ok();
 
     if (!targets.reserve(objCount))
         return abort(AbortReason::Alloc);
     for (unsigned i = 0; i < objCount; i++) {
         JSObject* obj = calleeTypes->getSingleton(i);
+        ObjectGroup* group = nullptr;
         if (obj) {
             MOZ_ASSERT(obj->isSingleton());
         } else {
-            ObjectGroup* group = calleeTypes->getGroup(i);
+            group = calleeTypes->getGroup(i);
             if (!group)
                 continue;
 
             obj = group->maybeInterpretedFunction();
             if (!obj) {
                 targets.clear();
                 return Ok();
             }
@@ -311,17 +312,17 @@ IonBuilder::getPolyCallTargets(Temporary
         // Don't optimize if the callee is not callable or constructable per
         // the manner it is being invoked, so that CallKnown does not have to
         // handle these cases (they will always throw).
         if (constructing ? !obj->isConstructor() : !obj->isCallable()) {
             targets.clear();
             return Ok();
         }
 
-        targets.infallibleAppend(obj);
+        targets.infallibleAppend(InliningTarget(obj, group));
     }
 
     return Ok();
 }
 
 IonBuilder::InliningDecision
 IonBuilder::DontInline(JSScript* targetScript, const char* reason)
 {
@@ -4006,33 +4007,33 @@ IonBuilder::makeInliningDecision(JSObjec
     // End of heuristics, we will inline this function.
 
     outerBuilder->inlinedBytecodeLength_ += targetScript->length();
 
     return InliningDecision_Inline;
 }
 
 AbortReasonOr<Ok>
-IonBuilder::selectInliningTargets(const ObjectVector& targets, CallInfo& callInfo, BoolVector& choiceSet,
-                                  uint32_t* numInlineable)
+IonBuilder::selectInliningTargets(const InliningTargets& targets, CallInfo& callInfo,
+                                  BoolVector& choiceSet, uint32_t* numInlineable)
 {
     *numInlineable = 0;
     uint32_t totalSize = 0;
 
     // For each target, ask whether it may be inlined.
     if (!choiceSet.reserve(targets.length()))
         return abort(AbortReason::Alloc);
 
     // Don't inline polymorphic sites during the definite properties analysis.
     // AddClearDefiniteFunctionUsesInScript depends on this for correctness.
     if (info().analysisMode() == Analysis_DefiniteProperties && targets.length() > 1)
         return Ok();
 
     for (size_t i = 0; i < targets.length(); i++) {
-        JSObject* target = targets[i];
+        JSObject* target = targets[i].target;
 
         trackOptimizationAttempt(TrackedStrategy::Call_Inline);
         trackTypeInfo(TrackedTypeSite::Call_Target, target);
 
         bool inlineable;
         InliningDecision decision = makeInliningDecision(target, callInfo);
         switch (decision) {
           case InliningDecision_Error:
@@ -4066,17 +4067,17 @@ IonBuilder::selectInliningTargets(const 
             *numInlineable += 1;
     }
 
     // If optimization tracking is turned on and one of the inlineable targets
     // is a native, track the type info of the call. Most native inlinings
     // depend on the types of the arguments and the return value.
     if (isOptimizationTrackingEnabled()) {
         for (size_t i = 0; i < targets.length(); i++) {
-            if (choiceSet[i] && targets[i]->as<JSFunction>().isNative()) {
+            if (choiceSet[i] && targets[i].target->as<JSFunction>().isNative()) {
                 trackTypeInfo(callInfo);
                 break;
             }
         }
     }
 
     MOZ_ASSERT(choiceSet.length() == targets.length());
     return Ok();
@@ -4218,34 +4219,34 @@ IonBuilder::inlineSingleCall(CallInfo& c
 
     // Track success now, as inlining a scripted call makes a new return block
     // which has a different pc than the current call pc.
     trackInlineSuccess();
     return inlineScriptedCall(callInfo, target);
 }
 
 IonBuilder::InliningResult
-IonBuilder::inlineCallsite(const ObjectVector& targets, CallInfo& callInfo)
+IonBuilder::inlineCallsite(const InliningTargets& targets, CallInfo& callInfo)
 {
     if (targets.empty()) {
         trackOptimizationAttempt(TrackedStrategy::Call_Inline);
         trackOptimizationOutcome(TrackedOutcome::CantInlineNoTarget);
         return InliningStatus_NotInlined;
     }
 
     // Is the function provided by an MGetPropertyCache?
     // If so, the cache may be movable to a fallback path, with a dispatch
     // instruction guarding on the incoming ObjectGroup.
     WrapMGetPropertyCache propCache(getInlineableGetPropertyCache(callInfo));
     keepFallbackFunctionGetter(propCache.get());
 
     // Inline single targets -- unless they derive from a cache, in which case
     // avoiding the cache and guarding is still faster.
     if (!propCache.get() && targets.length() == 1) {
-        JSObject* target = targets[0];
+        JSObject* target = targets[0].target;
 
         trackOptimizationAttempt(TrackedStrategy::Call_Inline);
         trackTypeInfo(TrackedTypeSite::Call_Target, target);
 
         InliningDecision decision = makeInliningDecision(target, callInfo);
         switch (decision) {
           case InliningDecision_Error:
             return abort(AbortReason::Alloc);
@@ -4404,17 +4405,17 @@ IonBuilder::inlineObjectGroupFallback(Ca
 
     // inlineGenericFallback() set the return block as |current|.
     preCallBlock->end(MGoto::New(alloc(), current));
     *fallbackTarget = prepBlock;
     return Ok();
 }
 
 AbortReasonOr<Ok>
-IonBuilder::inlineCalls(CallInfo& callInfo, const ObjectVector& targets, BoolVector& choiceSet,
+IonBuilder::inlineCalls(CallInfo& callInfo, const InliningTargets& targets, BoolVector& choiceSet,
                         MGetPropertyCache* maybeCache)
 {
     // Only handle polymorphic inlining.
     MOZ_ASSERT(IsIonInlinablePC(pc));
     MOZ_ASSERT(choiceSet.length() == targets.length());
     MOZ_ASSERT_IF(!maybeCache, targets.length() >= 2);
     MOZ_ASSERT_IF(maybeCache, targets.length() >= 1);
     MOZ_ASSERT_IF(maybeCache, maybeCache->value()->type() == MIRType::Object);
@@ -4477,17 +4478,17 @@ IonBuilder::inlineCalls(CallInfo& callIn
         if (!choiceSet[i])
             continue;
 
         // Even though we made one round of inline decisions already, we may
         // be amending them below.
         amendOptimizationAttempt(i);
 
         // Target must be reachable by the MDispatchInstruction.
-        JSFunction* target = &targets[i]->as<JSFunction>();
+        JSFunction* target = &targets[i].target->as<JSFunction>();
         if (maybeCache && !maybeCache->propTable()->hasFunction(target)) {
             choiceSet[i] = false;
             trackOptimizationOutcome(TrackedOutcome::CantInlineNotInDispatch);
             continue;
         }
 
         MBasicBlock* inlineBlock;
         MOZ_TRY_VAR(inlineBlock, newBlock(dispatchBlock, pc));
@@ -4543,18 +4544,17 @@ IonBuilder::inlineCalls(CallInfo& callIn
             continue;
         }
 
         // inlineSingleCall() changed |current| to the inline return block.
         MBasicBlock* inlineReturnBlock = current;
         setCurrent(dispatchBlock);
 
         // Connect the inline path to the returnBlock.
-        ObjectGroup* funcGroup = target->isSingleton() ? nullptr : target->group();
-        if (!dispatch->addCase(target, funcGroup, inlineBlock))
+        if (!dispatch->addCase(target, targets[i].group, inlineBlock))
             return abort(AbortReason::Alloc);
 
         MDefinition* retVal = inlineReturnBlock->peek(-1);
         retPhi->addInput(retVal);
         inlineReturnBlock->end(MGoto::New(alloc(), returnBlock));
         if (!returnBlock->addPredecessorWithoutPhis(inlineReturnBlock))
             return abort(AbortReason::Alloc);
     }
@@ -4629,18 +4629,19 @@ IonBuilder::inlineCalls(CallInfo& callIn
             // If there is only 1 remaining case, we can annotate the fallback call
             // with the target information.
             if (dispatch->numCases() + 1 == targets.length()) {
                 for (uint32_t i = 0; i < targets.length(); i++) {
                     if (choiceSet[i])
                         continue;
 
                     MOZ_ASSERT(!remaining);
-                    if (targets[i]->is<JSFunction>() && targets[i]->as<JSFunction>().isSingleton())
-                        remaining = &targets[i]->as<JSFunction>();
+                    JSObject* target = targets[i].target;
+                    if (target->is<JSFunction>() && target->isSingleton())
+                        remaining = &target->as<JSFunction>();
                     break;
                 }
             }
 
             MOZ_TRY(inlineGenericFallback(remaining, callInfo, dispatchBlock));
             dispatch->addFallback(current);
         }
 
@@ -5256,17 +5257,17 @@ IonBuilder::jsop_call(uint32_t argc, boo
             // See bug 870847.
             observed->addType(TypeSet::DoubleType(), alloc_->lifoAlloc());
         }
     }
 
     int calleeDepth = -((int)argc + 2 + constructing);
 
     // Acquire known call target if existent.
-    ObjectVector targets(alloc());
+    InliningTargets targets(alloc());
     TemporaryTypeSet* calleeTypes = current->peek(calleeDepth)->resultTypeSet();
     if (calleeTypes)
         MOZ_TRY(getPolyCallTargets(calleeTypes, constructing, targets, 4));
 
     CallInfo callInfo(alloc(), constructing, ignoresReturnValue);
     if (!callInfo.init(current, argc))
         return abort(AbortReason::Alloc);
 
@@ -5276,18 +5277,18 @@ IonBuilder::jsop_call(uint32_t argc, boo
     if (status == InliningStatus_Inlined)
         return Ok();
 
     // Discard unreferenced & pre-allocated resume points.
     replaceMaybeFallbackFunctionGetter(nullptr);
 
     // No inline, just make the call.
     JSFunction* target = nullptr;
-    if (targets.length() == 1 && targets[0]->is<JSFunction>())
-        target = &targets[0]->as<JSFunction>();
+    if (targets.length() == 1 && targets[0].target->is<JSFunction>())
+        target = &targets[0].target->as<JSFunction>();
 
     if (target && status == InliningStatus_WarmUpCountTooLow) {
         MRecompileCheck* check =
             MRecompileCheck::New(alloc(), target->nonLazyScript(),
                                  optimizationInfo().inliningRecompileThreshold(),
                                  MRecompileCheck::RecompileCheck_Inlining);
         current->add(check);
     }
--- a/js/src/jit/IonBuilder.h
+++ b/js/src/jit/IonBuilder.h
@@ -67,17 +67,17 @@ class IonBuilder
     uint32_t readIndex(jsbytecode* pc);
     JSAtom* readAtom(jsbytecode* pc);
 
     void trackActionableAbort(const char* message);
     void spew(const char* message);
 
     JSFunction* getSingleCallTarget(TemporaryTypeSet* calleeTypes);
     AbortReasonOr<Ok> getPolyCallTargets(TemporaryTypeSet* calleeTypes, bool constructing,
-                                         ObjectVector& targets, uint32_t maxTargets);
+                                         InliningTargets& targets, uint32_t maxTargets);
 
     AbortReasonOr<Ok> analyzeNewLoopTypes(const CFGBlock* loopEntryBlock);
 
     AbortReasonOr<MBasicBlock*> newBlock(MBasicBlock* predecessor, jsbytecode* pc);
     AbortReasonOr<MBasicBlock*> newBlock(MBasicBlock* predecessor, jsbytecode* pc,
                                          MResumePoint* priorResumePoint);
     AbortReasonOr<MBasicBlock*> newBlockPopN(MBasicBlock* predecessor, jsbytecode* pc,
                                              uint32_t popped);
@@ -610,17 +610,17 @@ class IonBuilder
     static InliningDecision DontInline(JSScript* targetScript, const char* reason);
 
     // Helper function for canInlineTarget
     bool hasCommonInliningPath(const JSScript* scriptToInline);
 
     // Oracles.
     InliningDecision canInlineTarget(JSFunction* target, CallInfo& callInfo);
     InliningDecision makeInliningDecision(JSObject* target, CallInfo& callInfo);
-    AbortReasonOr<Ok> selectInliningTargets(const ObjectVector& targets, CallInfo& callInfo,
+    AbortReasonOr<Ok> selectInliningTargets(const InliningTargets& targets, CallInfo& callInfo,
                                             BoolVector& choiceSet, uint32_t* numInlineable);
 
     // Native inlining helpers.
     // The typeset for the return value of our function.  These are
     // the types it's been observed returning in the past.
     TemporaryTypeSet* getInlineReturnTypeSet();
     // The known MIR type of getInlineReturnTypeSet.
     MIRType getInlineReturnType();
@@ -787,18 +787,18 @@ class IonBuilder
     // Main inlining functions
     InliningResult inlineNativeCall(CallInfo& callInfo, JSFunction* target);
     InliningResult inlineNativeGetter(CallInfo& callInfo, JSFunction* target);
     InliningResult inlineNonFunctionCall(CallInfo& callInfo, JSObject* target);
     InliningResult inlineScriptedCall(CallInfo& callInfo, JSFunction* target);
     InliningResult inlineSingleCall(CallInfo& callInfo, JSObject* target);
 
     // Call functions
-    InliningResult inlineCallsite(const ObjectVector& targets, CallInfo& callInfo);
-    AbortReasonOr<Ok> inlineCalls(CallInfo& callInfo, const ObjectVector& targets,
+    InliningResult inlineCallsite(const InliningTargets& targets, CallInfo& callInfo);
+    AbortReasonOr<Ok> inlineCalls(CallInfo& callInfo, const InliningTargets& targets,
                                   BoolVector& choiceSet, MGetPropertyCache* maybeCache);
 
     // Inlining helpers.
     AbortReasonOr<Ok> inlineGenericFallback(JSFunction* target, CallInfo& callInfo,
                                             MBasicBlock* dispatchBlock);
     AbortReasonOr<Ok> inlineObjectGroupFallback(CallInfo& callInfo, MBasicBlock* dispatchBlock,
                                                 MObjectGroupDispatch* dispatch,
                                                 MGetPropertyCache* cache,
--- a/js/src/jit/MIR.cpp
+++ b/js/src/jit/MIR.cpp
@@ -5439,52 +5439,52 @@ MGuardReceiverPolymorphic::congruentTo(c
         if (receiver(i) != other->receiver(i))
             return false;
     }
 
     return congruentIfOperandsEqual(ins);
 }
 
 void
-InlinePropertyTable::trimTo(const ObjectVector& targets, const BoolVector& choiceSet)
+InlinePropertyTable::trimTo(const InliningTargets& targets, const BoolVector& choiceSet)
 {
     for (size_t i = 0; i < targets.length(); i++) {
         // If the target was inlined, don't erase the entry.
         if (choiceSet[i])
             continue;
 
         // If the target wasn't a function we would have veto'ed it
         // and it will not be in the entries list.
-        if (!targets[i]->is<JSFunction>())
+        if (!targets[i].target->is<JSFunction>())
             continue;
 
-        JSFunction* target = &targets[i]->as<JSFunction>();
+        JSFunction* target = &targets[i].target->as<JSFunction>();
 
         // Eliminate all entries containing the vetoed function from the map.
         size_t j = 0;
         while (j < numEntries()) {
             if (entries_[j]->func == target)
                 entries_.erase(&entries_[j]);
             else
                 j++;
         }
     }
 }
 
 void
-InlinePropertyTable::trimToTargets(const ObjectVector& targets)
+InlinePropertyTable::trimToTargets(const InliningTargets& targets)
 {
     JitSpew(JitSpew_Inlining, "Got inlineable property cache with %d cases",
             (int)numEntries());
 
     size_t i = 0;
     while (i < numEntries()) {
         bool foundFunc = false;
         for (size_t j = 0; j < targets.length(); j++) {
-            if (entries_[i]->func == targets[j]) {
+            if (entries_[i]->func == targets[j].target) {
                 foundFunc = true;
                 break;
             }
         }
         if (!foundFunc)
             entries_.erase(&(entries_[i]));
         else
             i++;
--- a/js/src/jit/MIR.h
+++ b/js/src/jit/MIR.h
@@ -10731,18 +10731,35 @@ class MStoreFixedSlot
     }
     void setNeedsBarrier(bool needsBarrier = true) {
         needsBarrier_ = needsBarrier;
     }
 
     ALLOW_CLONE(MStoreFixedSlot)
 };
 
-typedef Vector<JSObject*, 4, JitAllocPolicy> ObjectVector;
-typedef Vector<bool, 4, JitAllocPolicy> BoolVector;
+struct InliningTarget
+{
+    JSObject* target;
+
+    // If target is a singleton, group is nullptr. If target is not a singleton,
+    // this is the group we need to guard on when doing a polymorphic inlining
+    // dispatch. Note that this can be different from target->group() due to
+    // proto mutation.
+    ObjectGroup* group;
+
+    InliningTarget(JSObject* target, ObjectGroup* group)
+      : target(target), group(group)
+    {
+        MOZ_ASSERT(target->isSingleton() == !group);
+    }
+};
+
+using InliningTargets = Vector<InliningTarget, 4, JitAllocPolicy>;
+using BoolVector = Vector<bool, 8, JitAllocPolicy>;
 
 class InlinePropertyTable : public TempObject
 {
     struct Entry : public TempObject {
         CompilerObjectGroup group;
         CompilerFunction func;
 
         Entry(ObjectGroup* group, JSFunction* func)
@@ -10795,20 +10812,20 @@ class InlinePropertyTable : public TempO
     }
 
     bool hasFunction(JSFunction* func) const;
     bool hasObjectGroup(ObjectGroup* group) const;
 
     TemporaryTypeSet* buildTypeSetForFunction(JSFunction* func) const;
 
     // Remove targets that vetoed inlining from the InlinePropertyTable.
-    void trimTo(const ObjectVector& targets, const BoolVector& choiceSet);
+    void trimTo(const InliningTargets& targets, const BoolVector& choiceSet);
 
     // Ensure that the InlinePropertyTable's domain is a subset of |targets|.
-    void trimToTargets(const ObjectVector& targets);
+    void trimToTargets(const InliningTargets& targets);
 
     bool appendRoots(MRootList& roots) const;
 };
 
 class MGetPropertyCache
   : public MBinaryInstruction,
     public MixPolicy<BoxExceptPolicy<0, MIRType::Object>, CacheIdPolicy<1>>::Data
 {