- Revision
- 194368
- Author
- [email protected]
- Date
- 2015-12-22 12:03:13 -0800 (Tue, 22 Dec 2015)
Log Message
FTL B3 should do tail calls
https://bugs.webkit.org/show_bug.cgi?id=152494
Reviewed by Michael Saboff.
OMG this was so easy.
The only shady part is that I broke a layering rule that we had so far been following: B3 was
sitting below the JSC runtime, and did not use JS-specific types. No more, since B3::ValueRep
can now turn itself into a ValueRecovery for a JSValue. This small feature makes a huge
difference for the readability of tail call code: it makes it plain that the call frame
shuffler is basically just directly consuming the stackmap generation params, and insofar as
there is any data transformation, it's just because it uses different classes to say the same
thing.
I think we should avoid adding too many JS-specific things to B3. But, so long as it's still
possible to use B3 to compile things that aren't JS, I think we'll be fine.
* b3/B3ValueRep.cpp:
(JSC::B3::ValueRep::dump):
(JSC::B3::ValueRep::emitRestore):
(JSC::B3::ValueRep::recoveryForJSValue):
* b3/B3ValueRep.h:
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::DFG::LowerDFGToLLVM::compileTailCall):
(JSC::FTL::DFG::LowerDFGToLLVM::compileCallOrConstructVarargs):
* test/stress/ftl-tail-call.js: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (194367 => 194368)
--- trunk/Source/_javascript_Core/ChangeLog 2015-12-22 19:37:50 UTC (rev 194367)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-12-22 20:03:13 UTC (rev 194368)
@@ -1,3 +1,33 @@
+2015-12-22 Filip Pizlo <[email protected]>
+
+ FTL B3 should do tail calls
+ https://bugs.webkit.org/show_bug.cgi?id=152494
+
+ Reviewed by Michael Saboff.
+
+ OMG this was so easy.
+
+ The only shady part is that I broke a layering rule that we had so far been following: B3 was
+ sitting below the JSC runtime, and did not use JS-specific types. No more, since B3::ValueRep
+ can now turn itself into a ValueRecovery for a JSValue. This small feature makes a huge
+ difference for the readability of tail call code: it makes it plain that the call frame
+ shuffler is basically just directly consuming the stackmap generation params, and insofar as
+ there is any data transformation, it's just because it uses different classes to say the same
+ thing.
+
+ I think we should avoid adding too many JS-specific things to B3. But, so long as it's still
+ possible to use B3 to compile things that aren't JS, I think we'll be fine.
+
+ * b3/B3ValueRep.cpp:
+ (JSC::B3::ValueRep::dump):
+ (JSC::B3::ValueRep::emitRestore):
+ (JSC::B3::ValueRep::recoveryForJSValue):
+ * b3/B3ValueRep.h:
+ * ftl/FTLLowerDFGToLLVM.cpp:
+ (JSC::FTL::DFG::LowerDFGToLLVM::compileTailCall):
+ (JSC::FTL::DFG::LowerDFGToLLVM::compileCallOrConstructVarargs):
+ * test/stress/ftl-tail-call.js: Added.
+
2015-12-21 Mark Lam <[email protected]>
Snippefy op_negate for the baseline JIT.
Modified: trunk/Source/_javascript_Core/b3/B3ValueRep.cpp (194367 => 194368)
--- trunk/Source/_javascript_Core/b3/B3ValueRep.cpp 2015-12-22 19:37:50 UTC (rev 194367)
+++ trunk/Source/_javascript_Core/b3/B3ValueRep.cpp 2015-12-22 20:03:13 UTC (rev 194368)
@@ -29,6 +29,7 @@
#if ENABLE(B3_JIT)
#include "AssemblyHelpers.h"
+#include "JSCInlines.h"
namespace JSC { namespace B3 {
@@ -57,7 +58,7 @@
RELEASE_ASSERT_NOT_REACHED();
}
-void ValueRep::emitRestore(AssemblyHelpers& jit, Reg reg)
+void ValueRep::emitRestore(AssemblyHelpers& jit, Reg reg) const
{
if (reg.isGPR()) {
switch (kind()) {
@@ -100,6 +101,24 @@
}
}
+ValueRecovery ValueRep::recoveryForJSValue() const
+{
+ switch (kind()) {
+ case Register:
+ return ValueRecovery::inGPR(gpr(), DataFormatJS);
+ case Stack:
+ RELEASE_ASSERT(!(offsetFromFP() % sizeof(EncodedJSValue)));
+ return ValueRecovery::displacedInJSStack(
+ VirtualRegister(offsetFromFP() / sizeof(EncodedJSValue)),
+ DataFormatJS);
+ case Constant:
+ return ValueRecovery::constant(JSValue::decode(value()));
+ default:
+ RELEASE_ASSERT_NOT_REACHED();
+ return { };
+ }
+}
+
} } // namespace JSC::B3
namespace WTF {
Modified: trunk/Source/_javascript_Core/b3/B3ValueRep.h (194367 => 194368)
--- trunk/Source/_javascript_Core/b3/B3ValueRep.h 2015-12-22 19:37:50 UTC (rev 194367)
+++ trunk/Source/_javascript_Core/b3/B3ValueRep.h 2015-12-22 20:03:13 UTC (rev 194368)
@@ -32,6 +32,7 @@
#include "GPRInfo.h"
#include "JSCJSValue.h"
#include "Reg.h"
+#include "ValueRecovery.h"
#include <wtf/PrintStream.h>
namespace JSC {
@@ -222,8 +223,14 @@
// This has a simple contract: it emits code to restore the value into the given register. This
// will work even if it requires moving between bits a GPR and a FPR.
- void emitRestore(AssemblyHelpers&, Reg);
+ void emitRestore(AssemblyHelpers&, Reg) const;
+ // Computes the ValueRecovery assuming that the Value* was for a JSValue (i.e. Int64).
+ // NOTE: We should avoid putting JSValue-related methods in B3, but this was hard to avoid
+ // because some parts of JSC use ValueRecovery like a general "where my bits at" object, almost
+ // exactly like ValueRep.
+ ValueRecovery recoveryForJSValue() const;
+
private:
Kind m_kind;
union U {
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp (194367 => 194368)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp 2015-12-22 19:37:50 UTC (rev 194367)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp 2015-12-22 20:03:13 UTC (rev 194368)
@@ -31,6 +31,7 @@
#include "AirGenerationContext.h"
#include "AllowMacroScratchRegisterUsage.h"
#include "B3StackmapGenerationParams.h"
+#include "CallFrameShuffler.h"
#include "CodeBlockWithJITType.h"
#include "DFGAbstractInterpreterInlines.h"
#include "DFGDominators.h"
@@ -5037,29 +5038,112 @@
void compileTailCall()
{
+ Node* node = m_node;
+ unsigned numArgs = node->numChildren() - 1;
+
+ LValue jsCallee = lowJSValue(m_graph.varArgChild(node, 0));
+
#if FTL_USES_B3
- if (verboseCompilationEnabled() || !verboseCompilationEnabled())
- CRASH();
+ // We want B3 to give us all of the arguments using whatever mechanism it thinks is
+ // convenient. The generator then shuffles those arguments into our own call frame,
+ // destroying our frame in the process.
+
+ Vector<ConstrainedValue> arguments;
+
+ arguments.append(ConstrainedValue(jsCallee, ValueRep::reg(GPRInfo::regT0)));
+
+ for (unsigned i = 0; i < numArgs; ++i) {
+ // Note: we could let the shuffler do boxing for us, but it's not super clear that this
+ // would be better. Also, if we wanted to do that, then we'd have to teach the shuffler
+ // that 32-bit values could land at 4-byte alignment but not 8-byte alignment.
+
+ ConstrainedValue constrainedValue(
+ lowJSValue(m_graph.varArgChild(node, 1 + i)),
+ ValueRep::WarmAny);
+ arguments.append(constrainedValue);
+ }
+
+ PatchpointValue* patchpoint = m_out.patchpoint(Void);
+ patchpoint->appendVector(arguments);
+
+ // Prevent any of the arguments from using the scratch register.
+ patchpoint->clobberEarly(RegisterSet::macroScratchRegisters());
+
+ // We don't have to tell the patchpoint that we will clobber registers, since we won't return
+ // anyway.
+
+ CodeOrigin codeOrigin = codeOriginDescriptionOfCallSite();
+ patchpoint->setGenerator(
+ [=] (CCallHelpers& jit, const StackmapGenerationParams& params) {
+ AllowMacroScratchRegisterUsage allowScratch(jit);
+
+ // FIXME: There may be some exception things that need to happen here.
+ // https://bugs.webkit.org/show_bug.cgi?id=151686
+
+ CallFrameShuffleData shuffleData;
+ shuffleData.callee = ValueRecovery::inGPR(GPRInfo::regT0, DataFormatJS);
+
+ for (unsigned i = 0; i < numArgs; ++i)
+ shuffleData.args.append(params[1 + i].recoveryForJSValue());
+
+ shuffleData.setupCalleeSaveRegisters(jit.codeBlock());
+
+ CallLinkInfo* callLinkInfo = jit.codeBlock()->addCallLinkInfo();
+
+ CCallHelpers::DataLabelPtr targetToCheck;
+ CCallHelpers::Jump slowPath = jit.branchPtrWithPatch(
+ CCallHelpers::NotEqual, GPRInfo::regT0, targetToCheck,
+ CCallHelpers::TrustedImmPtr(0));
+
+ callLinkInfo->setFrameShuffleData(shuffleData);
+ CallFrameShuffler(jit, shuffleData).prepareForTailCall();
+
+ CCallHelpers::Call fastCall = jit.nearTailCall();
+
+ slowPath.link(&jit);
+
+ CallFrameShuffler slowPathShuffler(jit, shuffleData);
+ slowPathShuffler.setCalleeJSValueRegs(JSValueRegs(GPRInfo::regT0));
+ slowPathShuffler.prepareForSlowPath();
+
+ jit.move(CCallHelpers::TrustedImmPtr(callLinkInfo), GPRInfo::regT2);
+ CCallHelpers::Call slowCall = jit.nearCall();
+
+ jit.abortWithReason(JITDidReturnFromTailCall);
+
+ callLinkInfo->setUpCall(CallLinkInfo::TailCall, codeOrigin, GPRInfo::regT0);
+
+ jit.addLinkTask(
+ [=] (LinkBuffer& linkBuffer) {
+ MacroAssemblerCodePtr linkCall =
+ linkBuffer.vm().getCTIStub(linkCallThunkGenerator).code();
+ linkBuffer.link(slowCall, FunctionPtr(linkCall.executableAddress()));
+
+ callLinkInfo->setCallLocations(
+ linkBuffer.locationOfNearCall(slowCall),
+ linkBuffer.locationOf(targetToCheck),
+ linkBuffer.locationOfNearCall(fastCall));
+ });
+ });
#else
- int numArgs = m_node->numChildren() - 1;
StackmapArgumentList exitArguments;
exitArguments.reserveCapacity(numArgs + 6);
unsigned stackmapID = m_stackmapIDs++;
- exitArguments.append(lowJSValue(m_graph.varArgChild(m_node, 0)));
+ exitArguments.append(jsCallee);
exitArguments.append(m_tagTypeNumber);
Vector<ExitValue> callArguments(numArgs);
bool needsTagTypeNumber { false };
- for (int i = 0; i < numArgs; ++i) {
+ for (unsigned i = 0; i < numArgs; ++i) {
callArguments[i] =
- exitValueForTailCall(exitArguments, m_graph.varArgChild(m_node, 1 + i).node());
+ exitValueForTailCall(exitArguments, m_graph.varArgChild(node, 1 + i).node());
if (callArguments[i].dataFormat() == DataFormatInt32)
needsTagTypeNumber = true;
}
- JSTailCall tailCall(stackmapID, m_node, WTF::move(callArguments));
+ JSTailCall tailCall(stackmapID, node, WTF::move(callArguments));
exitArguments.insert(0, m_out.constInt32(needsTagTypeNumber ? 2 : 1));
exitArguments.insert(0, constNull(m_out.ref8));
@@ -5068,10 +5152,11 @@
LValue call = m_out.call(m_out.voidType, m_out.patchpointVoidIntrinsic(), exitArguments);
setInstructionCallingConvention(call, LLVMAnyRegCallConv);
- m_out.unreachable();
m_ftlState.jsTailCalls.append(tailCall);
#endif
+
+ m_out.unreachable();
}
void compileCallOrConstructVarargs()
Added: trunk/Source/_javascript_Core/tests/stress/ftl-tail-call.js (0 => 194368)
--- trunk/Source/_javascript_Core/tests/stress/ftl-tail-call.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/ftl-tail-call.js 2015-12-22 20:03:13 UTC (rev 194368)
@@ -0,0 +1,20 @@
+"use strict";
+
+function foo(a, b, c) {
+ return a + b * 2 + c * 3;
+}
+
+noInline(foo);
+
+function bar(a, b, c) {
+ return foo(a.f, b.g, c.h);
+}
+
+noInline(bar);
+
+for (var i = 0; i < 10000; ++i) {
+ var result = bar({f: 4}, {g: 5}, {h: 6});
+ if (result != 4 + 5 * 2 + 6 * 3)
+ throw "Error: bad result: " + result;
+}
+
