Title: [192525] trunk/Source/_javascript_Core
Revision
192525
Author
sbar...@apple.com
Date
2015-11-17 12:52:26 -0800 (Tue, 17 Nov 2015)

Log Message

FTLLazySlowPaths should be able to handle being passed the zero register as a location
https://bugs.webkit.org/show_bug.cgi?id=151193

Reviewed by Geoffrey Garen.

On arm64, SP and ZR are the same register number. The meaning
of the register number being SP or ZR is dependent on context of
the instruction and the register within the instruction. LLVM may
prove that a value is zero, or sometimes, we will lower a
value as a constant zero (for example, we might compile
CreateDirectArguments of an inlined call frame and we might know
that the arguments have a length of zero). LazySlowPaths should
be able to gracefully handle being passed a stackmap location
with a gpr value of SP by moving zero into another register
and replacing the location's register with the new register.
This way, no lazy slow path will ever have access to a location with a GPR
value of SP.  This patch makes this work by using a scratch register
allocator when we need to do this maneuver of moving zero into a scratch
register.  Inside FTLCompile, we track if we need to move zero into a register,
and if so, into which register. We actually emit the necessary
instructions to move zero into this register, and to spill reused
registers if necessary, while generating the code for the lazy slow
path itself.

* ftl/FTLCompile.cpp:
(JSC::FTL::mmAllocateDataSection):
* ftl/FTLDWARFRegister.cpp:
(JSC::FTL::DWARFRegister::reg):
* ftl/FTLLazySlowPath.cpp:
(JSC::FTL::LazySlowPath::LazySlowPath):
(JSC::FTL::LazySlowPath::generate):
* ftl/FTLLazySlowPath.h:
(JSC::FTL::LazySlowPath::createGenerator):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (192524 => 192525)


--- trunk/Source/_javascript_Core/ChangeLog	2015-11-17 20:24:12 UTC (rev 192524)
+++ trunk/Source/_javascript_Core/ChangeLog	2015-11-17 20:52:26 UTC (rev 192525)
@@ -1,3 +1,39 @@
+2015-11-17  Saam barati  <sbar...@apple.com>
+
+        FTLLazySlowPaths should be able to handle being passed the zero register as a location
+        https://bugs.webkit.org/show_bug.cgi?id=151193
+
+        Reviewed by Geoffrey Garen.
+
+        On arm64, SP and ZR are the same register number. The meaning
+        of the register number being SP or ZR is dependent on context of
+        the instruction and the register within the instruction. LLVM may
+        prove that a value is zero, or sometimes, we will lower a
+        value as a constant zero (for example, we might compile
+        CreateDirectArguments of an inlined call frame and we might know
+        that the arguments have a length of zero). LazySlowPaths should
+        be able to gracefully handle being passed a stackmap location
+        with a gpr value of SP by moving zero into another register
+        and replacing the location's register with the new register.
+        This way, no lazy slow path will ever have access to a location with a GPR
+        value of SP.  This patch makes this work by using a scratch register 
+        allocator when we need to do this maneuver of moving zero into a scratch
+        register.  Inside FTLCompile, we track if we need to move zero into a register,
+        and if so, into which register. We actually emit the necessary
+        instructions to move zero into this register, and to spill reused
+        registers if necessary, while generating the code for the lazy slow
+        path itself.
+
+        * ftl/FTLCompile.cpp:
+        (JSC::FTL::mmAllocateDataSection):
+        * ftl/FTLDWARFRegister.cpp:
+        (JSC::FTL::DWARFRegister::reg):
+        * ftl/FTLLazySlowPath.cpp:
+        (JSC::FTL::LazySlowPath::LazySlowPath):
+        (JSC::FTL::LazySlowPath::generate):
+        * ftl/FTLLazySlowPath.h:
+        (JSC::FTL::LazySlowPath::createGenerator):
+
 2015-11-17  Mark Lam  <mark....@apple.com>
 
         [JSC] Support Doubles with B3's Mul.

Modified: trunk/Source/_javascript_Core/ftl/FTLCompile.cpp (192524 => 192525)


