Diff
Modified: trunk/JSTests/ChangeLog (231194 => 231195)
--- trunk/JSTests/ChangeLog 2018-05-01 08:47:56 UTC (rev 231194)
+++ trunk/JSTests/ChangeLog 2018-05-01 15:42:11 UTC (rev 231195)
@@ -1,3 +1,20 @@
+2018-05-01 Dominik Infuehr <[email protected]>
+
+ Add SetCallee as DFG-Operation
+ https://bugs.webkit.org/show_bug.cgi?id=184582
+
+ Reviewed by Filip Pizlo.
+
+ Added test that runs into infinite loop without updating the callee and
+ therefore emitting SetCallee in DFG for recursive tail calls.
+
+ * stress/closure-recursive-tail-call-infinite-loop.js: Added.
+ (Foo):
+ (second):
+ (first):
+ (return.closure):
+ (createClosure):
+
2018-04-30 Saam Barati <[email protected]>
ToString constant folds without preserving checks, causing us to break assumptions that the code would OSR exit
Added: trunk/JSTests/stress/closure-recursive-tail-call-infinite-loop.js (0 => 231195)
--- trunk/JSTests/stress/closure-recursive-tail-call-infinite-loop.js (rev 0)
+++ trunk/JSTests/stress/closure-recursive-tail-call-infinite-loop.js 2018-05-01 15:42:11 UTC (rev 231195)
@@ -0,0 +1,29 @@
+"use strict";
+
+function Foo() {}
+
+function second(next, cp) {
+ return 100;
+}
+
+function first(next, cp) {
+ return cp < 60 ? new Foo() : next(cp);
+}
+
+function createClosure(next, strategy) {
+ return function closure(cp) {
+ return strategy(next, cp);
+ };
+}
+
+var tmp = createClosure(null, second);
+var bar = createClosure(tmp, first);
+
+noInline(bar);
+
+for (var i=0; i<50000; i++) {
+ bar(32);
+ bar(32);
+ bar(32);
+ bar(100);
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (231194 => 231195)
--- trunk/Source/_javascript_Core/ChangeLog 2018-05-01 08:47:56 UTC (rev 231194)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-05-01 15:42:11 UTC (rev 231195)
@@ -1,3 +1,44 @@
+2018-05-01 Dominik Infuehr <[email protected]>
+
+ Add SetCallee as DFG-Operation
+ https://bugs.webkit.org/show_bug.cgi?id=184582
+
+ Reviewed by Filip Pizlo.
+
+ For recursive tail calls not only the argument count can change but also the
+ callee. Add SetCallee to DFG that sets the callee slot in the current call frame.
+ Also update the callee when optimizing a recursive tail call.
+ Enable recursive tail call optimization also for closures.
+
+ * dfg/DFGAbstractInterpreterInlines.h:
+ (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::handleRecursiveTailCall):
+ (JSC::DFG::ByteCodeParser::handleCallVariant):
+ * dfg/DFGClobberize.h:
+ (JSC::DFG::clobberize):
+ * dfg/DFGDoesGC.cpp:
+ (JSC::DFG::doesGC):
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::fixupNode):
+ * dfg/DFGMayExit.cpp:
+ * dfg/DFGNodeType.h:
+ * dfg/DFGPredictionPropagationPhase.cpp:
+ * dfg/DFGSafeToExecute.h:
+ (JSC::DFG::safeToExecute):
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::compileSetCallee):
+ * dfg/DFGSpeculativeJIT.h:
+ * dfg/DFGSpeculativeJIT32_64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * dfg/DFGSpeculativeJIT64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * ftl/FTLCapabilities.cpp:
+ (JSC::FTL::canCompile):
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::compileNode):
+ (JSC::FTL::DFG::LowerDFGToB3::compileSetCallee):
+
2018-05-01 Oleksandr Skachkov <[email protected]>
WebAssembly: add support for stream APIs - _javascript_ API
Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (231194 => 231195)
--- trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2018-05-01 08:47:56 UTC (rev 231194)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2018-05-01 15:42:11 UTC (rev 231195)
@@ -2448,6 +2448,7 @@
forNode(node).setType(SpecInt32Only);
break;
+ case SetCallee:
case SetArgumentCountIncludingThis:
break;
Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (231194 => 231195)
--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2018-05-01 08:47:56 UTC (rev 231194)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2018-05-01 15:42:11 UTC (rev 231195)
@@ -156,7 +156,7 @@
void emitArgumentPhantoms(int registerOffset, int argumentCountIncludingThis);
Node* getArgumentCount();
template<typename ChecksFunctor>
- bool handleRecursiveTailCall(CallVariant, int registerOffset, int argumentCountIncludingThis, const ChecksFunctor& emitFunctionCheckIfNeeded);
+ bool handleRecursiveTailCall(Node* callTargetNode, CallVariant, int registerOffset, int argumentCountIncludingThis, const ChecksFunctor& emitFunctionCheckIfNeeded);
unsigned inliningCost(CallVariant, int argumentCountIncludingThis, InlineCallFrame::Kind); // Return UINT_MAX if it's not an inlining candidate. By convention, intrinsics have a cost of 1.
// Handle inlining. Return true if it succeeded, false if we need to plant a call.
bool handleVarargsInlining(Node* callTargetNode, int resultOperand, const CallLinkStatus&, int registerOffset, VirtualRegister thisArgument, VirtualRegister argumentsArgument, unsigned argumentsOffset, NodeType callOp, InlineCallFrame::Kind);
@@ -1356,16 +1356,11 @@
}
template<typename ChecksFunctor>
-bool ByteCodeParser::handleRecursiveTailCall(CallVariant callVariant, int registerOffset, int argumentCountIncludingThis, const ChecksFunctor& emitFunctionCheckIfNeeded)
+bool ByteCodeParser::handleRecursiveTailCall(Node* callTargetNode, CallVariant callVariant, int registerOffset, int argumentCountIncludingThis, const ChecksFunctor& emitFunctionCheckIfNeeded)
{
if (UNLIKELY(!Options::optimizeRecursiveTailCalls()))
return false;
- // Currently we cannot do this optimisation for closures. The problem is that "emitFunctionChecks" which we use later is too coarse, only checking the executable
- // and not the value of captured variables.
- if (callVariant.isClosureCall())
- return false;
-
auto targetExecutable = callVariant.executable();
InlineStackEntry* stackEntry = m_inlineStackTop;
do {
@@ -1385,11 +1380,25 @@
return false;
}
+ // If an InlineCallFrame is not a closure, it was optimized using a constant callee.
+ // Check if this is the same callee that we try to inline here.
+ if (stackEntry->m_inlineCallFrame && !stackEntry->m_inlineCallFrame->isClosureCall) {
+ if (stackEntry->m_inlineCallFrame->calleeConstant() != callVariant.function())
+ continue;
+ }
+
// We must add some check that the profiling information was correct and the target of this call is what we thought.
emitFunctionCheckIfNeeded();
// We flush everything, as if we were in the backedge of a loop (see treatment of op_jmp in parseBlock).
flushForTerminal();
+ // We must set the callee to the right value
+ if (stackEntry->m_inlineCallFrame) {
+ if (stackEntry->m_inlineCallFrame->isClosureCall)
+ setDirect(stackEntry->remapOperand(VirtualRegister(CallFrameSlot::callee)), callTargetNode, NormalSet);
+ } else
+ addToGraph(SetCallee, callTargetNode);
+
// We must set the arguments to the right values
if (!stackEntry->m_inlineCallFrame)
addToGraph(SetArgumentCountIncludingThis, OpInfo(argumentCountIncludingThis));
@@ -1683,7 +1692,7 @@
didInsertChecks = true;
};
- if (kind == InlineCallFrame::TailCall && ByteCodeParser::handleRecursiveTailCall(callee, registerOffset, argumentCountIncludingThis, insertChecksWithAccounting)) {
+ if (kind == InlineCallFrame::TailCall && ByteCodeParser::handleRecursiveTailCall(callTargetNode, callee, registerOffset, argumentCountIncludingThis, insertChecksWithAccounting)) {
RELEASE_ASSERT(didInsertChecks);
return CallOptimizationResult::OptimizedToJump;
}
Modified: trunk/Source/_javascript_Core/dfg/DFGClobberize.h (231194 => 231195)
--- trunk/Source/_javascript_Core/dfg/DFGClobberize.h 2018-05-01 08:47:56 UTC (rev 231194)
+++ trunk/Source/_javascript_Core/dfg/DFGClobberize.h 2018-05-01 15:42:11 UTC (rev 231195)
@@ -695,6 +695,10 @@
read(AbstractHeap(Stack, CallFrameSlot::callee));
def(HeapLocation(StackLoc, AbstractHeap(Stack, CallFrameSlot::callee)), LazyNode(node));
return;
+
+ case SetCallee:
+ write(AbstractHeap(Stack, CallFrameSlot::callee));
+ return;
case GetArgumentCountIncludingThis: {
auto heap = AbstractHeap(Stack, remapOperand(node->argumentsInlineCallFrame(), VirtualRegister(CallFrameSlot::argumentCount)));
Modified: trunk/Source/_javascript_Core/dfg/DFGDoesGC.cpp (231194 => 231195)
--- trunk/Source/_javascript_Core/dfg/DFGDoesGC.cpp 2018-05-01 08:47:56 UTC (rev 231194)
+++ trunk/Source/_javascript_Core/dfg/DFGDoesGC.cpp 2018-05-01 15:42:11 UTC (rev 231195)
@@ -51,6 +51,7 @@
case Identity:
case IdentityWithProfile:
case GetCallee:
+ case SetCallee:
case GetArgumentCountIncludingThis:
case SetArgumentCountIncludingThis:
case GetRestLength:
Modified: trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (231194 => 231195)
--- trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2018-05-01 08:47:56 UTC (rev 231194)
+++ trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2018-05-01 15:42:11 UTC (rev 231195)
@@ -2134,6 +2134,10 @@
}
break;
+ case SetCallee:
+ fixEdge<CellUse>(node->child1());
+ break;
+
#if !ASSERT_DISABLED
// Have these no-op cases here to ensure that nobody forgets to add handlers for new opcodes.
case SetArgument:
Modified: trunk/Source/_javascript_Core/dfg/DFGMayExit.cpp (231194 => 231195)
--- trunk/Source/_javascript_Core/dfg/DFGMayExit.cpp 2018-05-01 08:47:56 UTC (rev 231194)
+++ trunk/Source/_javascript_Core/dfg/DFGMayExit.cpp 2018-05-01 15:42:11 UTC (rev 231195)
@@ -74,6 +74,7 @@
case KillStack:
case GetStack:
case GetCallee:
+ case SetCallee:
case GetArgumentCountIncludingThis:
case SetArgumentCountIncludingThis:
case GetRestLength:
Modified: trunk/Source/_javascript_Core/dfg/DFGNodeType.h (231194 => 231195)
--- trunk/Source/_javascript_Core/dfg/DFGNodeType.h 2018-05-01 08:47:56 UTC (rev 231194)
+++ trunk/Source/_javascript_Core/dfg/DFGNodeType.h 2018-05-01 15:42:11 UTC (rev 231195)
@@ -54,6 +54,7 @@
macro(ToThis, NodeResultJS) \
macro(CreateThis, NodeResultJS) /* Note this is not MustGenerate since we're returning it anyway. */ \
macro(GetCallee, NodeResultJS) \
+ macro(SetCallee, NodeMustGenerate) \
macro(GetArgumentCountIncludingThis, NodeResultInt32) \
macro(SetArgumentCountIncludingThis, NodeMustGenerate) \
\
Modified: trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp (231194 => 231195)
--- trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp 2018-05-01 08:47:56 UTC (rev 231194)
+++ trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp 2018-05-01 15:42:11 UTC (rev 231195)
@@ -776,6 +776,7 @@
break;
}
+ case SetCallee:
case SetArgumentCountIncludingThis:
break;
Modified: trunk/Source/_javascript_Core/dfg/DFGSafeToExecute.h (231194 => 231195)
--- trunk/Source/_javascript_Core/dfg/DFGSafeToExecute.h 2018-05-01 08:47:56 UTC (rev 231194)
+++ trunk/Source/_javascript_Core/dfg/DFGSafeToExecute.h 2018-05-01 15:42:11 UTC (rev 231195)
@@ -172,6 +172,7 @@
case ToThis:
case CreateThis:
case GetCallee:
+ case SetCallee:
case GetArgumentCountIncludingThis:
case SetArgumentCountIncludingThis:
case GetRestLength:
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (231194 => 231195)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2018-05-01 08:47:56 UTC (rev 231194)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2018-05-01 15:42:11 UTC (rev 231195)
@@ -11866,6 +11866,13 @@
cellResult(result.gpr(), node);
}
+void SpeculativeJIT::compileSetCallee(Node* node)
+{
+ SpeculateCellOperand callee(this, node->child1());
+ m_jit.storeCell(callee.gpr(), JITCompiler::payloadFor(CallFrameSlot::callee));
+ noResult(node);
+}
+
void SpeculativeJIT::compileGetArgumentCountIncludingThis(Node* node)
{
GPRTemporary result(this);
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h (231194 => 231195)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2018-05-01 08:47:56 UTC (rev 231194)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2018-05-01 15:42:11 UTC (rev 231195)
@@ -1459,6 +1459,7 @@
void compileGetGetter(Node*);
void compileGetSetter(Node*);
void compileGetCallee(Node*);
+ void compileSetCallee(Node*);
void compileGetArgumentCountIncludingThis(Node*);
void compileSetArgumentCountIncludingThis(Node*);
void compileStrCat(Node*);
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (231194 => 231195)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2018-05-01 08:47:56 UTC (rev 231194)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2018-05-01 15:42:11 UTC (rev 231195)
@@ -3255,6 +3255,11 @@
compileGetCallee(node);
break;
}
+
+ case SetCallee: {
+ compileSetCallee(node);
+ break;
+ }
case GetArgumentCountIncludingThis: {
compileGetArgumentCountIncludingThis(node);
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (231194 => 231195)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2018-05-01 08:47:56 UTC (rev 231194)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2018-05-01 15:42:11 UTC (rev 231195)
@@ -3476,6 +3476,11 @@
compileGetCallee(node);
break;
}
+
+ case SetCallee: {
+ compileSetCallee(node);
+ break;
+ }
case GetArgumentCountIncludingThis: {
compileGetArgumentCountIncludingThis(node);
Modified: trunk/Source/_javascript_Core/ftl/FTLCapabilities.cpp (231194 => 231195)
--- trunk/Source/_javascript_Core/ftl/FTLCapabilities.cpp 2018-05-01 08:47:56 UTC (rev 231194)
+++ trunk/Source/_javascript_Core/ftl/FTLCapabilities.cpp 2018-05-01 15:42:11 UTC (rev 231195)
@@ -182,6 +182,7 @@
case GetExecutable:
case GetScope:
case GetCallee:
+ case SetCallee:
case GetArgumentCountIncludingThis:
case SetArgumentCountIncludingThis:
case ToNumber:
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (231194 => 231195)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2018-05-01 08:47:56 UTC (rev 231194)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2018-05-01 15:42:11 UTC (rev 231195)
@@ -924,6 +924,9 @@
case GetCallee:
compileGetCallee();
break;
+ case SetCallee:
+ compileSetCallee();
+ break;
case GetArgumentCountIncludingThis:
compileGetArgumentCountIncludingThis();
break;
@@ -6626,6 +6629,12 @@
{
setJSValue(m_out.loadPtr(addressFor(CallFrameSlot::callee)));
}
+
+ void compileSetCallee()
+ {
+ auto callee = lowCell(m_node->child1());
+ m_out.storePtr(callee, payloadFor(CallFrameSlot::callee));
+ }
void compileGetArgumentCountIncludingThis()
{
