<!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>[195488] 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/195488">195488</a></dd>
<dt>Author</dt> <dd>mark.lam@apple.com</dd>
<dt>Date</dt> <dd>2016-01-22 15:30:07 -0800 (Fri, 22 Jan 2016)</dd>
</dl>

<h3>Log Message</h3>
<pre>We should OSR exit with Int52Overflow when we fail to make an Int52 where we expect one.
https://bugs.webkit.org/show_bug.cgi?id=153379

Reviewed by Filip Pizlo.

In DFG::Graph::addShouldSpeculateMachineInt(), we check
!hasExitSite(add, Int52Overflow) when determining whether it's ok to speculate
that an operand is of type Int52 or not.  However, the Int52Rep code that
converts a double to Int52 will OSR exit with exit kind BadType instead.
This renders the hasExitSite() check in addShouldSpeculateMachineInt() useless.
This patch fixes this by changing Int52Rep to OSR exit with exit kind
Int52Overflow instead when it fails to convert a double to an Int52.

* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::terminateSpeculativeExecution):
(JSC::DFG::SpeculativeJIT::typeCheck):
(JSC::DFG::SpeculativeJIT::usedRegisters):
* dfg/DFGSpeculativeJIT.h:
(JSC::DFG::SpeculativeJIT::needsTypeCheck):
(JSC::DFG::SpeculativeJIT::speculateStringObjectForStructure):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::DFG::LowerDFGToLLVM::typeCheck):
(JSC::FTL::DFG::LowerDFGToLLVM::appendTypeCheck):
(JSC::FTL::DFG::LowerDFGToLLVM::doubleToStrictInt52):</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGSpeculativeJITcpp">trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGSpeculativeJITh">trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGSpeculativeJIT64cpp">trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreftlFTLLowerDFGToLLVMcpp">trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (195487 => 195488)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog        2016-01-22 23:26:42 UTC (rev 195487)
+++ trunk/Source/JavaScriptCore/ChangeLog        2016-01-22 23:30:07 UTC (rev 195488)
</span><span class="lines">@@ -1,3 +1,32 @@
</span><ins>+2016-01-22  Mark Lam  &lt;mark.lam@apple.com&gt;
+
+        We should OSR exit with Int52Overflow when we fail to make an Int52 where we expect one.
+        https://bugs.webkit.org/show_bug.cgi?id=153379
+
+        Reviewed by Filip Pizlo.
+
+        In DFG::Graph::addShouldSpeculateMachineInt(), we check
+        !hasExitSite(add, Int52Overflow) when determining whether it's ok to speculate
+        that an operand is of type Int52 or not.  However, the Int52Rep code that
+        converts a double to Int52 will OSR exit with exit kind BadType instead.
+        This renders the hasExitSite() check in addShouldSpeculateMachineInt() useless.
+        This patch fixes this by changing Int52Rep to OSR exit with exit kind
+        Int52Overflow instead when it fails to convert a double to an Int52.
+
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::terminateSpeculativeExecution):
+        (JSC::DFG::SpeculativeJIT::typeCheck):
+        (JSC::DFG::SpeculativeJIT::usedRegisters):
+        * dfg/DFGSpeculativeJIT.h:
+        (JSC::DFG::SpeculativeJIT::needsTypeCheck):
+        (JSC::DFG::SpeculativeJIT::speculateStringObjectForStructure):
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * ftl/FTLLowerDFGToLLVM.cpp:
+        (JSC::FTL::DFG::LowerDFGToLLVM::typeCheck):
+        (JSC::FTL::DFG::LowerDFGToLLVM::appendTypeCheck):
+        (JSC::FTL::DFG::LowerDFGToLLVM::doubleToStrictInt52):
+
</ins><span class="cx"> 2016-01-22  Saam barati  &lt;sbarati@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Current implementation of Parser::createSavePoint is a foot gun
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGSpeculativeJITcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp (195487 => 195488)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp        2016-01-22 23:26:42 UTC (rev 195487)
+++ trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp        2016-01-22 23:30:07 UTC (rev 195488)
</span><span class="lines">@@ -298,11 +298,11 @@
</span><span class="cx">     terminateSpeculativeExecution(kind, jsValueRegs, nodeUse.node());
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-void SpeculativeJIT::typeCheck(JSValueSource source, Edge edge, SpeculatedType typesPassedThrough, MacroAssembler::Jump jumpToFail)
</del><ins>+void SpeculativeJIT::typeCheck(JSValueSource source, Edge edge, SpeculatedType typesPassedThrough, MacroAssembler::Jump jumpToFail, ExitKind exitKind)
</ins><span class="cx"> {
</span><span class="cx">     ASSERT(needsTypeCheck(edge, typesPassedThrough));
</span><span class="cx">     m_interpreter.filter(edge, typesPassedThrough);
</span><del>-    speculationCheck(BadType, source, edge.node(), jumpToFail);
</del><ins>+    speculationCheck(exitKind, source, edge.node(), jumpToFail);
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> RegisterSet SpeculativeJIT::usedRegisters()
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGSpeculativeJITh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h (195487 => 195488)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h        2016-01-22 23:26:42 UTC (rev 195487)
+++ trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h        2016-01-22 23:30:07 UTC (rev 195488)
</span><span class="lines">@@ -2447,7 +2447,7 @@
</span><span class="cx">     
</span><span class="cx">     // Helpers for performing type checks on an edge stored in the given registers.
</span><span class="cx">     bool needsTypeCheck(Edge edge, SpeculatedType typesPassedThrough) { return m_interpreter.needsTypeCheck(edge, typesPassedThrough); }
</span><del>-    void typeCheck(JSValueSource, Edge, SpeculatedType typesPassedThrough, MacroAssembler::Jump jumpToFail);
</del><ins>+    void typeCheck(JSValueSource, Edge, SpeculatedType typesPassedThrough, MacroAssembler::Jump jumpToFail, ExitKind = BadType);
</ins><span class="cx">     
</span><span class="cx">     void speculateCellTypeWithoutTypeFiltering(Edge, GPRReg cellGPR, JSType);
</span><span class="cx">     void speculateCellType(Edge, GPRReg cellGPR, SpeculatedType, JSType);
</span><span class="lines">@@ -3379,15 +3379,18 @@
</span><span class="cx">     }
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-#define DFG_TYPE_CHECK(source, edge, typesPassedThrough, jumpToFail) do { \
</del><ins>+#define DFG_TYPE_CHECK_WITH_EXIT_KIND(exitKind, source, edge, typesPassedThrough, jumpToFail) do { \
</ins><span class="cx">         JSValueSource _dtc_source = (source);                           \
</span><span class="cx">         Edge _dtc_edge = (edge);                                        \
</span><span class="cx">         SpeculatedType _dtc_typesPassedThrough = typesPassedThrough;    \
</span><span class="cx">         if (!needsTypeCheck(_dtc_edge, _dtc_typesPassedThrough))        \
</span><span class="cx">             break;                                                      \
</span><del>-        typeCheck(_dtc_source, _dtc_edge, _dtc_typesPassedThrough, (jumpToFail)); \
</del><ins>+        typeCheck(_dtc_source, _dtc_edge, _dtc_typesPassedThrough, (jumpToFail), exitKind); \
</ins><span class="cx">     } while (0)
</span><span class="cx"> 
</span><ins>+#define DFG_TYPE_CHECK(source, edge, typesPassedThrough, jumpToFail) \
+    DFG_TYPE_CHECK_WITH_EXIT_KIND(BadType, source, edge, typesPassedThrough, jumpToFail)
+
</ins><span class="cx"> } } // namespace JSC::DFG
</span><span class="cx"> 
</span><span class="cx"> #endif
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGSpeculativeJIT64cpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp (195487 => 195488)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp        2016-01-22 23:26:42 UTC (rev 195487)
+++ trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp        2016-01-22 23:30:07 UTC (rev 195488)
</span><span class="lines">@@ -2211,7 +2211,7 @@
</span><span class="cx">             
</span><span class="cx">             callOperation(operationConvertDoubleToInt52, resultGPR, valueFPR);
</span><span class="cx">             
</span><del>-            DFG_TYPE_CHECK(
</del><ins>+            DFG_TYPE_CHECK_WITH_EXIT_KIND(Int52Overflow,
</ins><span class="cx">                 JSValueRegs(), node-&gt;child1(), SpecInt52AsDouble,
</span><span class="cx">                 m_jit.branch64(
</span><span class="cx">                     JITCompiler::Equal, resultGPR,
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreftlFTLLowerDFGToLLVMcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp (195487 => 195488)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp        2016-01-22 23:26:42 UTC (rev 195487)
+++ trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp        2016-01-22 23:30:07 UTC (rev 195488)
</span><span class="lines">@@ -109,15 +109,18 @@
</span><span class="cx"> 
</span><span class="cx"> // Using this instead of typeCheck() helps to reduce the load on LLVM, by creating
</span><span class="cx"> // significantly less dead code.
</span><del>-#define FTL_TYPE_CHECK(lowValue, highValue, typesPassedThrough, failCondition) do { \
</del><ins>+#define FTL_TYPE_CHECK_WITH_EXIT_KIND(exitKind, lowValue, highValue, typesPassedThrough, failCondition) do { \
</ins><span class="cx">         FormattedValue _ftc_lowValue = (lowValue);                      \
</span><span class="cx">         Edge _ftc_highValue = (highValue);                              \
</span><span class="cx">         SpeculatedType _ftc_typesPassedThrough = (typesPassedThrough);  \
</span><span class="cx">         if (!m_interpreter.needsTypeCheck(_ftc_highValue, _ftc_typesPassedThrough)) \
</span><span class="cx">             break;                                                      \
</span><del>-        typeCheck(_ftc_lowValue, _ftc_highValue, _ftc_typesPassedThrough, (failCondition)); \
</del><ins>+        typeCheck(_ftc_lowValue, _ftc_highValue, _ftc_typesPassedThrough, (failCondition), exitKind); \
</ins><span class="cx">     } while (false)
</span><span class="cx"> 
</span><ins>+#define FTL_TYPE_CHECK(lowValue, highValue, typesPassedThrough, failCondition) \
+    FTL_TYPE_CHECK_WITH_EXIT_KIND(BadType, lowValue, highValue, typesPassedThrough, failCondition)
+
</ins><span class="cx"> class LowerDFGToLLVM {
</span><span class="cx">     WTF_MAKE_NONCOPYABLE(LowerDFGToLLVM);
</span><span class="cx"> public:
</span><span class="lines">@@ -8974,19 +8977,19 @@
</span><span class="cx">     
</span><span class="cx">     void typeCheck(
</span><span class="cx">         FormattedValue lowValue, Edge highValue, SpeculatedType typesPassedThrough,
</span><del>-        LValue failCondition)
</del><ins>+        LValue failCondition, ExitKind exitKind = BadType)
</ins><span class="cx">     {
</span><del>-        appendTypeCheck(lowValue, highValue, typesPassedThrough, failCondition);
</del><ins>+        appendTypeCheck(lowValue, highValue, typesPassedThrough, failCondition, exitKind);
</ins><span class="cx">     }
</span><span class="cx">     
</span><span class="cx">     void appendTypeCheck(
</span><span class="cx">         FormattedValue lowValue, Edge highValue, SpeculatedType typesPassedThrough,
</span><del>-        LValue failCondition)
</del><ins>+        LValue failCondition, ExitKind exitKind)
</ins><span class="cx">     {
</span><span class="cx">         if (!m_interpreter.needsTypeCheck(highValue, typesPassedThrough))
</span><span class="cx">             return;
</span><span class="cx">         ASSERT(mayHaveTypeCheck(highValue.useKind()));
</span><del>-        appendOSRExit(BadType, lowValue, highValue.node(), failCondition, m_origin);
</del><ins>+        appendOSRExit(exitKind, lowValue, highValue.node(), failCondition, m_origin);
</ins><span class="cx">         m_interpreter.filter(highValue, typesPassedThrough);
</span><span class="cx">     }
</span><span class="cx">     
</span><span class="lines">@@ -9399,7 +9402,7 @@
</span><span class="cx">     {
</span><span class="cx">         LValue possibleResult = m_out.call(
</span><span class="cx">             m_out.int64, m_out.operation(operationConvertDoubleToInt52), value);
</span><del>-        FTL_TYPE_CHECK(
</del><ins>+        FTL_TYPE_CHECK_WITH_EXIT_KIND(Int52Overflow,
</ins><span class="cx">             doubleValue(value), edge, SpecInt52AsDouble,
</span><span class="cx">             m_out.equal(possibleResult, m_out.constInt64(JSValue::notInt52)));
</span><span class="cx">         
</span></span></pre>
</div>
</div>

</body>
</html>