<!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>[188507] trunk/Source/JavaScriptCore</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/188507">188507</a></dd>
<dt>Author</dt> <dd>basile_clement@apple.com</dd>
<dt>Date</dt> <dd>2015-08-14 22:00:57 -0700 (Fri, 14 Aug 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>Occasional failure in v8-v6/v8-raytrace.js.ftl-eager
https://bugs.webkit.org/show_bug.cgi?id=147165
Reviewed by Saam Barati.
The object allocation sinking phase was not properly checking that a
MultiGetByOffset was safe to lower before lowering it.
This makes it so that we only lower MultiGetByOffset if it only loads
from direct properties of the object, and considers it as an escape in
any other case (e.g. a load from the prototype).
It also ensure proper conversion of MultiGetByOffset into
CheckStructureImmediate when needed.
* dfg/DFGObjectAllocationSinkingPhase.cpp:
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::DFG::LowerDFGToLLVM::checkStructure):
We were not compiling properly CheckStructure and
CheckStructureImmediate nodes with an empty StructureSet.
* tests/stress/sink-multigetbyoffset.js: Regression test.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGObjectAllocationSinkingPhasecpp">trunk/Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreftlFTLLowerDFGToLLVMcpp">trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoretestsstresssinkmultigetbyoffsetjs">trunk/Source/JavaScriptCore/tests/stress/sink-multigetbyoffset.js</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (188506 => 188507)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2015-08-15 03:47:04 UTC (rev 188506)
+++ trunk/Source/JavaScriptCore/ChangeLog 2015-08-15 05:00:57 UTC (rev 188507)
</span><span class="lines">@@ -1,3 +1,26 @@
</span><ins>+2015-08-14 Basile Clement <basile_clement@apple.com>
+
+ Occasional failure in v8-v6/v8-raytrace.js.ftl-eager
+ https://bugs.webkit.org/show_bug.cgi?id=147165
+
+ Reviewed by Saam Barati.
+
+ The object allocation sinking phase was not properly checking that a
+ MultiGetByOffset was safe to lower before lowering it.
+ This makes it so that we only lower MultiGetByOffset if it only loads
+ from direct properties of the object, and considers it as an escape in
+ any other case (e.g. a load from the prototype).
+
+ It also ensure proper conversion of MultiGetByOffset into
+ CheckStructureImmediate when needed.
+
+ * dfg/DFGObjectAllocationSinkingPhase.cpp:
+ * ftl/FTLLowerDFGToLLVM.cpp:
+ (JSC::FTL::DFG::LowerDFGToLLVM::checkStructure):
+ We were not compiling properly CheckStructure and
+ CheckStructureImmediate nodes with an empty StructureSet.
+ * tests/stress/sink-multigetbyoffset.js: Regression test.
+
</ins><span class="cx"> 2015-08-14 Filip Pizlo <fpizlo@apple.com>
</span><span class="cx">
</span><span class="cx"> Use WTF::Lock and WTF::Condition instead of WTF::Mutex, WTF::ThreadCondition, std::mutex, and std::condition_variable
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGObjectAllocationSinkingPhasecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp (188506 => 188507)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp 2015-08-15 03:47:04 UTC (rev 188506)
+++ trunk/Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp 2015-08-15 05:00:57 UTC (rev 188507)
</span><span class="lines">@@ -921,14 +921,67 @@
</span><span class="cx"> }
</span><span class="cx"> break;
</span><span class="cx">
</span><del>- case MultiGetByOffset:
- target = m_heap.onlyLocalAllocation(node->child1().node());
- if (target && target->isObjectAllocation()) {
- unsigned identifierNumber = node->multiGetByOffsetData().identifierNumber;
- exactRead = PromotedLocationDescriptor(NamedPropertyPLoc, identifierNumber);
</del><ins>+ case MultiGetByOffset: {
+ Allocation* allocation = m_heap.onlyLocalAllocation(node->child1().node());
+ if (allocation && allocation->isObjectAllocation()) {
+ MultiGetByOffsetData& data = node->multiGetByOffsetData();
+ StructureSet validStructures;
+ bool hasInvalidStructures = false;
+ for (const auto& multiGetByOffsetCase : data.cases) {
+ if (!allocation->structures().overlaps(multiGetByOffsetCase.set()))
+ continue;
+
+ switch (multiGetByOffsetCase.method().kind()) {
+ case GetByOffsetMethod::LoadFromPrototype: // We need to escape those
+ case GetByOffsetMethod::Constant: // We don't really have a way of expressing this
+ hasInvalidStructures = true;
+ break;
+
+ case GetByOffsetMethod::Load: // We're good
+ validStructures.merge(multiGetByOffsetCase.set());
+ break;
+
+ default:
+ RELEASE_ASSERT_NOT_REACHED();
+ }
+ }
+ if (hasInvalidStructures) {
+ m_heap.escape(node->child1().node());
+ break;
+ }
+ unsigned identifierNumber = data.identifierNumber;
+ PromotedHeapLocation location(NamedPropertyPLoc, allocation->identifier(), identifierNumber);
+ if (Node* value = heapResolve(location)) {
+ if (allocation->structures().isSubsetOf(validStructures))
+ node->replaceWith(value);
+ else {
+ Node* structure = heapResolve(PromotedHeapLocation(allocation->identifier(), StructurePLoc));
+ ASSERT(structure);
+ allocation->filterStructures(validStructures);
+ node->convertToCheckStructure(m_graph.addStructureSet(allocation->structures()));
+ node->convertToCheckStructureImmediate(structure);
+ node->setReplacement(value);
+ }
+ } else if (!allocation->structures().isSubsetOf(validStructures)) {
+ // Even though we don't need the result here, we still need
+ // to make the call to tell our caller that we could need
+ // the StructurePLoc.
+ // The reason for this is that when we decide not to sink a
+ // node, we will still lower any read to its fields before
+ // it escapes (which are usually reads across a function
+ // call that DFGClobberize can't handle) - but we only do
+ // this for PromotedHeapLocations that we have seen read
+ // during the analysis!
+ heapResolve(PromotedHeapLocation(allocation->identifier(), StructurePLoc));
+ allocation->filterStructures(validStructures);
+ }
+ Node* identifier = allocation->get(location.descriptor());
+ if (identifier)
+ m_heap.newPointer(node, identifier);
</ins><span class="cx"> } else
</span><span class="cx"> m_heap.escape(node->child1().node());
</span><span class="cx"> break;
</span><ins>+ }
</ins><span class="cx">
</span><span class="cx"> case PutByOffset:
</span><span class="cx"> target = m_heap.onlyLocalAllocation(node->child2().node());
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreftlFTLLowerDFGToLLVMcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp (188506 => 188507)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp 2015-08-15 03:47:04 UTC (rev 188506)
+++ trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp 2015-08-15 05:00:57 UTC (rev 188507)
</span><span class="lines">@@ -5515,6 +5515,11 @@
</span><span class="cx"> LValue structureDiscriminant, const FormattedValue& formattedValue, ExitKind exitKind,
</span><span class="cx"> const StructureSet& set, const Functor& weakStructureDiscriminant)
</span><span class="cx"> {
</span><ins>+ if (set.isEmpty()) {
+ terminate(exitKind);
+ return;
+ }
+
</ins><span class="cx"> if (set.size() == 1) {
</span><span class="cx"> speculate(
</span><span class="cx"> exitKind, formattedValue, 0,
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoretestsstresssinkmultigetbyoffsetjs"></a>
<div class="addfile"><h4>Added: trunk/Source/JavaScriptCore/tests/stress/sink-multigetbyoffset.js (0 => 188507)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/tests/stress/sink-multigetbyoffset.js (rev 0)
+++ trunk/Source/JavaScriptCore/tests/stress/sink-multigetbyoffset.js 2015-08-15 05:00:57 UTC (rev 188507)
</span><span class="lines">@@ -0,0 +1,27 @@
</span><ins>+// Regression test for https://bugs.webkit.org/show_bug.cgi?id=147165
+
+function Foo() { }
+Foo.prototype.f = 42;
+
+function get(o, p) {
+ if (p)
+ return o.f;
+ return 42;
+}
+
+for (var i = 0; i < 100000; ++i) {
+ get({ f: 42 }, i % 2);
+ get({ o: 10, f: 42 }, i % 2);
+}
+
+function foo() {
+ var o = new Foo();
+ return get(o, isFinalTier());
+}
+noInline(foo);
+
+for (var i = 0; i < 1000000; ++i) {
+ var result = foo();
+ if (result !== 42)
+ throw new Error("Result should be 42 but was " + result);
+}
</ins></span></pre>
</div>
</div>
</body>
</html>