Fix handling of indirect stackmap locations in FTL OSR exit
https://bugs.webkit.org/show_bug.cgi?id=122666
Reviewed by Mark Hahnenberg.
With this change, the llvm.webkit.stackmap-based OSR exit only fails one test, down from
five tests previously.
* ftl/FTLLocation.cpp:
(JSC::FTL::Location::gpr): It's OK to call this method when kind() == Indirect, so asserting that isGPR() is wrong; change to assert that involvesGPR().
(JSC::FTL::Location::restoreInto): Stack-related registers aren't saved to the scratch buffer, so use them directly.
* ftl/FTLLocation.h: Add comment about requirements for stack layout.
* ftl/FTLOSRExitCompiler.cpp:
(JSC::FTL::compileStubWithOSRExitStackmap): Make enough room on the stack so that saveAllRegisters() has a scratchpad to save things to. Without this, saveAllRegisters() may clobber a spilled value.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@157326 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index c47234a..c5c7d06 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,20 @@
+2013-10-11 Filip Pizlo <fpizlo@apple.com>
+
+ Fix handling of indirect stackmap locations in FTL OSR exit
+ https://bugs.webkit.org/show_bug.cgi?id=122666
+
+ Reviewed by Mark Hahnenberg.
+
+ With this change, the llvm.webkit.stackmap-based OSR exit only fails one test, down from
+ five tests previously.
+
+ * ftl/FTLLocation.cpp:
+ (JSC::FTL::Location::gpr): It's OK to call this method when kind() == Indirect, so asserting that isGPR() is wrong; change to assert that involvesGPR().
+ (JSC::FTL::Location::restoreInto): Stack-related registers aren't saved to the scratch buffer, so use them directly.
+ * ftl/FTLLocation.h: Add comment about requirements for stack layout.
+ * ftl/FTLOSRExitCompiler.cpp:
+ (JSC::FTL::compileStubWithOSRExitStackmap): Make enough room on the stack so that saveAllRegisters() has a scratchpad to save things to. Without this, saveAllRegisters() may clobber a spilled value.
+
2013-10-11 Commit Queue <commit-queue@webkit.org>
Unreviewed, rolling out r157307.
diff --git a/Source/JavaScriptCore/ftl/FTLLocation.cpp b/Source/JavaScriptCore/ftl/FTLLocation.cpp
index fbabf57..cd1084e 100644
--- a/Source/JavaScriptCore/ftl/FTLLocation.cpp
+++ b/Source/JavaScriptCore/ftl/FTLLocation.cpp
@@ -87,7 +87,7 @@
// for example, the architecture encodes CX as 1 and DX as 2 while Dwarf does the
// opposite. Hence we need the switch.
- ASSERT(isGPR());
+ ASSERT(involvesGPR());
switch (dwarfRegNum()) {
case 0:
@@ -127,6 +127,12 @@
void Location::restoreInto(MacroAssembler& jit, char* savedRegisters, GPRReg result) const
{
if (isGPR()) {
+ if (MacroAssembler::isStackRelated(gpr())) {
+ // These don't get saved.
+ jit.move(gpr(), result);
+ return;
+ }
+
jit.load64(savedRegisters + offsetOfGPR(gpr()), result);
return;
}
@@ -143,6 +149,12 @@
return;
case Indirect:
+ if (MacroAssembler::isStackRelated(gpr())) {
+ // These don't get saved.
+ jit.load64(MacroAssembler::Address(gpr(), offset()), result);
+ return;
+ }
+
jit.load64(savedRegisters + offsetOfGPR(gpr()), result);
jit.load64(MacroAssembler::Address(result, offset()), result);
return;
diff --git a/Source/JavaScriptCore/ftl/FTLLocation.h b/Source/JavaScriptCore/ftl/FTLLocation.h
index 74fcf16..2ec22f3 100644
--- a/Source/JavaScriptCore/ftl/FTLLocation.h
+++ b/Source/JavaScriptCore/ftl/FTLLocation.h
@@ -155,7 +155,11 @@
// Assuming that all registers are saved to the savedRegisters buffer according
// to FTLSaveRestore convention, this loads the value into the given register.
- // The code that this generates isn't exactly super fast.
+ // The code that this generates isn't exactly super fast. This assumes that FP
+ // and SP contain the same values that they would have contained in the original
+ // frame. If we did push things onto the stack then probably we'll have to change
+ // the signature of this method to take a stack offset for stack-relative
+ // indirects.
void restoreInto(MacroAssembler&, char* savedRegisters, GPRReg result) const;
private:
diff --git a/Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp b/Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp
index c596832..a3a7c3b 100644
--- a/Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp
+++ b/Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp
@@ -67,10 +67,17 @@
EncodedJSValue* scratch = scratchBuffer ? static_cast<EncodedJSValue*>(scratchBuffer->dataBuffer()) : 0;
char* registerScratch = bitwise_cast<char*>(scratch + exit.m_values.size());
+ // 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);
saveAllRegisters(jit, registerScratch);
// Bring the stack back into a sane form.
jit.pop(GPRInfo::regT0);
+ jit.pop(GPRInfo::regT0);
+
+ // The remaining code assumes that SP/FP are in the same state that they were in the FTL's
+ // call frame.
// Get the call frame and tag thingies.
record->locations[0].restoreInto(jit, jitCode->stackmaps, registerScratch, GPRInfo::callFrameRegister);
