Title: [161450] trunk/Source/_javascript_Core
Revision
161450
Author
[email protected]
Date
2014-01-07 13:05:30 -0800 (Tue, 07 Jan 2014)

Log Message

Repatch write barrier slow path call doesn't align the stack in the presence of saved registers
https://bugs.webkit.org/show_bug.cgi?id=126093

Reviewed by Geoffrey Garen.

* jit/Repatch.cpp: Reworked the stack alignment code for calling out to C code on the write barrier slow path.
We need to properly account for the number of reused registers that were saved to the stack, so we have to 
pass the ScratchRegisterAllocator around.
(JSC::storeToWriteBarrierBuffer):
(JSC::writeBarrier):
(JSC::emitPutReplaceStub):
(JSC::emitPutTransitionStub):
* jit/ScratchRegisterAllocator.h: Previously the ScratchRegisterAllocator only knew whether or not it had
reused registers, but not how many. In order to correctly align the stack for calls to C slow paths for 
the write barriers in inline caches we need to know how the stack is aligned. So now ScratchRegisterAllocator
tracks how many registers it has reused.
(JSC::ScratchRegisterAllocator::ScratchRegisterAllocator):
(JSC::ScratchRegisterAllocator::allocateScratch):
(JSC::ScratchRegisterAllocator::didReuseRegisters):
(JSC::ScratchRegisterAllocator::numberOfReusedRegisters):
(JSC::ScratchRegisterAllocator::preserveReusedRegistersByPushing):
(JSC::ScratchRegisterAllocator::restoreReusedRegistersByPopping):
* llint/LowLevelInterpreter64.asm: Random typo fix.

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (161449 => 161450)


--- trunk/Source/_javascript_Core/ChangeLog	2014-01-07 20:59:43 UTC (rev 161449)
+++ trunk/Source/_javascript_Core/ChangeLog	2014-01-07 21:05:30 UTC (rev 161450)
@@ -1,3 +1,29 @@
+2014-01-07  Mark Hahnenberg  <[email protected]>
+
+        Repatch write barrier slow path call doesn't align the stack in the presence of saved registers
+        https://bugs.webkit.org/show_bug.cgi?id=126093
+
+        Reviewed by Geoffrey Garen.
+
+        * jit/Repatch.cpp: Reworked the stack alignment code for calling out to C code on the write barrier slow path.
+        We need to properly account for the number of reused registers that were saved to the stack, so we have to 
+        pass the ScratchRegisterAllocator around.
+        (JSC::storeToWriteBarrierBuffer):
+        (JSC::writeBarrier):
+        (JSC::emitPutReplaceStub):
+        (JSC::emitPutTransitionStub):
+        * jit/ScratchRegisterAllocator.h: Previously the ScratchRegisterAllocator only knew whether or not it had
+        reused registers, but not how many. In order to correctly align the stack for calls to C slow paths for 
+        the write barriers in inline caches we need to know how the stack is aligned. So now ScratchRegisterAllocator
+        tracks how many registers it has reused.
+        (JSC::ScratchRegisterAllocator::ScratchRegisterAllocator):
+        (JSC::ScratchRegisterAllocator::allocateScratch):
+        (JSC::ScratchRegisterAllocator::didReuseRegisters):
+        (JSC::ScratchRegisterAllocator::numberOfReusedRegisters):
+        (JSC::ScratchRegisterAllocator::preserveReusedRegistersByPushing):
+        (JSC::ScratchRegisterAllocator::restoreReusedRegistersByPopping):
+        * llint/LowLevelInterpreter64.asm: Random typo fix.
+
 2014-01-07  Mark Lam  <[email protected]>
 
         r161364 caused JSC tests regression on non-DFG builds (e.g. C Loop and Windows).

Modified: trunk/Source/_javascript_Core/jit/Repatch.cpp (161449 => 161450)


