<!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>[194431] 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/194431">194431</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2015-12-28 14:46:51 -0800 (Mon, 28 Dec 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>FTL B3 should know that used registers are not the same thing as used registers. Rename the
latter to unavailable registers to avoid future confusion.
https://bugs.webkit.org/show_bug.cgi?id=152572
Reviewed by Saam Barati.
Prior to this change, we used the term "used registers" in two different senses:
- The set of registers that are live at some point in the current compilation unit. A
register is live at some point if it is read after that point on some path through that
point.
- The set of registers that are not available for scratch register use at some point. A
register may not be available if it is live or if it is a callee-save register but it is
not being saved by the current compilation.
In the old FTL LLVM code, we had some translations from the first sense into the second
sense. We forgot to do those in FTL B3, and so we get crashes, for example in V8/splay. That
benchmark highlighted this issue because it fired some lazy slow paths, and then used an
unsaved callee-save for scratch.
Curiously, we could merge these two definitions by observing that, in some sense, an unsaved
callee save is live at every point in a compilation in the sense that it may contain a value
that will be read when the compilation returns. That's pretty cool, but it feels strange to
me. This isn't how we would normally define liveness of registers. It's not how the
Air::TmpLiveness analysis would do it for any of its other clients.
So, this changes B3 to have two different concepts:
- Used registers. These are the registers that are live.
- Unavailable registers. These are the registers that are not available for scratch. It's
always a superset of used registers.
This also changes FTLLower to use unavailableRegisters() pretty much everywhere that it
previously used usedRegisters().
This makes it possible to run V8/splay.
* b3/B3StackmapGenerationParams.cpp:
(JSC::B3::StackmapGenerationParams::usedRegisters):
(JSC::B3::StackmapGenerationParams::unavailableRegisters):
(JSC::B3::StackmapGenerationParams::proc):
* b3/B3StackmapGenerationParams.h:
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::DFG::LowerDFGToLLVM::compilePutById):
(JSC::FTL::DFG::LowerDFGToLLVM::getById):
(JSC::FTL::DFG::LowerDFGToLLVM::lazySlowPath):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreb3B3StackmapGenerationParamscpp">trunk/Source/JavaScriptCore/b3/B3StackmapGenerationParams.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreb3B3StackmapGenerationParamsh">trunk/Source/JavaScriptCore/b3/B3StackmapGenerationParams.h</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 (194430 => 194431)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2015-12-28 17:22:51 UTC (rev 194430)
+++ trunk/Source/JavaScriptCore/ChangeLog 2015-12-28 22:46:51 UTC (rev 194431)
</span><span class="lines">@@ -1,3 +1,54 @@
</span><ins>+2015-12-27 Filip Pizlo <fpizlo@apple.com>
+
+ FTL B3 should know that used registers are not the same thing as used registers. Rename the
+ latter to unavailable registers to avoid future confusion.
+ https://bugs.webkit.org/show_bug.cgi?id=152572
+
+ Reviewed by Saam Barati.
+
+ Prior to this change, we used the term "used registers" in two different senses:
+
+ - The set of registers that are live at some point in the current compilation unit. A
+ register is live at some point if it is read after that point on some path through that
+ point.
+
+ - The set of registers that are not available for scratch register use at some point. A
+ register may not be available if it is live or if it is a callee-save register but it is
+ not being saved by the current compilation.
+
+ In the old FTL LLVM code, we had some translations from the first sense into the second
+ sense. We forgot to do those in FTL B3, and so we get crashes, for example in V8/splay. That
+ benchmark highlighted this issue because it fired some lazy slow paths, and then used an
+ unsaved callee-save for scratch.
+
+ Curiously, we could merge these two definitions by observing that, in some sense, an unsaved
+ callee save is live at every point in a compilation in the sense that it may contain a value
+ that will be read when the compilation returns. That's pretty cool, but it feels strange to
+ me. This isn't how we would normally define liveness of registers. It's not how the
+ Air::TmpLiveness analysis would do it for any of its other clients.
+
+ So, this changes B3 to have two different concepts:
+
+ - Used registers. These are the registers that are live.
+
+ - Unavailable registers. These are the registers that are not available for scratch. It's
+ always a superset of used registers.
+
+ This also changes FTLLower to use unavailableRegisters() pretty much everywhere that it
+ previously used usedRegisters().
+
+ This makes it possible to run V8/splay.
+
+ * b3/B3StackmapGenerationParams.cpp:
+ (JSC::B3::StackmapGenerationParams::usedRegisters):
+ (JSC::B3::StackmapGenerationParams::unavailableRegisters):
+ (JSC::B3::StackmapGenerationParams::proc):
+ * b3/B3StackmapGenerationParams.h:
+ * ftl/FTLLowerDFGToLLVM.cpp:
+ (JSC::FTL::DFG::LowerDFGToLLVM::compilePutById):
+ (JSC::FTL::DFG::LowerDFGToLLVM::getById):
+ (JSC::FTL::DFG::LowerDFGToLLVM::lazySlowPath):
+
</ins><span class="cx"> 2015-12-25 Andy Estes <aestes@apple.com>
</span><span class="cx">
</span><span class="cx"> Stop moving local objects in return statements
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreb3B3StackmapGenerationParamscpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/b3/B3StackmapGenerationParams.cpp (194430 => 194431)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/b3/B3StackmapGenerationParams.cpp 2015-12-28 17:22:51 UTC (rev 194430)
+++ trunk/Source/JavaScriptCore/b3/B3StackmapGenerationParams.cpp 2015-12-28 22:46:51 UTC (rev 194431)
</span><span class="lines">@@ -41,6 +41,18 @@
</span><span class="cx"> return m_value->m_usedRegisters;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+RegisterSet StackmapGenerationParams::unavailableRegisters() const
+{
+ RegisterSet result = usedRegisters();
+
+ RegisterSet unsavedCalleeSaves = RegisterSet::vmCalleeSaveRegisters();
+ for (const RegisterAtOffset& regAtOffset : m_context.code->calleeSaveRegisters())
+ unsavedCalleeSaves.clear(regAtOffset.reg());
+
+ result.merge(unsavedCalleeSaves);
+ return result;
+}
+
</ins><span class="cx"> Procedure& StackmapGenerationParams::proc() const
</span><span class="cx"> {
</span><span class="cx"> return m_context.code->proc();
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreb3B3StackmapGenerationParamsh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/b3/B3StackmapGenerationParams.h (194430 => 194431)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/b3/B3StackmapGenerationParams.h 2015-12-28 17:22:51 UTC (rev 194430)
+++ trunk/Source/JavaScriptCore/b3/B3StackmapGenerationParams.h 2015-12-28 22:46:51 UTC (rev 194431)
</span><span class="lines">@@ -59,6 +59,13 @@
</span><span class="cx"> // This tells you the registers that were used.
</span><span class="cx"> const RegisterSet& usedRegisters() const;
</span><span class="cx">
</span><ins>+ // This is a useful helper if you want to do register allocation inside of a patchpoint. You
+ // can only use callee-save registers if they were saved in the prologue. This gives you the
+ // used register set that's useful for such settings by returning:
+ //
+ // usedRegisters() | (RegisterSet::calleeSaveRegisters() - proc.calleeSaveRegisters())
+ RegisterSet unavailableRegisters() const;
+
</ins><span class="cx"> // This is provided for convenience; it means that you don't have to capture it if you don't want to.
</span><span class="cx"> Procedure& proc() const;
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreftlFTLLowerDFGToLLVMcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp (194430 => 194431)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp 2015-12-28 17:22:51 UTC (rev 194430)
+++ trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp 2015-12-28 22:46:51 UTC (rev 194431)
</span><span class="lines">@@ -2586,8 +2586,8 @@
</span><span class="cx"> auto generator = Box<JITPutByIdGenerator>::create(
</span><span class="cx"> jit.codeBlock(), node->origin.semantic,
</span><span class="cx"> state->jitCode->common.addUniqueCallSiteIndex(node->origin.semantic),
</span><del>- params.usedRegisters(), JSValueRegs(params[0].gpr()), JSValueRegs(params[1].gpr()),
- GPRInfo::patchpointScratchRegister, ecmaMode,
</del><ins>+ params.unavailableRegisters(), JSValueRegs(params[0].gpr()),
+ JSValueRegs(params[1].gpr()), GPRInfo::patchpointScratchRegister, ecmaMode,
</ins><span class="cx"> node->op() == PutByIdDirect ? Direct : NotDirect);
</span><span class="cx">
</span><span class="cx"> generator->generateFastPath(jit);
</span><span class="lines">@@ -2603,8 +2603,8 @@
</span><span class="cx"> generator->slowPathJump().link(&jit);
</span><span class="cx"> CCallHelpers::Label slowPathBegin = jit.label();
</span><span class="cx"> CCallHelpers::Call slowPathCall = callOperation(
</span><del>- *state, params.usedRegisters(), jit, node->origin.semantic, &exceptions,
- generator->slowPathFunction(), InvalidGPRReg,
</del><ins>+ *state, params.unavailableRegisters(), jit, node->origin.semantic,
+ &exceptions, generator->slowPathFunction(), InvalidGPRReg,
</ins><span class="cx"> CCallHelpers::TrustedImmPtr(generator->stubInfo()), params[1].gpr(),
</span><span class="cx"> params[0].gpr(), CCallHelpers::TrustedImmPtr(uid)).call();
</span><span class="cx"> jit.jump().linkTo(done, &jit);
</span><span class="lines">@@ -7091,7 +7091,8 @@
</span><span class="cx"> auto generator = Box<JITGetByIdGenerator>::create(
</span><span class="cx"> jit.codeBlock(), node->origin.semantic,
</span><span class="cx"> state->jitCode->common.addUniqueCallSiteIndex(node->origin.semantic),
</span><del>- params.usedRegisters(), JSValueRegs(params[1].gpr()), JSValueRegs(params[0].gpr()));
</del><ins>+ params.unavailableRegisters(), JSValueRegs(params[1].gpr()),
+ JSValueRegs(params[0].gpr()));
</ins><span class="cx">
</span><span class="cx"> generator->generateFastPath(jit);
</span><span class="cx"> CCallHelpers::Label done = jit.label();
</span><span class="lines">@@ -7106,8 +7107,8 @@
</span><span class="cx"> generator->slowPathJump().link(&jit);
</span><span class="cx"> CCallHelpers::Label slowPathBegin = jit.label();
</span><span class="cx"> CCallHelpers::Call slowPathCall = callOperation(
</span><del>- *state, params.usedRegisters(), jit, node->origin.semantic, &exceptions,
- operationGetByIdOptimize, params[0].gpr(),
</del><ins>+ *state, params.unavailableRegisters(), jit, node->origin.semantic,
+ &exceptions, operationGetByIdOptimize, params[0].gpr(),
</ins><span class="cx"> CCallHelpers::TrustedImmPtr(generator->stubInfo()), params[1].gpr(),
</span><span class="cx"> CCallHelpers::TrustedImmPtr(uid)).call();
</span><span class="cx"> jit.jump().linkTo(done, &jit);
</span><span class="lines">@@ -8516,7 +8517,7 @@
</span><span class="cx"> CCallHelpers::PatchableJump patchableJump = jit.patchableJump();
</span><span class="cx"> CCallHelpers::Label done = jit.label();
</span><span class="cx">
</span><del>- RegisterSet usedRegisters = params.usedRegisters();
</del><ins>+ RegisterSet usedRegisters = params.unavailableRegisters();
</ins><span class="cx">
</span><span class="cx"> // FIXME: As part of handling exceptions, we need to create a concrete OSRExit here.
</span><span class="cx"> // Doing so should automagically register late paths that emit exit thunks.
</span><span class="lines">@@ -8544,7 +8545,7 @@
</span><span class="cx"> generatorJump, CodeLocationLabel(
</span><span class="cx"> vm->getCTIStub(
</span><span class="cx"> lazySlowPathGenerationThunkGenerator).code()));
</span><del>-
</del><ins>+
</ins><span class="cx"> CodeLocationJump linkedPatchableJump = CodeLocationJump(
</span><span class="cx"> linkBuffer.locationOf(patchableJump));
</span><span class="cx"> CodeLocationLabel linkedDone = linkBuffer.locationOf(done);
</span></span></pre>
</div>
</div>
</body>
</html>