Modified: trunk/Source/_javascript_Core/ChangeLog (223111 => 223112)
--- trunk/Source/_javascript_Core/ChangeLog 2017-10-10 01:20:34 UTC (rev 223111)
+++ trunk/Source/_javascript_Core/ChangeLog 2017-10-10 01:30:02 UTC (rev 223112)
@@ -1,5 +1,20 @@
2017-10-09 Robin Morisset <[email protected]>
+ Evaluate the benefit of skipping dead code in the DFGByteCodeParser when a function returns in its first block
+ https://bugs.webkit.org/show_bug.cgi?id=177925
+
+ Reviewed by Saam Barati.
+
+ We used to do a rather weird "optimisation" in the bytecode parser: when a function would return in its first block,
+ the rest of the function was skipped. Since it has no actual impact on any benchmarks from what I could see, I removed
+ that code. It allows some changes to parseBlock(), since it now returns void and no-longer bool (it was returning a boolean that said whether that case happened or not).
+
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::parseBlock):
+ (JSC::DFG::ByteCodeParser::parseCodeBlock):
+
+2017-10-09 Robin Morisset <[email protected]>
+
Refactor the inliner to simplify block linking
https://bugs.webkit.org/show_bug.cgi?id=177922
Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (223111 => 223112)
--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2017-10-10 01:20:34 UTC (rev 223111)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2017-10-10 01:30:02 UTC (rev 223112)
@@ -303,7 +303,7 @@
void clearCaches();
// Parse a single basic block of bytecode instructions.
- bool parseBlock(unsigned limit);
+ void parseBlock(unsigned limit);
// Link block successors.
void linkBlock(BasicBlock*, Vector<BasicBlock*>& possibleTargets);
void linkBlocks(Vector<BasicBlock*>& unlinkedBlocks, Vector<BasicBlock*>& possibleTargets);
@@ -4086,11 +4086,11 @@
WTF_CONCAT(NEXT_OPCODE_, __LINE__): \
continue
-#define LAST_OPCODE_LINKED(name) \
- return \
- m_currentIndex += OPCODE_LENGTH(name), \
- m_exitOK = false, \
- shouldContinueParsing
+#define LAST_OPCODE_LINKED(name) do { \
+ m_currentIndex += OPCODE_LENGTH(name); \
+ m_exitOK = false; \
+ return; \
+ } while (false)
#define LAST_OPCODE(name) \
do { \
@@ -4108,10 +4108,8 @@
LAST_OPCODE_LINKED(name); \
} while (false)
-bool ByteCodeParser::parseBlock(unsigned limit)
+void ByteCodeParser::parseBlock(unsigned limit)
{
- bool shouldContinueParsing = true;
-
Instruction* instructionsBegin = m_inlineStackTop->m_codeBlock->instructions().begin();
unsigned blockBegin = m_currentIndex;
@@ -4160,7 +4158,7 @@
if (!m_currentBlock->isEmpty())
addJumpTo(m_currentIndex);
- return shouldContinueParsing;
+ return;
}
// Switch on the current bytecode opcode.
@@ -5176,17 +5174,10 @@
setDirect(m_inlineStackTop->m_returnValue, get(VirtualRegister(currentInstruction[1].u.operand)), ImmediateSetWithFlush);
if (!m_inlineStackTop->m_continuationBlock && m_currentIndex + OPCODE_LENGTH(op_ret) != m_inlineStackTop->m_codeBlock->instructions().size()) {
- if (m_inlineStackTop->m_entryBlock == m_currentBlock) {
- // This is a rather weird case: we are returning from the first block in the function, so all the rest of the function is dead code.
- // So we treat this as a normal return instead of an early return, and signal to skip the rest of the function.
- // FIXME https://bugs.webkit.org/show_bug.cgi?id=177925: evaluate whether that optimization is useful, it seems like a really weird corner case.
- shouldContinueParsing = false;
- } else {
- // This is an early return from an inlined function and we do not have a continuation block, so we must allocate one.
- // It is untargetable, because we do not know the appropriate index.
- // If this block turns out to be a jump target, parseCodeBlock will fix its bytecodeIndex before putting it in m_blockLinkingTargets
- m_inlineStackTop->m_continuationBlock = allocateUntargetableBlock();
- }
+ // This is an early return from an inlined function and we do not have a continuation block, so we must allocate one.
+ // It is untargetable, because we do not know the appropriate index.
+ // If this block turns out to be a jump target, parseCodeBlock will fix its bytecodeIndex before putting it in m_blockLinkingTargets
+ m_inlineStackTop->m_continuationBlock = allocateUntargetableBlock();
}
if (m_inlineStackTop->m_continuationBlock)
@@ -6104,7 +6095,7 @@
// Parse failed! This should not happen because the capabilities checker
// should have caught it.
RELEASE_ASSERT_NOT_REACHED();
- return false;
+ return;
}
}
}
@@ -6327,16 +6318,11 @@
prepareToParseBlock();
}
- bool shouldContinueParsing = parseBlock(limit);
+ parseBlock(limit);
// We should not have gone beyond the limit.
ASSERT(m_currentIndex <= limit);
- if (!shouldContinueParsing) {
- VERBOSE_LOG("Done parsing ", *codeBlock, "\n");
- return;
- }
-
if (m_currentBlock->isEmpty()) {
// This case only happens if the last instruction was an inlined call with early returns
// or polymorphic (creating an empty continuation block),
