<!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>[192353] 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/192353">192353</a></dd>
<dt>Author</dt> <dd>mark.lam@apple.com</dd>
<dt>Date</dt> <dd>2015-11-11 23:17:09 -0800 (Wed, 11 Nov 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>The BinarySnippetRegisterContext needs to copy the result back from the scratch register.
https://bugs.webkit.org/show_bug.cgi?id=150712
Reviewed by Geoffrey Garen.
If the BinarySnippetRegisterContext had re-assigned the result register to a scratch
before emitting the snippet, it needs to copy the result back from the scratch after
the snippet is done.
This fixes the cdjs-tests.yaml/main.js.ftl failure reported in
https://bugs.webkit.org/show_bug.cgi?id=150687.
Also added an optimization to check for the case where any of the left, right,
or result registers are aliased together, and to map them to the corresponding
allocated scratch register for their alias instead of allocating separate ones.
* JavaScriptCore.vcxproj/JavaScriptCore.vcxproj:
* JavaScriptCore.vcxproj/JavaScriptCore.vcxproj.filters:
- Added JITSubGenerator.h to these project files for completeness.
* ftl/FTLCapabilities.cpp:
(JSC::FTL::canCompile):
- Re-enable ArithSub handling of UntypedUse operands.
* ftl/FTLCompile.cpp:
* ftl/FTLInlineCacheSize.cpp:
(JSC::FTL::sizeOfArithSub):
- Adjusted IC sizes to account for the snippet changes.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreJavaScriptCorevcxprojJavaScriptCorevcxproj">trunk/Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj</a></li>
<li><a href="#trunkSourceJavaScriptCoreJavaScriptCorevcxprojJavaScriptCorevcxprojfilters">trunk/Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj.filters</a></li>
<li><a href="#trunkSourceJavaScriptCoreftlFTLCapabilitiescpp">trunk/Source/JavaScriptCore/ftl/FTLCapabilities.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreftlFTLCompilecpp">trunk/Source/JavaScriptCore/ftl/FTLCompile.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreftlFTLInlineCacheSizecpp">trunk/Source/JavaScriptCore/ftl/FTLInlineCacheSize.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (192352 => 192353)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2015-11-12 06:59:13 UTC (rev 192352)
+++ trunk/Source/JavaScriptCore/ChangeLog 2015-11-12 07:17:09 UTC (rev 192353)
</span><span class="lines">@@ -1,5 +1,37 @@
</span><span class="cx"> 2015-11-11 Mark Lam <mark.lam@apple.com>
</span><span class="cx">
</span><ins>+ The BinarySnippetRegisterContext needs to copy the result back from the scratch register.
+ https://bugs.webkit.org/show_bug.cgi?id=150712
+
+ Reviewed by Geoffrey Garen.
+
+ If the BinarySnippetRegisterContext had re-assigned the result register to a scratch
+ before emitting the snippet, it needs to copy the result back from the scratch after
+ the snippet is done.
+
+ This fixes the cdjs-tests.yaml/main.js.ftl failure reported in
+ https://bugs.webkit.org/show_bug.cgi?id=150687.
+
+ Also added an optimization to check for the case where any of the left, right,
+ or result registers are aliased together, and to map them to the corresponding
+ allocated scratch register for their alias instead of allocating separate ones.
+
+ * JavaScriptCore.vcxproj/JavaScriptCore.vcxproj:
+ * JavaScriptCore.vcxproj/JavaScriptCore.vcxproj.filters:
+ - Added JITSubGenerator.h to these project files for completeness.
+
+ * ftl/FTLCapabilities.cpp:
+ (JSC::FTL::canCompile):
+ - Re-enable ArithSub handling of UntypedUse operands.
+
+ * ftl/FTLCompile.cpp:
+
+ * ftl/FTLInlineCacheSize.cpp:
+ (JSC::FTL::sizeOfArithSub):
+ - Adjusted IC sizes to account for the snippet changes.
+
+2015-11-11 Mark Lam <mark.lam@apple.com>
+
</ins><span class="cx"> Crash in sorting-benchmark.js on ARM64 with full op_sub FTL coverage.
</span><span class="cx"> https://bugs.webkit.org/show_bug.cgi?id=150936
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreJavaScriptCorevcxprojJavaScriptCorevcxproj"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj (192352 => 192353)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj 2015-11-12 06:59:13 UTC (rev 192352)
+++ trunk/Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj 2015-11-12 07:17:09 UTC (rev 192353)
</span><span class="lines">@@ -1469,6 +1469,7 @@
</span><span class="cx"> <ClInclude Include="..\jit\JITInlines.h" />
</span><span class="cx"> <ClInclude Include="..\jit\JITOperations.h" />
</span><span class="cx"> <ClInclude Include="..\jit\JITStubRoutine.h" />
</span><ins>+ <ClInclude Include="..\jit\JITSubGenerator.h" />
</ins><span class="cx"> <ClInclude Include="..\jit\JITThunks.h" />
</span><span class="cx"> <ClInclude Include="..\jit\JITToDFGDeferredCompilationCallback.h" />
</span><span class="cx"> <ClInclude Include="..\jit\JITWriteBarrier.h" />
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreJavaScriptCorevcxprojJavaScriptCorevcxprojfilters"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj.filters (192352 => 192353)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj.filters 2015-11-12 06:59:13 UTC (rev 192352)
+++ trunk/Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj.filters 2015-11-12 07:17:09 UTC (rev 192353)
</span><span class="lines">@@ -2522,6 +2522,9 @@
</span><span class="cx"> <ClInclude Include="..\jit\JITStubRoutine.h">
</span><span class="cx"> <Filter>jit</Filter>
</span><span class="cx"> </ClInclude>
</span><ins>+ <ClInclude Include="..\jit\JITSubGenerator.h">
+ <Filter>jit</Filter>
+ </ClInclude>
</ins><span class="cx"> <ClInclude Include="..\jit\JITThunks.h">
</span><span class="cx"> <Filter>jit</Filter>
</span><span class="cx"> </ClInclude>
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreftlFTLCapabilitiescpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ftl/FTLCapabilities.cpp (192352 => 192353)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ftl/FTLCapabilities.cpp 2015-11-12 06:59:13 UTC (rev 192352)
+++ trunk/Source/JavaScriptCore/ftl/FTLCapabilities.cpp 2015-11-12 07:17:09 UTC (rev 192353)
</span><span class="lines">@@ -84,6 +84,7 @@
</span><span class="cx"> case StrCat:
</span><span class="cx"> case ArithAdd:
</span><span class="cx"> case ArithClz32:
</span><ins>+ case ArithSub:
</ins><span class="cx"> case ArithMul:
</span><span class="cx"> case ArithDiv:
</span><span class="cx"> case ArithMod:
</span><span class="lines">@@ -211,10 +212,6 @@
</span><span class="cx"> case PutSetterByVal:
</span><span class="cx"> // These are OK.
</span><span class="cx"> break;
</span><del>- case ArithSub:
- if (node->result() == NodeResultJS)
- return CannotCompile;
- break;
</del><span class="cx">
</span><span class="cx"> case Identity:
</span><span class="cx"> // No backend handles this because it will be optimized out. But we may check
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreftlFTLCompilecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ftl/FTLCompile.cpp (192352 => 192353)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ftl/FTLCompile.cpp 2015-11-12 06:59:13 UTC (rev 192352)
+++ trunk/Source/JavaScriptCore/ftl/FTLCompile.cpp 2015-11-12 07:17:09 UTC (rev 192353)
</span><span class="lines">@@ -310,13 +310,17 @@
</span><span class="cx"> class BinarySnippetRegisterContext {
</span><span class="cx"> // The purpose of this class is to shuffle registers to get them into the state
</span><span class="cx"> // that baseline code expects so that we can use the baseline snippet generators i.e.
</span><del>- // 1. ensure that the inputs and outputs are not in tag or scratch registers.
- // 2. tag registers are loaded with the expected values.
</del><ins>+ // 1. Ensure that the inputs and output are not in reserved registers (which
+ // include the tag registers). The snippet will use these reserved registers.
+ // Hence, we need to put the inputs and output in other scratch registers.
+ // 2. Tag registers are loaded with the expected values.
</ins><span class="cx"> //
</span><del>- // We also need to:
- // 1. restore the input and tag registers to the values that LLVM put there originally.
- // 2. that is except when one of the input registers is also the result register.
- // In this case, we don't want to trash the result, and hence, should not restore into it.
</del><ins>+ // When the snippet is done:
+ // 1. If we had re-assigned the result register to a scratch, we need to copy the
+ // result back from the scratch.
+ // 2. Restore the input and tag registers to the values that LLVM put there originally.
+ // That is unless when one of them is also the result register. In that case, we
+ // don't want to trash the result, and hence, should not restore into it.
</ins><span class="cx">
</span><span class="cx"> public:
</span><span class="cx"> BinarySnippetRegisterContext(ScratchRegisterAllocator& allocator, GPRReg& result, GPRReg& left, GPRReg& right)
</span><span class="lines">@@ -332,20 +336,28 @@
</span><span class="cx"> m_allocator.lock(m_left);
</span><span class="cx"> m_allocator.lock(m_right);
</span><span class="cx">
</span><del>- RegisterSet inputRegisters = RegisterSet(m_left, m_right);
- RegisterSet inputAndOutputRegisters = RegisterSet(inputRegisters, m_result);
-
</del><ins>+ RegisterSet inputAndOutputRegisters = RegisterSet(m_left, m_right, m_result);
</ins><span class="cx"> RegisterSet reservedRegisters;
</span><span class="cx"> for (GPRReg reg : GPRInfo::reservedRegisters())
</span><span class="cx"> reservedRegisters.set(reg);
</span><span class="cx">
</span><span class="cx"> if (reservedRegisters.get(m_left))
</span><span class="cx"> m_left = m_allocator.allocateScratchGPR();
</span><del>- if (reservedRegisters.get(m_right))
- m_right = m_allocator.allocateScratchGPR();
- if (!inputRegisters.get(m_result) && reservedRegisters.get(m_result))
- m_result = m_allocator.allocateScratchGPR();
-
</del><ins>+ if (reservedRegisters.get(m_right)) {
+ if (m_origRight == m_origLeft)
+ m_right = m_left;
+ else
+ m_right = m_allocator.allocateScratchGPR();
+ }
+ if (reservedRegisters.get(m_result)) {
+ if (m_origResult == m_origLeft)
+ m_result = m_left;
+ else if (m_origResult == m_origRight)
+ m_result = m_right;
+ else
+ m_result = m_allocator.allocateScratchGPR();
+ }
+
</ins><span class="cx"> if (!inputAndOutputRegisters.get(GPRInfo::tagMaskRegister))
</span><span class="cx"> m_savedTagMaskRegister = m_allocator.allocateScratchGPR();
</span><span class="cx"> if (!inputAndOutputRegisters.get(GPRInfo::tagTypeNumberRegister))
</span><span class="lines">@@ -356,7 +368,7 @@
</span><span class="cx"> {
</span><span class="cx"> if (m_left != m_origLeft)
</span><span class="cx"> jit.move(m_origLeft, m_left);
</span><del>- if (m_right != m_origRight)
</del><ins>+ if (m_right != m_origRight && m_origRight != m_origLeft)
</ins><span class="cx"> jit.move(m_origRight, m_right);
</span><span class="cx">
</span><span class="cx"> if (m_savedTagMaskRegister != InvalidGPRReg)
</span><span class="lines">@@ -369,15 +381,28 @@
</span><span class="cx">
</span><span class="cx"> void restoreRegisters(CCallHelpers& jit)
</span><span class="cx"> {
</span><ins>+ if (m_origResult != m_result)
+ jit.move(m_result, m_origResult);
</ins><span class="cx"> if (m_origLeft != m_left && m_origLeft != m_origResult)
</span><span class="cx"> jit.move(m_left, m_origLeft);
</span><del>- if (m_origRight != m_right && m_origRight != m_origResult)
</del><ins>+ if (m_origRight != m_right && m_origRight != m_origResult && m_origRight != m_origLeft)
</ins><span class="cx"> jit.move(m_right, m_origRight);
</span><del>-
- if (m_savedTagMaskRegister != InvalidGPRReg)
</del><ins>+
+ // We are guaranteed that the tag registers are not the same as the original input
+ // or output registers. Otherwise, we would not have allocated a scratch for them.
+ // Hence, we don't need to need to check for overlap like we do for the input registers.
+ if (m_savedTagMaskRegister != InvalidGPRReg) {
+ ASSERT(GPRInfo::tagMaskRegister != m_origLeft);
+ ASSERT(GPRInfo::tagMaskRegister != m_origRight);
+ ASSERT(GPRInfo::tagMaskRegister != m_origResult);
</ins><span class="cx"> jit.move(m_savedTagMaskRegister, GPRInfo::tagMaskRegister);
</span><del>- if (m_savedTagTypeNumberRegister != InvalidGPRReg)
</del><ins>+ }
+ if (m_savedTagTypeNumberRegister != InvalidGPRReg) {
+ ASSERT(GPRInfo::tagTypeNumberRegister != m_origLeft);
+ ASSERT(GPRInfo::tagTypeNumberRegister != m_origRight);
+ ASSERT(GPRInfo::tagTypeNumberRegister != m_origResult);
</ins><span class="cx"> jit.move(m_savedTagTypeNumberRegister, GPRInfo::tagTypeNumberRegister);
</span><ins>+ }
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> private:
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreftlFTLInlineCacheSizecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ftl/FTLInlineCacheSize.cpp (192352 => 192353)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ftl/FTLInlineCacheSize.cpp 2015-11-12 06:59:13 UTC (rev 192352)
+++ trunk/Source/JavaScriptCore/ftl/FTLInlineCacheSize.cpp 2015-11-12 07:17:09 UTC (rev 192353)
</span><span class="lines">@@ -132,15 +132,15 @@
</span><span class="cx"> {
</span><span class="cx"> #if CPU(ARM64)
</span><span class="cx"> #ifdef NDEBUG
</span><del>- return 192; // ARM64 release.
</del><ins>+ return 216; // ARM64 release.
</ins><span class="cx"> #else
</span><del>- return 288; // ARM64 debug.
</del><ins>+ return 312; // ARM64 debug.
</ins><span class="cx"> #endif
</span><span class="cx"> #else // CPU(X86_64)
</span><span class="cx"> #ifdef NDEBUG
</span><del>- return 184; // X86_64 release.
</del><ins>+ return 223; // X86_64 release.
</ins><span class="cx"> #else
</span><del>- return 259; // X86_64 debug.
</del><ins>+ return 298; // X86_64 debug.
</ins><span class="cx"> #endif
</span><span class="cx"> #endif
</span><span class="cx"> }
</span></span></pre>
</div>
</div>
</body>
</html>