<!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>[165912] 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/165912">165912</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2014-03-19 13:36:45 -0700 (Wed, 19 Mar 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>REGRESSION(<a href="http://trac.webkit.org/projects/webkit/changeset/165459">r165459</a>): It broke 109 jsc stress test on ARM Thumb2 and Mac 32 bit
https://bugs.webkit.org/show_bug.cgi?id=130134
Reviewed by Mark Hahnenberg.
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode): Can't do some optimizations if you don't have a lot of registers.
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::cachedGetById): Move stuff around before going into the IC code to ensure that we give the IC code the invariants it needs. This only happens in case of GetByIdFlush, where we are forced into using weird combinations of registers because the results have to be in t0/t1.
(JSC::DFG::SpeculativeJIT::compile): For a normal GetById, the register allocator should just do the right thing so nobody has to move anything around.
* jit/JITInlineCacheGenerator.cpp:
(JSC::JITGetByIdGenerator::JITGetByIdGenerator): Assert the things we want.
* jit/JITInlineCacheGenerator.h:
* jit/Repatch.cpp:
(JSC::generateGetByIdStub): Remove a previous incomplete hack to try to work around the DFG's problem.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGFixupPhasecpp">trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGSpeculativeJIT32_64cpp">trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorejitJITInlineCacheGeneratorcpp">trunk/Source/JavaScriptCore/jit/JITInlineCacheGenerator.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorejitJITInlineCacheGeneratorh">trunk/Source/JavaScriptCore/jit/JITInlineCacheGenerator.h</a></li>
<li><a href="#trunkSourceJavaScriptCorejitRepatchcpp">trunk/Source/JavaScriptCore/jit/Repatch.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (165911 => 165912)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2014-03-19 20:12:24 UTC (rev 165911)
+++ trunk/Source/JavaScriptCore/ChangeLog 2014-03-19 20:36:45 UTC (rev 165912)
</span><span class="lines">@@ -1,3 +1,21 @@
</span><ins>+2014-03-19 Filip Pizlo <fpizlo@apple.com>
+
+ REGRESSION(r165459): It broke 109 jsc stress test on ARM Thumb2 and Mac 32 bit
+ https://bugs.webkit.org/show_bug.cgi?id=130134
+
+ Reviewed by Mark Hahnenberg.
+
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::fixupNode): Can't do some optimizations if you don't have a lot of registers.
+ * dfg/DFGSpeculativeJIT32_64.cpp:
+ (JSC::DFG::SpeculativeJIT::cachedGetById): Move stuff around before going into the IC code to ensure that we give the IC code the invariants it needs. This only happens in case of GetByIdFlush, where we are forced into using weird combinations of registers because the results have to be in t0/t1.
+ (JSC::DFG::SpeculativeJIT::compile): For a normal GetById, the register allocator should just do the right thing so nobody has to move anything around.
+ * jit/JITInlineCacheGenerator.cpp:
+ (JSC::JITGetByIdGenerator::JITGetByIdGenerator): Assert the things we want.
+ * jit/JITInlineCacheGenerator.h:
+ * jit/Repatch.cpp:
+ (JSC::generateGetByIdStub): Remove a previous incomplete hack to try to work around the DFG's problem.
+
</ins><span class="cx"> 2014-03-19 Mark Hahnenberg <mhahnenberg@apple.com>
</span><span class="cx">
</span><span class="cx"> Normalize some of the older JSC options
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGFixupPhasecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp (165911 => 165912)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp 2014-03-19 20:12:24 UTC (rev 165911)
+++ trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp 2014-03-19 20:36:45 UTC (rev 165912)
</span><span class="lines">@@ -418,7 +418,7 @@
</span><span class="cx"> fixEdge<StringIdentUse>(node->child2());
</span><span class="cx"> break;
</span><span class="cx"> }
</span><del>- if (node->child1()->shouldSpeculateString() && node->child2()->shouldSpeculateString() && GPRInfo::numberOfRegisters >= 7) {
</del><ins>+ if (node->child1()->shouldSpeculateString() && node->child2()->shouldSpeculateString() && (GPRInfo::numberOfRegisters >= 7 || isFTL(m_graph.m_plan.mode))) {
</ins><span class="cx"> fixEdge<StringUse>(node->child1());
</span><span class="cx"> fixEdge<StringUse>(node->child2());
</span><span class="cx"> break;
</span><span class="lines">@@ -448,11 +448,11 @@
</span><span class="cx"> fixEdge<NotStringVarUse>(node->child1());
</span><span class="cx"> break;
</span><span class="cx"> }
</span><del>- if (node->child1()->shouldSpeculateString()) {
</del><ins>+ if (node->child1()->shouldSpeculateString() && (GPRInfo::numberOfRegisters >= 8 || isFTL(m_graph.m_plan.mode))) {
</ins><span class="cx"> fixEdge<StringUse>(node->child1());
</span><span class="cx"> break;
</span><span class="cx"> }
</span><del>- if (node->child2()->shouldSpeculateString()) {
</del><ins>+ if (node->child2()->shouldSpeculateString() && (GPRInfo::numberOfRegisters >= 8 || isFTL(m_graph.m_plan.mode))) {
</ins><span class="cx"> fixEdge<StringUse>(node->child2());
</span><span class="cx"> break;
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGSpeculativeJIT32_64cpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp (165911 => 165912)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp 2014-03-19 20:12:24 UTC (rev 165911)
+++ trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp 2014-03-19 20:36:45 UTC (rev 165912)
</span><span class="lines">@@ -169,8 +169,24 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void SpeculativeJIT::cachedGetById(CodeOrigin codeOrigin, GPRReg baseTagGPROrNone, GPRReg basePayloadGPR, GPRReg resultTagGPR, GPRReg resultPayloadGPR, unsigned identifierNumber, JITCompiler::Jump slowPathTarget, SpillRegistersMode spillMode)
</del><ins>+void SpeculativeJIT::cachedGetById(
+ CodeOrigin codeOrigin, GPRReg baseTagGPROrNone, GPRReg basePayloadGPR, GPRReg resultTagGPR, GPRReg resultPayloadGPR,
+ unsigned identifierNumber, JITCompiler::Jump slowPathTarget, SpillRegistersMode spillMode)
</ins><span class="cx"> {
</span><ins>+ // This is a hacky fix for when the register allocator decides to alias the base payload with the result tag. This only happens
+ // in the case of GetByIdFlush, which has a relatively expensive register allocation story already so we probably don't need to
+ // trip over one move instruction.
+ if (basePayloadGPR == resultTagGPR) {
+ RELEASE_ASSERT(basePayloadGPR != resultPayloadGPR);
+
+ if (baseTagGPROrNone == resultPayloadGPR) {
+ m_jit.swap(basePayloadGPR, baseTagGPROrNone);
+ baseTagGPROrNone = resultTagGPR;
+ } else
+ m_jit.move(basePayloadGPR, resultPayloadGPR);
+ basePayloadGPR = resultPayloadGPR;
+ }
+
</ins><span class="cx"> JITGetByIdGenerator gen(
</span><span class="cx"> m_jit.codeBlock(), codeOrigin, usedRegisters(),
</span><span class="cx"> JSValueRegs(baseTagGPROrNone, basePayloadGPR),
</span><span class="lines">@@ -3659,8 +3675,8 @@
</span><span class="cx"> switch (node->child1().useKind()) {
</span><span class="cx"> case CellUse: {
</span><span class="cx"> SpeculateCellOperand base(this, node->child1());
</span><del>- GPRTemporary resultTag(this, Reuse, base);
- GPRTemporary resultPayload(this);
</del><ins>+ GPRTemporary resultTag(this);
+ GPRTemporary resultPayload(this, Reuse, base);
</ins><span class="cx">
</span><span class="cx"> GPRReg baseGPR = base.gpr();
</span><span class="cx"> GPRReg resultTagGPR = resultTag.gpr();
</span><span class="lines">@@ -3676,8 +3692,8 @@
</span><span class="cx">
</span><span class="cx"> case UntypedUse: {
</span><span class="cx"> JSValueOperand base(this, node->child1());
</span><del>- GPRTemporary resultTag(this, Reuse, base, TagWord);
- GPRTemporary resultPayload(this);
</del><ins>+ GPRTemporary resultTag(this);
+ GPRTemporary resultPayload(this, Reuse, base, TagWord);
</ins><span class="cx">
</span><span class="cx"> GPRReg baseTagGPR = base.tagGPR();
</span><span class="cx"> GPRReg basePayloadGPR = base.payloadGPR();
</span><span class="lines">@@ -3713,10 +3729,10 @@
</span><span class="cx">
</span><span class="cx"> GPRReg baseGPR = base.gpr();
</span><span class="cx">
</span><del>- GPRResult resultTag(this);
- GPRResult2 resultPayload(this);
</del><ins>+ GPRResult resultPayload(this);
+ GPRResult2 resultTag(this);
+ GPRReg resultPayloadGPR = resultPayload.gpr();
</ins><span class="cx"> GPRReg resultTagGPR = resultTag.gpr();
</span><del>- GPRReg resultPayloadGPR = resultPayload.gpr();
</del><span class="cx">
</span><span class="cx"> base.use();
</span><span class="cx">
</span><span class="lines">@@ -3733,10 +3749,10 @@
</span><span class="cx"> GPRReg baseTagGPR = base.tagGPR();
</span><span class="cx"> GPRReg basePayloadGPR = base.payloadGPR();
</span><span class="cx">
</span><del>- GPRResult resultTag(this);
- GPRResult2 resultPayload(this);
</del><ins>+ GPRResult resultPayload(this);
+ GPRResult2 resultTag(this);
+ GPRReg resultPayloadGPR = resultPayload.gpr();
</ins><span class="cx"> GPRReg resultTagGPR = resultTag.gpr();
</span><del>- GPRReg resultPayloadGPR = resultPayload.gpr();
</del><span class="cx">
</span><span class="cx"> base.use();
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorejitJITInlineCacheGeneratorcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/jit/JITInlineCacheGenerator.cpp (165911 => 165912)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/jit/JITInlineCacheGenerator.cpp 2014-03-19 20:12:24 UTC (rev 165911)
+++ trunk/Source/JavaScriptCore/jit/JITInlineCacheGenerator.cpp 2014-03-19 20:36:45 UTC (rev 165912)
</span><span class="lines">@@ -110,6 +110,14 @@
</span><span class="cx"> MacroAssembler::Address(m_base.payloadGPR(), JSObject::butterflyOffset()), butterfly);
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+JITGetByIdGenerator::JITGetByIdGenerator(
+ CodeBlock* codeBlock, CodeOrigin codeOrigin, const RegisterSet& usedRegisters,
+ JSValueRegs base, JSValueRegs value, SpillRegistersMode spillMode)
+ : JITByIdGenerator(codeBlock, codeOrigin, usedRegisters, base, value, spillMode)
+{
+ RELEASE_ASSERT(base.payloadGPR() != value.tagGPR());
+}
+
</ins><span class="cx"> void JITGetByIdGenerator::generateFastPath(MacroAssembler& jit)
</span><span class="cx"> {
</span><span class="cx"> generateFastPathChecks(jit, m_value.payloadGPR());
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorejitJITInlineCacheGeneratorh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/jit/JITInlineCacheGenerator.h (165911 => 165912)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/jit/JITInlineCacheGenerator.h 2014-03-19 20:12:24 UTC (rev 165911)
+++ trunk/Source/JavaScriptCore/jit/JITInlineCacheGenerator.h 2014-03-19 20:36:45 UTC (rev 165912)
</span><span class="lines">@@ -95,11 +95,8 @@
</span><span class="cx"> JITGetByIdGenerator() { }
</span><span class="cx">
</span><span class="cx"> JITGetByIdGenerator(
</span><del>- CodeBlock* codeBlock, CodeOrigin codeOrigin, const RegisterSet& usedRegisters,
- JSValueRegs base, JSValueRegs value, SpillRegistersMode spillMode)
- : JITByIdGenerator(codeBlock, codeOrigin, usedRegisters, base, value, spillMode)
- {
- }
</del><ins>+ CodeBlock*, CodeOrigin, const RegisterSet& usedRegisters, JSValueRegs base,
+ JSValueRegs value, SpillRegistersMode spillMode);
</ins><span class="cx">
</span><span class="cx"> void generateFastPath(MacroAssembler&);
</span><span class="cx"> };
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorejitRepatchcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/jit/Repatch.cpp (165911 => 165912)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/jit/Repatch.cpp 2014-03-19 20:12:24 UTC (rev 165911)
+++ trunk/Source/JavaScriptCore/jit/Repatch.cpp 2014-03-19 20:36:45 UTC (rev 165912)
</span><span class="lines">@@ -301,13 +301,8 @@
</span><span class="cx"> #if USE(JSVALUE64)
</span><span class="cx"> stubJit.load64(MacroAssembler::Address(storageGPR, offsetRelativeToBase(offset)), loadedValueGPR);
</span><span class="cx"> #else
</span><del>- GPRReg copyOfStorageGPR = storageGPR;
- if (storageGPR == resultTagGPR) {
- copyOfStorageGPR = loadedValueGPR;
- stubJit.move(storageGPR, copyOfStorageGPR);
- }
</del><span class="cx"> stubJit.load32(MacroAssembler::Address(storageGPR, offsetRelativeToBase(offset) + TagOffset), resultTagGPR);
</span><del>- stubJit.load32(MacroAssembler::Address(copyOfStorageGPR, offsetRelativeToBase(offset) + PayloadOffset), loadedValueGPR);
</del><ins>+ stubJit.load32(MacroAssembler::Address(storageGPR, offsetRelativeToBase(offset) + PayloadOffset), loadedValueGPR);
</ins><span class="cx"> #endif
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>