<!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>[199675] 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/199675">199675</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2016-04-18 10:13:33 -0700 (Mon, 18 Apr 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>FTL should pin the tag registers at inline caches
https://bugs.webkit.org/show_bug.cgi?id=156678
Reviewed by Saam Barati.
This is a long-overdue fix to our inline caches. Back when we had LLVM, we couldn't rely on the tags
being pinned to any registers. So, if the inline caches needed tags, they'd have to materialize them.
This removes those materializations. This should reduce the amount of code generated in inline caches
and it should make inline caches faster. The effect appears to be small.
It may be that after this change, we'll even be able to kill the
HaveTagRegisters/DoNotHaveTagRegisters logic.
* bytecode/PolymorphicAccess.cpp:
(JSC::AccessCase::generateWithGuard):
(JSC::AccessCase::generateImpl):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compilePutById):
(JSC::FTL::DFG::LowerDFGToB3::compileCallOrConstruct):
(JSC::FTL::DFG::LowerDFGToB3::compileTailCall):
(JSC::FTL::DFG::LowerDFGToB3::compileCallOrConstructVarargs):
(JSC::FTL::DFG::LowerDFGToB3::compileIn):
(JSC::FTL::DFG::LowerDFGToB3::getById):
* jit/Repatch.cpp:
(JSC::readCallTarget):
(JSC::linkPolymorphicCall):
* jit/ThunkGenerators.cpp:
(JSC::virtualThunkFor):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecodePolymorphicAccesscpp">trunk/Source/JavaScriptCore/bytecode/PolymorphicAccess.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreftlFTLLowerDFGToB3cpp">trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorejitRepatchcpp">trunk/Source/JavaScriptCore/jit/Repatch.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorejitThunkGeneratorscpp">trunk/Source/JavaScriptCore/jit/ThunkGenerators.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (199674 => 199675)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-04-18 17:13:04 UTC (rev 199674)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-04-18 17:13:33 UTC (rev 199675)
</span><span class="lines">@@ -1,3 +1,35 @@
</span><ins>+2016-04-17 Filip Pizlo <fpizlo@apple.com>
+
+ FTL should pin the tag registers at inline caches
+ https://bugs.webkit.org/show_bug.cgi?id=156678
+
+ Reviewed by Saam Barati.
+
+ This is a long-overdue fix to our inline caches. Back when we had LLVM, we couldn't rely on the tags
+ being pinned to any registers. So, if the inline caches needed tags, they'd have to materialize them.
+
+ This removes those materializations. This should reduce the amount of code generated in inline caches
+ and it should make inline caches faster. The effect appears to be small.
+
+ It may be that after this change, we'll even be able to kill the
+ HaveTagRegisters/DoNotHaveTagRegisters logic.
+
+ * bytecode/PolymorphicAccess.cpp:
+ (JSC::AccessCase::generateWithGuard):
+ (JSC::AccessCase::generateImpl):
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::compilePutById):
+ (JSC::FTL::DFG::LowerDFGToB3::compileCallOrConstruct):
+ (JSC::FTL::DFG::LowerDFGToB3::compileTailCall):
+ (JSC::FTL::DFG::LowerDFGToB3::compileCallOrConstructVarargs):
+ (JSC::FTL::DFG::LowerDFGToB3::compileIn):
+ (JSC::FTL::DFG::LowerDFGToB3::getById):
+ * jit/Repatch.cpp:
+ (JSC::readCallTarget):
+ (JSC::linkPolymorphicCall):
+ * jit/ThunkGenerators.cpp:
+ (JSC::virtualThunkFor):
+
</ins><span class="cx"> 2016-04-18 Yusuke Suzuki <utatane.tea@gmail.com>
</span><span class="cx">
</span><span class="cx"> [ES7] yield star should not return if the inner iterator.throw returns { done: true }
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecodePolymorphicAccesscpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecode/PolymorphicAccess.cpp (199674 => 199675)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecode/PolymorphicAccess.cpp 2016-04-18 17:13:04 UTC (rev 199674)
+++ trunk/Source/JavaScriptCore/bytecode/PolymorphicAccess.cpp 2016-04-18 17:13:33 UTC (rev 199675)
</span><span class="lines">@@ -610,7 +610,7 @@
</span><span class="cx"> jit.load32(
</span><span class="cx"> CCallHelpers::Address(baseGPR, DirectArguments::offsetOfLength()),
</span><span class="cx"> valueRegs.payloadGPR());
</span><del>- jit.boxInt32(valueRegs.payloadGPR(), valueRegs, CCallHelpers::DoNotHaveTagRegisters);
</del><ins>+ jit.boxInt32(valueRegs.payloadGPR(), valueRegs);
</ins><span class="cx"> state.succeed();
</span><span class="cx"> return;
</span><span class="cx"> }
</span><span class="lines">@@ -630,7 +630,7 @@
</span><span class="cx"> jit.load32(
</span><span class="cx"> CCallHelpers::Address(baseGPR, ScopedArguments::offsetOfTotalLength()),
</span><span class="cx"> valueRegs.payloadGPR());
</span><del>- jit.boxInt32(valueRegs.payloadGPR(), valueRegs, CCallHelpers::DoNotHaveTagRegisters);
</del><ins>+ jit.boxInt32(valueRegs.payloadGPR(), valueRegs);
</ins><span class="cx"> state.succeed();
</span><span class="cx"> return;
</span><span class="cx"> }
</span><span class="lines">@@ -1126,7 +1126,7 @@
</span><span class="cx"> dataLog("Have type: ", type->descriptor(), "\n");
</span><span class="cx"> state.failAndRepatch.append(
</span><span class="cx"> jit.branchIfNotType(
</span><del>- valueRegs, scratchGPR, type->descriptor(), CCallHelpers::DoNotHaveTagRegisters));
</del><ins>+ valueRegs, scratchGPR, type->descriptor(), CCallHelpers::HaveTagRegisters));
</ins><span class="cx"> } else if (verbose)
</span><span class="cx"> dataLog("Don't have type.\n");
</span><span class="cx">
</span><span class="lines">@@ -1157,7 +1157,7 @@
</span><span class="cx"> dataLog("Have type: ", type->descriptor(), "\n");
</span><span class="cx"> state.failAndRepatch.append(
</span><span class="cx"> jit.branchIfNotType(
</span><del>- valueRegs, scratchGPR, type->descriptor(), CCallHelpers::DoNotHaveTagRegisters));
</del><ins>+ valueRegs, scratchGPR, type->descriptor(), CCallHelpers::HaveTagRegisters));
</ins><span class="cx"> } else if (verbose)
</span><span class="cx"> dataLog("Don't have type.\n");
</span><span class="cx">
</span><span class="lines">@@ -1361,14 +1361,14 @@
</span><span class="cx"> jit.load32(CCallHelpers::Address(scratchGPR, ArrayStorage::lengthOffset()), scratchGPR);
</span><span class="cx"> state.failAndIgnore.append(
</span><span class="cx"> jit.branch32(CCallHelpers::LessThan, scratchGPR, CCallHelpers::TrustedImm32(0)));
</span><del>- jit.boxInt32(scratchGPR, valueRegs, CCallHelpers::DoNotHaveTagRegisters);
</del><ins>+ jit.boxInt32(scratchGPR, valueRegs);
</ins><span class="cx"> state.succeed();
</span><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> case StringLength: {
</span><span class="cx"> jit.load32(CCallHelpers::Address(baseGPR, JSString::offsetOfLength()), valueRegs.payloadGPR());
</span><del>- jit.boxInt32(valueRegs.payloadGPR(), valueRegs, CCallHelpers::DoNotHaveTagRegisters);
</del><ins>+ jit.boxInt32(valueRegs.payloadGPR(), valueRegs);
</ins><span class="cx"> state.succeed();
</span><span class="cx"> return;
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreftlFTLLowerDFGToB3cpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp (199674 => 199675)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp 2016-04-18 17:13:04 UTC (rev 199674)
+++ trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp 2016-04-18 17:13:33 UTC (rev 199675)
</span><span class="lines">@@ -2403,6 +2403,8 @@
</span><span class="cx"> B3::PatchpointValue* patchpoint = m_out.patchpoint(Void);
</span><span class="cx"> patchpoint->appendSomeRegister(base);
</span><span class="cx"> patchpoint->appendSomeRegister(value);
</span><ins>+ patchpoint->append(m_tagMask, ValueRep::reg(GPRInfo::tagMaskRegister));
+ patchpoint->append(m_tagTypeNumber, ValueRep::reg(GPRInfo::tagTypeNumberRegister));
</ins><span class="cx"> patchpoint->clobber(RegisterSet::macroScratchRegisters());
</span><span class="cx">
</span><span class="cx"> // FIXME: If this is a PutByIdFlush, we might want to late-clobber volatile registers.
</span><span class="lines">@@ -4865,6 +4867,8 @@
</span><span class="cx"> RefPtr<PatchpointExceptionHandle> exceptionHandle =
</span><span class="cx"> preparePatchpointForExceptions(patchpoint);
</span><span class="cx">
</span><ins>+ patchpoint->append(m_tagMask, ValueRep::reg(GPRInfo::tagMaskRegister));
+ patchpoint->append(m_tagTypeNumber, ValueRep::reg(GPRInfo::tagTypeNumberRegister));
</ins><span class="cx"> patchpoint->clobber(RegisterSet::macroScratchRegisters());
</span><span class="cx"> patchpoint->clobberLate(RegisterSet::volatileRegistersForJSCall());
</span><span class="cx"> patchpoint->resultConstraint = ValueRep::reg(GPRInfo::returnValueGPR);
</span><span class="lines">@@ -4954,6 +4958,9 @@
</span><span class="cx"> PatchpointValue* patchpoint = m_out.patchpoint(Void);
</span><span class="cx"> patchpoint->appendVector(arguments);
</span><span class="cx">
</span><ins>+ patchpoint->append(m_tagMask, ValueRep::reg(GPRInfo::tagMaskRegister));
+ patchpoint->append(m_tagTypeNumber, ValueRep::reg(GPRInfo::tagTypeNumberRegister));
+
</ins><span class="cx"> // Prevent any of the arguments from using the scratch register.
</span><span class="cx"> patchpoint->clobberEarly(RegisterSet::macroScratchRegisters());
</span><span class="cx">
</span><span class="lines">@@ -5071,6 +5078,9 @@
</span><span class="cx"> RefPtr<PatchpointExceptionHandle> exceptionHandle =
</span><span class="cx"> preparePatchpointForExceptions(patchpoint);
</span><span class="cx">
</span><ins>+ patchpoint->append(m_tagMask, ValueRep::reg(GPRInfo::tagMaskRegister));
+ patchpoint->append(m_tagTypeNumber, ValueRep::reg(GPRInfo::tagTypeNumberRegister));
+
</ins><span class="cx"> patchpoint->clobber(RegisterSet::macroScratchRegisters());
</span><span class="cx"> patchpoint->clobberLate(RegisterSet::volatileRegistersForJSCall());
</span><span class="cx"> patchpoint->resultConstraint = ValueRep::reg(GPRInfo::returnValueGPR);
</span><span class="lines">@@ -5934,6 +5944,8 @@
</span><span class="cx"> UniquedStringImpl* str = bitwise_cast<UniquedStringImpl*>(string->tryGetValueImpl());
</span><span class="cx"> B3::PatchpointValue* patchpoint = m_out.patchpoint(Int64);
</span><span class="cx"> patchpoint->appendSomeRegister(cell);
</span><ins>+ patchpoint->append(m_tagMask, ValueRep::reg(GPRInfo::tagMaskRegister));
+ patchpoint->append(m_tagTypeNumber, ValueRep::reg(GPRInfo::tagTypeNumberRegister));
</ins><span class="cx"> patchpoint->clobber(RegisterSet::macroScratchRegisters());
</span><span class="cx">
</span><span class="cx"> State* state = &m_ftlState;
</span><span class="lines">@@ -7393,6 +7405,8 @@
</span><span class="cx">
</span><span class="cx"> B3::PatchpointValue* patchpoint = m_out.patchpoint(Int64);
</span><span class="cx"> patchpoint->appendSomeRegister(base);
</span><ins>+ patchpoint->append(m_tagMask, ValueRep::reg(GPRInfo::tagMaskRegister));
+ patchpoint->append(m_tagTypeNumber, ValueRep::reg(GPRInfo::tagTypeNumberRegister));
</ins><span class="cx">
</span><span class="cx"> // FIXME: If this is a GetByIdFlush, we might get some performance boost if we claim that it
</span><span class="cx"> // clobbers volatile registers late. It's not necessary for correctness, though, since the
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorejitRepatchcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/jit/Repatch.cpp (199674 => 199675)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/jit/Repatch.cpp 2016-04-18 17:13:04 UTC (rev 199674)
+++ trunk/Source/JavaScriptCore/jit/Repatch.cpp 2016-04-18 17:13:33 UTC (rev 199675)
</span><span class="lines">@@ -55,11 +55,6 @@
</span><span class="cx">
</span><span class="cx"> namespace JSC {
</span><span class="cx">
</span><del>-// Beware: in this code, it is not safe to assume anything about the following registers
-// that would ordinarily have well-known values:
-// - tagTypeNumberRegister
-// - tagMaskRegister
-
</del><span class="cx"> static FunctionPtr readCallTarget(CodeBlock* codeBlock, CodeLocationCall call)
</span><span class="cx"> {
</span><span class="cx"> FunctionPtr result = MacroAssembler::readCallTarget(call);
</span><span class="lines">@@ -795,10 +790,7 @@
</span><span class="cx"> // Verify that we have a function and stash the executable in scratchGPR.
</span><span class="cx">
</span><span class="cx"> #if USE(JSVALUE64)
</span><del>- // We can't rely on tagMaskRegister being set, so we do this the hard
- // way.
- stubJit.move(MacroAssembler::TrustedImm64(TagMask), scratchGPR);
- slowPath.append(stubJit.branchTest64(CCallHelpers::NonZero, calleeGPR, scratchGPR));
</del><ins>+ slowPath.append(stubJit.branchTest64(CCallHelpers::NonZero, calleeGPR, GPRInfo::tagMaskRegister));
</ins><span class="cx"> #else
</span><span class="cx"> // We would have already checked that the callee is a cell.
</span><span class="cx"> #endif
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorejitThunkGeneratorscpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/jit/ThunkGenerators.cpp (199674 => 199675)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/jit/ThunkGenerators.cpp 2016-04-18 17:13:04 UTC (rev 199674)
+++ trunk/Source/JavaScriptCore/jit/ThunkGenerators.cpp 2016-04-18 17:13:33 UTC (rev 199675)
</span><span class="lines">@@ -180,11 +180,9 @@
</span><span class="cx"> // the DFG knows that the value is definitely a cell, or definitely a function.
</span><span class="cx">
</span><span class="cx"> #if USE(JSVALUE64)
</span><del>- jit.move(CCallHelpers::TrustedImm64(TagMask), GPRInfo::regT4);
-
</del><span class="cx"> slowCase.append(
</span><span class="cx"> jit.branchTest64(
</span><del>- CCallHelpers::NonZero, GPRInfo::regT0, GPRInfo::regT4));
</del><ins>+ CCallHelpers::NonZero, GPRInfo::regT0, GPRInfo::tagMaskRegister));
</ins><span class="cx"> #else
</span><span class="cx"> slowCase.append(
</span><span class="cx"> jit.branch32(
</span></span></pre>
</div>
</div>
</body>
</html>