Bug 1128490 - Mark formal arguments in Ion frames which use lazy arguments, r=jandem.
☠☠ backed out by 6f42cfe37f5c ☠ ☠
authorBrian Hackett <bhackett1024@gmail.com>
Tue, 10 Feb 2015 14:41:59 -0700
changeset 255618 7152dc5221cb10bc9c44d331778bf52610778a75
parent 255582 ee093ca706662491d356484bf729ef32d55bc19c
child 255619 91c1bd885ad98929c0eda0f22ed516e84f504322
push id4610
push userjlund@mozilla.com
push dateMon, 30 Mar 2015 18:32:55 +0000
treeherdermozilla-beta@4df54044d9ef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1128490
milestone38.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 1128490 - Mark formal arguments in Ion frames which use lazy arguments, r=jandem.
js/src/jit-test/tests/ion/bug1128490.js
js/src/jit/BacktrackingAllocator.cpp
js/src/jit/JitFrames.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ion/bug1128490.js
@@ -0,0 +1,10 @@
+
+for (var i = 0; i < 1000; ++i) {
+    function isNotEmpty(obj) {
+        for (var i = 0 ; i < arguments.length; i++) {
+            minorgc();
+            var o = arguments[i];
+        }
+    };
+    isNotEmpty([1]);
+}
--- a/js/src/jit/BacktrackingAllocator.cpp
+++ b/js/src/jit/BacktrackingAllocator.cpp
@@ -167,20 +167,25 @@ BacktrackingAllocator::canAddToGroup(Vir
     for (size_t i = 0; i < group->registers.length(); i++) {
         if (LifetimesOverlap(reg, &vregs[group->registers[i]]))
             return false;
     }
     return true;
 }
 
 static bool
+IsArgumentSlotDefinition(LDefinition *def)
+{
+    return def->policy() == LDefinition::FIXED && def->output()->isArgument();
+}
+
+static bool
 IsThisSlotDefinition(LDefinition *def)
 {
-    return def->policy() == LDefinition::FIXED &&
-           def->output()->isArgument() &&
+    return IsArgumentSlotDefinition(def) &&
            def->output()->toArgument()->index() < THIS_FRAME_ARGSLOT + sizeof(Value);
 }
 
 bool
 BacktrackingAllocator::tryGroupRegisters(uint32_t vreg0, uint32_t vreg1)
 {
     // See if reg0 and reg1 can be placed in the same group, following the
     // restrictions imposed by VirtualRegisterGroup and any other registers
@@ -194,16 +199,26 @@ BacktrackingAllocator::tryGroupRegisters
     // grouped with other such registers. The frame's |this| slot must always
     // hold the |this| value, as required by JitFrame tracing and by the Ion
     // constructor calling convention.
     if (IsThisSlotDefinition(reg0->def()) || IsThisSlotDefinition(reg1->def())) {
         if (*reg0->def()->output() != *reg1->def()->output())
             return true;
     }
 
+    // Registers which might spill to the frame's argument slots can only be
+    // grouped with other such registers if the frame might access those
+    // arguments through a lazy arguments object.
+    if ((IsArgumentSlotDefinition(reg0->def()) || IsArgumentSlotDefinition(reg1->def())) &&
+        graph.mir().entryBlock()->info().script()->argumentsAliasesFormals())
+    {
+        if (*reg0->def()->output() != *reg1->def()->output())
+            return true;
+    }
+
     VirtualRegisterGroup *group0 = reg0->group(), *group1 = reg1->group();
 
     if (!group0 && group1)
         return tryGroupRegisters(vreg1, vreg0);
 
     if (group0) {
         if (group1) {
             if (group0 == group1) {
--- a/js/src/jit/JitFrames.cpp
+++ b/js/src/jit/JitFrames.cpp
@@ -966,27 +966,31 @@ ReadAllocation(const JitFrameIterator &f
         Register reg = a->toGeneralReg()->reg();
         return frame.machineState().read(reg);
     }
     return *frame.jsFrame()->slotRef(SafepointSlotEntry(a));
 }
 #endif
 
 static void
-MarkThisAndExtraActualArguments(JSTracer *trc, const JitFrameIterator &frame)
+MarkThisAndArguments(JSTracer *trc, const JitFrameIterator &frame)
 {
     // Mark |this| and any extra actual arguments for an Ion frame. Marking of
-    // formal arguments is taken care of by the frame's safepoint/snapshot.
+    // formal arguments is taken care of by the frame's safepoint/snapshot,
+    // except when the script's lazy arguments object aliases those formals,
+    // in which case we mark them as well.
 
     JitFrameLayout *layout = frame.jsFrame();
 
     size_t nargs = frame.numActualArgs();
     size_t nformals = 0;
-    if (CalleeTokenIsFunction(layout->calleeToken()))
-        nformals = CalleeTokenToFunction(layout->calleeToken())->nargs();
+    if (CalleeTokenIsFunction(layout->calleeToken())) {
+        JSFunction *fun = CalleeTokenToFunction(layout->calleeToken());
+        nformals = fun->nonLazyScript()->argumentsAliasesFormals() ? 0 : fun->nargs();
+    }
 
     Value *argv = layout->argv();
 
     // Trace |this|.
     gc::MarkValueRoot(trc, argv, "ion-thisv");
 
     // Trace actual arguments beyond the formals. Note + 1 for thisv.
     for (size_t i = nformals + 1; i < nargs + 1; i++)
@@ -1018,17 +1022,17 @@ MarkIonJSFrame(JSTracer *trc, const JitF
         // This frame has been invalidated, meaning that its IonScript is no
         // longer reachable through the callee token (JSFunction/JSScript->ion
         // is now nullptr or recompiled). Manually trace it here.
         IonScript::Trace(trc, ionScript);
     } else {
         ionScript = frame.ionScriptFromCalleeToken();
     }
 
-    MarkThisAndExtraActualArguments(trc, frame);
+    MarkThisAndArguments(trc, frame);
 
     const SafepointIndex *si = ionScript->getSafepointIndex(frame.returnAddressToFp());
 
     SafepointReader safepoint(ionScript, si);
 
     // Scan through slots which contain pointers (or on punboxing systems,
     // actual values).
     SafepointSlotEntry entry;
@@ -1077,17 +1081,17 @@ static void
 MarkBailoutFrame(JSTracer *trc, const JitFrameIterator &frame)
 {
     JitFrameLayout *layout = (JitFrameLayout *)frame.fp();
 
     layout->replaceCalleeToken(MarkCalleeToken(trc, layout->calleeToken()));
 
     // We have to mark the list of actual arguments, as only formal arguments
     // are represented in the Snapshot.
-    MarkThisAndExtraActualArguments(trc, frame);
+    MarkThisAndArguments(trc, frame);
 
     // Under a bailout, do not have a Safepoint to only iterate over GC-things.
     // Thus we use a SnapshotIterator to trace all the locations which would be
     // used to reconstruct the Baseline frame.
     //
     // Note that at the time where this function is called, we have not yet
     // started to reconstruct baseline frames.