Remove instructions from dead blocks when building the dominator tree am: 7023bf8227 am: 8880c14298 am: 4adb3eb29b am: 65d72a1c79
Original change: https://android-review.googlesource.com/c/platform/art/+/2190021
Change-Id: Id73317329995f50bcf3427d9686fc1dcbcba363d
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
diff --git a/compiler/optimizing/nodes.cc b/compiler/optimizing/nodes.cc
index 4fbb033..cb07d2e 100644
--- a/compiler/optimizing/nodes.cc
+++ b/compiler/optimizing/nodes.cc
@@ -183,6 +183,7 @@
}
void HGraph::RemoveDeadBlocks(const ArenaBitVector& visited) {
+ DCHECK(reverse_post_order_.empty()) << "We shouldn't have dominance information.";
for (size_t i = 0; i < blocks_.size(); ++i) {
if (!visited.IsBitSet(i)) {
HBasicBlock* block = blocks_[i];
@@ -190,7 +191,7 @@
// Disconnect from its sucessors, and remove all remaining uses.
block->DisconnectFromSuccessors(&visited);
- block->RemoveCatchPhiUses(/* remove_instruction = */ false);
+ block->RemoveCatchPhiUsesAndInstruction(/* building_dominator_tree = */ true);
// Remove the block from the list of blocks, so that further analyses
// never see it.
@@ -2435,7 +2436,7 @@
// remove `index`-th input of all phis in the catch block since they are
// guaranteed dead. Note that we may miss dead inputs this way but the
// graph will always remain consistent.
- RemoveCatchPhiUses(/* remove_instruction = */ true);
+ RemoveCatchPhiUsesAndInstruction(/* building_dominator_tree = */ false);
// (4) Disconnect the block from its predecessors and update their
// control-flow instructions.
@@ -2544,20 +2545,32 @@
successors_.clear();
}
-void HBasicBlock::RemoveCatchPhiUses(bool remove_instruction) {
+void HBasicBlock::RemoveCatchPhiUsesAndInstruction(bool building_dominator_tree) {
for (HBackwardInstructionIterator it(GetInstructions()); !it.Done(); it.Advance()) {
HInstruction* insn = it.Current();
RemoveCatchPhiUsesOfDeadInstruction(insn);
- if (remove_instruction) {
- RemoveInstruction(insn);
+
+ // If we are building the dominator tree, we removed all input records previously.
+ // `RemoveInstruction` will try to remove them again but that's not something we support and we
+ // will crash. We check here since we won't be checking that in RemoveInstruction.
+ if (building_dominator_tree) {
+ DCHECK(insn->GetUses().empty());
+ DCHECK(insn->GetEnvUses().empty());
}
+ RemoveInstruction(insn, /* ensure_safety= */ !building_dominator_tree);
}
for (HInstructionIterator it(GetPhis()); !it.Done(); it.Advance()) {
HPhi* insn = it.Current()->AsPhi();
RemoveCatchPhiUsesOfDeadInstruction(insn);
- if (remove_instruction) {
- RemovePhi(insn);
+
+ // If we are building the dominator tree, we removed all input records previously.
+ // `RemovePhi` will try to remove them again but that's not something we support and we
+ // will crash. We check here since we won't be checking that in RemovePhi.
+ if (building_dominator_tree) {
+ DCHECK(insn->GetUses().empty());
+ DCHECK(insn->GetEnvUses().empty());
}
+ RemovePhi(insn, /* ensure_safety= */ !building_dominator_tree);
}
}
diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h
index 103d318..98bcd38 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -1351,11 +1351,14 @@
// skip updating those phis.
void DisconnectFromSuccessors(const ArenaBitVector* visited = nullptr);
- // Removes the catch phi uses of the instructions in `this`. If `remove_instruction` is set to
- // true, it will also remove the instructions themselves. This method assumes the instructions
- // have been removed from all users with the exception of catch phis because of missing
- // exceptional edges in the graph.
- void RemoveCatchPhiUses(bool remove_instruction);
+ // Removes the catch phi uses of the instructions in `this`, and then remove the instruction
+ // itself. If `building_dominator_tree` is true, it will not remove the instruction as user, since
+ // we do it in a previous step. This is a special case for building up the dominator tree: we want
+ // to eliminate uses before inputs but we don't have domination information, so we remove all
+ // connections from input/uses first before removing any instruction.
+ // This method assumes the instructions have been removed from all users with the exception of
+ // catch phis because of missing exceptional edges in the graph.
+ void RemoveCatchPhiUsesAndInstruction(bool building_dominator_tree);
void AddInstruction(HInstruction* instruction);
// Insert `instruction` before/after an existing instruction `cursor`.