--- trunk/Source/_javascript_Core/jit/Repatch.cpp	2014-01-07 20:59:43 UTC (rev 161449)
+++ trunk/Source/_javascript_Core/jit/Repatch.cpp	2014-01-07 21:05:30 UTC (rev 161450)
@@ -774,7 +774,7 @@
 }
 
 #if ENABLE(GGC)
-static MacroAssembler::Call storeToWriteBarrierBuffer(CCallHelpers& jit, GPRReg cell, GPRReg scratch1, GPRReg scratch2, ScratchRegisterAllocator& allocator)
+static MacroAssembler::Call storeToWriteBarrierBuffer(CCallHelpers& jit, GPRReg cell, GPRReg scratch1, GPRReg scratch2, GPRReg callFrameRegister, ScratchRegisterAllocator& allocator)
 {
     ASSERT(scratch1 != scratch2);
     WriteBarrierBuffer* writeBarrierBuffer = &jit.vm()->heap.writeBarrierBuffer();
@@ -795,17 +795,23 @@
     ScratchBuffer* scratchBuffer = jit.vm()->scratchBufferForSize(allocator.desiredScratchBufferSize());
     allocator.preserveUsedRegistersToScratchBuffer(jit, scratchBuffer, scratch1);
 
-    // We need these extra slots because setupArgumentsWithExecState will use poke on x86.
+    unsigned bytesFromBase = allocator.numberOfReusedRegisters() * sizeof(void*);
+    unsigned bytesToSubtract = 0;
 #if CPU(X86)
-    jit.subPtr(MacroAssembler::TrustedImm32(sizeof(void*) * 3), MacroAssembler::stackPointerRegister);
+    bytesToSubtract += 2 * sizeof(void*);
+    bytesFromBase += bytesToSubtract;
 #endif
+    unsigned currentAlignment = bytesFromBase % stackAlignmentBytes();
+    bytesToSubtract += currentAlignment;
 
-    jit.setupArgumentsWithExecState(cell);
+    if (bytesToSubtract)
+        jit.subPtr(MacroAssembler::TrustedImm32(bytesToSubtract), MacroAssembler::stackPointerRegister); 
+
+    jit.setupArguments(callFrameRegister, cell);
     MacroAssembler::Call call = jit.call();
 
-#if CPU(X86)
-    jit.addPtr(MacroAssembler::TrustedImm32(sizeof(void*) * 3), MacroAssembler::stackPointerRegister);
-#endif
+    if (bytesToSubtract)
+        jit.addPtr(MacroAssembler::TrustedImm32(bytesToSubtract), MacroAssembler::stackPointerRegister);
     allocator.restoreUsedRegistersFromScratchBuffer(jit, scratchBuffer, scratch1);
 
     done.link(&jit);
@@ -813,13 +819,13 @@
     return call;
 }
 
-static MacroAssembler::Call writeBarrier(CCallHelpers& jit, GPRReg owner, GPRReg scratch1, GPRReg scratch2, ScratchRegisterAllocator& allocator)
+static MacroAssembler::Call writeBarrier(CCallHelpers& jit, GPRReg owner, GPRReg scratch1, GPRReg scratch2, GPRReg callFrameRegister, ScratchRegisterAllocator& allocator)
 {
     ASSERT(owner != scratch1);
     ASSERT(owner != scratch2);
 
     MacroAssembler::Jump definitelyNotMarked = DFG::SpeculativeJIT::genericWriteBarrier(jit, owner, scratch1, scratch2);
-    MacroAssembler::Call call = storeToWriteBarrierBuffer(jit, owner, scratch1, scratch2, allocator);
+    MacroAssembler::Call call = storeToWriteBarrierBuffer(jit, owner, scratch1, scratch2, callFrameRegister, allocator);
     definitelyNotMarked.link(&jit);
     return call;
 }
