<!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>[179746] 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/179746">179746</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2015-02-06 08:04:39 -0800 (Fri, 06 Feb 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>Remove BytecodeGenerator::preserveLastVar() and replace it with a more robust mechanism for preserving non-temporary registers
https://bugs.webkit.org/show_bug.cgi?id=141211
Reviewed by Mark Lam.
Previously, the way non-temporary registers were preserved (i.e. not reclaimed anytime
we did newTemporary()) by calling preserveLastVar() after all non-temps are created. It
would raise the refcount on the last (highest-numbered) variable created, and rely on
the fact that register reclamation started at higher-numbered registers and worked its
way down. So any retained register would block any lower-numbered registers from being
reclaimed.
Also, preserveLastVar() sets a thing called m_firstConstantIndex. It's unused.
This removes preserveLastVar() and makes addVar() retain each register it creates. This
is more explicit, since addVar() is the mechanism for creating non-temporary registers.
To make this work I had to remove an assertion that Register::setIndex() can only be
called when the refcount is zero. This method might be called after a var is created to
change its index. This previously worked because preserveLastVar() would be called after
we had already made all index changes, so the vars would still have refcount zero. Now
they have refcount 1. I think it's OK to lose this assertion; I can't remember this
assertion ever firing in a way that alerted me to a serious issue.
* bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::BytecodeGenerator):
(JSC::BytecodeGenerator::preserveLastVar): Deleted.
* bytecompiler/BytecodeGenerator.h:
(JSC::BytecodeGenerator::addVar):
* bytecompiler/RegisterID.h:
(JSC::RegisterID::setIndex):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecompilerBytecodeGeneratorcpp">trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecompilerBytecodeGeneratorh">trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecompilerRegisterIDh">trunk/Source/JavaScriptCore/bytecompiler/RegisterID.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (179745 => 179746)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2015-02-06 14:50:20 UTC (rev 179745)
+++ trunk/Source/JavaScriptCore/ChangeLog 2015-02-06 16:04:39 UTC (rev 179746)
</span><span class="lines">@@ -1,3 +1,37 @@
</span><ins>+2015-02-04 Filip Pizlo <fpizlo@apple.com>
+
+ Remove BytecodeGenerator::preserveLastVar() and replace it with a more robust mechanism for preserving non-temporary registers
+ https://bugs.webkit.org/show_bug.cgi?id=141211
+
+ Reviewed by Mark Lam.
+
+ Previously, the way non-temporary registers were preserved (i.e. not reclaimed anytime
+ we did newTemporary()) by calling preserveLastVar() after all non-temps are created. It
+ would raise the refcount on the last (highest-numbered) variable created, and rely on
+ the fact that register reclamation started at higher-numbered registers and worked its
+ way down. So any retained register would block any lower-numbered registers from being
+ reclaimed.
+
+ Also, preserveLastVar() sets a thing called m_firstConstantIndex. It's unused.
+
+ This removes preserveLastVar() and makes addVar() retain each register it creates. This
+ is more explicit, since addVar() is the mechanism for creating non-temporary registers.
+
+ To make this work I had to remove an assertion that Register::setIndex() can only be
+ called when the refcount is zero. This method might be called after a var is created to
+ change its index. This previously worked because preserveLastVar() would be called after
+ we had already made all index changes, so the vars would still have refcount zero. Now
+ they have refcount 1. I think it's OK to lose this assertion; I can't remember this
+ assertion ever firing in a way that alerted me to a serious issue.
+
+ * bytecompiler/BytecodeGenerator.cpp:
+ (JSC::BytecodeGenerator::BytecodeGenerator):
+ (JSC::BytecodeGenerator::preserveLastVar): Deleted.
+ * bytecompiler/BytecodeGenerator.h:
+ (JSC::BytecodeGenerator::addVar):
+ * bytecompiler/RegisterID.h:
+ (JSC::RegisterID::setIndex):
+
</ins><span class="cx"> 2015-02-06 Andreas Kling <akling@apple.com>
</span><span class="cx">
</span><span class="cx"> Remove WTF::fastMallocGoodSize().
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecompilerBytecodeGeneratorcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp (179745 => 179746)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp 2015-02-06 14:50:20 UTC (rev 179745)
+++ trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp 2015-02-06 16:04:39 UTC (rev 179746)
</span><span class="lines">@@ -151,12 +151,6 @@
</span><span class="cx"> return true;
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void BytecodeGenerator::preserveLastVar()
-{
- if ((m_firstConstantIndex = m_calleeRegisters.size()) != 0)
- m_lastVar = &m_calleeRegisters.last();
-}
-
</del><span class="cx"> BytecodeGenerator::BytecodeGenerator(VM& vm, ProgramNode* programNode, UnlinkedProgramCodeBlock* codeBlock, DebuggerMode debuggerMode, ProfilerMode profilerMode)
</span><span class="cx"> : m_shouldEmitDebugHooks(Options::forceDebuggerBytecodeGeneration() || debuggerMode == DebuggerOn)
</span><span class="cx"> , m_shouldEmitProfileHooks(Options::forceProfilerBytecodeGeneration() || profilerMode == ProfilerOn)
</span><span class="lines">@@ -428,7 +422,6 @@
</span><span class="cx"> }
</span><span class="cx"> addParameter(simpleParameter->boundProperty(), index);
</span><span class="cx"> }
</span><del>- preserveLastVar();
</del><span class="cx">
</span><span class="cx"> // We declare the callee's name last because it should lose to a var, function, and/or parameter declaration.
</span><span class="cx"> addCallee(functionNode, calleeRegister);
</span><span class="lines">@@ -493,7 +486,6 @@
</span><span class="cx"> variables.append(varStack[i].first);
</span><span class="cx"> }
</span><span class="cx"> codeBlock->adoptVariables(variables);
</span><del>- preserveLastVar();
</del><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> BytecodeGenerator::~BytecodeGenerator()
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecompilerBytecodeGeneratorh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h (179745 => 179746)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h 2015-02-06 14:50:20 UTC (rev 179745)
+++ trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h 2015-02-06 16:04:39 UTC (rev 179746)
</span><span class="lines">@@ -630,7 +630,10 @@
</span><span class="cx"> RegisterID* addVar()
</span><span class="cx"> {
</span><span class="cx"> ++m_codeBlock->m_numVars;
</span><del>- return newRegister();
</del><ins>+ RegisterID* result = newRegister();
+ ASSERT(VirtualRegister(result->index()).toLocal() == m_codeBlock->m_numVars - 1);
+ result->ref(); // We should never free this slot.
+ return result;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> // Returns the index of the added var.
</span><span class="lines">@@ -777,7 +780,6 @@
</span><span class="cx"> SegmentedVector<RegisterID, 32> m_parameters;
</span><span class="cx"> SegmentedVector<Label, 32> m_labels;
</span><span class="cx"> LabelScopeStore m_labelScopes;
</span><del>- RefPtr<RegisterID> m_lastVar;
</del><span class="cx"> int m_finallyDepth;
</span><span class="cx"> int m_localScopeDepth;
</span><span class="cx"> CodeType m_codeType;
</span><span class="lines">@@ -791,7 +793,6 @@
</span><span class="cx"> Vector<TryRange> m_tryRanges;
</span><span class="cx"> SegmentedVector<TryData, 8> m_tryData;
</span><span class="cx">
</span><del>- int m_firstConstantIndex;
</del><span class="cx"> int m_nextConstantOffset;
</span><span class="cx">
</span><span class="cx"> int m_firstLazyFunction;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecompilerRegisterIDh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecompiler/RegisterID.h (179745 => 179746)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecompiler/RegisterID.h 2015-02-06 14:50:20 UTC (rev 179745)
+++ trunk/Source/JavaScriptCore/bytecompiler/RegisterID.h 2015-02-06 16:04:39 UTC (rev 179746)
</span><span class="lines">@@ -70,7 +70,6 @@
</span><span class="cx">
</span><span class="cx"> void setIndex(int index)
</span><span class="cx"> {
</span><del>- ASSERT(!m_refCount);
</del><span class="cx"> #ifndef NDEBUG
</span><span class="cx"> m_didSetIndex = true;
</span><span class="cx"> #endif
</span></span></pre>
</div>
</div>
</body>
</html>