Bug 1398105 - CallInfo::pushFormals is now responsible for allocating space fun.apply arguments. r=jandem
authorNicolas B. Pierron <nicolas.b.pierron@mozilla.com>
Wed, 13 Sep 2017 09:37:19 +0000
changeset 380670 d7bb391e9644
parent 380669 1a53ff436568
child 380671 fa027280892f
push id32492
push userarchaeopteryx@coole-files.de
push date2017-09-13 21:59 +0000
treeherdermozilla-central@8645a74bbbd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1398105
milestone57.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 1398105 - CallInfo::pushFormals is now responsible for allocating space fun.apply arguments. r=jandem
js/src/jit-test/tests/ion/array-push-multiple-with-funapply.js
js/src/jit/IonBuilder.cpp
js/src/jit/IonBuilder.h
js/src/jit/MCallOptimize.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ion/array-push-multiple-with-funapply.js
@@ -0,0 +1,80 @@
+// |jit-test| --no-threads
+
+// This test case check's Ion ability to inline Array.prototype.push, when
+// fun.apply is used and inlined with the set of arguments of the current
+// function. Note, that the following are not equivalent in case of failures:
+//
+//   arr = [];
+//   arr.push(1,2,3); // OOM ---> arr == []
+//
+//   arr = [];
+//   arr.push(1);
+//   arr.push(2); // OOM --> arr == [1]
+//   arr.push(3);
+
+function canIoncompile() {
+  while (true) {
+    var i = inIon();
+    if (i)
+      return i;
+  }
+}
+
+if (!("oomAtAllocation" in this))
+  quit();
+if (canIoncompile() != true)
+  quit();
+if ("gczeal" in this)
+  gczeal(0);
+
+function pushLimits(limit, offset) {
+  function pusher() {
+    Array.prototype.push.apply(arr, arguments)
+  }
+  var arr = [0,1,2,3,4,5,6,7];
+  arr.length = offset;
+  var l = arr.length;
+  var was = inIon();
+  oomAtAllocation(limit);
+  try {
+    for (var i = 0; i < 100; i++)
+      pusher(0,1,2,3,4,5,6,7);
+  } catch (e) {
+    // Catch OOM.
+  }
+  resetOOMFailure();
+  assertEq(arr.length % 8, l);
+  // Check for a bailout.
+  var is = inIon();
+  return was ? is ? 1 : 2 : 0;
+}
+
+
+
+// We need this limit to be high enough to be able to OSR in the for-loop of
+// pushLimits.
+var limit = 1024 * 1024 * 1024;
+while(true) {
+  var res = pushLimits(limit, 0);
+  print(limit, res);
+
+  if (res == 0) {
+    limit = 1024 * 1024 * 1024;
+  } else if (res == 1) { // Started and finished in Ion.
+    // We want to converge quickly to a state where the memory is limited
+    // enough to cause failures in array.prototype.push.
+    limit = (limit / 1.5) | 0;
+    if (limit == 0) // If we are not in the Jit.
+      break;
+  } else if (res == 2) { // Started in Ion, and finished in Baseline.
+    if (limit < 10) {
+      // This is used to offset the OOM location, such that we can test
+      // each steps of the Array.push function, when it is jitted.
+      for (var off = 1; off < 8; off++)
+        pushLimits(limit, off);
+    }
+    if (limit == 1)
+      break;
+    limit--;
+  }
+}
--- a/js/src/jit/IonBuilder.cpp
+++ b/js/src/jit/IonBuilder.cpp
@@ -3726,33 +3726,26 @@ IonBuilder::inlineScriptedCall(CallInfo&
     MOZ_ASSERT(IsIonInlinablePC(pc));
 
     MBasicBlock::BackupPoint backup(current);
     if (!backup.init(alloc()))
         return abort(AbortReason::Alloc);
 
     callInfo.setImplicitlyUsedUnchecked();
 
-    // Ensure sufficient space in the slots: needed for inlining from FUNAPPLY.
-    uint32_t depth = current->stackDepth() + callInfo.numFormals();
-    if (depth > current->nslots()) {
-        if (!current->increaseSlots(depth - current->nslots()))
-            return abort(AbortReason::Alloc);
-    }
-
     // Create new |this| on the caller-side for inlined constructors.
     if (callInfo.constructing()) {
         MDefinition* thisDefn = createThis(target, callInfo.fun(), callInfo.getNewTarget());
         if (!thisDefn)
             return abort(AbortReason::Alloc);
         callInfo.setThis(thisDefn);
     }
 
     // Capture formals in the outer resume point.
-    callInfo.pushFormals(current);
+    MOZ_TRY(callInfo.pushFormals(this, current));
 
     MResumePoint* outerResumePoint =
         MResumePoint::New(alloc(), current, pc, MResumePoint::Outer);
     if (!outerResumePoint)
         return abort(AbortReason::Alloc);
     current->setOuterResumePoint(outerResumePoint);
 
     // Pop formals again, except leave |fun| on stack for duration of call.
@@ -4405,17 +4398,17 @@ AbortReasonOr<Ok>
 IonBuilder::inlineGenericFallback(JSFunction* target, CallInfo& callInfo, MBasicBlock* dispatchBlock)
 {
     // Generate a new block with all arguments on-stack.
     MBasicBlock* fallbackBlock;
     MOZ_TRY_VAR(fallbackBlock, newBlock(dispatchBlock, pc));
     graph().addBlock(fallbackBlock);
 
     // Create a new CallInfo to track modified state within this block.
-    CallInfo fallbackInfo(alloc(), callInfo.constructing(), callInfo.ignoresReturnValue());
+    CallInfo fallbackInfo(alloc(), pc, callInfo.constructing(), callInfo.ignoresReturnValue());
     if (!fallbackInfo.init(callInfo))
         return abort(AbortReason::Alloc);
     fallbackInfo.popFormals(fallbackBlock);
 
     // Generate an MCall, which uses stateful |current|.
     MOZ_TRY(setCurrentAndSpecializePhis(fallbackBlock));
     MOZ_TRY(makeCall(target, fallbackInfo));
 
@@ -4442,17 +4435,17 @@ IonBuilder::inlineObjectGroupFallback(Ca
     MOZ_ASSERT_IF(callInfo.fun()->isTypeBarrier(), cache->hasOneUse());
 
     // This means that no resume points yet capture the MGetPropertyCache,
     // so everything from the MGetPropertyCache up until the call is movable.
     // We now move the MGetPropertyCache and friends into a fallback path.
     MOZ_ASSERT(cache->idempotent());
 
     // Create a new CallInfo to track modified state within the fallback path.
-    CallInfo fallbackInfo(alloc(), callInfo.constructing(), callInfo.ignoresReturnValue());
+    CallInfo fallbackInfo(alloc(), pc, callInfo.constructing(), callInfo.ignoresReturnValue());
     if (!fallbackInfo.init(callInfo))
         return abort(AbortReason::Alloc);
 
     // Capture stack prior to the call operation. This captures the function.
     MResumePoint* preCallResumePoint =
         MResumePoint::New(alloc(), dispatchBlock, pc, MResumePoint::ResumeAt);
     if (!preCallResumePoint)
         return abort(AbortReason::Alloc);
@@ -4528,17 +4521,17 @@ IonBuilder::inlineCalls(CallInfo& callIn
     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);
 
     MBasicBlock* dispatchBlock = current;
     callInfo.setImplicitlyUsedUnchecked();
-    callInfo.pushFormals(dispatchBlock);
+    MOZ_TRY(callInfo.pushFormals(this, dispatchBlock));
 
     // Patch any InlinePropertyTable to only contain functions that are
     // inlineable. The InlinePropertyTable will also be patched at the end to
     // exclude native functions that vetoed inlining.
     if (maybeCache) {
         InlinePropertyTable* propTable = maybeCache->propTable();
         propTable->trimToTargets(targets);
         if (propTable->numEntries() == 0)
@@ -4621,17 +4614,17 @@ IonBuilder::inlineCalls(CallInfo& callIn
         dispatchBlock->add(funcDef);
 
         // Use the inlined callee in the inline resume point and on stack.
         int funIndex = inlineBlock->entryResumePoint()->stackDepth() - callInfo.numFormals();
         inlineBlock->entryResumePoint()->replaceOperand(funIndex, funcDef);
         inlineBlock->rewriteSlot(funIndex, funcDef);
 
         // Create a new CallInfo to track modified state within the inline block.
-        CallInfo inlineInfo(alloc(), callInfo.constructing(), callInfo.ignoresReturnValue());
+        CallInfo inlineInfo(alloc(), pc, callInfo.constructing(), callInfo.ignoresReturnValue());
         if (!inlineInfo.init(callInfo))
             return abort(AbortReason::Alloc);
         inlineInfo.popFormals(inlineBlock);
         inlineInfo.setFun(funcDef);
 
         if (maybeCache) {
             // Assign the 'this' value a TypeSet specialized to the groups that
             // can generate this inlining target.
@@ -5068,17 +5061,17 @@ IonBuilder::jsop_funcall(uint32_t argc)
 
     int calleeDepth = -((int)argc + 2);
     int funcDepth = -((int)argc + 1);
 
     // If |Function.prototype.call| may be overridden, don't optimize callsite.
     TemporaryTypeSet* calleeTypes = current->peek(calleeDepth)->resultTypeSet();
     JSFunction* native = getSingleCallTarget(calleeTypes);
     if (!native || !native->isNative() || native->native() != &fun_call) {
-        CallInfo callInfo(alloc(), /* constructing = */ false,
+        CallInfo callInfo(alloc(), pc, /* constructing = */ false,
                           /* ignoresReturnValue = */ BytecodeIsPopped(pc));
         if (!callInfo.init(current, argc))
             return abort(AbortReason::Alloc);
         return makeCall(native, callInfo);
     }
     current->peek(calleeDepth)->setImplicitlyUsedUnchecked();
 
     // Extract call target.
@@ -5094,17 +5087,17 @@ IonBuilder::jsop_funcall(uint32_t argc)
     // Pushing is safe here, since one stack slot has been removed.
     if (zeroArguments) {
         pushConstant(UndefinedValue());
     } else {
         // |this| becomes implicit in the call.
         argc -= 1;
     }
 
-    CallInfo callInfo(alloc(), /* constructing = */ false,
+    CallInfo callInfo(alloc(), pc, /* constructing = */ false,
                       /* ignoresReturnValue = */ BytecodeIsPopped(pc));
     if (!callInfo.init(current, argc))
         return abort(AbortReason::Alloc);
 
     // Try to inline the call.
     if (!zeroArguments) {
         InliningDecision decision = makeInliningDecision(target, callInfo);
         switch (decision) {
@@ -5130,17 +5123,17 @@ IonBuilder::jsop_funcall(uint32_t argc)
 AbortReasonOr<Ok>
 IonBuilder::jsop_funapply(uint32_t argc)
 {
     int calleeDepth = -((int)argc + 2);
 
     TemporaryTypeSet* calleeTypes = current->peek(calleeDepth)->resultTypeSet();
     JSFunction* native = getSingleCallTarget(calleeTypes);
     if (argc != 2 || info().analysisMode() == Analysis_ArgumentsUsage) {
-        CallInfo callInfo(alloc(), /* constructing = */ false,
+        CallInfo callInfo(alloc(), pc, /* constructing = */ false,
                           /* ignoresReturnValue = */ BytecodeIsPopped(pc));
         if (!callInfo.init(current, argc))
             return abort(AbortReason::Alloc);
         return makeCall(native, callInfo);
     }
 
     // Disable compilation if the second argument to |apply| cannot be guaranteed
     // to be either definitely |arguments| or definitely not |arguments|.
@@ -5160,17 +5153,17 @@ IonBuilder::jsop_funapply(uint32_t argc)
             objTypes &&
             objTypes->getKnownClass(constraints()) == &ArrayObject::class_ &&
             !objTypes->hasObjectFlags(constraints(), OBJECT_FLAG_LENGTH_OVERFLOW) &&
             ElementAccessIsPacked(constraints(), argument))
         {
             return jsop_funapplyarray(argc);
         }
 
-        CallInfo callInfo(alloc(), /* constructing = */ false,
+        CallInfo callInfo(alloc(), pc, /* constructing = */ false,
                           /* ignoresReturnValue = */ BytecodeIsPopped(pc));
         if (!callInfo.init(current, argc))
             return abort(AbortReason::Alloc);
         return makeCall(native, callInfo);
     }
 
     if ((!native || !native->isNative() ||
         native->native() != fun_apply) &&
@@ -5307,17 +5300,17 @@ IonBuilder::jsop_funapplyarguments(uint3
     }
 
     // When inlining we have the arguments the function gets called with
     // and can optimize even more, by just calling the functions with the args.
     // We also try this path when doing the definite properties analysis, as we
     // can inline the apply() target and don't care about the actual arguments
     // that were passed in.
 
-    CallInfo callInfo(alloc(), /* constructing = */ false,
+    CallInfo callInfo(alloc(), pc, /* constructing = */ false,
                       /* ignoresReturnValue = */ BytecodeIsPopped(pc));
 
     // Vp
     MDefinition* vp = current->pop();
     vp->setImplicitlyUsedUnchecked();
 
     // Arguments
     if (inliningDepth_) {
@@ -5378,17 +5371,17 @@ IonBuilder::jsop_call(uint32_t argc, boo
     int calleeDepth = -((int)argc + 2 + constructing);
 
     // Acquire known call target if existent.
     InliningTargets targets(alloc());
     TemporaryTypeSet* calleeTypes = current->peek(calleeDepth)->resultTypeSet();
     if (calleeTypes)
         MOZ_TRY(getPolyCallTargets(calleeTypes, constructing, targets, 4));
 
-    CallInfo callInfo(alloc(), constructing, ignoresReturnValue);
+    CallInfo callInfo(alloc(), pc, constructing, ignoresReturnValue);
     if (!callInfo.init(current, argc))
         return abort(AbortReason::Alloc);
 
     // Try inlining
     InliningStatus status;
     MOZ_TRY_VAR(status, inlineCallsite(targets, callInfo));
     if (status == InliningStatus_Inlined)
         return Ok();
@@ -5630,17 +5623,17 @@ IonBuilder::jsop_eval(uint32_t argc)
             return abort(AbortReason::Disable, "Direct eval with more than one argument");
 
         if (!info().funMaybeLazy())
             return abort(AbortReason::Disable, "Direct eval in global code");
 
         if (info().funMaybeLazy()->isArrow())
             return abort(AbortReason::Disable, "Direct eval from arrow function");
 
-        CallInfo callInfo(alloc(), /* constructing = */ false,
+        CallInfo callInfo(alloc(), pc, /* constructing = */ false,
                           /* ignoresReturnValue = */ BytecodeIsPopped(pc));
         if (!callInfo.init(current, argc))
             return abort(AbortReason::Alloc);
         callInfo.setImplicitlyUsedUnchecked();
 
         callInfo.fun()->setImplicitlyUsedUnchecked();
 
         MDefinition* envChain = current->environmentChain();
@@ -5669,17 +5662,17 @@ IonBuilder::jsop_eval(uint32_t argc)
             if (StringEqualsAscii(atom, "()")) {
                 MDefinition* name = string->getOperand(0);
                 MInstruction* dynamicName = MGetDynamicName::New(alloc(), envChain, name);
                 current->add(dynamicName);
 
                 current->push(dynamicName);
                 current->push(constant(UndefinedValue())); // thisv
 
-                CallInfo evalCallInfo(alloc(), /* constructing = */ false,
+                CallInfo evalCallInfo(alloc(), pc, /* constructing = */ false,
                                       /* ignoresReturnValue = */ BytecodeIsPopped(pc));
                 if (!evalCallInfo.init(current, /* argc = */ 0))
                     return abort(AbortReason::Alloc);
 
                 return makeCall(nullptr, evalCallInfo);
             }
         }
 
@@ -11096,17 +11089,17 @@ IonBuilder::getPropTryCommonGetter(bool*
 
     // Make sure there's enough room
     if (!current->ensureHasSlots(2))
         return abort(AbortReason::Alloc);
     current->push(constant(ObjectValue(*commonGetter)));
 
     current->push(obj);
 
-    CallInfo callInfo(alloc(), /* constructing = */ false,
+    CallInfo callInfo(alloc(), pc, /* constructing = */ false,
                       /* ignoresReturnValue = */ BytecodeIsPopped(pc));
     if (!callInfo.init(current, 0))
         return abort(AbortReason::Alloc);
 
     if (commonGetter->isNative()) {
         InliningStatus status;
         MOZ_TRY_VAR(status, inlineNativeGetter(callInfo, commonGetter));
         switch (status) {
@@ -11637,17 +11630,17 @@ IonBuilder::setPropTryCommonSetter(bool*
         return abort(AbortReason::Alloc);
 
     current->push(constant(ObjectValue(*commonSetter)));
     current->push(obj);
     current->push(value);
 
     // Call the setter. Note that we have to push the original value, not
     // the setter's return value.
-    CallInfo callInfo(alloc(), /* constructing = */ false,
+    CallInfo callInfo(alloc(), pc, /* constructing = */ false,
                       /* ignoresReturnValue = */ BytecodeIsPopped(pc));
     if (!callInfo.init(current, 1))
         return abort(AbortReason::Alloc);
 
     // Ensure that we know we are calling a setter in case we inline it.
     callInfo.markAsSetter();
 
     // Inline the setter if we can.
--- a/js/src/jit/IonBuilder.h
+++ b/js/src/jit/IonBuilder.h
@@ -1190,26 +1190,28 @@ class CallInfo
     MDefinitionVector args_;
 
     bool constructing_:1;
 
     // True if the caller does not use the return value.
     bool ignoresReturnValue_:1;
 
     bool setter_:1;
+    bool apply_:1;
 
   public:
-    CallInfo(TempAllocator& alloc, bool constructing, bool ignoresReturnValue)
+    CallInfo(TempAllocator& alloc, jsbytecode* pc, bool constructing, bool ignoresReturnValue)
       : fun_(nullptr),
         thisArg_(nullptr),
         newTargetArg_(nullptr),
         args_(alloc),
         constructing_(constructing),
         ignoresReturnValue_(ignoresReturnValue),
-        setter_(false)
+        setter_(false),
+        apply_(JSOp(*pc) == JSOP_FUNAPPLY)
     { }
 
     MOZ_MUST_USE bool init(CallInfo& callInfo) {
         MOZ_ASSERT(constructing_ == callInfo.constructing());
 
         fun_ = callInfo.fun();
         thisArg_ = callInfo.thisArg();
         ignoresReturnValue_ = callInfo.ignoresReturnValue();
@@ -1243,25 +1245,36 @@ class CallInfo
 
         return true;
     }
 
     void popFormals(MBasicBlock* current) {
         current->popn(numFormals());
     }
 
-    void pushFormals(MBasicBlock* current) {
+    AbortReasonOr<Ok> pushFormals(MIRGenerator* mir, MBasicBlock* current) {
+        // Ensure sufficient space in the slots: needed for inlining from FUNAPPLY.
+        if (apply_) {
+            uint32_t depth = current->stackDepth() + numFormals();
+            if (depth > current->nslots()) {
+                if (!current->increaseSlots(depth - current->nslots()))
+                    return mir->abort(AbortReason::Alloc);
+            }
+        }
+
         current->push(fun());
         current->push(thisArg());
 
         for (uint32_t i = 0; i < argc(); i++)
             current->push(getArg(i));
 
         if (constructing())
             current->push(getNewTarget());
+
+        return Ok();
     }
 
     uint32_t argc() const {
         return args_.length();
     }
     uint32_t numFormals() const {
         return argc() + 2 + constructing();
     }
--- a/js/src/jit/MCallOptimize.cpp
+++ b/js/src/jit/MCallOptimize.cpp
@@ -873,17 +873,17 @@ IonBuilder::inlineArrayPush(CallInfo& ca
         truncate->setRecoveredOnBailout();
 
         current->add(elements);
         current->add(length);
         current->add(truncate);
 
         // Restore the stack, such that resume points are created with the stack
         // as it was before the call.
-        callInfo.pushFormals(current);
+        MOZ_TRY(callInfo.pushFormals(this, current));
     }
 
     MInstruction* ins = nullptr;
     for (uint32_t i = 0; i < callInfo.argc(); i++) {
         MDefinition* value = callInfo.getArg(i);
         if (toDouble) {
             MInstruction* valueDouble = MToDouble::New(alloc(), value);
             current->add(valueDouble);