<!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>[174224] 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/174224">174224</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2014-10-02 12:38:08 -0700 (Thu, 02 Oct 2014)</dd>
</dl>

<h3>Log Message</h3>
<pre>Object allocation sinking should have a sound story for picking materialization points
https://bugs.webkit.org/show_bug.cgi?id=137315

Reviewed by Oliver Hunt.
        
The only missing piece was having the object allocation sinking phase locate materialization
points that were at CFG edges.
        
The logic for how and why this &quot;just works&quot; relies on some properties of critical edge
breaking, so I was fairly careful in how I did this. Also, this requires inserting things at
the &quot;first origin node&quot; of a block - that is the first node in a block that has a NodeOrigin
and therefore is allowed to exit. We basically had support for such a notion before, but
didn't close the loop on it; this patch does that.
        
Also I added the ability to provide a BasicBlock* as context for a DFG_ASSERT().

* dfg/DFGBasicBlock.cpp:
(JSC::DFG::BasicBlock::firstOriginNode):
(JSC::DFG::BasicBlock::firstOrigin):
* dfg/DFGBasicBlock.h:
* dfg/DFGCriticalEdgeBreakingPhase.cpp:
(JSC::DFG::CriticalEdgeBreakingPhase::breakCriticalEdge):
* dfg/DFGGraph.cpp:
(JSC::DFG::crash):
(JSC::DFG::Graph::handleAssertionFailure):
* dfg/DFGGraph.h:
* dfg/DFGLoopPreHeaderCreationPhase.cpp:
(JSC::DFG::createPreHeader):
* dfg/DFGNodeOrigin.h:
(JSC::DFG::NodeOrigin::isSet):
* dfg/DFGObjectAllocationSinkingPhase.cpp:
(JSC::DFG::ObjectAllocationSinkingPhase::determineMaterializationPoints):
(JSC::DFG::ObjectAllocationSinkingPhase::placeMaterializationPoints):
(JSC::DFG::ObjectAllocationSinkingPhase::promoteSunkenFields):
(JSC::DFG::ObjectAllocationSinkingPhase::createMaterialize):
* dfg/DFGValidate.cpp:
(JSC::DFG::Validate::validate):
* runtime/Options.h:</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGBasicBlockcpp">trunk/Source/JavaScriptCore/dfg/DFGBasicBlock.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGBasicBlockh">trunk/Source/JavaScriptCore/dfg/DFGBasicBlock.h</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGCriticalEdgeBreakingPhasecpp">trunk/Source/JavaScriptCore/dfg/DFGCriticalEdgeBreakingPhase.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGGraphcpp">trunk/Source/JavaScriptCore/dfg/DFGGraph.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGGraphh">trunk/Source/JavaScriptCore/dfg/DFGGraph.h</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGLoopPreHeaderCreationPhasecpp">trunk/Source/JavaScriptCore/dfg/DFGLoopPreHeaderCreationPhase.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGNodeOriginh">trunk/Source/JavaScriptCore/dfg/DFGNodeOrigin.h</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGObjectAllocationSinkingPhasecpp">trunk/Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGValidatecpp">trunk/Source/JavaScriptCore/dfg/DFGValidate.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeOptionsh">trunk/Source/JavaScriptCore/runtime/Options.h</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (174223 => 174224)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog        2014-10-02 19:14:57 UTC (rev 174223)
+++ trunk/Source/JavaScriptCore/ChangeLog        2014-10-02 19:38:08 UTC (rev 174224)
</span><span class="lines">@@ -1,3 +1,44 @@
</span><ins>+2014-10-02  Filip Pizlo  &lt;fpizlo@apple.com&gt;
+
+        Object allocation sinking should have a sound story for picking materialization points
+        https://bugs.webkit.org/show_bug.cgi?id=137315
+
+        Reviewed by Oliver Hunt.
+        
+        The only missing piece was having the object allocation sinking phase locate materialization
+        points that were at CFG edges.
+        
+        The logic for how and why this &quot;just works&quot; relies on some properties of critical edge
+        breaking, so I was fairly careful in how I did this. Also, this requires inserting things at
+        the &quot;first origin node&quot; of a block - that is the first node in a block that has a NodeOrigin
+        and therefore is allowed to exit. We basically had support for such a notion before, but
+        didn't close the loop on it; this patch does that.
+        
+        Also I added the ability to provide a BasicBlock* as context for a DFG_ASSERT().
+
+        * dfg/DFGBasicBlock.cpp:
+        (JSC::DFG::BasicBlock::firstOriginNode):
+        (JSC::DFG::BasicBlock::firstOrigin):
+        * dfg/DFGBasicBlock.h:
+        * dfg/DFGCriticalEdgeBreakingPhase.cpp:
+        (JSC::DFG::CriticalEdgeBreakingPhase::breakCriticalEdge):
+        * dfg/DFGGraph.cpp:
+        (JSC::DFG::crash):
+        (JSC::DFG::Graph::handleAssertionFailure):
+        * dfg/DFGGraph.h:
+        * dfg/DFGLoopPreHeaderCreationPhase.cpp:
+        (JSC::DFG::createPreHeader):
+        * dfg/DFGNodeOrigin.h:
+        (JSC::DFG::NodeOrigin::isSet):
+        * dfg/DFGObjectAllocationSinkingPhase.cpp:
+        (JSC::DFG::ObjectAllocationSinkingPhase::determineMaterializationPoints):
+        (JSC::DFG::ObjectAllocationSinkingPhase::placeMaterializationPoints):
+        (JSC::DFG::ObjectAllocationSinkingPhase::promoteSunkenFields):
+        (JSC::DFG::ObjectAllocationSinkingPhase::createMaterialize):
+        * dfg/DFGValidate.cpp:
+        (JSC::DFG::Validate::validate):
+        * runtime/Options.h:
+
</ins><span class="cx"> 2014-10-02  Daniel Bates  &lt;dabates@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Clean up: Move XPC forward declarations in JavaScriptCore to WTF SPI wrapper header
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGBasicBlockcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGBasicBlock.cpp (174223 => 174224)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGBasicBlock.cpp        2014-10-02 19:14:57 UTC (rev 174223)
+++ trunk/Source/JavaScriptCore/dfg/DFGBasicBlock.cpp        2014-10-02 19:38:08 UTC (rev 174224)
</span><span class="lines">@@ -89,6 +89,21 @@
</span><span class="cx">     return false;
</span><span class="cx"> }
</span><span class="cx"> 
</span><ins>+Node* BasicBlock::firstOriginNode()
+{
+    for (Node* node : *this) {
+        if (node-&gt;origin.isSet())
+            return node;
+    }
+    RELEASE_ASSERT_NOT_REACHED();
+    return nullptr;
+}
+
+NodeOrigin BasicBlock::firstOrigin()
+{
+    return firstOriginNode()-&gt;origin;
+}
+
</ins><span class="cx"> void BasicBlock::removePredecessor(BasicBlock* block)
</span><span class="cx"> {
</span><span class="cx">     for (unsigned i = 0; i &lt; predecessors.size(); ++i) {
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGBasicBlockh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGBasicBlock.h (174223 => 174224)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGBasicBlock.h        2014-10-02 19:14:57 UTC (rev 174223)
+++ trunk/Source/JavaScriptCore/dfg/DFGBasicBlock.h        2014-10-02 19:38:08 UTC (rev 174224)
</span><span class="lines">@@ -34,6 +34,7 @@
</span><span class="cx"> #include &quot;DFGBranchDirection.h&quot;
</span><span class="cx"> #include &quot;DFGFlushedAt.h&quot;
</span><span class="cx"> #include &quot;DFGNode.h&quot;
</span><ins>+#include &quot;DFGNodeOrigin.h&quot;
</ins><span class="cx"> #include &quot;DFGStructureClobberState.h&quot;
</span><span class="cx"> #include &quot;Operands.h&quot;
</span><span class="cx"> #include &lt;wtf/HashMap.h&gt;
</span><span class="lines">@@ -90,6 +91,9 @@
</span><span class="cx">     BlockNodeList::iterator begin() { return m_nodes.begin(); }
</span><span class="cx">     BlockNodeList::iterator end() { return m_nodes.end(); }
</span><span class="cx">     
</span><ins>+    Node* firstOriginNode();
+    NodeOrigin firstOrigin();
+    
</ins><span class="cx">     unsigned numSuccessors() { return last()-&gt;numSuccessors(); }
</span><span class="cx">     
</span><span class="cx">     BasicBlock*&amp; successor(unsigned index)
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGCriticalEdgeBreakingPhasecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGCriticalEdgeBreakingPhase.cpp (174223 => 174224)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGCriticalEdgeBreakingPhase.cpp        2014-10-02 19:14:57 UTC (rev 174223)
+++ trunk/Source/JavaScriptCore/dfg/DFGCriticalEdgeBreakingPhase.cpp        2014-10-02 19:38:08 UTC (rev 174224)
</span><span class="lines">@@ -77,7 +77,7 @@
</span><span class="cx">         // don't know its execution frequency.
</span><span class="cx">         BasicBlock* pad = m_insertionSet.insertBefore(*successor, PNaN);
</span><span class="cx">         pad-&gt;appendNode(
</span><del>-            m_graph, SpecNone, Jump, (*successor)-&gt;at(0)-&gt;origin, OpInfo(*successor));
</del><ins>+            m_graph, SpecNone, Jump, (*successor)-&gt;firstOrigin(), OpInfo(*successor));
</ins><span class="cx">         pad-&gt;predecessors.append(predecessor);
</span><span class="cx">         (*successor)-&gt;replacePredecessor(predecessor, pad);
</span><span class="cx">         
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGGraphcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGGraph.cpp (174223 => 174224)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGGraph.cpp        2014-10-02 19:14:57 UTC (rev 174223)
+++ trunk/Source/JavaScriptCore/dfg/DFGGraph.cpp        2014-10-02 19:38:08 UTC (rev 174224)
</span><span class="lines">@@ -1212,25 +1212,41 @@
</span><span class="cx">     DFG_CRASH(*this, nullptr, toCString(&quot;Structure &quot;, pointerDump(structure), &quot; is watchable but isn't being watched.&quot;).data());
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-void Graph::handleAssertionFailure(
-    Node* node, const char* file, int line, const char* function, const char* assertion)
</del><ins>+NO_RETURN_DUE_TO_CRASH static void crash(
+    Graph&amp; graph, const CString&amp; whileText, const char* file, int line, const char* function,
+    const char* assertion)
</ins><span class="cx"> {
</span><span class="cx">     startCrashing();
</span><span class="cx">     dataLog(&quot;DFG ASSERTION FAILED: &quot;, assertion, &quot;\n&quot;);
</span><span class="cx">     dataLog(file, &quot;(&quot;, line, &quot;) : &quot;, function, &quot;\n&quot;);
</span><span class="cx">     dataLog(&quot;\n&quot;);
</span><del>-    if (node) {
-        dataLog(&quot;While handling node &quot;, node, &quot;\n&quot;);
-        dataLog(&quot;\n&quot;);
-    }
</del><ins>+    dataLog(whileText);
</ins><span class="cx">     dataLog(&quot;Graph at time of failure:\n&quot;);
</span><del>-    dump();
</del><ins>+    graph.dump();
</ins><span class="cx">     dataLog(&quot;\n&quot;);
</span><span class="cx">     dataLog(&quot;DFG ASSERTION FAILED: &quot;, assertion, &quot;\n&quot;);
</span><span class="cx">     dataLog(file, &quot;(&quot;, line, &quot;) : &quot;, function, &quot;\n&quot;);
</span><span class="cx">     CRASH_WITH_SECURITY_IMPLICATION();
</span><span class="cx"> }
</span><span class="cx"> 
</span><ins>+void Graph::handleAssertionFailure(
+    std::nullptr_t, const char* file, int line, const char* function, const char* assertion)
+{
+    crash(*this, &quot;&quot;, file, line, function, assertion);
+}
+
+void Graph::handleAssertionFailure(
+    Node* node, const char* file, int line, const char* function, const char* assertion)
+{
+    crash(*this, toCString(&quot;While handling node &quot;, node, &quot;\n\n&quot;), file, line, function, assertion);
+}
+
+void Graph::handleAssertionFailure(
+    BasicBlock* block, const char* file, int line, const char* function, const char* assertion)
+{
+    crash(*this, toCString(&quot;While handling block &quot;, pointerDump(block), &quot;\n\n&quot;), file, line, function, assertion);
+}
+
</ins><span class="cx"> } } // namespace JSC::DFG
</span><span class="cx"> 
</span><span class="cx"> #endif // ENABLE(DFG_JIT)
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGGraphh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGGraph.h (174223 => 174224)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGGraph.h        2014-10-02 19:14:57 UTC (rev 174223)
+++ trunk/Source/JavaScriptCore/dfg/DFGGraph.h        2014-10-02 19:38:08 UTC (rev 174224)
</span><span class="lines">@@ -845,8 +845,14 @@
</span><span class="cx">     virtual void visitChildren(SlotVisitor&amp;) override;
</span><span class="cx">     
</span><span class="cx">     NO_RETURN_DUE_TO_CRASH void handleAssertionFailure(
</span><del>-        Node* node, const char* file, int line, const char* function,
</del><ins>+        std::nullptr_t, const char* file, int line, const char* function,
</ins><span class="cx">         const char* assertion);
</span><ins>+    NO_RETURN_DUE_TO_CRASH void handleAssertionFailure(
+        Node*, const char* file, int line, const char* function,
+        const char* assertion);
+    NO_RETURN_DUE_TO_CRASH void handleAssertionFailure(
+        BasicBlock*, const char* file, int line, const char* function,
+        const char* assertion);
</ins><span class="cx">     
</span><span class="cx">     VM&amp; m_vm;
</span><span class="cx">     Plan&amp; m_plan;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGLoopPreHeaderCreationPhasecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGLoopPreHeaderCreationPhase.cpp (174223 => 174224)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGLoopPreHeaderCreationPhase.cpp        2014-10-02 19:14:57 UTC (rev 174223)
+++ trunk/Source/JavaScriptCore/dfg/DFGLoopPreHeaderCreationPhase.cpp        2014-10-02 19:38:08 UTC (rev 174224)
</span><span class="lines">@@ -42,7 +42,7 @@
</span><span class="cx">     // Don't bother to preserve execution frequencies for now.
</span><span class="cx">     BasicBlock* preHeader = insertionSet.insertBefore(block, PNaN);
</span><span class="cx">     preHeader-&gt;appendNode(
</span><del>-        graph, SpecNone, Jump, block-&gt;at(0)-&gt;origin, OpInfo(block));
</del><ins>+        graph, SpecNone, Jump, block-&gt;firstOrigin(), OpInfo(block));
</ins><span class="cx">     
</span><span class="cx">     for (unsigned predecessorIndex = 0; predecessorIndex &lt; block-&gt;predecessors.size(); predecessorIndex++) {
</span><span class="cx">         BasicBlock* predecessor = block-&gt;predecessors[predecessorIndex];
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGNodeOriginh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGNodeOrigin.h (174223 => 174224)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGNodeOrigin.h        2014-10-02 19:14:57 UTC (rev 174223)
+++ trunk/Source/JavaScriptCore/dfg/DFGNodeOrigin.h        2014-10-02 19:38:08 UTC (rev 174224)
</span><span class="lines">@@ -49,6 +49,7 @@
</span><span class="cx">     
</span><span class="cx">     bool isSet() const
</span><span class="cx">     {
</span><ins>+        ASSERT(semantic.isSet() == forExit.isSet());
</ins><span class="cx">         return semantic.isSet();
</span><span class="cx">     }
</span><span class="cx">     
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGObjectAllocationSinkingPhasecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp (174223 => 174224)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp        2014-10-02 19:14:57 UTC (rev 174223)
+++ trunk/Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp        2014-10-02 19:38:08 UTC (rev 174224)
</span><span class="lines">@@ -145,7 +145,8 @@
</span><span class="cx">         // materializations.
</span><span class="cx">         
</span><span class="cx">         m_sinkCandidates.clear();
</span><del>-        m_edgeToMaterializationPoint.clear();
</del><ins>+        m_materializationToEscapee.clear();
+        m_materializationSiteToMaterializations.clear();
</ins><span class="cx">         
</span><span class="cx">         BlockMap&lt;HashMap&lt;Node*, bool&gt;&gt; materializedAtHead(m_graph);
</span><span class="cx">         BlockMap&lt;HashMap&lt;Node*, bool&gt;&gt; materializedAtTail(m_graph);
</span><span class="lines">@@ -235,7 +236,14 @@
</span><span class="cx">             return;
</span><span class="cx">         
</span><span class="cx">         // A materialization edge exists at any point where a node escapes but hasn't been
</span><del>-        // materialized yet.
</del><ins>+        // materialized yet. We do this in two parts. First we find all of the nodes that cause
+        // escaping to happen, where the escapee had not yet been materialized. This catches
+        // everything but loops. We then catch loops - as well as weirder control flow constructs -
+        // in a subsequent pass that looks at places in the CFG where an edge exists from a block
+        // that hasn't materialized to a block that has. We insert the materialization along such an
+        // edge, and we rely on the fact that critical edges were already broken so that we actually
+        // either insert the materialization at the head of the successor or the tail of the
+        // predecessor.
</ins><span class="cx">         //
</span><span class="cx">         // FIXME: This can create duplicate allocations when we really only needed to perform one.
</span><span class="cx">         // For example:
</span><span class="lines">@@ -254,6 +262,8 @@
</span><span class="cx">         // materialization point right at the top of the then case of &quot;if (rare)&quot;. To do this, we can
</span><span class="cx">         // find the LCA of the various materializations in the dom tree.
</span><span class="cx">         // https://bugs.webkit.org/show_bug.cgi?id=137124
</span><ins>+        
+        // First pass: find intra-block materialization points.
</ins><span class="cx">         for (BasicBlock* block : m_graph.blocksInNaturalOrder()) {
</span><span class="cx">             HashSet&lt;Node*&gt; materialized;
</span><span class="cx">             for (auto pair : materializedAtHead[block]) {
</span><span class="lines">@@ -274,20 +284,70 @@
</span><span class="cx">                         if (!materialized.add(escapee).isNewEntry)
</span><span class="cx">                             return;
</span><span class="cx">                         
</span><del>-                        Node* materialize = createMaterialize(escapee, node-&gt;origin);
-                        if (verbose)
-                            dataLog(&quot;Adding materialization point: &quot;, node, &quot;-&gt;&quot;, escapee, &quot; = &quot;, materialize, &quot;\n&quot;);
-                        m_edgeToMaterializationPoint.add(
-                            std::make_pair(node, escapee), materialize);
</del><ins>+                        createMaterialize(escapee, node);
</ins><span class="cx">                     });
</span><span class="cx">             }
</span><span class="cx">         }
</span><ins>+        
+        // Second pass: find CFG edge materialization points.
+        for (BasicBlock* block : m_graph.blocksInNaturalOrder()) {
+            for (BasicBlock* successorBlock : block-&gt;successors()) {
+                for (auto pair : materializedAtHead[successorBlock]) {
+                    Node* allocation = pair.key;
+                    
+                    // We only care if it is materialized in the successor.
+                    if (!pair.value)
+                        continue;
+                    
+                    // We only care about sinking candidates.
+                    if (!m_sinkCandidates.contains(allocation))
+                        continue;
+                    
+                    // We only care if it isn't materialized in the predecessor.
+                    if (materializedAtTail[block].get(allocation))
+                        continue;
+                    
+                    // We need to decide if we put the materialization at the head of the successor,
+                    // or the tail of the predecessor. It needs to be placed so that the allocation
+                    // is never materialized before, and always materialized after.
+                    
+                    // Is it never materialized in any of successor's predecessors? I like to think
+                    // of &quot;successors' predecessors&quot; and &quot;predecessor's successors&quot; as the &quot;shadow&quot;,
+                    // because of what it looks like when you draw it.
+                    bool neverMaterializedInShadow = true;
+                    for (BasicBlock* shadowBlock : successorBlock-&gt;predecessors) {
+                        if (materializedAtTail[shadowBlock].get(allocation)) {
+                            neverMaterializedInShadow = false;
+                            break;
+                        }
+                    }
+                    
+                    if (neverMaterializedInShadow) {
+                        createMaterialize(allocation, successorBlock-&gt;firstOriginNode());
+                        continue;
+                    }
+                    
+                    // Because we had broken critical edges, it must be the case that the
+                    // predecessor's successors all materialize the object. This is because the
+                    // previous case is guaranteed to catch the case where the successor only has
+                    // one predecessor. When critical edges are broken, this is also the only case
+                    // where the predecessor could have had multiple successors. Therefore we have
+                    // already handled the case where the predecessor has multiple successors.
+                    DFG_ASSERT(m_graph, block, block-&gt;numSuccessors() == 1);
+                    
+                    createMaterialize(allocation, block-&gt;last());
+                }
+            }
+        }
</ins><span class="cx">     }
</span><span class="cx">     
</span><span class="cx">     void placeMaterializationPoints()
</span><span class="cx">     {
</span><span class="cx">         m_ssaCalculator.reset();
</span><span class="cx">         
</span><ins>+        // The &quot;variables&quot; are the object allocations that we are sinking. So, nodeToVariable maps
+        // sink candidates (aka escapees) to the SSACalculator's notion of Variable, and indexToNode
+        // maps in the opposite direction using the SSACalculator::Variable::index() as the key.
</ins><span class="cx">         HashMap&lt;Node*, SSACalculator::Variable*&gt; nodeToVariable;
</span><span class="cx">         Vector&lt;Node*&gt; indexToNode;
</span><span class="cx">         
</span><span class="lines">@@ -303,17 +363,11 @@
</span><span class="cx">                 if (SSACalculator::Variable* variable = nodeToVariable.get(node))
</span><span class="cx">                     m_ssaCalculator.newDef(variable, block, node);
</span><span class="cx">                 
</span><del>-                m_graph.doToChildren(
-                    node,
-                    [&amp;] (Edge edge) {
-                        Node* materialize =
-                            m_edgeToMaterializationPoint.get(std::make_pair(node, edge.node()));
-                        if (!materialize)
-                            return;
-                        
-                        m_ssaCalculator.newDef(
-                            nodeToVariable.get(edge.node()), block, materialize);
-                    });
</del><ins>+                for (Node* materialize : m_materializationSiteToMaterializations.get(node)) {
+                    m_ssaCalculator.newDef(
+                        nodeToVariable.get(m_materializationToEscapee.get(materialize)),
+                        block, materialize);
+                }
</ins><span class="cx">             }
</span><span class="cx">         }
</span><span class="cx">         
</span><span class="lines">@@ -361,20 +415,15 @@
</span><span class="cx">             for (unsigned nodeIndex = 0; nodeIndex &lt; block-&gt;size(); ++nodeIndex) {
</span><span class="cx">                 Node* node = block-&gt;at(nodeIndex);
</span><span class="cx"> 
</span><del>-                m_graph.doToChildren(
-                    node,
-                    [&amp;] (Edge edge) {
-                        Node* materialize = m_edgeToMaterializationPoint.get(
-                            std::make_pair(node, edge.node()));
-                        if (materialize) {
-                            m_insertionSet.insert(nodeIndex, materialize);
-                            insertOSRHintsForUpdate(
-                                m_insertionSet, nodeIndex, node-&gt;origin,
-                                availabilityCalculator.m_availability, edge.node(), materialize);
-                            mapping.set(edge.node(), materialize);
-                        }
-                    });
-
</del><ins>+                for (Node* materialize : m_materializationSiteToMaterializations.get(node)) {
+                    Node* escapee = m_materializationToEscapee.get(materialize);
+                    m_insertionSet.insert(nodeIndex, materialize);
+                    insertOSRHintsForUpdate(
+                        m_insertionSet, nodeIndex, node-&gt;origin,
+                        availabilityCalculator.m_availability, escapee, materialize);
+                    mapping.set(escapee, materialize);
+                }
+                    
</ins><span class="cx">                 availabilityCalculator.executeNode(node);
</span><span class="cx">                 
</span><span class="cx">                 m_graph.doToChildren(
</span><span class="lines">@@ -386,7 +435,8 @@
</span><span class="cx">             }
</span><span class="cx">             
</span><span class="cx">             size_t upsilonInsertionPoint = block-&gt;size() - 1;
</span><del>-            NodeOrigin upsilonOrigin = block-&gt;last()-&gt;origin;
</del><ins>+            Node* upsilonWhere = block-&gt;last();
+            NodeOrigin upsilonOrigin = upsilonWhere-&gt;origin;
</ins><span class="cx">             for (BasicBlock* successorBlock : block-&gt;successors()) {
</span><span class="cx">                 for (SSACalculator::Def* phiDef : m_ssaCalculator.phisForBlock(successorBlock)) {
</span><span class="cx">                     Node* phiNode = phiDef-&gt;value();
</span><span class="lines">@@ -399,7 +449,7 @@
</span><span class="cx">                         // If we have a Phi that combines materializations with the original
</span><span class="cx">                         // phantom object, then the path with the phantom object must materialize.
</span><span class="cx">                         
</span><del>-                        incoming = createMaterialize(allocation, upsilonOrigin);
</del><ins>+                        incoming = createMaterialize(allocation, upsilonWhere);
</ins><span class="cx">                         m_insertionSet.insert(upsilonInsertionPoint, incoming);
</span><span class="cx">                         insertOSRHintsForUpdate(
</span><span class="cx">                             m_insertionSet, upsilonInsertionPoint, upsilonOrigin,
</span><span class="lines">@@ -407,14 +457,9 @@
</span><span class="cx">                     } else
</span><span class="cx">                         incoming = originalIncoming;
</span><span class="cx">                     
</span><del>-                    Node* upsilon = m_insertionSet.insertNode(
</del><ins>+                    m_insertionSet.insertNode(
</ins><span class="cx">                         upsilonInsertionPoint, SpecNone, Upsilon, upsilonOrigin,
</span><span class="cx">                         OpInfo(phiNode), incoming-&gt;defaultEdge());
</span><del>-                    
-                    if (originalIncoming == allocation) {
-                        m_edgeToMaterializationPoint.add(
-                            std::make_pair(upsilon, allocation), incoming);
-                    }
</del><span class="cx">                 }
</span><span class="cx">             }
</span><span class="cx">             
</span><span class="lines">@@ -502,12 +547,6 @@
</span><span class="cx">     
</span><span class="cx">     void promoteSunkenFields()
</span><span class="cx">     {
</span><del>-        // Henceforth when we encounter a materialization point, we will want to ask *who* it is
-        // a materialization for. Invert the map to be able to answer such questions.
-        m_materializationPointToEscapee.clear();
-        for (auto pair : m_edgeToMaterializationPoint)
-            m_materializationPointToEscapee.add(pair.value, pair.key.second);
-        
</del><span class="cx">         // Collect the set of heap locations that we will be operating over.
</span><span class="cx">         HashSet&lt;PromotedHeapLocation&gt; locations;
</span><span class="cx">         for (BasicBlock* block : m_graph.blocksInNaturalOrder()) {
</span><span class="lines">@@ -615,7 +654,7 @@
</span><span class="cx">             for (Node* node : *block) {
</span><span class="cx">                 m_graph.performSubstitution(node);
</span><span class="cx">                 
</span><del>-                if (Node* escapee = m_materializationPointToEscapee.get(node))
</del><ins>+                if (Node* escapee = m_materializationToEscapee.get(node))
</ins><span class="cx">                     populateMaterialize(block, node, escapee);
</span><span class="cx">                 
</span><span class="cx">                 promoteHeapAccess(
</span><span class="lines">@@ -717,26 +756,37 @@
</span><span class="cx">         }
</span><span class="cx">     }
</span><span class="cx">     
</span><del>-    Node* createMaterialize(Node* escapee, const NodeOrigin whereOrigin)
</del><ins>+    Node* createMaterialize(Node* escapee, Node* where)
</ins><span class="cx">     {
</span><ins>+        Node* result = nullptr;
+        
</ins><span class="cx">         switch (escapee-&gt;op()) {
</span><span class="cx">         case NewObject:
</span><span class="cx">         case MaterializeNewObject: {
</span><span class="cx">             ObjectMaterializationData* data = m_graph.m_objectMaterializationData.add();
</span><span class="cx">             
</span><del>-            Node* result = m_graph.addNode(
</del><ins>+            result = m_graph.addNode(
</ins><span class="cx">                 escapee-&gt;prediction(), Node::VarArg, MaterializeNewObject,
</span><span class="cx">                 NodeOrigin(
</span><span class="cx">                     escapee-&gt;origin.semantic,
</span><del>-                    whereOrigin.forExit),
</del><ins>+                    where-&gt;origin.forExit),
</ins><span class="cx">                 OpInfo(data), OpInfo(), 0, 0);
</span><del>-            return result;
</del><ins>+            break;
</ins><span class="cx">         }
</span><span class="cx">             
</span><span class="cx">         default:
</span><span class="cx">             DFG_CRASH(m_graph, escapee, &quot;Bad escapee op&quot;);
</span><del>-            return nullptr;
</del><ins>+            break;
</ins><span class="cx">         }
</span><ins>+        
+        if (verbose)
+            dataLog(&quot;Creating materialization point at &quot;, where, &quot; for &quot;, escapee, &quot;: &quot;, result, &quot;\n&quot;);
+        
+        m_materializationToEscapee.add(result, escapee);
+        m_materializationSiteToMaterializations.add(
+            where, Vector&lt;Node*&gt;()).iterator-&gt;value.append(result);
+        
+        return result;
</ins><span class="cx">     }
</span><span class="cx">     
</span><span class="cx">     void populateMaterialize(BasicBlock* block, Node* node, Node* escapee)
</span><span class="lines">@@ -791,8 +841,8 @@
</span><span class="cx">     
</span><span class="cx">     SSACalculator m_ssaCalculator;
</span><span class="cx">     HashSet&lt;Node*&gt; m_sinkCandidates;
</span><del>-    HashMap&lt;std::pair&lt;Node*, Node*&gt;, Node*&gt; m_edgeToMaterializationPoint;
-    HashMap&lt;Node*, Node*&gt; m_materializationPointToEscapee;
</del><ins>+    HashMap&lt;Node*, Node*&gt; m_materializationToEscapee;
+    HashMap&lt;Node*, Vector&lt;Node*&gt;&gt; m_materializationSiteToMaterializations;
</ins><span class="cx">     HashMap&lt;Node*, Vector&lt;PromotedHeapLocation&gt;&gt; m_locationsForAllocation;
</span><span class="cx">     HashMap&lt;PromotedHeapLocation, SSACalculator::Variable*&gt; m_locationToVariable;
</span><span class="cx">     Vector&lt;PromotedHeapLocation&gt; m_indexToLocation;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGValidatecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGValidate.cpp (174223 => 174224)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGValidate.cpp        2014-10-02 19:14:57 UTC (rev 174223)
+++ trunk/Source/JavaScriptCore/dfg/DFGValidate.cpp        2014-10-02 19:38:08 UTC (rev 174224)
</span><span class="lines">@@ -213,6 +213,23 @@
</span><span class="cx">                 case Identity:
</span><span class="cx">                     VALIDATE((node), canonicalResultRepresentation(node-&gt;result()) == canonicalResultRepresentation(node-&gt;child1()-&gt;result()));
</span><span class="cx">                     break;
</span><ins>+                case SetLocal:
+                case PutLocal:
+                case Upsilon:
+                    VALIDATE((node), !!node-&gt;child1());
+                    switch (node-&gt;child1().useKind()) {
+                    case UntypedUse:
+                    case CellUse:
+                    case Int32Use:
+                    case Int52RepUse:
+                    case DoubleRepUse:
+                    case BooleanUse:
+                        break;
+                    default:
+                        VALIDATE((node), !&quot;Bad use kind&quot;);
+                        break;
+                    }
+                    break;
</ins><span class="cx">                 case MakeRope:
</span><span class="cx">                 case ValueAdd:
</span><span class="cx">                 case ArithAdd:
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeOptionsh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/Options.h (174223 => 174224)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/Options.h        2014-10-02 19:14:57 UTC (rev 174223)
+++ trunk/Source/JavaScriptCore/runtime/Options.h        2014-10-02 19:38:08 UTC (rev 174224)
</span><span class="lines">@@ -178,7 +178,7 @@
</span><span class="cx">     v(bool, enableCallEdgeProfiling, true) \
</span><span class="cx">     v(unsigned, frequentCallThreshold, 2) \
</span><span class="cx">     v(bool, optimizeNativeCalls, false) \
</span><del>-    v(bool, enableObjectAllocationSinking, false) \
</del><ins>+    v(bool, enableObjectAllocationSinking, true) \
</ins><span class="cx">     \
</span><span class="cx">     v(bool, enableConcurrentJIT, true) \
</span><span class="cx">     v(unsigned, numberOfDFGCompilerThreads, computeNumberOfWorkerThreads(2, 2) - 1) \
</span></span></pre>
</div>
</div>

</body>
</html>