<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[163789] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/163789">163789</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2014-02-10 09:04:28 -0800 (Mon, 10 Feb 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre><1/100 probability FTL failure: v8-v6/v8-deltablue.js.ftl-eager: Exception: TypeError: undefined is not an object (evaluating 'c.isInput')
https://bugs.webkit.org/show_bug.cgi?id=128278
Reviewed by Mark Hahnenberg.
Fix another FTL flake due to bytecode liveness corner cases. Hopefully it's the last
one.
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::parseBlock): Make sure that inside a constructor, the 'this' result is always set. This makes it easier to unify the treatment of 'this' for OSR exit: we just say that it's always live.
* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::isLiveInBytecode): Assume that 'this' is live. We were already sort of doing this for calls because the callsite would claim it to be live. But we didn't do it for constructors. It's true that *at the callsite* 'this' won't be live, but inside the inlined constructor, it almost certainly will be.
* dfg/DFGTierUpCheckInjectionPhase.cpp:
(JSC::DFG::TierUpCheckInjectionPhase::run): I just noticed this benign bug. We should only return 'true' if we actually injected checks.
* ftl/FTLOSRExitCompiler.cpp:
(JSC::FTL::compileStub): Make it easier to just dump disassembly for FTL OSR exits.
* runtime/Options.h: Ditto.
* tests/stress/inlined-constructor-this-liveness.js: Added.
(Foo):
(foo):
* tests/stress/inlined-function-this-liveness.js: Added.
(bar):
(foo):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkPerformanceTestsSunSpidertestsv8v6v8deltabluejs">trunk/PerformanceTests/SunSpider/tests/v8-v6/v8-deltablue.js</a></li>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGByteCodeParsercpp">trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGGraphcpp">trunk/Source/JavaScriptCore/dfg/DFGGraph.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGTierUpCheckInjectionPhasecpp">trunk/Source/JavaScriptCore/dfg/DFGTierUpCheckInjectionPhase.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreftlFTLOSRExitCompilercpp">trunk/Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeOptionsh">trunk/Source/JavaScriptCore/runtime/Options.h</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoretestsstressinlinedconstructorthislivenessjs">trunk/Source/JavaScriptCore/tests/stress/inlined-constructor-this-liveness.js</a></li>
<li><a href="#trunkSourceJavaScriptCoretestsstressinlinedfunctionthislivenessjs">trunk/Source/JavaScriptCore/tests/stress/inlined-function-this-liveness.js</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkPerformanceTestsSunSpidertestsv8v6v8deltabluejs"></a>
<div class="modfile"><h4>Modified: trunk/PerformanceTests/SunSpider/tests/v8-v6/v8-deltablue.js (163788 => 163789)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/PerformanceTests/SunSpider/tests/v8-v6/v8-deltablue.js 2014-02-10 16:53:30 UTC (rev 163788)
+++ trunk/PerformanceTests/SunSpider/tests/v8-v6/v8-deltablue.js 2014-02-10 17:04:28 UTC (rev 163789)
</span><span class="lines">@@ -669,6 +669,8 @@
</span><span class="cx"> return this.makePlan(sources);
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+noInline(Planner.prototype.extractPlanFromConstraints);
+
</ins><span class="cx"> /**
</span><span class="cx"> * Recompute the walkabout strengths and stay flags of all variables
</span><span class="cx"> * downstream of the given constraint and recompute the actual
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (163788 => 163789)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2014-02-10 16:53:30 UTC (rev 163788)
+++ trunk/Source/JavaScriptCore/ChangeLog 2014-02-10 17:04:28 UTC (rev 163789)
</span><span class="lines">@@ -1,5 +1,31 @@
</span><span class="cx"> 2014-02-10 Filip Pizlo <fpizlo@apple.com>
</span><span class="cx">
</span><ins>+ <1/100 probability FTL failure: v8-v6/v8-deltablue.js.ftl-eager: Exception: TypeError: undefined is not an object (evaluating 'c.isInput')
+ https://bugs.webkit.org/show_bug.cgi?id=128278
+
+ Reviewed by Mark Hahnenberg.
+
+ Fix another FTL flake due to bytecode liveness corner cases. Hopefully it's the last
+ one.
+
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::parseBlock): Make sure that inside a constructor, the 'this' result is always set. This makes it easier to unify the treatment of 'this' for OSR exit: we just say that it's always live.
+ * dfg/DFGGraph.cpp:
+ (JSC::DFG::Graph::isLiveInBytecode): Assume that 'this' is live. We were already sort of doing this for calls because the callsite would claim it to be live. But we didn't do it for constructors. It's true that *at the callsite* 'this' won't be live, but inside the inlined constructor, it almost certainly will be.
+ * dfg/DFGTierUpCheckInjectionPhase.cpp:
+ (JSC::DFG::TierUpCheckInjectionPhase::run): I just noticed this benign bug. We should only return 'true' if we actually injected checks.
+ * ftl/FTLOSRExitCompiler.cpp:
+ (JSC::FTL::compileStub): Make it easier to just dump disassembly for FTL OSR exits.
+ * runtime/Options.h: Ditto.
+ * tests/stress/inlined-constructor-this-liveness.js: Added.
+ (Foo):
+ (foo):
+ * tests/stress/inlined-function-this-liveness.js: Added.
+ (bar):
+ (foo):
+
+2014-02-10 Filip Pizlo <fpizlo@apple.com>
+
</ins><span class="cx"> Actually register those DFG::Safepoints
</span><span class="cx"> https://bugs.webkit.org/show_bug.cgi?id=128521
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGByteCodeParsercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp (163788 => 163789)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp 2014-02-10 16:53:30 UTC (rev 163788)
+++ trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp 2014-02-10 17:04:28 UTC (rev 163789)
</span><span class="lines">@@ -1956,6 +1956,8 @@
</span><span class="cx"> // Initialize all locals to undefined.
</span><span class="cx"> for (int i = 0; i < m_inlineStackTop->m_codeBlock->m_numVars; ++i)
</span><span class="cx"> set(virtualRegisterForLocal(i), constantUndefined(), ImmediateSet);
</span><ins>+ if (m_inlineStackTop->m_codeBlock->specializationKind() == CodeForConstruct)
+ set(virtualRegisterForArgument(0), constantUndefined(), ImmediateSet);
</ins><span class="cx"> NEXT_OPCODE(op_enter);
</span><span class="cx">
</span><span class="cx"> case op_touch_entry:
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGGraphcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGGraph.cpp (163788 => 163789)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGGraph.cpp 2014-02-10 16:53:30 UTC (rev 163788)
+++ trunk/Source/JavaScriptCore/dfg/DFGGraph.cpp 2014-02-10 17:04:28 UTC (rev 163789)
</span><span class="lines">@@ -711,8 +711,9 @@
</span><span class="cx">
</span><span class="cx"> // Arguments are always live. This would be redundant if it wasn't for our
</span><span class="cx"> // op_call_varargs inlining.
</span><ins>+ // FIXME: 'this' might not be live, but we don't have a way of knowing.
+ // https://bugs.webkit.org/show_bug.cgi?id=128519
</ins><span class="cx"> if (reg.isArgument()
</span><del>- && reg.toArgument()
</del><span class="cx"> && static_cast<size_t>(reg.toArgument()) < inlineCallFrame->arguments.size())
</span><span class="cx"> return true;
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGTierUpCheckInjectionPhasecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGTierUpCheckInjectionPhase.cpp (163788 => 163789)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGTierUpCheckInjectionPhase.cpp 2014-02-10 16:53:30 UTC (rev 163788)
+++ trunk/Source/JavaScriptCore/dfg/DFGTierUpCheckInjectionPhase.cpp 2014-02-10 17:04:28 UTC (rev 163789)
</span><span class="lines">@@ -52,7 +52,7 @@
</span><span class="cx"> return false;
</span><span class="cx">
</span><span class="cx"> if (m_graph.m_profiledBlock->m_didFailFTLCompilation)
</span><del>- return true;
</del><ins>+ return false;
</ins><span class="cx">
</span><span class="cx"> #if ENABLE(FTL_JIT)
</span><span class="cx"> FTL::CapabilityLevel level = FTL::canCompile(m_graph);
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreftlFTLOSRExitCompilercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp (163788 => 163789)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp 2014-02-10 16:53:30 UTC (rev 163788)
+++ trunk/Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp 2014-02-10 17:04:28 UTC (rev 163789)
</span><span class="lines">@@ -341,7 +341,7 @@
</span><span class="cx">
</span><span class="cx"> LinkBuffer patchBuffer(*vm, &jit, codeBlock);
</span><span class="cx"> exit.m_code = FINALIZE_CODE_IF(
</span><del>- shouldShowDisassembly() || Options::verboseOSR(),
</del><ins>+ shouldShowDisassembly() || Options::verboseOSR() || Options::verboseFTLOSRExit(),
</ins><span class="cx"> patchBuffer,
</span><span class="cx"> ("FTL OSR exit #%u (%s, %s) from %s, with operands = %s, and record = %s",
</span><span class="cx"> exitID, toCString(exit.m_codeOrigin).data(),
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeOptionsh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/Options.h (163788 => 163789)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/Options.h 2014-02-10 16:53:30 UTC (rev 163788)
+++ trunk/Source/JavaScriptCore/runtime/Options.h 2014-02-10 17:04:28 UTC (rev 163789)
</span><span class="lines">@@ -122,6 +122,7 @@
</span><span class="cx"> v(bool, validateGraph, false) \
</span><span class="cx"> v(bool, validateGraphAtEachPhase, false) \
</span><span class="cx"> v(bool, verboseOSR, false) \
</span><ins>+ v(bool, verboseFTLOSRExit, false) \
</ins><span class="cx"> v(bool, verboseCallLink, false) \
</span><span class="cx"> v(bool, verboseCompilationQueue, false) \
</span><span class="cx"> v(bool, reportCompileTimes, false) \
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoretestsstressinlinedconstructorthislivenessjs"></a>
<div class="addfile"><h4>Added: trunk/Source/JavaScriptCore/tests/stress/inlined-constructor-this-liveness.js (0 => 163789)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/tests/stress/inlined-constructor-this-liveness.js (rev 0)
+++ trunk/Source/JavaScriptCore/tests/stress/inlined-constructor-this-liveness.js 2014-02-10 17:04:28 UTC (rev 163789)
</span><span class="lines">@@ -0,0 +1,24 @@
</span><ins>+function Foo(a, b) {
+ this.f = a.f;
+ this.g = b.f + 1;
+}
+
+function foo(a, b) {
+ return new Foo(a, b);
+}
+
+noInline(foo);
+
+for (var i = 0; i < 100000; ++i) {
+ var result = foo({f:1}, {f:2});
+ if (result.f != 1)
+ throw "Error: bad result.f: " + result.f;
+ if (result.g != 3)
+ throw "Error: bad result.g: " + result.g;
+}
+
+var result = foo({f:1}, {f:2.5});
+if (result.f != 1)
+ throw "Error: bad result.f: " + result.f;
+if (result.g != 3.5)
+ throw "Error: bad result.f: " + result.g;
</ins></span></pre></div>
<a id="trunkSourceJavaScriptCoretestsstressinlinedfunctionthislivenessjs"></a>
<div class="addfile"><h4>Added: trunk/Source/JavaScriptCore/tests/stress/inlined-function-this-liveness.js (0 => 163789)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/tests/stress/inlined-function-this-liveness.js (rev 0)
+++ trunk/Source/JavaScriptCore/tests/stress/inlined-function-this-liveness.js 2014-02-10 17:04:28 UTC (rev 163789)
</span><span class="lines">@@ -0,0 +1,26 @@
</span><ins>+function bar(a, b) {
+ this.f = a.f;
+ this.g = b.f + 1;
+ return this;
+}
+
+function foo(a, b) {
+ var o = {f:bar};
+ return o.f(a, b);
+}
+
+noInline(foo);
+
+for (var i = 0; i < 100000; ++i) {
+ var result = foo({f:1}, {f:2});
+ if (result.f != 1)
+ throw "Error: bad result.f: " + result.f;
+ if (result.g != 3)
+ throw "Error: bad result.g: " + result.g;
+}
+
+var result = foo({f:1}, {f:2.5});
+if (result.f != 1)
+ throw "Error: bad result.f: " + result.f;
+if (result.g != 3.5)
+ throw "Error: bad result.f: " + result.g;
</ins></span></pre>
</div>
</div>
</body>
</html>