<!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>[202518] 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/202518">202518</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2016-06-27 15:26:41 -0700 (Mon, 27 Jun 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>Clean up resetting reachability in B3/Air
https://bugs.webkit.org/show_bug.cgi?id=159170
Reviewed by Geoffrey Garen.
When I fixed bug 159165, I took the brute force approach. I still used the
B3::resetReachability() method, and changed the callback to record the set of deleted values
instead of deleting them eagerly. But this means tracking the set of deleted values, even
though resetReachability() already internally tracks the set of deleted blocks. You can find
out if a value is deleted by asking if its owning block was deleted.
So, this change refactors B3::resetReachability() into a new helper called
B3::recomputePredecessors(). This new helper skips the block deletion step, and lets the
client delete blocks. This lets Air delete blocks the same way that it did before, and it
lets B3 use the isBlockDead() method (which is a glorified proxy for
block->predecessors().isEmpty()) to track which values are deleted. This allows B3 to turn
Upsilons that point to dead Phis into Nops before deleting the blocks.
This shouldn't affect performance or anything real. It just makes the code cleaner.
* b3/B3BasicBlockUtils.h:
(JSC::B3::updatePredecessorsAfter):
(JSC::B3::recomputePredecessors):
(JSC::B3::isBlockDead):
(JSC::B3::resetReachability): Deleted.
* b3/B3Procedure.cpp:
(JSC::B3::Procedure::resetReachability):
(JSC::B3::Procedure::invalidateCFG):
* b3/air/AirCode.cpp:
(JSC::B3::Air::Code::resetReachability):
(JSC::B3::Air::Code::dump):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreb3B3BasicBlockUtilsh">trunk/Source/JavaScriptCore/b3/B3BasicBlockUtils.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreb3B3Procedurecpp">trunk/Source/JavaScriptCore/b3/B3Procedure.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreb3airAirCodecpp">trunk/Source/JavaScriptCore/b3/air/AirCode.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (202517 => 202518)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-06-27 22:20:17 UTC (rev 202517)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-06-27 22:26:41 UTC (rev 202518)
</span><span class="lines">@@ -1,3 +1,37 @@
</span><ins>+2016-06-27 Filip Pizlo <fpizlo@apple.com>
+
+ Clean up resetting reachability in B3/Air
+ https://bugs.webkit.org/show_bug.cgi?id=159170
+
+ Reviewed by Geoffrey Garen.
+
+ When I fixed bug 159165, I took the brute force approach. I still used the
+ B3::resetReachability() method, and changed the callback to record the set of deleted values
+ instead of deleting them eagerly. But this means tracking the set of deleted values, even
+ though resetReachability() already internally tracks the set of deleted blocks. You can find
+ out if a value is deleted by asking if its owning block was deleted.
+
+ So, this change refactors B3::resetReachability() into a new helper called
+ B3::recomputePredecessors(). This new helper skips the block deletion step, and lets the
+ client delete blocks. This lets Air delete blocks the same way that it did before, and it
+ lets B3 use the isBlockDead() method (which is a glorified proxy for
+ block->predecessors().isEmpty()) to track which values are deleted. This allows B3 to turn
+ Upsilons that point to dead Phis into Nops before deleting the blocks.
+
+ This shouldn't affect performance or anything real. It just makes the code cleaner.
+
+ * b3/B3BasicBlockUtils.h:
+ (JSC::B3::updatePredecessorsAfter):
+ (JSC::B3::recomputePredecessors):
+ (JSC::B3::isBlockDead):
+ (JSC::B3::resetReachability): Deleted.
+ * b3/B3Procedure.cpp:
+ (JSC::B3::Procedure::resetReachability):
+ (JSC::B3::Procedure::invalidateCFG):
+ * b3/air/AirCode.cpp:
+ (JSC::B3::Air::Code::resetReachability):
+ (JSC::B3::Air::Code::dump):
+
</ins><span class="cx"> 2016-06-27 Brian Burg <bburg@apple.com>
</span><span class="cx">
</span><span class="cx"> Web Inspector: CRASH in backend at Inspector::HeapFrontendDispatcher::garbageCollected + 552 when closing frontend/inspected page
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreb3B3BasicBlockUtilsh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/b3/B3BasicBlockUtils.h (202517 => 202518)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/b3/B3BasicBlockUtils.h 2016-06-27 22:20:17 UTC (rev 202517)
+++ trunk/Source/JavaScriptCore/b3/B3BasicBlockUtils.h 2016-06-27 22:26:41 UTC (rev 202518)
</span><span class="lines">@@ -1,5 +1,5 @@
</span><span class="cx"> /*
</span><del>- * Copyright (C) 2015 Apple Inc. All rights reserved.
</del><ins>+ * Copyright (C) 2015-2016 Apple Inc. All rights reserved.
</ins><span class="cx"> *
</span><span class="cx"> * Redistribution and use in source and binary forms, with or without
</span><span class="cx"> * modification, are permitted provided that the following conditions
</span><span class="lines">@@ -85,10 +85,8 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><del>-// This recomputes predecessors and removes blocks that aren't reachable.
-template<typename BasicBlock, typename DeleteFunctor>
-void resetReachability(
- Vector<std::unique_ptr<BasicBlock>>& blocks, const DeleteFunctor& deleteFunctor)
</del><ins>+template<typename BasicBlock>
+void recomputePredecessors(Vector<std::unique_ptr<BasicBlock>>& blocks)
</ins><span class="cx"> {
</span><span class="cx"> // Clear all predecessor lists first.
</span><span class="cx"> for (auto& block : blocks) {
</span><span class="lines">@@ -97,15 +95,16 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> updatePredecessorsAfter(blocks[0].get());
</span><ins>+}
</ins><span class="cx">
</span><del>- for (unsigned i = 1; i < blocks.size(); ++i) {
- if (!blocks[i])
- continue;
- if (blocks[i]->predecessors().isEmpty()) {
- deleteFunctor(blocks[i].get());
- blocks[i] = nullptr;
- }
- }
</del><ins>+template<typename BasicBlock>
+bool isBlockDead(BasicBlock* block)
+{
+ if (!block)
+ return false;
+ if (!block->index())
+ return false;
+ return block->predecessors().isEmpty();
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> template<typename BasicBlock>
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreb3B3Procedurecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/b3/B3Procedure.cpp (202517 => 202518)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/b3/B3Procedure.cpp 2016-06-27 22:20:17 UTC (rev 202517)
+++ trunk/Source/JavaScriptCore/b3/B3Procedure.cpp 2016-06-27 22:26:41 UTC (rev 202518)
</span><span class="lines">@@ -173,31 +173,35 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><del>- IndexSet<Value> valuesToDelete;
</del><ins>+ recomputePredecessors(m_blocks);
</ins><span class="cx">
</span><del>- B3::resetReachability(
- m_blocks,
- [&] (BasicBlock* deleted) {
- // Gotta delete the values in this block.
- for (Value* value : *deleted)
- valuesToDelete.add(value);
- });
-
- if (valuesToDelete.isEmpty())
</del><ins>+ // The common case is that this does not find any dead blocks.
+ bool foundDead = false;
+ for (auto& block : m_blocks) {
+ if (isBlockDead(block.get())) {
+ foundDead = true;
+ break;
+ }
+ }
+ if (!foundDead)
</ins><span class="cx"> return;
</span><span class="cx">
</span><del>- for (BasicBlock* block : *this) {
- for (Value* value : *block) {
- ASSERT(!valuesToDelete.contains(value)); // The block should have been deleted already.
- if (UpsilonValue* upsilon = value->as<UpsilonValue>()) {
- if (valuesToDelete.contains(upsilon->phi()))
- upsilon->replaceWithNop();
- }
</del><ins>+ resetValueOwners();
+
+ for (Value* value : values()) {
+ if (UpsilonValue* upsilon = value->as<UpsilonValue>()) {
+ if (isBlockDead(upsilon->phi()->owner))
+ upsilon->replaceWithNop();
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><del>- for (Value* value : valuesToDelete.values(values()))
- deleteValue(value);
</del><ins>+ for (auto& block : m_blocks) {
+ if (isBlockDead(block.get())) {
+ for (Value* value : *block)
+ deleteValue(value);
+ block = nullptr;
+ }
+ }
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void Procedure::invalidateCFG()
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreb3airAirCodecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/b3/air/AirCode.cpp (202517 => 202518)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/b3/air/AirCode.cpp 2016-06-27 22:20:17 UTC (rev 202517)
+++ trunk/Source/JavaScriptCore/b3/air/AirCode.cpp 2016-06-27 22:26:41 UTC (rev 202518)
</span><span class="lines">@@ -80,11 +80,12 @@
</span><span class="cx">
</span><span class="cx"> void Code::resetReachability()
</span><span class="cx"> {
</span><del>- B3::resetReachability(
- m_blocks,
- [&] (BasicBlock*) {
- // We don't have to do anything special for deleted blocks.
- });
</del><ins>+ recomputePredecessors(m_blocks);
+
+ for (auto& block : m_blocks) {
+ if (isBlockDead(block.get()))
+ block = nullptr;
+ }
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void Code::dump(PrintStream& out) const
</span></span></pre>
</div>
</div>
</body>
</html>