+2017-10-25 Commit Queue <commit-queue@webkit.org>
+
+ Unreviewed, rolling out r223691 and r223729.
+ https://bugs.webkit.org/show_bug.cgi?id=178834
+
+ Broke Speedometer 2 React-Redux-TodoMVC test case (Requested
+ by rniwa on #webkit).
+
+ Reverted changesets:
+
+ "Turn recursive tail calls into loops"
+ https://bugs.webkit.org/show_bug.cgi?id=176601
+ https://trac.webkit.org/changeset/223691
+
+ "REGRESSION(r223691): DFGByteCodeParser.cpp:1483:83: warning:
+ comparison is always false due to limited range of data type
+ [-Wtype-limits]"
+ https://bugs.webkit.org/show_bug.cgi?id=178543
+ https://trac.webkit.org/changeset/223729
+
2017-10-25 Ryan Haddad <ryanhaddad@apple.com>
Mark test262.yaml/test262/test/language/statements/try/tco-catch.js as passing.
+++ /dev/null
-"use strict";
-
-// factorial calls aux that tail-calls back into factorial.
-// It is not OK to turn that tail call into a jump back to the top of the function, because the call to aux is not a tail call.
-function factorial(n) {
- function aux(n) {
- if (n == 1)
- return 1;
- return factorial(n - 1);
- }
-
- return n * aux(n);
-}
-
-// Same, but this time with an irrelevant tail call in factorial.
-// This exercises a different code path because the recursive tail call optimization aborts early if the op_enter block is not split, and it is split by the detection of a tail call.
-function factorial2(n) {
- function aux2(n) {
- if (n == 1)
- return 1;
- return factorial2(n - 1);
- }
- function id(n) {
- return n;
- }
-
- return id(n * aux2(n));
-}
-
-// This time, aux is tail-calling itself: the optimization is valid but must jump to the start of aux3, not of factorial3.
-function factorial3(n) {
- function aux3(n, acc) {
- if (n == 1)
- return acc;
- return aux3 (n-1, n*acc);
- }
-
- return n * aux3(n-1, 1);
-}
-
-// In this case, it is valid to jump all the way to the top of factorial4, because all calls are tail calls.
-function factorial4(n, acc) {
- function aux4(n, acc) {
- if (n == 1)
- return acc;
- return factorial4(n-1, n*acc);
- }
-
- if (acc)
- return aux4(n, acc);
- return aux4(n, 1);
-}
-
-// This function is used to test that recursing with a different number of arguments doesn't mess up with the stack.
-// The first two tail calls should be optimized, but not the last one (no place on the stack for the third argument).
-function foo(a, b) {
- if (a == 0)
- return 42;
- if (a == 1)
- return foo(a - 1);
- if (a == 2)
- return foo(b - 1, a);
- return foo (b - 1, a, 43);
-}
-
-// Same deal as foo, just with an inlining thrown into the mix.
-// Even the first tail call should not be optimized in this case, for subtle reasons.
-function bar(x, y) {
- function auxBar(a, b) {
- if (a == 0)
- return 42;
- if (a == 1)
- return foo(a - 1);
- if (a == 2)
- return foo(b - 1, a);
- return foo (b - 1, a, 43);
- }
-
- return auxBar(x, y);
-}
-
-function test(result, expected, name) {
- if (result != expected)
- throw "Wrong result for " + name + ": " + result + " instead of " + expected;
-}
-
-for (var i = 0; i < 10000; ++i) {
- test(factorial(20), 2432902008176640000, "factorial");
- test(factorial2(20), 2432902008176640000, "factorial2");
- test(factorial3(20), 2432902008176640000, "factorial3");
- test(factorial4(20), 2432902008176640000, "factorial4");
- test(foo(10, 10), 42, "foo");
- test(bar(10, 10), 42, "bar");
-}
+2017-10-25 Commit Queue <commit-queue@webkit.org>
+
+ Unreviewed, rolling out r223691 and r223729.
+ https://bugs.webkit.org/show_bug.cgi?id=178834
+
+ Broke Speedometer 2 React-Redux-TodoMVC test case (Requested
+ by rniwa on #webkit).
+
+ Reverted changesets:
+
+ "Turn recursive tail calls into loops"
+ https://bugs.webkit.org/show_bug.cgi?id=176601
+ https://trac.webkit.org/changeset/223691
+
+ "REGRESSION(r223691): DFGByteCodeParser.cpp:1483:83: warning:
+ comparison is always false due to limited range of data type
+ [-Wtype-limits]"
+ https://bugs.webkit.org/show_bug.cgi?id=178543
+ https://trac.webkit.org/changeset/223729
+
2017-10-25 Michael Saboff <msaboff@apple.com>
REGRESSION(r223937): Use of -fobjc-weak causes build failures with older compilers
// Call this to cause an optimization trigger to fire soon, but
// not necessarily the next one. This makes sense if optimization
- // succeeds. Successful optimization means that all calls are
+ // succeeds. Successfuly optimization means that all calls are
// relinked to the optimized code, so this only affects call
// frames that are still executing this CodeBlock. The value here
// is tuned to strike a balance between the cost of OSR entry
std::optional<CodeOrigin> findPC(void* pc);
#endif
- bool hasTailCalls() const { return m_unlinkedCode->hasTailCalls(); }
-
protected:
void finalizeLLIntInlineCaches();
void finalizeBaselineJITInlineCaches();
// op_loop_hint does not have jump target stored in bytecode instructions.
if (opcodeID == op_loop_hint)
out.append(bytecodeOffset);
- else if (opcodeID == op_enter && codeBlock->hasTailCalls()) {
- // We need to insert a jump after op_enter, so recursive tail calls have somewhere to jump to.
- // But we only want to pay that price for functions that have at least one tail call.
- out.append(bytecodeOffset + opcodeLengths[op_enter]);
- }
}
enum class ComputePreciseJumpTargetsMode {
void computePreciseJumpTargetsInternal(Block* codeBlock, Instruction* instructionsBegin, unsigned instructionCount, Vector<unsigned, vectorSize>& out)
{
ASSERT(out.isEmpty());
-
- // The code block has a superset of the jump targets. So if it claims to have none, we are done.
+
+ // We will derive a superset of the jump targets that the code block thinks it has.
+ // So, if the code block claims there are none, then we are done.
if (Mode == ComputePreciseJumpTargetsMode::FollowCodeBlockClaim && !codeBlock->numberOfJumpTargets())
return;
, m_constructorKind(static_cast<unsigned>(info.constructorKind()))
, m_derivedContextType(static_cast<unsigned>(info.derivedContextType()))
, m_evalContextType(static_cast<unsigned>(info.evalContextType()))
- , m_hasTailCalls(false)
, m_lineCount(0)
, m_endColumn(UINT_MAX)
, m_didOptimize(MixedTriState)
EvalContextType evalContextType() const { return static_cast<EvalContextType>(m_evalContextType); }
bool isArrowFunctionContext() const { return m_isArrowFunctionContext; }
bool isClassContext() const { return m_isClassContext; }
- bool hasTailCalls() const { return m_hasTailCalls; }
- void setHasTailCalls() { m_hasTailCalls = true; }
void addExpressionInfo(unsigned instructionOffset, int divot,
int startOffset, int endOffset, unsigned line, unsigned column);
unsigned m_constructorKind : 2;
unsigned m_derivedContextType : 2;
unsigned m_evalContextType : 2;
- unsigned m_hasTailCalls : 1;
unsigned m_lineCount;
unsigned m_endColumn;
void BytecodeGenerator::emitEnter()
{
emitOpcode(op_enter);
-
- // We must add the end of op_enter as a potential jump target, because the bytecode parser may decide to split its basic block
- // to have somewhere to jump to if there is a recursive tail-call that points to this function.
- m_codeBlock->addJumpTarget(instructions().size());
- // This disables peephole optimizations when an instruction is a jump target
- m_lastOpcodeID = op_end;
}
void BytecodeGenerator::emitLoopHint()
RegisterID* BytecodeGenerator::emitCallInTailPosition(RegisterID* dst, RegisterID* func, ExpectedFunction expectedFunction, CallArguments& callArguments, const JSTextPosition& divot, const JSTextPosition& divotStart, const JSTextPosition& divotEnd, DebuggableCall debuggableCall)
{
- if (m_inTailPosition) {
- m_codeBlock->setHasTailCalls();
- return emitCall(op_tail_call, dst, func, expectedFunction, callArguments, divot, divotStart, divotEnd, debuggableCall);
- }
- return emitCall(op_call, dst, func, expectedFunction, callArguments, divot, divotStart, divotEnd, debuggableCall);
+ return emitCall(m_inTailPosition ? op_tail_call : op_call, dst, func, expectedFunction, callArguments, divot, divotStart, divotEnd, debuggableCall);
}
RegisterID* BytecodeGenerator::emitCallEval(RegisterID* dst, RegisterID* func, CallArguments& callArguments, const JSTextPosition& divot, const JSTextPosition& divotStart, const JSTextPosition& divotEnd, DebuggableCall debuggableCall)
void emitFunctionChecks(CallVariant, Node* callTarget, VirtualRegister thisArgumnt);
void emitArgumentPhantoms(int registerOffset, int argumentCountIncludingThis);
Node* getArgumentCount();
- bool handleRecursiveTailCall(Node* callTargetNode, const CallLinkStatus&, int registerOffset, VirtualRegister thisArgument, int argumentCountIncludingThis);
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 handleInlining(Node* callTargetNode, int resultOperand, const CallLinkStatus&, int registerOffset, VirtualRegister thisArgument, VirtualRegister argumentsArgument, unsigned argumentsOffset, int argumentCountIncludingThis, unsigned nextOffset, NodeType callOp, InlineCallFrame::Kind, SpeculatedType prediction);
bool attemptToInlineCall(Node* callTargetNode, int resultOperand, CallVariant, int registerOffset, int argumentCountIncludingThis, unsigned nextOffset, InlineCallFrame::Kind, SpeculatedType prediction, unsigned& inliningBalance, BasicBlock* continuationBlock, const ChecksFunctor& insertChecks);
template<typename ChecksFunctor>
void inlineCall(Node* callTargetNode, int resultOperand, CallVariant, int registerOffset, int argumentCountIncludingThis, unsigned nextOffset, InlineCallFrame::Kind, BasicBlock* continuationBlock, const ChecksFunctor& insertChecks);
+ void cancelLinkingForBlock(InlineStackEntry*, BasicBlock*); // Only works when the given block is the last one to have been added for that inline stack entry.
// Handle intrinsic functions. Return true if it succeeded, false if we need to plant a call.
template<typename ChecksFunctor>
bool handleIntrinsicCall(Node* callee, int resultOperand, Intrinsic, int registerOffset, int argumentCountIncludingThis, SpeculatedType prediction, const ChecksFunctor& insertChecks);
// cannot have two blocks that have the same bytecodeBegin.
Vector<BasicBlock*> m_blockLinkingTargets;
+ // This is set by op_enter in parseBlock(), and indicates the first block of the function.
+ BasicBlock* m_entryBlock;
+
// Optional: a continuation block for returns to jump to. It is set by early returns if it does not exist.
BasicBlock* m_continuationBlock;
ASSERT(bytecodeIndex != UINT_MAX);
Ref<BasicBlock> block = adoptRef(*new BasicBlock(bytecodeIndex, m_numArguments, m_numLocals, 1));
BasicBlock* blockPtr = block.ptr();
- // m_blockLinkingTargets must always be sorted in increasing order of bytecodeBegin
- if (m_inlineStackTop->m_blockLinkingTargets.size())
- ASSERT(m_inlineStackTop->m_blockLinkingTargets.last()->bytecodeBegin < bytecodeIndex);
m_inlineStackTop->m_blockLinkingTargets.append(blockPtr);
m_graph.appendBlock(WTFMove(block));
return blockPtr;
{
ASSERT(block->bytecodeBegin == UINT_MAX);
block->bytecodeBegin = bytecodeIndex;
- // m_blockLinkingTargets must always be sorted in increasing order of bytecodeBegin
- if (m_inlineStackTop->m_blockLinkingTargets.size())
- ASSERT(m_inlineStackTop->m_blockLinkingTargets.last()->bytecodeBegin < bytecodeIndex);
m_inlineStackTop->m_blockLinkingTargets.append(block);
}
if (callLinkStatus.canOptimize()) {
VirtualRegister thisArgument = virtualRegisterForArgument(0, registerOffset);
- if (op == TailCall && handleRecursiveTailCall(callTarget, callLinkStatus, registerOffset, thisArgument, argumentCountIncludingThis))
- return Terminal;
-
// Inlining is quite complex, and managed by a pipeline of functions:
// handle(Varargs)Call -> handleInlining -> attemptToInlineCall -> inlineCall
// - handleCall and handleVarargsCall deal with the case where no inlining happens, and do some sanity checks on their arguments
addToGraph(Phantom, get(virtualRegisterForArgument(i, registerOffset)));
}
-bool ByteCodeParser::handleRecursiveTailCall(Node* callTargetNode, const CallLinkStatus& callLinkStatus, int registerOffset, VirtualRegister thisArgument, int argumentCountIncludingThis)
-{
- // FIXME: We currently only do this optimisation in the simple, non-polymorphic case.
- // https://bugs.webkit.org/show_bug.cgi?id=178390
- if (callLinkStatus.couldTakeSlowPath() || callLinkStatus.size() != 1)
- return false;
-
- auto targetExecutable = callLinkStatus[0].executable();
- InlineStackEntry* stackEntry = m_inlineStackTop;
- do {
- if (targetExecutable != stackEntry->executable())
- continue;
- VERBOSE_LOG(" We found a recursive tail call, trying to optimize it into a jump.\n");
-
- if (auto* callFrame = stackEntry->m_inlineCallFrame) {
- // Some code may statically use the argument count from the InlineCallFrame, so it would be invalid to loop back if it does not match.
- // We "continue" instead of returning false in case another stack entry further on the stack has the right number of arguments.
- if (argumentCountIncludingThis != static_cast<int>(callFrame->argumentCountIncludingThis))
- continue;
- } else {
- // We are in the machine code entry (i.e. the original caller).
- // If we have more arguments than the number of parameters to the function, it is not clear where we could put them on the stack.
- if (argumentCountIncludingThis > m_codeBlock->numParameters())
- return false;
- }
-
- // We must add some check that the profiling information was correct and the target of this call is what we thought
- emitFunctionChecks(callLinkStatus[0], callTargetNode, thisArgument);
- // 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 arguments to the right values
- int argIndex = 0;
- for (; argIndex < argumentCountIncludingThis; ++argIndex) {
- Node* value = get(virtualRegisterForArgument(argIndex, registerOffset));
- setDirect(stackEntry->remapOperand(virtualRegisterForArgument(argIndex)), value, NormalSet);
- }
- Node* undefined = addToGraph(JSConstant, OpInfo(m_constantUndefined));
- for (; argIndex < stackEntry->m_codeBlock->numParameters(); ++argIndex)
- setDirect(stackEntry->remapOperand(virtualRegisterForArgument(argIndex)), undefined, NormalSet);
-
- // We must repeat the work of op_enter here as we will jump right after it.
- // We jump right after it and not before it, because of some invariant saying that a CFG root cannot have predecessors in the IR.
- for (int i = 0; i < stackEntry->m_codeBlock->m_numVars; ++i)
- setDirect(stackEntry->remapOperand(virtualRegisterForLocal(i)), undefined, NormalSet);
-
- // We want to emit the SetLocals with an exit origin that points to the place we are jumping to.
- unsigned oldIndex = m_currentIndex;
- auto oldStackTop = m_inlineStackTop;
- m_inlineStackTop = stackEntry;
- m_currentIndex = OPCODE_LENGTH(op_enter);
- m_exitOK = true;
- processSetLocalQueue();
- m_currentIndex = oldIndex;
- m_inlineStackTop = oldStackTop;
- m_exitOK = false;
-
- BasicBlock** entryBlockPtr = tryBinarySearch<BasicBlock*, unsigned>(stackEntry->m_blockLinkingTargets, stackEntry->m_blockLinkingTargets.size(), OPCODE_LENGTH(op_enter), getBytecodeBeginForBlock);
- RELEASE_ASSERT(entryBlockPtr);
- addJumpTo(*entryBlockPtr);
- return true;
- // It would be unsound to jump over a non-tail call: the "tail" call is not really a tail call in that case.
- } while (stackEntry->m_inlineCallFrame && stackEntry->m_inlineCallFrame->kind == InlineCallFrame::TailCall && (stackEntry = stackEntry->m_caller));
-
- // The tail call was not recursive
- return false;
-}
-
unsigned ByteCodeParser::inliningCost(CallVariant callee, int argumentCountIncludingThis, InlineCallFrame::Kind kind)
{
CallMode callMode = InlineCallFrame::callModeFor(kind);
for (int i = 0; i < m_inlineStackTop->m_codeBlock->m_numVars; ++i)
set(virtualRegisterForLocal(i), undefined, ImmediateNakedSet);
+ m_inlineStackTop->m_entryBlock = m_currentBlock;
+
NEXT_OPCODE(op_enter);
}
if (terminality == NonTerminal)
NEXT_OPCODE(op_tail_call);
else
- LAST_OPCODE_LINKED(op_tail_call);
- // We use LAST_OPCODE_LINKED instead of LAST_OPCODE because if the tail call was optimized, it may now be a jump to a bytecode index in a different InlineStackEntry.
+ LAST_OPCODE(op_tail_call);
}
case op_construct:
linkBlocks(inlineStackEntry.m_unlinkedBlocks, inlineStackEntry.m_blockLinkingTargets);
m_graph.determineReachability();
+ if (Options::verboseDFGBytecodeParsing())
+ m_graph.dump();
m_graph.killUnreachableBlocks();
for (BlockIndex blockIndex = m_graph.numBlocks(); blockIndex--;) {
git://git.webkit.org / WebKit-https.git / commitdiff