@@ -837,6 +843,9 @@
     RefPtr<JITStubRoutine>& stubRoutine)
 {
     VM* vm = &exec->vm();
+#if ENABLE(GGC)
+    GPRReg callFrameRegister = static_cast<GPRReg>(stubInfo.patch.callFrameRegister);
+#endif
     GPRReg baseGPR = static_cast<GPRReg>(stubInfo.patch.baseGPR);
 #if USE(JSVALUE32_64)
     GPRReg valueTagGPR = static_cast<GPRReg>(stubInfo.patch.valueTagGPR);
@@ -883,7 +892,7 @@
 #endif
     
 #if ENABLE(GGC)
-    MacroAssembler::Call writeBarrierOperation = writeBarrier(stubJit, baseGPR, scratchGPR1, scratchGPR2, allocator);
+    MacroAssembler::Call writeBarrierOperation = writeBarrier(stubJit, baseGPR, scratchGPR1, scratchGPR2, callFrameRegister, allocator);
 #endif
     
     MacroAssembler::Jump success;
@@ -1050,7 +1059,7 @@
 #endif
     
 #if ENABLE(GGC)
-    MacroAssembler::Call writeBarrierOperation = writeBarrier(stubJit, baseGPR, scratchGPR1, scratchGPR2, allocator);
+    MacroAssembler::Call writeBarrierOperation = writeBarrier(stubJit, baseGPR, scratchGPR1, scratchGPR2, callFrameRegister, allocator);
 #endif
     
     MacroAssembler::Jump success;

Modified: trunk/Source/_javascript_Core/jit/ScratchRegisterAllocator.h (161449 => 161450)


--- trunk/Source/_javascript_Core/jit/ScratchRegisterAllocator.h	2014-01-07 20:59:43 UTC (rev 161449)
+++ trunk/Source/_javascript_Core/jit/ScratchRegisterAllocator.h	2014-01-07 21:05:30 UTC (rev 161450)
@@ -41,7 +41,7 @@
 public:
     ScratchRegisterAllocator(const TempRegisterSet& usedRegisters)
         : m_usedRegisters(usedRegisters)
-        , m_didReuseRegisters(false)
+        , m_numberOfReusedRegisters(0)
     {
     }
 
@@ -80,7 +80,7 @@
             typename BankInfo::RegisterType reg = BankInfo::toRegister(i);
             if (!m_lockedRegisters.get(reg) && !m_scratchRegisters.get(reg)) {
                 m_scratchRegisters.set(reg);
-                m_didReuseRegisters = true;
+                m_numberOfReusedRegisters++;
                 return reg;
             }
         }
@@ -96,12 +96,17 @@
     
     bool didReuseRegisters() const
     {
-        return m_didReuseRegisters;
+        return !!m_numberOfReusedRegisters;
     }
     
+    unsigned numberOfReusedRegisters() const
+    {
+        return m_numberOfReusedRegisters;
+    }
+    
     void preserveReusedRegistersByPushing(MacroAssembler& jit)
     {
-        if (!m_didReuseRegisters)
+        if (!didReuseRegisters())
             return;
         
         for (unsigned i = 0; i < FPRInfo::numberOfRegisters; ++i) {
@@ -116,7 +121,7 @@
     
     void restoreReusedRegistersByPopping(MacroAssembler& jit)
     {
-        if (!m_didReuseRegisters)
+        if (!didReuseRegisters())
             return;
         
         for (unsigned i = GPRInfo::numberOfRegisters; i--;) {
@@ -199,7 +204,7 @@
     TempRegisterSet m_usedRegisters;
     TempRegisterSet m_lockedRegisters;
     TempRegisterSet m_scratchRegisters;
-    bool m_didReuseRegisters;
+    unsigned m_numberOfReusedRegisters;
 };
 
 } // namespace JSC

Modified: trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm (161449 => 161450)


--- trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm	2014-01-07 20:59:43 UTC (rev 161449)
+++ trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm	2014-01-07 21:05:30 UTC (rev 161450)
@@ -340,7 +340,7 @@
                 btbz marked, .writeBarrierDone
                 push PB, PC
                 cCall2(_llint_write_barrier_slow, cfr, t0)
-                push PC, PB
+                pop PC, PB
             end
         )
     .writeBarrierDone:
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to