--- trunk/Source/_javascript_Core/ftl/FTLCompile.cpp	2015-11-17 20:24:12 UTC (rev 192524)
+++ trunk/Source/_javascript_Core/ftl/FTLCompile.cpp	2015-11-17 20:52:26 UTC (rev 192525)
@@ -922,10 +922,6 @@
             for (unsigned i = 0; i < iter->value.size(); ++i) {
                 StackMaps::Record& record = iter->value[i].record;
                 RegisterSet usedRegisters = usedRegistersFor(record);
-                Vector<Location> locations;
-                for (auto location : record.locations)
-                    locations.append(Location::forStackmaps(&stackmaps, location));
-
                 char* startOfIC =
                     bitwise_cast<char*>(generatedFunction) + record.instructionOffset;
                 CodeLocationLabel patchpoint((MacroAssemblerCodePtr(startOfIC)));
@@ -933,9 +929,28 @@
                 if (!exceptionTarget)
                     exceptionTarget = state.finalizer->handleExceptionsLinkBuffer->entrypoint();
 
+                ScratchRegisterAllocator scratchAllocator(usedRegisters);
+                GPRReg newZero = InvalidGPRReg;
+                Vector<Location> locations;
+                for (auto stackmapLocation : record.locations) {
+                    FTL::Location location = Location::forStackmaps(&stackmaps, stackmapLocation);
+                    if (isARM64()) {
+                        // If LLVM proves that something is zero, it may pass us the zero register (aka, the stack pointer). Our assembler
+                        // isn't prepared to handle this well. We need to move it into a different register if such a case arises.
+                        if (location.isGPR() && location.gpr() == MacroAssembler::stackPointerRegister) {
+                            if (newZero == InvalidGPRReg) {
+                                newZero = scratchAllocator.allocateScratchGPR();
+                                usedRegisters.set(newZero);
+                            }
+                            location = FTL::Location::forRegister(DWARFRegister(static_cast<uint16_t>(newZero)), 0); // DWARF GPRs for arm64 are sensibly numbered.
+                        }
+                    }
+                    locations.append(location);
+                }
+
                 std::unique_ptr<LazySlowPath> lazySlowPath = std::make_unique<LazySlowPath>(
                     patchpoint, exceptionTarget, usedRegisters, exceptionHandlerManager.procureCallSiteIndex(iter->value[i].index, codeOrigin),
-                    descriptor.m_linker->run(locations));
+                    descriptor.m_linker->run(locations), newZero, scratchAllocator);
 
                 CCallHelpers::Label begin = slowPathJIT.label();
 

Modified: trunk/Source/_javascript_Core/ftl/FTLDWARFRegister.cpp (192524 => 192525)


--- trunk/Source/_javascript_Core/ftl/FTLDWARFRegister.cpp	2015-11-17 20:24:12 UTC (rev 192524)
+++ trunk/Source/_javascript_Core/ftl/FTLDWARFRegister.cpp	2015-11-17 20:52:26 UTC (rev 192525)
@@ -55,7 +55,6 @@
         case 7:
             return X86Registers::esp;
         default:
-            RELEASE_ASSERT(m_dwarfRegNum < 16);
             // Registers r8..r15 are numbered sensibly.
             return static_cast<GPRReg>(m_dwarfRegNum);
         }

Modified: trunk/Source/_javascript_Core/ftl/FTLLazySlowPath.cpp (192524 => 192525)


--- trunk/Source/_javascript_Core/ftl/FTLLazySlowPath.cpp	2015-11-17 20:24:12 UTC (rev 192524)
+++ trunk/Source/_javascript_Core/ftl/FTLLazySlowPath.cpp	2015-11-17 20:52:26 UTC (rev 192525)
@@ -35,12 +35,15 @@
 
 LazySlowPath::LazySlowPath(
     CodeLocationLabel patchpoint, CodeLocationLabel exceptionTarget,
-    const RegisterSet& usedRegisters, CallSiteIndex callSiteIndex, RefPtr<Generator> generator)
+    const RegisterSet& usedRegisters, CallSiteIndex callSiteIndex, RefPtr<Generator> generator,
+    GPRReg newZeroReg, ScratchRegisterAllocator scratchRegisterAllocator)
     : m_patchpoint(patchpoint)
     , m_exceptionTarget(exceptionTarget)
     , m_usedRegisters(usedRegisters)
     , m_callSiteIndex(callSiteIndex)
     , m_generator(generator)
