Bug 968931: IonMonkey: Loosen the verifyOsiPointRegs checks, r=jandem
authorHannes Verschore <hv1989@gmail.com>
Thu, 27 Mar 2014 20:08:34 +0100
changeset 175680 2dc1dab474abf06e205b19e27ef7edbf368f9540
parent 175679 036eab21ad2a3e2465f05125d3222d27ce63ad29
child 175681 281d420eb9b726dc717c59fbcef9101ee2907c52
push id41590
push userhv1989@gmail.com
push dateThu, 27 Mar 2014 19:10:13 +0000
treeherdermozilla-inbound@2dc1dab474ab [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs968931
milestone31.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 968931: IonMonkey: Loosen the verifyOsiPointRegs checks, r=jandem
js/src/jit/LIR.h
js/src/jit/LiveRangeAllocator.h
js/src/jit/shared/CodeGenerator-shared.cpp
--- a/js/src/jit/LIR.h
+++ b/js/src/jit/LIR.h
@@ -982,19 +982,19 @@ class LSafepoint : public TempObject
     // MarkJitExitFrame, and no registers can be live after the instruction
     // except its outputs.
     RegisterSet liveRegs_;
 
     // The subset of liveRegs which contains gcthing pointers.
     GeneralRegisterSet gcRegs_;
 
 #ifdef CHECK_OSIPOINT_REGISTERS
-    // Temp regs of the current instruction. This set is never written to the
-    // safepoint; it's only used by assertions during compilation.
-    RegisterSet tempRegs_;
+    // Clobbered regs of the current instruction. This set is never written to
+    // the safepoint; it's only used by assertions during compilation.
+    RegisterSet clobberedRegs_;
 #endif
 
     // Offset to a position in the safepoint stream, or
     // INVALID_SAFEPOINT_OFFSET.
     uint32_t safepointOffset_;
 
     // Assembler buffer displacement to OSI point's call location.
     uint32_t osiCallPointOffset_;
@@ -1047,22 +1047,22 @@ class LSafepoint : public TempObject
     void addLiveRegister(AnyRegister reg) {
         liveRegs_.addUnchecked(reg);
         assertInvariants();
     }
     const RegisterSet &liveRegs() const {
         return liveRegs_;
     }
 #ifdef CHECK_OSIPOINT_REGISTERS
-    void addTempRegister(AnyRegister reg) {
-        tempRegs_.addUnchecked(reg);
+    void addClobberedRegister(AnyRegister reg) {
+        clobberedRegs_.addUnchecked(reg);
         assertInvariants();
     }
-    const RegisterSet &tempRegs() const {
-        return tempRegs_;
+    const RegisterSet &clobberedRegs() const {
+        return clobberedRegs_;
     }
 #endif
     void addGcRegister(Register reg) {
         gcRegs_.addUnchecked(reg);
         assertInvariants();
     }
     GeneralRegisterSet gcRegs() const {
         return gcRegs_;
--- a/js/src/jit/LiveRangeAllocator.h
+++ b/js/src/jit/LiveRangeAllocator.h
@@ -681,18 +681,26 @@ class LiveRangeAllocator : protected Reg
     {
         // Fill in the live register sets for all non-call safepoints.
         LAllocation *a = interval->getAllocation();
         if (!a->isRegister())
             return;
 
         // Don't add output registers to the safepoint.
         CodePosition start = interval->start();
-        if (interval->index() == 0 && !reg->isTemp())
+        if (interval->index() == 0 && !reg->isTemp()) {
+#ifdef CHECK_OSIPOINT_REGISTERS
+            // We don't add the output register to the safepoint,
+            // but it still might get added as one of the inputs.
+            // So eagerly add this reg to the safepoint clobbered registers.
+            if (LSafepoint *safepoint = reg->ins()->safepoint())
+                safepoint->addClobberedRegister(a->toRegister());
+#endif
             start = start.next();
+        }
 
         size_t i = findFirstNonCallSafepoint(start);
         for (; i < graph.numNonCallSafepoints(); i++) {
             LInstruction *ins = graph.getNonCallSafepoint(i);
             CodePosition pos = inputOf(ins);
 
             // Safepoints are sorted, so we can shortcut out of this loop
             // if we go out of range.
@@ -702,17 +710,17 @@ class LiveRangeAllocator : protected Reg
             if (!interval->covers(pos))
                 continue;
 
             LSafepoint *safepoint = ins->safepoint();
             safepoint->addLiveRegister(a->toRegister());
 
 #ifdef CHECK_OSIPOINT_REGISTERS
             if (reg->isTemp())
-                safepoint->addTempRegister(a->toRegister());
+                safepoint->addClobberedRegister(a->toRegister());
 #endif
         }
     }
 
     // Finds the first safepoint that is within range of an interval.
     size_t findFirstSafepoint(const LiveInterval *interval, size_t startFrom) const
     {
         size_t i = startFrom;
--- a/js/src/jit/shared/CodeGenerator-shared.cpp
+++ b/js/src/jit/shared/CodeGenerator-shared.cpp
@@ -555,21 +555,23 @@ CodeGeneratorShared::verifyOsiPointRegs(
 
     // Having more than one VM function call made in one visit function at
     // runtime is a sec-ciritcal error, because if we conservatively assume that
     // one of the function call can re-enter Ion, then the invalidation process
     // will potentially add a call at a random location, by patching the code
     // before the return address.
     masm.branch32(Assembler::NotEqual, checkRegs, Imm32(1), &failure);
 
-    // Ignore temp registers. Some instructions (like LValueToInt32) modify
+    // Ignore clobbered registers. Some instructions (like LValueToInt32) modify
     // temps after calling into the VM. This is fine because no other
-    // instructions (including this OsiPoint) will depend on them.
+    // instructions (including this OsiPoint) will depend on them. Also
+    // backtracking can also use the same register for an input and an output.
+    // These are marked as clobbered and shouldn't get checked.
     RegisterSet liveRegs = safepoint->liveRegs();
-    liveRegs = RegisterSet::Intersect(liveRegs, RegisterSet::Not(safepoint->tempRegs()));
+    liveRegs = RegisterSet::Intersect(liveRegs, RegisterSet::Not(safepoint->clobberedRegs()));
 
     VerifyOp op(masm, &failure);
     HandleRegisterDump<VerifyOp>(op, masm, liveRegs, scratch, allRegs.getAny());
 
     masm.jump(&done);
 
     // Do not profile the callWithABI that occurs below.  This is to avoid a
     // rare corner case that occurs when profiling interacts with itself: