Bug 1125770 - Always trace |this| slot in Ion frames, r=jandem.
authorBrian Hackett <bhackett1024@gmail.com>
Tue, 27 Jan 2015 18:11:44 -0700
changeset 226165 05f7505159ebfadcf03a7890ca14969aa85eadd0
parent 226164 58222952d07345035416a192a1c521f337dc250b
child 226166 1c324316ea7a505bc200d2d207e9b513e6c1ffa4
push id28187
push usercbook@mozilla.com
push dateWed, 28 Jan 2015 13:20:48 +0000
treeherdermozilla-central@fc21937ca612 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1125770
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 1125770 - Always trace |this| slot in Ion frames, r=jandem.
js/src/jit/BacktrackingAllocator.cpp
js/src/jit/JitFrames.cpp
js/src/jit/LinearScan.cpp
js/src/jit/RegisterAllocator.cpp
--- a/js/src/jit/BacktrackingAllocator.cpp
+++ b/js/src/jit/BacktrackingAllocator.cpp
@@ -163,27 +163,44 @@ 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
+IsThisSlotDefinition(LDefinition *def)
+{
+    return def->policy() == LDefinition::FIXED &&
+           def->output()->isArgument() &&
+           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
     // already grouped with reg0 or reg1.
     BacktrackingVirtualRegister *reg0 = &vregs[vreg0], *reg1 = &vregs[vreg1];
 
     if (!reg0->isCompatibleVReg(*reg1))
         return true;
 
+    // Registers which might spill to the frame's |this| slot can only be
+    // 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;
+    }
+
     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
@@ -967,33 +967,34 @@ ReadAllocation(const JitFrameIterator &f
         Register reg = a->toGeneralReg()->reg();
         return frame.machineState().read(reg);
     }
     return *frame.jsFrame()->slotRef(SafepointSlotEntry(a));
 }
 #endif
 
 static void
-MarkExtraActualArguments(JSTracer *trc, const JitFrameIterator &frame)
+MarkThisAndExtraActualArguments(JSTracer *trc, const JitFrameIterator &frame)
 {
-    // Mark any extra actual arguments for an Ion frame. Marking of |this| and
-    // the formal arguments is taken care of by the frame's safepoint/snapshot.
+    // 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.
 
     JitFrameLayout *layout = frame.jsFrame();
 
-    if (!CalleeTokenIsFunction(layout->calleeToken())) {
-        MOZ_ASSERT(frame.numActualArgs() == 0);
-        return;
-    }
-
     size_t nargs = frame.numActualArgs();
-    size_t nformals = CalleeTokenToFunction(layout->calleeToken())->nargs();
-
-    // Trace actual arguments. Note + 1 for thisv.
+    size_t nformals = 0;
+    if (CalleeTokenIsFunction(layout->calleeToken()))
+        nformals = CalleeTokenToFunction(layout->calleeToken())->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++)
         gc::MarkValueRoot(trc, &argv[i], "ion-argv");
 }
 
 #ifdef JS_NUNBOX32
 static inline void
 WriteAllocation(const JitFrameIterator &frame, const LAllocation *a, uintptr_t value)
 {
@@ -1018,17 +1019,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();
     }
 
-    MarkExtraActualArguments(trc, frame);
+    MarkThisAndExtraActualArguments(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 +1078,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.
-    MarkExtraActualArguments(trc, frame);
+    MarkThisAndExtraActualArguments(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.
 
--- a/js/src/jit/LinearScan.cpp
+++ b/js/src/jit/LinearScan.cpp
@@ -479,25 +479,21 @@ LinearScanAllocator::isSpilledAt(LiveInt
     }
 
     return interval->getAllocation() == reg->canonicalSpill();
 }
 
 bool
 LinearScanAllocator::populateSafepoints()
 {
-    // Populate all safepoints with this/argument slots. These are never changed
+    // Populate all safepoints with argument slots. These are never changed
     // by the allocator and are not necessarily populated by the code below.
     size_t nargs = graph.getBlock(0)->mir()->info().nargs();
     for (size_t i = 0; i < graph.numSafepoints(); i++) {
         LSafepoint *safepoint = graph.getSafepoint(i)->safepoint();
-
-        if (!safepoint->addValueSlot(/* stack = */ false, THIS_FRAME_ARGSLOT * sizeof(Value)))
-            return false;
-
         for (size_t j = 0; j < nargs; j++) {
             if (!safepoint->addValueSlot(/* stack = */ false, (j + 1) * sizeof(Value)))
                 return false;
         }
     }
 
     size_t firstSafepoint = 0;
 
--- a/js/src/jit/RegisterAllocator.cpp
+++ b/js/src/jit/RegisterAllocator.cpp
@@ -273,16 +273,20 @@ AllocationIntegrityState::checkSafepoint
     if (alloc.isRegister()) {
         AnyRegister reg = alloc.toRegister();
         if (populateSafepoints)
             safepoint->addLiveRegister(reg);
 
         MOZ_ASSERT(safepoint->liveRegs().has(reg));
     }
 
+    // The |this| argument slot is implicitly included in all safepoints.
+    if (alloc.isArgument() && alloc.toArgument()->index() < THIS_FRAME_ARGSLOT + sizeof(Value))
+        return true;
+
     LDefinition::Type type = virtualRegisters[vreg]
                              ? virtualRegisters[vreg]->type()
                              : LDefinition::GENERAL;
 
     switch (type) {
       case LDefinition::OBJECT:
         if (populateSafepoints) {
             JitSpew(JitSpew_RegAlloc, "Safepoint object v%u i%u %s",