FTL OSR exit shouldn't make X86-specific assumptions
https://bugs.webkit.org/show_bug.cgi?id=128890
Reviewed by Mark Hahnenberg.
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):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@164228 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 78af889..82f71e5 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -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 Mark Hahnenberg.
+
+ 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-17 Filip Pizlo <fpizlo@apple.com>
Unreviewed, make this test pass without DFG. It was assuming that you always have DFG
diff --git a/Source/JavaScriptCore/assembler/MacroAssembler.h b/Source/JavaScriptCore/assembler/MacroAssembler.h
index 57fc3a6..35b2ba7 100644
--- a/Source/JavaScriptCore/assembler/MacroAssembler.h
+++ b/Source/JavaScriptCore/assembler/MacroAssembler.h
@@ -258,6 +258,10 @@
{
push(src);
}
+ void pushToSaveImmediateWithoutTouchingRegisters(TrustedImm32 imm)
+ {
+ push(imm);
+ }
void popToRestore(RegisterID dest)
{
pop(dest);
@@ -272,6 +276,8 @@
loadDouble(stackPointerRegister, dest);
addPtr(TrustedImm32(sizeof(double)), stackPointerRegister);
}
+
+ static ptrdiff_t pushToSaveByteOffset() { return sizeof(void*); }
#endif // !CPU(ARM64)
#if CPU(X86_64) || CPU(ARM64)
diff --git a/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h b/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h
index 42857b4..fbc9461 100644
--- a/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h
+++ b/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h
@@ -1459,6 +1459,15 @@
{
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 @@
storeDouble(src, stackPointerRegister);
}
+ static ptrdiff_t pushToSaveByteOffset() { return 16; }
// Register move operations:
diff --git a/Source/JavaScriptCore/ftl/FTLExitThunkGenerator.cpp b/Source/JavaScriptCore/ftl/FTLExitThunkGenerator.cpp
index 86c8723..3d40bdf 100644
--- a/Source/JavaScriptCore/ftl/FTLExitThunkGenerator.cpp
+++ b/Source/JavaScriptCore/ftl/FTLExitThunkGenerator.cpp
@@ -51,7 +51,7 @@
OSRExitCompilationInfo& info = m_state.finalizer->osrExit[index];
info.m_thunkLabel = label();
- push(TrustedImm32(index));
+ pushToSaveImmediateWithoutTouchingRegisters(TrustedImm32(index));
info.m_thunkJump = patchableJump();
m_didThings = true;
diff --git a/Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp b/Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp
index f37a6f6..c1eb2f9 100644
--- a/Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp
+++ b/Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp
@@ -70,14 +70,14 @@
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;
diff --git a/Source/JavaScriptCore/ftl/FTLThunks.cpp b/Source/JavaScriptCore/ftl/FTLThunks.cpp
index e25407f..0c306c6 100644
--- a/Source/JavaScriptCore/ftl/FTLThunks.cpp
+++ b/Source/JavaScriptCore/ftl/FTLThunks.cpp
@@ -45,11 +45,20 @@
// 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 @@
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 @@
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
