Attachment 224393 Details for Bug 128890 – the patch

[patch] the patch

blah.patch (text/plain), 7.25 KB, created by Filip Pizlo on 2014-02-17 10:26:24 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Filip Pizlo

Created: 2014-02-17 10:26:24 PST

Size: 7.25 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 164207)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,25 @@
>+2014-02-16  Filip Pizlo  <fpizlo@apple.com>
>+
>+        FTL OSR exit shouldn't make X86-specific assumptions
>+        https://bugs.webkit.org/show_bug.cgi?id=128890
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Mostly this is about not using push/pop, but instead using the more abstract pushToSave() and popToRestore() while reflecting on the stack alignment.
>+
>+        * assembler/MacroAssembler.h:
>+        (JSC::MacroAssembler::pushToSaveImmediateWithoutTouchingRegisters):
>+        (JSC::MacroAssembler::pushToSaveByteOffset):
>+        * assembler/MacroAssemblerARM64.h:
>+        (JSC::MacroAssemblerARM64::pushToSaveImmediateWithoutTouchingRegisters):
>+        (JSC::MacroAssemblerARM64::pushToSaveByteOffset):
>+        * ftl/FTLExitThunkGenerator.cpp:
>+        (JSC::FTL::ExitThunkGenerator::emitThunk):
>+        * ftl/FTLOSRExitCompiler.cpp:
>+        (JSC::FTL::compileStub):
>+        * ftl/FTLThunks.cpp:
>+        (JSC::FTL::osrExitGenerationThunkGenerator):
>+
> 2014-02-15  Filip Pizlo  <fpizlo@apple.com>
> 
>         FTL should inline polymorphic heap accesses
>Index: Source/JavaScriptCore/assembler/MacroAssembler.h
>===================================================================
>--- Source/JavaScriptCore/assembler/MacroAssembler.h	(revision 164207)
>+++ Source/JavaScriptCore/assembler/MacroAssembler.h	(working copy)
>@@ -258,6 +258,10 @@ public:
>     {
>         push(src);
>     }
>+    void pushToSaveImmediateWithoutTouchingRegisters(TrustedImm32 imm)
>+    {
>+        push(imm);
>+    }
>     void popToRestore(RegisterID dest)
>     {
>         pop(dest);
>@@ -272,6 +276,8 @@ public:
>         loadDouble(stackPointerRegister, dest);
>         addPtr(TrustedImm32(sizeof(double)), stackPointerRegister);
>     }
>+    
>+    static ptrdiff_t pushToSaveByteOffset() { return sizeof(void*); }
> #endif // !CPU(ARM64)
> 
> #if CPU(X86_64) || CPU(ARM64)
>Index: Source/JavaScriptCore/assembler/MacroAssemblerARM64.h
>===================================================================
>--- Source/JavaScriptCore/assembler/MacroAssemblerARM64.h	(revision 164207)
>+++ Source/JavaScriptCore/assembler/MacroAssemblerARM64.h	(working copy)
>@@ -1459,6 +1459,15 @@ public:
>     {
>         m_assembler.str<64>(src, ARM64Registers::sp, PreIndex(-16));
>     }
>+    
>+    void pushToSaveImmediateWithoutTouchingRegisters(TrustedImm32 imm)
>+    {
>+        RegisterID reg = dataTempRegister;
>+        pushPair(reg, reg);
>+        move(imm, reg);
>+        store64(reg, stackPointerRegister);
>+        load64(Address(stackPointerRegister, 8), reg);
>+    }
> 
>     void pushToSave(Address address)
>     {
>@@ -1484,6 +1493,7 @@ public:
>         storeDouble(src, stackPointerRegister);
>     }
> 
>+    static ptrdiff_t pushToSaveByteOffset() { return 16; }
> 
>     // Register move operations:
> 
>Index: Source/JavaScriptCore/ftl/FTLExitThunkGenerator.cpp
>===================================================================
>--- Source/JavaScriptCore/ftl/FTLExitThunkGenerator.cpp	(revision 164207)
>+++ Source/JavaScriptCore/ftl/FTLExitThunkGenerator.cpp	(working copy)
>@@ -51,7 +51,7 @@ void ExitThunkGenerator::emitThunk(unsig
>     OSRExitCompilationInfo& info = m_state.finalizer->osrExit[index];
>     
>     info.m_thunkLabel = label();
>-    push(TrustedImm32(index));
>+    pushToSaveImmediateWithoutTouchingRegisters(TrustedImm32(index));
>     info.m_thunkJump = patchableJump();
>     
>     m_didThings = true;
>Index: Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp
>===================================================================
>--- Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp	(revision 164207)
>+++ Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp	(working copy)
>@@ -70,14 +70,14 @@ static void compileStub(
>     char* registerScratch = bitwise_cast<char*>(scratch + exit.m_values.size());
>     uint64_t* unwindScratch = bitwise_cast<uint64_t*>(registerScratch + requiredScratchMemorySizeInBytes());
>     
>-    // Make sure that saveAllRegisters() has a place on top of the stack to spill things. That
>-    // function expects to be able to use top of stack for scratch memory.
>-    jit.push(GPRInfo::regT0);
>+    // Note that we come in here, the stack used to be as LLVM left it except that someone called pushToSave().
>+    // We don't care about the value they saved. But, we do appreciate the fact that they did it, because we use
>+    // that slot for saveAllRegisters().
>+
>     saveAllRegisters(jit, registerScratch);
>     
>     // Bring the stack back into a sane form.
>-    jit.pop(GPRInfo::regT0);
>-    jit.pop(GPRInfo::regT0);
>+    jit.popToRestore(GPRInfo::regT0);
>     
>     if (vm->m_perBytecodeProfiler && codeBlock->jitCode()->dfgCommon()->compilation) {
>         Profiler::Database& database = *vm->m_perBytecodeProfiler;
>Index: Source/JavaScriptCore/ftl/FTLThunks.cpp
>===================================================================
>--- Source/JavaScriptCore/ftl/FTLThunks.cpp	(revision 164207)
>+++ Source/JavaScriptCore/ftl/FTLThunks.cpp	(working copy)
>@@ -45,11 +45,20 @@ MacroAssemblerCodeRef osrExitGenerationT
>     
>     // Note that the "return address" will be the OSR exit ID.
>     
>+    ptrdiff_t stackMisalignment = MacroAssembler::pushToSaveByteOffset();
>+    
>     // Pretend that we're a C call frame.
>-    jit.push(MacroAssembler::framePointerRegister);
>+    jit.pushToSave(MacroAssembler::framePointerRegister);
>     jit.move(MacroAssembler::stackPointerRegister, MacroAssembler::framePointerRegister);
>-    jit.push(GPRInfo::regT0);
>-    jit.push(GPRInfo::regT0);
>+    stackMisalignment += MacroAssembler::pushToSaveByteOffset();
>+    
>+    // Now create ourselves enough stack space to give saveAllRegisters() a scratch slot.
>+    unsigned numberOfRequiredPops = 0;
>+    do {
>+        jit.pushToSave(GPRInfo::regT0);
>+        stackMisalignment += MacroAssembler::pushToSaveByteOffset();
>+        numberOfRequiredPops++;
>+    } while (stackMisalignment % stackAlignmentBytes());
>     
>     ScratchBuffer* scratchBuffer = vm->scratchBufferForSize(requiredScratchMemorySizeInBytes());
>     char* buffer = static_cast<char*>(scratchBuffer->dataBuffer());
>@@ -61,7 +70,7 @@ MacroAssemblerCodeRef osrExitGenerationT
>     jit.storePtr(MacroAssembler::TrustedImmPtr(requiredScratchMemorySizeInBytes()), GPRInfo::nonArgGPR1);
> 
>     jit.loadPtr(GPRInfo::callFrameRegister, GPRInfo::argumentGPR0);
>-    jit.peek(GPRInfo::argumentGPR1, 3);
>+    jit.peek(GPRInfo::argumentGPR1, (stackMisalignment / sizeof(void*)) - 1);
>     MacroAssembler::Call functionCall = jit.call();
>     
>     // At this point we want to make a tail call to what was returned to us in the
>@@ -76,9 +85,9 @@ MacroAssemblerCodeRef osrExitGenerationT
>     jit.storePtr(MacroAssembler::TrustedImmPtr(0), GPRInfo::regT1);
>     
>     // Prepare for tail call.
>-    jit.pop(GPRInfo::regT1);
>-    jit.pop(GPRInfo::regT1);
>-    jit.pop(MacroAssembler::framePointerRegister);
>+    while (numberOfRequiredPops--)
>+        jit.popToRestore(GPRInfo::regT1);
>+    jit.popToRestore(MacroAssembler::framePointerRegister);
>     
>     // At this point we're sitting on the return address - so if we did a jump right now, the
>     // tail-callee would be happy. Instead we'll stash the callee in the return address and then

Flags:

mhahnenberg: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 128890: 224326 | 224393