Title: [202518] trunk/Source/_javascript_Core
- Revision
- 202518
- Author
- [email protected]
- Date
- 2016-06-27 15:26:41 -0700 (Mon, 27 Jun 2016)
Log Message
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):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (202517 => 202518)
--- trunk/Source/_javascript_Core/ChangeLog 2016-06-27 22:20:17 UTC (rev 202517)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-06-27 22:26:41 UTC (rev 202518)
@@ -1,3 +1,37 @@
+2016-06-27 Filip Pizlo <[email protected]>
+
+ 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):
+
2016-06-27 Brian Burg <[email protected]>
Web Inspector: CRASH in backend at Inspector::HeapFrontendDispatcher::garbageCollected + 552 when closing frontend/inspected page
Modified: trunk/Source/_javascript_Core/b3/B3BasicBlockUtils.h (202517 => 202518)
--- trunk/Source/_javascript_Core/b3/B3BasicBlockUtils.h 2016-06-27 22:20:17 UTC (rev 202517)
+++ trunk/Source/_javascript_Core/b3/B3BasicBlockUtils.h 2016-06-27 22:26:41 UTC (rev 202518)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2015-2016 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -85,10 +85,8 @@
}
}
-// 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)
+template<typename BasicBlock>
+void recomputePredecessors(Vector<std::unique_ptr<BasicBlock>>& blocks)
{
// Clear all predecessor lists first.
for (auto& block : blocks) {
@@ -97,15 +95,16 @@
}
updatePredecessorsAfter(blocks[0].get());
+}
- for (unsigned i = 1; i < blocks.size(); ++i) {
- if (!blocks[i])
- continue;
- if (blocks[i]->predecessors().isEmpty()) {
- deleteFunctor(blocks[i].get());
- blocks[i] = nullptr;
- }
- }
+template<typename BasicBlock>
+bool isBlockDead(BasicBlock* block)
+{
+ if (!block)
+ return false;
+ if (!block->index())
+ return false;
+ return block->predecessors().isEmpty();
}
template<typename BasicBlock>
Modified: trunk/Source/_javascript_Core/b3/B3Procedure.cpp (202517 => 202518)
--- trunk/Source/_javascript_Core/b3/B3Procedure.cpp 2016-06-27 22:20:17 UTC (rev 202517)
+++ trunk/Source/_javascript_Core/b3/B3Procedure.cpp 2016-06-27 22:26:41 UTC (rev 202518)
@@ -173,31 +173,35 @@
}
}
- IndexSet<Value> valuesToDelete;
+ recomputePredecessors(m_blocks);
- B3::resetReachability(
- m_blocks,
- [&] (BasicBlock* deleted) {
- // Gotta delete the values in this block.
- for (Value* value : *deleted)
- valuesToDelete.add(value);
- });
-
- if (valuesToDelete.isEmpty())
+ // 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)
return;
- 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();
- }
+ resetValueOwners();
+
+ for (Value* value : values()) {
+ if (UpsilonValue* upsilon = value->as<UpsilonValue>()) {
+ if (isBlockDead(upsilon->phi()->owner))
+ upsilon->replaceWithNop();
}
}
- for (Value* value : valuesToDelete.values(values()))
- deleteValue(value);
+ for (auto& block : m_blocks) {
+ if (isBlockDead(block.get())) {
+ for (Value* value : *block)
+ deleteValue(value);
+ block = nullptr;
+ }
+ }
}
void Procedure::invalidateCFG()
Modified: trunk/Source/_javascript_Core/b3/air/AirCode.cpp (202517 => 202518)
--- trunk/Source/_javascript_Core/b3/air/AirCode.cpp 2016-06-27 22:20:17 UTC (rev 202517)
+++ trunk/Source/_javascript_Core/b3/air/AirCode.cpp 2016-06-27 22:26:41 UTC (rev 202518)
@@ -80,11 +80,12 @@
void Code::resetReachability()
{
- B3::resetReachability(
- m_blocks,
- [&] (BasicBlock*) {
- // We don't have to do anything special for deleted blocks.
- });
+ recomputePredecessors(m_blocks);
+
+ for (auto& block : m_blocks) {
+ if (isBlockDead(block.get()))
+ block = nullptr;
+ }
}
void Code::dump(PrintStream& out) const
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
