<!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>[196616] 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/196616">196616</a></dd>
<dt>Author</dt> <dd>commit-queue@webkit.org</dd>
<dt>Date</dt> <dd>2016-02-15 20:51:20 -0800 (Mon, 15 Feb 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>[JSC] BranchAdd can override arguments of its stackmap
https://bugs.webkit.org/show_bug.cgi?id=154274
Patch by Benjamin Poulain <bpoulain@apple.com> on 2016-02-15
Reviewed by Filip Pizlo.
With the 3 operands BranchAdd added in <a href="http://trac.webkit.org/projects/webkit/changeset/196513">r196513</a>, we can run into
a register allocation such that the destination register is also
used by a value in the stack map.
It use to be that BranchAdd was a 2 operand instruction.
In that form, the destination is also one of the source and
can be recovered through Sub. There is no conflict between
destination and the stackmap.
After <a href="http://trac.webkit.org/projects/webkit/changeset/196513">r196513</a>, the destination has its own value. It is uncommon
on x86 because of the aggressive aliasing but that can happen.
On ARM, that's a standard form since there is no need for aliasing.
Since the arguments of the stackmap are of type EarlyUse,
they appeared as not interfering with the destination. When the register
allocator gives the same register to the destination and something in
the stack map, the result of BranchAdd destroys the value kept alive
for the stackmap.
In this patch, I introduce a concept very similar to ForceLateUse
to keep the argument of the stackmap live in CheckAdd. The new
role is "ForceLateUseUnlessRecoverable".
In this mode, anything that is not also an input argument becomes
LateUse. As such, it interferes with the destination of CheckAdd.
The arguments are recovered by the slow patch of CheckAdd. They
remain Early use.
This new modes ensure that destination can be aliased to the source
when that's useful, while making sure it is not aliased with another
value that needs to be live on exit.
* b3/B3CheckSpecial.cpp:
(JSC::B3::CheckSpecial::forEachArg):
* b3/B3LowerToAir.cpp:
(JSC::B3::Air::LowerToAir::lower):
* b3/B3PatchpointSpecial.cpp:
(JSC::B3::PatchpointSpecial::forEachArg):
* b3/B3StackmapSpecial.cpp:
(JSC::B3::StackmapSpecial::forEachArgImpl):
(WTF::printInternal):
* b3/B3StackmapSpecial.h:
* b3/B3StackmapValue.h:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreb3B3CheckSpecialcpp">trunk/Source/JavaScriptCore/b3/B3CheckSpecial.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreb3B3LowerToAircpp">trunk/Source/JavaScriptCore/b3/B3LowerToAir.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreb3B3PatchpointSpecialcpp">trunk/Source/JavaScriptCore/b3/B3PatchpointSpecial.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreb3B3StackmapSpecialcpp">trunk/Source/JavaScriptCore/b3/B3StackmapSpecial.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreb3B3StackmapSpecialh">trunk/Source/JavaScriptCore/b3/B3StackmapSpecial.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreb3B3StackmapValueh">trunk/Source/JavaScriptCore/b3/B3StackmapValue.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (196615 => 196616)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-02-16 04:41:58 UTC (rev 196615)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-02-16 04:51:20 UTC (rev 196616)
</span><span class="lines">@@ -1,3 +1,54 @@
</span><ins>+2016-02-15 Benjamin Poulain <bpoulain@apple.com>
+
+ [JSC] BranchAdd can override arguments of its stackmap
+ https://bugs.webkit.org/show_bug.cgi?id=154274
+
+ Reviewed by Filip Pizlo.
+
+ With the 3 operands BranchAdd added in r196513, we can run into
+ a register allocation such that the destination register is also
+ used by a value in the stack map.
+
+ It use to be that BranchAdd was a 2 operand instruction.
+ In that form, the destination is also one of the source and
+ can be recovered through Sub. There is no conflict between
+ destination and the stackmap.
+
+ After r196513, the destination has its own value. It is uncommon
+ on x86 because of the aggressive aliasing but that can happen.
+ On ARM, that's a standard form since there is no need for aliasing.
+
+ Since the arguments of the stackmap are of type EarlyUse,
+ they appeared as not interfering with the destination. When the register
+ allocator gives the same register to the destination and something in
+ the stack map, the result of BranchAdd destroys the value kept alive
+ for the stackmap.
+
+ In this patch, I introduce a concept very similar to ForceLateUse
+ to keep the argument of the stackmap live in CheckAdd. The new
+ role is "ForceLateUseUnlessRecoverable".
+
+ In this mode, anything that is not also an input argument becomes
+ LateUse. As such, it interferes with the destination of CheckAdd.
+ The arguments are recovered by the slow patch of CheckAdd. They
+ remain Early use.
+
+ This new modes ensure that destination can be aliased to the source
+ when that's useful, while making sure it is not aliased with another
+ value that needs to be live on exit.
+
+ * b3/B3CheckSpecial.cpp:
+ (JSC::B3::CheckSpecial::forEachArg):
+ * b3/B3LowerToAir.cpp:
+ (JSC::B3::Air::LowerToAir::lower):
+ * b3/B3PatchpointSpecial.cpp:
+ (JSC::B3::PatchpointSpecial::forEachArg):
+ * b3/B3StackmapSpecial.cpp:
+ (JSC::B3::StackmapSpecial::forEachArgImpl):
+ (WTF::printInternal):
+ * b3/B3StackmapSpecial.h:
+ * b3/B3StackmapValue.h:
+
</ins><span class="cx"> 2016-02-15 Joseph Pecoraro <pecoraro@apple.com>
</span><span class="cx">
</span><span class="cx"> Web Inspector: Web Workers have no access to console for debugging
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreb3B3CheckSpecialcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/b3/B3CheckSpecial.cpp (196615 => 196616)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/b3/B3CheckSpecial.cpp 2016-02-16 04:41:58 UTC (rev 196615)
+++ trunk/Source/JavaScriptCore/b3/B3CheckSpecial.cpp 2016-02-16 04:51:20 UTC (rev 196616)
</span><span class="lines">@@ -113,7 +113,11 @@
</span><span class="cx"> unsigned index = &arg - &hidden.args[0];
</span><span class="cx"> callback(inst.args[1 + index], role, type, width);
</span><span class="cx"> });
</span><del>- forEachArgImpl(numB3Args(inst), m_numCheckArgs + 1, inst, m_stackmapRole, callback);
</del><ins>+
+ Optional<unsigned> firstRecoverableIndex;
+ if (m_checkOpcode == BranchAdd32 || m_checkOpcode == BranchAdd64)
+ firstRecoverableIndex = 1;
+ forEachArgImpl(numB3Args(inst), m_numCheckArgs + 1, inst, m_stackmapRole, firstRecoverableIndex, callback);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> bool CheckSpecial::isValid(Inst& inst)
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreb3B3LowerToAircpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/b3/B3LowerToAir.cpp (196615 => 196616)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/b3/B3LowerToAir.cpp 2016-02-16 04:41:58 UTC (rev 196615)
+++ trunk/Source/JavaScriptCore/b3/B3LowerToAir.cpp 2016-02-16 04:51:20 UTC (rev 196616)
</span><span class="lines">@@ -2120,6 +2120,7 @@
</span><span class="cx"> switch (m_value->opcode()) {
</span><span class="cx"> case CheckAdd:
</span><span class="cx"> opcode = opcodeForType(BranchAdd32, BranchAdd64, m_value->type());
</span><ins>+ stackmapRole = StackmapSpecial::ForceLateUseUnlessRecoverable;
</ins><span class="cx"> commutativity = Commutative;
</span><span class="cx"> break;
</span><span class="cx"> case CheckSub:
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreb3B3PatchpointSpecialcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/b3/B3PatchpointSpecial.cpp (196615 => 196616)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/b3/B3PatchpointSpecial.cpp 2016-02-16 04:41:58 UTC (rev 196615)
+++ trunk/Source/JavaScriptCore/b3/B3PatchpointSpecial.cpp 2016-02-16 04:51:20 UTC (rev 196616)
</span><span class="lines">@@ -51,7 +51,7 @@
</span><span class="cx"> if (inst.origin->type() != Void)
</span><span class="cx"> callback(inst.args[argIndex++], Arg::Def, inst.origin->airType(), inst.origin->airWidth());
</span><span class="cx">
</span><del>- forEachArgImpl(0, argIndex, inst, SameAsRep, callback);
</del><ins>+ forEachArgImpl(0, argIndex, inst, SameAsRep, Nullopt, callback);
</ins><span class="cx"> argIndex += inst.origin->numChildren();
</span><span class="cx">
</span><span class="cx"> for (unsigned i = inst.origin->as<PatchpointValue>()->numGPScratchRegisters; i--;)
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreb3B3StackmapSpecialcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/b3/B3StackmapSpecial.cpp (196615 => 196616)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/b3/B3StackmapSpecial.cpp 2016-02-16 04:41:58 UTC (rev 196615)
+++ trunk/Source/JavaScriptCore/b3/B3StackmapSpecial.cpp 2016-02-16 04:51:20 UTC (rev 196616)
</span><span class="lines">@@ -73,7 +73,8 @@
</span><span class="cx">
</span><span class="cx"> void StackmapSpecial::forEachArgImpl(
</span><span class="cx"> unsigned numIgnoredB3Args, unsigned numIgnoredAirArgs,
</span><del>- Inst& inst, RoleMode roleMode, const ScopedLambda<Inst::EachArgCallback>& callback)
</del><ins>+ Inst& inst, RoleMode roleMode, Optional<unsigned> firstRecoverableIndex,
+ const ScopedLambda<Inst::EachArgCallback>& callback)
</ins><span class="cx"> {
</span><span class="cx"> StackmapValue* value = inst.origin->as<StackmapValue>();
</span><span class="cx"> ASSERT(value);
</span><span class="lines">@@ -89,6 +90,13 @@
</span><span class="cx">
</span><span class="cx"> Arg::Role role;
</span><span class="cx"> switch (roleMode) {
</span><ins>+ case ForceLateUseUnlessRecoverable:
+ ASSERT(firstRecoverableIndex);
+ if (arg != inst.args[*firstRecoverableIndex] && arg != inst.args[*firstRecoverableIndex + 1]) {
+ role = Arg::LateColdUse;
+ break;
+ }
+ FALLTHROUGH;
</ins><span class="cx"> case SameAsRep:
</span><span class="cx"> switch (child.rep().kind()) {
</span><span class="cx"> case ValueRep::WarmAny:
</span><span class="lines">@@ -273,6 +281,9 @@
</span><span class="cx"> case StackmapSpecial::SameAsRep:
</span><span class="cx"> out.print("SameAsRep");
</span><span class="cx"> return;
</span><ins>+ case StackmapSpecial::ForceLateUseUnlessRecoverable:
+ out.print("ForceLateUseUnlessRecoverable");
+ return;
</ins><span class="cx"> case StackmapSpecial::ForceLateUse:
</span><span class="cx"> out.print("ForceLateUse");
</span><span class="cx"> return;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreb3B3StackmapSpecialh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/b3/B3StackmapSpecial.h (196615 => 196616)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/b3/B3StackmapSpecial.h 2016-02-16 04:41:58 UTC (rev 196615)
+++ trunk/Source/JavaScriptCore/b3/B3StackmapSpecial.h 2016-02-16 04:51:20 UTC (rev 196616)
</span><span class="lines">@@ -47,6 +47,7 @@
</span><span class="cx">
</span><span class="cx"> enum RoleMode : int8_t {
</span><span class="cx"> SameAsRep,
</span><ins>+ ForceLateUseUnlessRecoverable,
</ins><span class="cx"> ForceLateUse
</span><span class="cx"> };
</span><span class="cx">
</span><span class="lines">@@ -59,7 +60,8 @@
</span><span class="cx"> // subclasses that implement that.
</span><span class="cx"> void forEachArgImpl(
</span><span class="cx"> unsigned numIgnoredB3Args, unsigned numIgnoredAirArgs,
</span><del>- Air::Inst&, RoleMode, const ScopedLambda<Air::Inst::EachArgCallback>&);
</del><ins>+ Air::Inst&, RoleMode, Optional<unsigned> firstRecoverableIndex,
+ const ScopedLambda<Air::Inst::EachArgCallback>&);
</ins><span class="cx">
</span><span class="cx"> bool isValidImpl(
</span><span class="cx"> unsigned numIgnoredB3Args, unsigned numIgnoredAirArgs,
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreb3B3StackmapValueh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/b3/B3StackmapValue.h (196615 => 196616)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/b3/B3StackmapValue.h 2016-02-16 04:41:58 UTC (rev 196615)
+++ trunk/Source/JavaScriptCore/b3/B3StackmapValue.h 2016-02-16 04:51:20 UTC (rev 196616)
</span><span class="lines">@@ -91,6 +91,11 @@
</span><span class="cx"> {
</span><span class="cx"> appendVectorWithRep(vector, ValueRep::ColdAny);
</span><span class="cx"> }
</span><ins>+ template<typename VectorType>
+ void appendLateColdAnys(const VectorType& vector)
+ {
+ appendVectorWithRep(vector, ValueRep::LateColdAny);
+ }
</ins><span class="cx">
</span><span class="cx"> // This is a helper for something you might do a lot of: append a value that should be constrained
</span><span class="cx"> // to SomeRegister.
</span></span></pre>
</div>
</div>
</body>
</html>