Diff
Modified: branches/jsc-tailcall/Source/_javascript_Core/ChangeLog (187617 => 187618)
--- branches/jsc-tailcall/Source/_javascript_Core/ChangeLog 2015-07-30 23:17:45 UTC (rev 187617)
+++ branches/jsc-tailcall/Source/_javascript_Core/ChangeLog 2015-07-30 23:19:30 UTC (rev 187618)
@@ -1,5 +1,35 @@
2015-07-30 Basile Clement <basile_clem...@apple.com>
+ jsc-tailcall: Don't waste stack space when arity fixup was performed
+ https://bugs.webkit.org/show_bug.cgi?id=147447
+
+ Reviewed by Michael Saboff.
+
+ When doing a tail call, we overwrite an amount of stack space based on
+ the number of arguments in the call frame. If we entered the tail
+ caller by performing an arity fixup, this is incorrect and leads to
+ wasted stack space - we must use the CodeBlock's number of parameters
+ instead in that case.
+
+ This patch is also moving the prepareForTailCall() function from
+ jit/ThunkGenerators.h to the place where it should have always been,
+ namely jit/CCallHelpers.h
+
+ * jit/CCallHelpers.h:
+ (JSC::CCallHelpers::prepareForTailCallSlow):
+ * jit/JITCall.cpp:
+ (JSC::JIT::compileOpCall):
+ * jit/Repatch.cpp:
+ (JSC::linkPolymorphicCall):
+ * jit/ThunkGenerators.cpp:
+ (JSC::slowPathFor):
+ (JSC::virtualThunkFor):
+ * jit/ThunkGenerators.h:
+ * tests/stress/tail-call-no-stack-overflow.js:
+ (strictLoopArityFixup):
+
+2015-07-30 Basile Clement <basile_clem...@apple.com>
+
jsc-tailcall: We should consider a tail call as an exit in the LLInt for the purpose of switching to the JIT
https://bugs.webkit.org/show_bug.cgi?id=147449
Modified: branches/jsc-tailcall/Source/_javascript_Core/jit/CCallHelpers.h (187617 => 187618)
--- branches/jsc-tailcall/Source/_javascript_Core/jit/CCallHelpers.h 2015-07-30 23:17:45 UTC (rev 187617)
+++ branches/jsc-tailcall/Source/_javascript_Core/jit/CCallHelpers.h 2015-07-30 23:19:30 UTC (rev 187618)
@@ -30,6 +30,7 @@
#include "AssemblyHelpers.h"
#include "GPRInfo.h"
+#include "StackAlignment.h"
namespace JSC {
@@ -2020,6 +2021,70 @@
loadPtr(&vm()->targetMachinePCForThrow, GPRInfo::regT1);
jump(GPRInfo::regT1);
}
+
+ void prepareForTailCallSlow(const TempRegisterSet& usedRegisters = { RegisterSet::specialRegisters() })
+ {
+ GPRReg temp1 = usedRegisters.getFreeGPR(0);
+ GPRReg temp2 = usedRegisters.getFreeGPR(1);
+ ASSERT(temp2 != InvalidGPRReg);
+
+ subPtr(TrustedImm32(sizeof(CallerFrameAndPC)), stackPointerRegister);
+ loadPtr(Address(GPRInfo::callFrameRegister), temp1);
+ storePtr(temp1, Address(stackPointerRegister));
+ loadPtr(Address(GPRInfo::callFrameRegister, sizeof(void*)), temp1);
+ storePtr(temp1, Address(stackPointerRegister, sizeof(void*)));
+
+ // Now stackPointerRegister points to a valid call frame for the callee
+ // and callFrameRegister points to our own call frame.
+ // We now slide the callee's call frame over our own call frame,
+ // starting with the top to avoid unwanted overwrites
+
+ // Move the callFrameRegister to the top of our (trashed) call frame
+ load32(Address(GPRInfo::callFrameRegister, JSStack::ArgumentCount * static_cast<int>(sizeof(Register)) + PayloadOffset),
+ temp1);
+
+ // We need to use the number of parameters instead of the argument count in case of arity fixup
+ loadPtr(Address(GPRInfo::callFrameRegister, JSStack::CodeBlock * static_cast<int>(sizeof(Register))), temp2);
+ load32(Address(temp2, CodeBlock::offsetOfNumParameters()), temp2);
+ MacroAssembler::Jump argumentCountWasNotFixedUp = branch32(BelowOrEqual, temp2, temp1);
+ move(temp2, temp1);
+ argumentCountWasNotFixedUp.link(this);
+
+ add32(TrustedImm32(stackAlignmentRegisters() + JSStack::CallFrameHeaderSize - 1), temp1);
+ and32(TrustedImm32(-stackAlignmentRegisters()), temp1);
+ mul32(TrustedImm32(sizeof(Register)), temp1, temp1);
+ addPtr(temp1, GPRInfo::callFrameRegister);
+
+ // Compute the call frame size of the callee's call frame
+ load32(Address(stackPointerRegister, JSStack::ArgumentCount * static_cast<int>(sizeof(Register)) + PayloadOffset),
+ temp2);
+ add32(TrustedImm32(stackAlignmentRegisters() + JSStack::CallFrameHeaderSize - 1), temp2);
+ and32(TrustedImm32(-stackAlignmentRegisters()), temp2);
+
+#if USE(JSVALUE32_64)
+ COMPILE_ASSERT(sizeof(void*) * 2 == sizeof(Register), Register_is_two_pointers_sized);
+ lshift32(TrustedImm32(1), temp2);
+#endif
+
+ // Do the sliding
+ MacroAssembler::Label copyLoop(label());
+
+ subPtr(TrustedImm32(sizeof(void*)), GPRInfo::callFrameRegister);
+ sub32(TrustedImm32(1), temp2);
+ loadPtr(BaseIndex(stackPointerRegister, temp2, MacroAssembler::timesPtr()), temp1);
+ storePtr(temp1, Address(GPRInfo::callFrameRegister));
+
+ branchTest32(MacroAssembler::NonZero, temp2).linkTo(copyLoop, this);
+
+ emitFunctionEpilogue();
+ }
+
+ void prepareForTailCallSlow(GPRReg calleeGPR)
+ {
+ TempRegisterSet usedRegisters { RegisterSet::specialRegisters() };
+ usedRegisters.set(calleeGPR);
+ prepareForTailCallSlow(usedRegisters);
+ }
};
} // namespace JSC
Modified: branches/jsc-tailcall/Source/_javascript_Core/jit/JITCall.cpp (187617 => 187618)
--- branches/jsc-tailcall/Source/_javascript_Core/jit/JITCall.cpp 2015-07-30 23:17:45 UTC (rev 187617)
+++ branches/jsc-tailcall/Source/_javascript_Core/jit/JITCall.cpp 2015-07-30 23:19:30 UTC (rev 187618)
@@ -192,7 +192,7 @@
m_callCompilationInfo[callLinkInfoIndex].callLinkInfo = info;
if (opcodeID == op_tail_call || opcodeID == op_tail_call_varargs) {
- prepareForTailCall(*this);
+ prepareForTailCallSlow();
m_callCompilationInfo[callLinkInfoIndex].hotPathOther = emitNakedTailCall();
// We must never come back here
breakpoint();
Modified: branches/jsc-tailcall/Source/_javascript_Core/jit/JITCall32_64.cpp (187617 => 187618)
--- branches/jsc-tailcall/Source/_javascript_Core/jit/JITCall32_64.cpp 2015-07-30 23:17:45 UTC (rev 187617)
+++ branches/jsc-tailcall/Source/_javascript_Core/jit/JITCall32_64.cpp 2015-07-30 23:19:30 UTC (rev 187618)
@@ -276,7 +276,7 @@
checkStackPointerAlignment();
if (opcodeID == op_tail_call || opcodeID == op_tail_call_varargs) {
- prepareForTailCall(*this);
+ prepareForTailCallSlow();
m_callCompilationInfo[callLinkInfoIndex].hotPathOther = emitNakedTailCall();
// We must never come back here
breakpoint();
Modified: branches/jsc-tailcall/Source/_javascript_Core/jit/Repatch.cpp (187617 => 187618)
--- branches/jsc-tailcall/Source/_javascript_Core/jit/Repatch.cpp 2015-07-30 23:17:45 UTC (rev 187617)
+++ branches/jsc-tailcall/Source/_javascript_Core/jit/Repatch.cpp 2015-07-30 23:19:30 UTC (rev 187618)
@@ -1881,7 +1881,7 @@
CCallHelpers::Address(fastCountsBaseGPR, caseIndex * sizeof(uint32_t)));
}
if (CallLinkInfo::isTailCallType(callLinkInfo.callType())) {
- prepareForTailCall(stubJit);
+ stubJit.prepareForTailCallSlow();
calls[caseIndex].call = stubJit.nearTailCall();
} else
calls[caseIndex].call = stubJit.nearCall();
Modified: branches/jsc-tailcall/Source/_javascript_Core/jit/ThunkGenerators.cpp (187617 => 187618)
--- branches/jsc-tailcall/Source/_javascript_Core/jit/ThunkGenerators.cpp 2015-07-30 23:17:45 UTC (rev 187617)
+++ branches/jsc-tailcall/Source/_javascript_Core/jit/ThunkGenerators.cpp 2015-07-30 23:19:30 UTC (rev 187618)
@@ -102,9 +102,7 @@
CCallHelpers::Jump doNotTrash = jit.branchTestPtr(CCallHelpers::Zero, GPRInfo::returnValueGPR2);
jit.preserveReturnAddressAfterCall(GPRInfo::nonPreservedNonReturnGPR);
- jit.move(GPRInfo::returnValueGPR, GPRInfo::regT4); // FIXME
- prepareForTailCall(jit);
- jit.jump(GPRInfo::regT4);
+ jit.prepareForTailCallSlow(GPRInfo::returnValueGPR);
doNotTrash.link(&jit);
jit.jump(GPRInfo::returnValueGPR);
@@ -195,7 +193,7 @@
emitPointerValidation(jit, GPRInfo::regT4);
if (CallLinkInfo::isTailCallType(callLinkInfo.callType())) {
jit.preserveReturnAddressAfterCall(GPRInfo::regT0);
- prepareForTailCall(jit);
+ jit.prepareForTailCallSlow(GPRInfo::regT4);
}
jit.jump(GPRInfo::regT4);
@@ -461,51 +459,6 @@
return FINALIZE_CODE(patchBuffer, ("unreachable thunk"));
}
-void prepareForTailCall(CCallHelpers& jit)
-{
- jit.subPtr(CCallHelpers::TrustedImm32(sizeof(CallerFrameAndPC)), JSInterfaceJIT::stackPointerRegister);
- jit.loadPtr(CCallHelpers::Address(JSInterfaceJIT::callFrameRegister), JSInterfaceJIT::regT1);
- jit.storePtr(JSInterfaceJIT::regT1, CCallHelpers::Address(JSInterfaceJIT::stackPointerRegister));
- jit.loadPtr(CCallHelpers::Address(JSInterfaceJIT::callFrameRegister, sizeof(void*)), JSInterfaceJIT::regT1);
- jit.storePtr(JSInterfaceJIT::regT1, CCallHelpers::Address(JSInterfaceJIT::stackPointerRegister, sizeof(void*)));
-
- // Now stackPointerRegister points to a valid call frame for the callee
- // and callFrameRegister points to our own call frame.
- // We now slide the callee's call frame over our own call frame,
- // starting with the top to avoid unwanted overwrites
-
- // Move the callFrameRegister to the top of our (trashed) call frame
- jit.load32(CCallHelpers::Address(JSInterfaceJIT::callFrameRegister, JSStack::ArgumentCount * static_cast<int>(sizeof(Register)) + PayloadOffset),
- JSInterfaceJIT::regT1);
- jit.add32(CCallHelpers::TrustedImm32(stackAlignmentRegisters() + JSStack::CallFrameHeaderSize - 1), JSInterfaceJIT::regT1);
- jit.and32(CCallHelpers::TrustedImm32(-stackAlignmentRegisters()), JSInterfaceJIT::regT1);
- jit.mul32(CCallHelpers::TrustedImm32(sizeof(Register)), JSInterfaceJIT::regT1, JSInterfaceJIT::regT1);
- jit.addPtr(JSInterfaceJIT::regT1, JSInterfaceJIT::callFrameRegister);
-
- // Compute the call frame size of the callee's call frame
- jit.load32(CCallHelpers::Address(JSInterfaceJIT::stackPointerRegister, JSStack::ArgumentCount * static_cast<int>(sizeof(Register)) + PayloadOffset),
- JSInterfaceJIT::regT2);
- jit.add32(CCallHelpers::TrustedImm32(stackAlignmentRegisters() + JSStack::CallFrameHeaderSize - 1), JSInterfaceJIT::regT2);
- jit.and32(CCallHelpers::TrustedImm32(-stackAlignmentRegisters()), JSInterfaceJIT::regT2);
-
-#if USE(JSVALUE32_64)
- COMPILE_ASSERT(sizeof(void*) * 2 == sizeof(Register), Register_is_two_pointers_sized);
- jit.lshift32(CCallHelpers::TrustedImm32(1), JSInterfaceJIT::regT2);
-#endif
-
- // Do the sliding
- MacroAssembler::Label copyLoop(jit.label());
-
- jit.subPtr(CCallHelpers::TrustedImm32(sizeof(void*)), JSInterfaceJIT::callFrameRegister);
- jit.sub32(CCallHelpers::TrustedImm32(1), JSInterfaceJIT::regT2);
- jit.loadPtr(CCallHelpers::BaseIndex(JSInterfaceJIT::stackPointerRegister, JSInterfaceJIT::regT2, MacroAssembler::timesPtr()), JSInterfaceJIT::regT1);
- jit.storePtr(JSInterfaceJIT::regT1, CCallHelpers::Address(JSInterfaceJIT::callFrameRegister));
-
- jit.branchTest32(MacroAssembler::NonZero, JSInterfaceJIT::regT2).linkTo(copyLoop, &jit);
-
- jit.emitFunctionEpilogue();
-}
-
MacroAssemblerCodeRef baselineGetterReturnThunkGenerator(VM* vm)
{
JSInterfaceJIT jit(vm);
Modified: branches/jsc-tailcall/Source/_javascript_Core/jit/ThunkGenerators.h (187617 => 187618)
--- branches/jsc-tailcall/Source/_javascript_Core/jit/ThunkGenerators.h 2015-07-30 23:17:45 UTC (rev 187617)
+++ branches/jsc-tailcall/Source/_javascript_Core/jit/ThunkGenerators.h 2015-07-30 23:19:30 UTC (rev 187618)
@@ -49,8 +49,6 @@
MacroAssemblerCodeRef arityFixupGenerator(VM*);
MacroAssemblerCodeRef unreachableGenerator(VM*);
-void prepareForTailCall(CCallHelpers& jit);
-
MacroAssemblerCodeRef baselineGetterReturnThunkGenerator(VM* vm);
MacroAssemblerCodeRef baselineSetterReturnThunkGenerator(VM* vm);
Modified: branches/jsc-tailcall/Source/_javascript_Core/tests/stress/tail-call-no-stack-overflow.js (187617 => 187618)
--- branches/jsc-tailcall/Source/_javascript_Core/tests/stress/tail-call-no-stack-overflow.js 2015-07-30 23:17:45 UTC (rev 187617)
+++ branches/jsc-tailcall/Source/_javascript_Core/tests/stress/tail-call-no-stack-overflow.js 2015-07-30 23:19:30 UTC (rev 187618)
@@ -24,5 +24,14 @@
return strictLoop(n - 1);
}
+function strictLoopArityFixup(n, dummy) {
+ "use strict";
+ if (n > 0)
+ return strictLoopArityFixup(n - 1);
+}
+
shouldThrow(function () { sloppyLoop(100000); }, 'RangeError: Maximum call stack size exceeded.');
+
+// These should not throw
strictLoop(100000);
+strictLoopArityFixup(100000);