+    , m_newZeroValueRegister(newZeroReg)
+    , m_scratchRegisterAllocator(scratchRegisterAllocator)
 {
 }
 
@@ -59,11 +62,32 @@
     CCallHelpers::JumpList exceptionJumps;
     params.exceptionJumps = m_exceptionTarget ? &exceptionJumps : nullptr;
     params.lazySlowPath = this;
+
+    unsigned bytesSaved = m_scratchRegisterAllocator.preserveReusedRegistersByPushing(jit, ScratchRegisterAllocator::ExtraStackSpace::NoExtraSpace);
+    // This is needed because LLVM may create a stackmap location that is the register SP.
+    // But on arm64, SP is also the same register number as ZR, so LLVM is telling us that it has
+    // proven something is zero. Our MASM isn't universally compatible with arm64's context dependent
+    // notion of SP meaning ZR. We just make things easier by ensuring we do the necessary move of zero
+    // into a non-SP register.
+    if (m_newZeroValueRegister != InvalidGPRReg)
+        jit.move(CCallHelpers::TrustedImm32(0), m_newZeroValueRegister);
+
     m_generator->run(jit, params);
 
+    CCallHelpers::Label doneLabel;
+    CCallHelpers::Jump jumpToEndOfPatchpoint;
+    if (bytesSaved) {
+        doneLabel = jit.label();
+        m_scratchRegisterAllocator.restoreReusedRegistersByPopping(jit, bytesSaved, ScratchRegisterAllocator::ExtraStackSpace::NoExtraSpace);
+        jumpToEndOfPatchpoint = jit.jump();
+    }
+
     LinkBuffer linkBuffer(vm, jit, codeBlock, JITCompilationMustSucceed);
-    linkBuffer.link(
-        params.doneJumps, m_patchpoint.labelAtOffset(MacroAssembler::maxJumpReplacementSize()));
+    if (bytesSaved) {
+        linkBuffer.link(params.doneJumps, linkBuffer.locationOf(doneLabel));
+        linkBuffer.link(jumpToEndOfPatchpoint, m_patchpoint.labelAtOffset(MacroAssembler::maxJumpReplacementSize()));
+    } else
+        linkBuffer.link(params.doneJumps, m_patchpoint.labelAtOffset(MacroAssembler::maxJumpReplacementSize()));
     if (m_exceptionTarget)
         linkBuffer.link(exceptionJumps, m_exceptionTarget);
     m_stub = FINALIZE_CODE_FOR(codeBlock, linkBuffer, ("Lazy slow path call stub"));

Modified: trunk/Source/_javascript_Core/ftl/FTLLazySlowPath.h (192524 => 192525)


--- trunk/Source/_javascript_Core/ftl/FTLLazySlowPath.h	2015-11-17 20:24:12 UTC (rev 192524)
+++ trunk/Source/_javascript_Core/ftl/FTLLazySlowPath.h	2015-11-17 20:52:26 UTC (rev 192525)
@@ -34,6 +34,7 @@
 #include "GPRInfo.h"
 #include "MacroAssemblerCodeRef.h"
 #include "RegisterSet.h"
+#include "ScratchRegisterAllocator.h"
 #include <wtf/SharedTask.h>
 
 namespace JSC { namespace FTL {
@@ -65,8 +66,8 @@
     }
     
     LazySlowPath(
-        CodeLocationLabel patchpoint, CodeLocationLabel exceptionTarget,
-        const RegisterSet& usedRegisters, CallSiteIndex callSiteIndex, RefPtr<Generator>);
+        CodeLocationLabel patchpoint, CodeLocationLabel exceptionTarget, const RegisterSet& usedRegisters,
+        CallSiteIndex, RefPtr<Generator>, GPRReg newZeroReg, ScratchRegisterAllocator);
 
     ~LazySlowPath();
 
@@ -85,6 +86,8 @@
     CallSiteIndex m_callSiteIndex;
     MacroAssemblerCodeRef m_stub;
     RefPtr<Generator> m_generator;
+    GPRReg m_newZeroValueRegister;
+    ScratchRegisterAllocator m_scratchRegisterAllocator;
 };
 
 } } // namespace JSC::FTL
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to