<!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>[226208] 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/226208">226208</a></dd>
<dt>Author</dt> <dd>sbarati@apple.com</dd>
<dt>Date</dt> <dd>2017-12-20 17:54:46 -0800 (Wed, 20 Dec 2017)</dd>
</dl>

<h3>Log Message</h3>
<pre>GetPropertyEnumerator in DFG/FTL should not unconditionally speculate cell
https://bugs.webkit.org/show_bug.cgi?id=181054

Reviewed by Mark Lam.

Speedometer's react subtest has a function that is in an OSR exit loop because
we used to unconditionally speculate cell for the operand to GetPropertyEnumerator.
This fix doesn't seem to speed up Speedometer at all, but it's good hygiene
for our compiler to not have this pathology. This patch adds a generic
GetPropertyEnumerator to prevent the exit loop.

* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileGetPropertyEnumerator):
* jit/JITOperations.cpp:
* jit/JITOperations.h:</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGFixupPhasecpp">trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGSpeculativeJIT32_64cpp">trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGSpeculativeJIT64cpp">trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreftlFTLLowerDFGToB3cpp">trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorejitJITOperationscpp">trunk/Source/JavaScriptCore/jit/JITOperations.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorejitJITOperationsh">trunk/Source/JavaScriptCore/jit/JITOperations.h</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (226207 => 226208)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog    2017-12-21 01:49:00 UTC (rev 226207)
+++ trunk/Source/JavaScriptCore/ChangeLog       2017-12-21 01:54:46 UTC (rev 226208)
</span><span class="lines">@@ -1,3 +1,27 @@
</span><ins>+2017-12-20  Saam Barati  <sbarati@apple.com>
+
+        GetPropertyEnumerator in DFG/FTL should not unconditionally speculate cell
+        https://bugs.webkit.org/show_bug.cgi?id=181054
+
+        Reviewed by Mark Lam.
+
+        Speedometer's react subtest has a function that is in an OSR exit loop because
+        we used to unconditionally speculate cell for the operand to GetPropertyEnumerator.
+        This fix doesn't seem to speed up Speedometer at all, but it's good hygiene 
+        for our compiler to not have this pathology. This patch adds a generic
+        GetPropertyEnumerator to prevent the exit loop.
+
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::fixupNode):
+        * dfg/DFGSpeculativeJIT32_64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileGetPropertyEnumerator):
+        * jit/JITOperations.cpp:
+        * jit/JITOperations.h:
+
</ins><span class="cx"> 2017-12-20  Daniel Bates  <dabates@apple.com>
</span><span class="cx"> 
</span><span class="cx">         Remove Alternative Presentation Button
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGFixupPhasecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp (226207 => 226208)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp        2017-12-21 01:49:00 UTC (rev 226207)
+++ trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp   2017-12-21 01:54:46 UTC (rev 226208)
</span><span class="lines">@@ -1725,7 +1725,8 @@
</span><span class="cx">             break;
</span><span class="cx">         }
</span><span class="cx">         case GetPropertyEnumerator: {
</span><del>-            fixEdge<CellUse>(node->child1());
</del><ins>+            if (node->child1()->shouldSpeculateCell())
+                fixEdge<CellUse>(node->child1());
</ins><span class="cx">             break;
</span><span class="cx">         }
</span><span class="cx">         case GetEnumeratorStructurePname: {
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGSpeculativeJIT32_64cpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp (226207 => 226208)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp       2017-12-21 01:49:00 UTC (rev 226207)
+++ trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp  2017-12-21 01:54:46 UTC (rev 226208)
</span><span class="lines">@@ -5358,14 +5358,25 @@
</span><span class="cx">         break;
</span><span class="cx">     }
</span><span class="cx">     case GetPropertyEnumerator: {
</span><del>-        SpeculateCellOperand base(this, node->child1());
-        GPRFlushedCallResult result(this);
-        GPRReg resultGPR = result.gpr();
</del><ins>+        if (node->child1().useKind() == CellUse) {
+            SpeculateCellOperand base(this, node->child1());
+            GPRFlushedCallResult result(this);
+            GPRReg resultGPR = result.gpr();
</ins><span class="cx"> 
</span><del>-        flushRegisters();
-        callOperation(operationGetPropertyEnumerator, resultGPR, base.gpr());
-        m_jit.exceptionCheck();
-        cellResult(resultGPR, node);
</del><ins>+            flushRegisters();
+            callOperation(operationGetPropertyEnumeratorCell, resultGPR, base.gpr());
+            m_jit.exceptionCheck();
+            cellResult(resultGPR, node);
+        } else {
+            JSValueOperand base(this, node->child1());
+            GPRFlushedCallResult result(this);
+            GPRReg resultGPR = result.gpr();
+
+            flushRegisters();
+            callOperation(operationGetPropertyEnumerator, resultGPR, base.jsValueRegs());
+            m_jit.exceptionCheck();
+            cellResult(resultGPR, node);
+        }
</ins><span class="cx">         break;
</span><span class="cx">     }
</span><span class="cx">     case GetEnumeratorStructurePname:
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGSpeculativeJIT64cpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp (226207 => 226208)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp  2017-12-21 01:49:00 UTC (rev 226207)
+++ trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp     2017-12-21 01:54:46 UTC (rev 226208)
</span><span class="lines">@@ -5802,12 +5802,19 @@
</span><span class="cx">         break;
</span><span class="cx">     }
</span><span class="cx">     case GetPropertyEnumerator: {
</span><del>-        SpeculateCellOperand base(this, node->child1());
</del><ins>+        JSValueOperand base(this, node->child1(), ManualOperandSpeculation);
</ins><span class="cx">         GPRFlushedCallResult result(this);
</span><span class="cx">         GPRReg resultGPR = result.gpr();
</span><ins>+        GPRReg baseGPR = base.gpr();
</ins><span class="cx"> 
</span><ins>+        if (node->child1().useKind() == CellUse)
+            speculateCell(node->child1());
+
</ins><span class="cx">         flushRegisters();
</span><del>-        callOperation(operationGetPropertyEnumerator, resultGPR, base.gpr());
</del><ins>+        if (node->child1().useKind() == CellUse)
+            callOperation(operationGetPropertyEnumeratorCell, resultGPR, baseGPR);
+        else
+            callOperation(operationGetPropertyEnumerator, resultGPR, baseGPR);
</ins><span class="cx">         m_jit.exceptionCheck();
</span><span class="cx">         cellResult(resultGPR, node);
</span><span class="cx">         break;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreftlFTLLowerDFGToB3cpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp (226207 => 226208)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp      2017-12-21 01:49:00 UTC (rev 226207)
+++ trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp 2017-12-21 01:54:46 UTC (rev 226208)
</span><span class="lines">@@ -9733,8 +9733,10 @@
</span><span class="cx"> 
</span><span class="cx">     void compileGetPropertyEnumerator()
</span><span class="cx">     {
</span><del>-        LValue base = lowCell(m_node->child1());
-        setJSValue(vmCall(Int64, m_out.operation(operationGetPropertyEnumerator), m_callFrame, base));
</del><ins>+        if (m_node->child1().useKind() == CellUse)
+            setJSValue(vmCall(Int64, m_out.operation(operationGetPropertyEnumeratorCell), m_callFrame, lowCell(m_node->child1())));
+        else
+            setJSValue(vmCall(Int64, m_out.operation(operationGetPropertyEnumerator), m_callFrame, lowJSValue(m_node->child1())));
</ins><span class="cx">     }
</span><span class="cx"> 
</span><span class="cx">     void compileGetEnumeratorStructurePname()
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorejitJITOperationscpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/jit/JITOperations.cpp (226207 => 226208)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/jit/JITOperations.cpp        2017-12-21 01:49:00 UTC (rev 226207)
+++ trunk/Source/JavaScriptCore/jit/JITOperations.cpp   2017-12-21 01:54:46 UTC (rev 226208)
</span><span class="lines">@@ -2394,7 +2394,7 @@
</span><span class="cx">     return JSValue::encode(jsBoolean(base->hasPropertyGeneric(exec, asString(propertyName)->toIdentifier(exec), PropertySlot::InternalMethodType::GetOwnProperty)));
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-JSCell* JIT_OPERATION operationGetPropertyEnumerator(ExecState* exec, JSCell* cell)
</del><ins>+JSCell* JIT_OPERATION operationGetPropertyEnumeratorCell(ExecState* exec, JSCell* cell)
</ins><span class="cx"> {
</span><span class="cx">     VM& vm = exec->vm();
</span><span class="cx">     NativeCallFrameTracer tracer(&vm, exec);
</span><span class="lines">@@ -2407,6 +2407,23 @@
</span><span class="cx">     return propertyNameEnumerator(exec, base);
</span><span class="cx"> }
</span><span class="cx"> 
</span><ins>+JSCell* JIT_OPERATION operationGetPropertyEnumerator(ExecState* exec, EncodedJSValue encodedBase)
+{
+    VM& vm = exec->vm();
+    NativeCallFrameTracer tracer(&vm, exec);
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
+    JSValue base = JSValue::decode(encodedBase);
+    if (base.isUndefinedOrNull())
+        return JSPropertyNameEnumerator::create(vm);
+
+    JSObject* baseObject = base.toObject(exec);
+    RETURN_IF_EXCEPTION(scope, { });
+
+    scope.release();
+    return propertyNameEnumerator(exec, baseObject);
+}
+
</ins><span class="cx"> EncodedJSValue JIT_OPERATION operationNextEnumeratorPname(ExecState* exec, JSCell* enumeratorCell, int32_t index)
</span><span class="cx"> {
</span><span class="cx">     VM& vm = exec->vm();
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorejitJITOperationsh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/jit/JITOperations.h (226207 => 226208)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/jit/JITOperations.h  2017-12-21 01:49:00 UTC (rev 226207)
+++ trunk/Source/JavaScriptCore/jit/JITOperations.h     2017-12-21 01:54:46 UTC (rev 226208)
</span><span class="lines">@@ -461,7 +461,8 @@
</span><span class="cx"> int32_t JIT_OPERATION operationInstanceOfCustom(ExecState*, EncodedJSValue encodedValue, JSObject* constructor, EncodedJSValue encodedHasInstance) WTF_INTERNAL;
</span><span class="cx"> 
</span><span class="cx"> EncodedJSValue JIT_OPERATION operationHasGenericProperty(ExecState*, EncodedJSValue, JSCell*);
</span><del>-JSCell* JIT_OPERATION operationGetPropertyEnumerator(ExecState*, JSCell*);
</del><ins>+JSCell* JIT_OPERATION operationGetPropertyEnumeratorCell(ExecState*, JSCell*);
+JSCell* JIT_OPERATION operationGetPropertyEnumerator(ExecState*, EncodedJSValue);
</ins><span class="cx"> EncodedJSValue JIT_OPERATION operationNextEnumeratorPname(ExecState*, JSCell*, int32_t);
</span><span class="cx"> JSCell* JIT_OPERATION operationToIndexString(ExecState*, int32_t);
</span><span class="cx"> 
</span></span></pre>
</div>
</div>

</body>
</html>