Title: [204360] trunk
- Revision
- 204360
- Author
- [email protected]
- Date
- 2016-08-10 16:19:49 -0700 (Wed, 10 Aug 2016)
Log Message
DFG's flushForTerminal() needs to add PhantomLocals for bytecode live locals.
https://bugs.webkit.org/show_bug.cgi?id=160755
<rdar://problem/27488507>
Reviewed by Filip Pizlo.
JSTests:
* stress/need-bytecode-liveness-for-unreachable-blocks-at-dfg-time.js: Added.
Source/_javascript_Core:
If the DFG sees that an inlined function will result in an OSR exit every time,
it will treat all downstream blocks as dead. However, it still needs to keep
locals that are alive in the bytecode alive for the compiled function so that
those locals are properly written to the stack by the OSR exit ramp.
The existing code neglected to do this. This patch remedies this issue.
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::flushDirect):
(JSC::DFG::ByteCodeParser::addFlushOrPhantomLocal):
(JSC::DFG::ByteCodeParser::phantomLocalDirect):
(JSC::DFG::ByteCodeParser::flushForTerminal):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (204359 => 204360)
--- trunk/JSTests/ChangeLog 2016-08-10 22:37:47 UTC (rev 204359)
+++ trunk/JSTests/ChangeLog 2016-08-10 23:19:49 UTC (rev 204360)
@@ -1,3 +1,13 @@
+2016-08-10 Mark Lam <[email protected]>
+
+ DFG's flushForTerminal() needs to add PhantomLocals for bytecode live locals.
+ https://bugs.webkit.org/show_bug.cgi?id=160755
+ <rdar://problem/27488507>
+
+ Reviewed by Filip Pizlo.
+
+ * stress/need-bytecode-liveness-for-unreachable-blocks-at-dfg-time.js: Added.
+
2016-08-09 Skachkov Oleksandr <[email protected]>
[ES2016] Implement Object.values
Added: trunk/JSTests/stress/need-bytecode-liveness-for-unreachable-blocks-at-dfg-time.js (0 => 204360)
--- trunk/JSTests/stress/need-bytecode-liveness-for-unreachable-blocks-at-dfg-time.js (rev 0)
+++ trunk/JSTests/stress/need-bytecode-liveness-for-unreachable-blocks-at-dfg-time.js 2016-08-10 23:19:49 UTC (rev 204360)
@@ -0,0 +1,31 @@
+//@ run("--useConcurrentJIT=false")
+
+// This test is set up delicately to:
+// 1. cause the test() function to DFG compile with just the right amount of profiling
+// so that ...
+// 2. the DFG identifies the "return Function()" path as dead, and ...
+// 3. the DFG compiled function doesn't OSR exit too many times before ...
+// 4. we change the implementation of the inlined foo() and execute test() again.
+//
+// This test should not crash.
+
+eval("\"use strict\"; var w;");
+foo = function() { throw 0; }
+var x;
+
+(function() {
+ eval("test = function() { ~foo(~(0 ? ~x : x) ? 0 : 0); return Function(); }");
+})();
+
+// This loop count of 2000 was empirically determined to be the right amount to get this
+// this issue to manifest. Dropping or raising it may mask the issue and prevent it from
+// manifesting.
+for (var i = 0; i < 2000; ++i) {
+ try {
+ test();
+ } catch(e) {
+ }
+}
+
+foo = function() { };
+test();
Modified: trunk/Source/_javascript_Core/ChangeLog (204359 => 204360)
--- trunk/Source/_javascript_Core/ChangeLog 2016-08-10 22:37:47 UTC (rev 204359)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-08-10 23:19:49 UTC (rev 204360)
@@ -1,3 +1,24 @@
+2016-08-10 Mark Lam <[email protected]>
+
+ DFG's flushForTerminal() needs to add PhantomLocals for bytecode live locals.
+ https://bugs.webkit.org/show_bug.cgi?id=160755
+ <rdar://problem/27488507>
+
+ Reviewed by Filip Pizlo.
+
+ If the DFG sees that an inlined function will result in an OSR exit every time,
+ it will treat all downstream blocks as dead. However, it still needs to keep
+ locals that are alive in the bytecode alive for the compiled function so that
+ those locals are properly written to the stack by the OSR exit ramp.
+
+ The existing code neglected to do this. This patch remedies this issue.
+
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::flushDirect):
+ (JSC::DFG::ByteCodeParser::addFlushOrPhantomLocal):
+ (JSC::DFG::ByteCodeParser::phantomLocalDirect):
+ (JSC::DFG::ByteCodeParser::flushForTerminal):
+
2016-08-09 Skachkov Oleksandr <[email protected]>
[ES2016] Implement Object.values
Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (204359 => 204360)
--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2016-08-10 22:37:47 UTC (rev 204359)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2016-08-10 23:19:49 UTC (rev 204360)
@@ -573,9 +573,15 @@
{
flushDirect(operand, findArgumentPosition(operand));
}
-
+
void flushDirect(VirtualRegister operand, ArgumentPosition* argumentPosition)
{
+ addFlushOrPhantomLocal<Flush>(operand, argumentPosition);
+ }
+
+ template<NodeType nodeType>
+ void addFlushOrPhantomLocal(VirtualRegister operand, ArgumentPosition* argumentPosition)
+ {
ASSERT(!operand.isConstant());
Node* node = m_currentBlock->variablesAtTail.operand(operand);
@@ -587,12 +593,17 @@
else
variable = newVariableAccessData(operand);
- node = addToGraph(Flush, OpInfo(variable));
+ node = addToGraph(nodeType, OpInfo(variable));
m_currentBlock->variablesAtTail.operand(operand) = node;
if (argumentPosition)
argumentPosition->addVariable(variable);
}
-
+
+ void phantomLocalDirect(VirtualRegister operand)
+ {
+ addFlushOrPhantomLocal<PhantomLocal>(operand, findArgumentPosition(operand));
+ }
+
void flush(InlineStackEntry* inlineStackEntry)
{
int numArguments;
@@ -615,8 +626,32 @@
void flushForTerminal()
{
- for (InlineStackEntry* inlineStackEntry = m_inlineStackTop; inlineStackEntry; inlineStackEntry = inlineStackEntry->m_caller)
+ CodeOrigin origin = currentCodeOrigin();
+ unsigned bytecodeIndex = origin.bytecodeIndex;
+
+ for (InlineStackEntry* inlineStackEntry = m_inlineStackTop; inlineStackEntry; inlineStackEntry = inlineStackEntry->m_caller) {
flush(inlineStackEntry);
+
+ ASSERT(origin.inlineCallFrame == inlineStackEntry->m_inlineCallFrame);
+ InlineCallFrame* inlineCallFrame = inlineStackEntry->m_inlineCallFrame;
+ CodeBlock* codeBlock = m_graph.baselineCodeBlockFor(inlineCallFrame);
+ FullBytecodeLiveness& fullLiveness = m_graph.livenessFor(codeBlock);
+ const FastBitVector& livenessAtBytecode = fullLiveness.getLiveness(bytecodeIndex);
+
+ for (unsigned local = codeBlock->m_numCalleeLocals; local--;) {
+ if (livenessAtBytecode.get(local)) {
+ VirtualRegister reg = virtualRegisterForLocal(local);
+ if (inlineCallFrame)
+ reg = inlineStackEntry->remapOperand(reg);
+ phantomLocalDirect(reg);
+ }
+ }
+
+ if (inlineCallFrame) {
+ bytecodeIndex = inlineCallFrame->directCaller.bytecodeIndex;
+ origin = inlineCallFrame->directCaller;
+ }
+ }
}
void flushForReturn()
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes