Merge "Fix performance of unneeded catch handler removal."
diff --git a/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java b/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
index ee2c6ab..163c20b 100644
--- a/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
+++ b/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
@@ -569,13 +569,15 @@
}
/**
- * Unlinks this block from a single predecessor and successor.
+ * Unlinks the current block based on the assumption that it is a catch handler.
*
- * @return Returns the unlinked successor
+ * Catch handlers always have only one predecessor and at most one successor.
+ * That is because we have edge-split form for all exceptional flow.
*/
- public BasicBlock unlinkSingle() {
- unlinkSinglePredecessor();
- return unlinkSingleSuccessor();
+ public void unlinkCatchHandler() {
+ assert predecessors.size() == 1;
+ predecessors.get(0).removeSuccessor(this);
+ predecessors.clear();
}
public void detachAllSuccessors() {
@@ -590,34 +592,41 @@
assert successor.predecessors.contains(this);
List<BasicBlock> removedBlocks = new ArrayList<>();
for (BasicBlock dominated : dominator.dominatedBlocks(successor)) {
+ dominated.cleanForRemoval();
removedBlocks.add(dominated);
- for (BasicBlock block : dominated.successors) {
- block.removePredecessor(dominated);
- }
- dominated.successors.clear();
- for (BasicBlock block : dominated.predecessors) {
- block.removeSuccessor(dominated);
- }
- dominated.predecessors.clear();
- for (Phi phi : dominated.getPhis()) {
- for (Value operand : phi.getOperands()) {
- operand.removePhiUser(phi);
- }
- }
- dominated.getPhis().clear();
- for (Instruction instruction : dominated.getInstructions()) {
- for (Value value : instruction.inValues) {
- value.removeUser(instruction);
- }
- for (Value value : instruction.getDebugValues()) {
- value.removeDebugUser(instruction);
- }
- }
}
assert blocksClean(removedBlocks);
return removedBlocks;
}
+ public void cleanForRemoval() {
+ for (BasicBlock block : successors) {
+ block.removePredecessor(this);
+ }
+ successors.clear();
+ for (BasicBlock block : predecessors) {
+ block.removeSuccessor(this);
+ }
+ predecessors.clear();
+ for (Phi phi : getPhis()) {
+ for (Value operand : phi.getOperands()) {
+ operand.removePhiUser(phi);
+ }
+ }
+ getPhis().clear();
+ for (Instruction instruction : getInstructions()) {
+ if (instruction.outValue != null) {
+ instruction.outValue.clearUsers();
+ }
+ for (Value value : instruction.inValues) {
+ value.removeUser(instruction);
+ }
+ for (Value value : instruction.getDebugValues()) {
+ value.removeDebugUser(instruction);
+ }
+ }
+ }
+
public void linkCatchSuccessors(List<DexType> guards, List<BasicBlock> targets) {
List<Integer> successorIndexes = new ArrayList<>(targets.size());
for (BasicBlock target : targets) {
diff --git a/src/main/java/com/android/tools/r8/ir/code/IRCode.java b/src/main/java/com/android/tools/r8/ir/code/IRCode.java
index bf62fed..2613aae 100644
--- a/src/main/java/com/android/tools/r8/ir/code/IRCode.java
+++ b/src/main/java/com/android/tools/r8/ir/code/IRCode.java
@@ -663,4 +663,31 @@
public boolean verifyNoColorsInUse() {
return usedMarkingColors == 0;
}
+
+ public void removeUnreachableBlocks() {
+ int color = reserveMarkingColor();
+ Queue<BasicBlock> worklist = new ArrayDeque<>();
+ worklist.add(blocks.getFirst());
+ while (!worklist.isEmpty()) {
+ BasicBlock block = worklist.poll();
+ if (block.isMarked(color)) {
+ continue;
+ }
+ block.mark(color);
+ for (BasicBlock successor : block.getSuccessors()) {
+ if (!successor.isMarked(color)) {
+ worklist.add(successor);
+ }
+ }
+ }
+ ListIterator<BasicBlock> blockIterator = listIterator();
+ while (blockIterator.hasNext()) {
+ BasicBlock current = blockIterator.next();
+ if (!current.isMarked(color)) {
+ current.cleanForRemoval();
+ blockIterator.remove();
+ }
+ }
+ returnMarkingColor(color);
+ }
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/DeadCodeRemover.java b/src/main/java/com/android/tools/r8/ir/optimize/DeadCodeRemover.java
index f767e1b..531989e 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/DeadCodeRemover.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/DeadCodeRemover.java
@@ -5,7 +5,6 @@
import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.CatchHandlers;
-import com.android.tools.r8.ir.code.DominatorTree;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.InstructionListIterator;
@@ -20,50 +19,43 @@
public static void removeDeadCode(
IRCode code, CodeRewriter codeRewriter, InternalOptions options) {
+ removeUnneededCatchHandlers(code);
Queue<BasicBlock> worklist = new LinkedList<>();
- int color = code.reserveMarkingColor();
worklist.addAll(code.blocks);
for (BasicBlock block = worklist.poll(); block != null; block = worklist.poll()) {
- if (block.isMarked(color)) {
- // Ignore marked blocks, as they are scheduled for removal.
- continue;
- }
- removeDeadInstructions(worklist, code, block, options, color);
- removeDeadPhis(worklist, block, options, color);
- removeUnneededCatchHandlers(worklist, block, code, color);
+ removeDeadInstructions(worklist, code, block, options);
+ removeDeadPhis(worklist, block, options);
}
- code.removeMarkedBlocks(color);
- code.returnMarkingColor(color);
assert code.isConsistentSSA();
codeRewriter.rewriteMoveResult(code);
}
// Add the block from where the value originates to the worklist.
- private static void updateWorklist(Queue<BasicBlock> worklist, Value value, int color) {
+ private static void updateWorklist(Queue<BasicBlock> worklist, Value value) {
BasicBlock block = null;
if (value.isPhi()) {
block = value.asPhi().getBlock();
} else if (value.definition.hasBlock()) {
block = value.definition.getBlock();
}
- if (block != null && !block.isMarked(color)) {
+ if (block != null) {
worklist.add(block);
}
}
// Add all blocks from where the in/debug-values to the instruction originates.
private static void updateWorklist(
- Queue<BasicBlock> worklist, Instruction instruction, int color) {
+ Queue<BasicBlock> worklist, Instruction instruction) {
for (Value inValue : instruction.inValues()) {
- updateWorklist(worklist, inValue, color);
+ updateWorklist(worklist, inValue);
}
for (Value debugValue : instruction.getDebugValues()) {
- updateWorklist(worklist, debugValue, color);
+ updateWorklist(worklist, debugValue);
}
}
private static void removeDeadPhis(Queue<BasicBlock> worklist, BasicBlock block,
- InternalOptions options, int color) {
+ InternalOptions options) {
Iterator<Phi> phiIt = block.getPhis().iterator();
while (phiIt.hasNext()) {
Phi phi = phiIt.next();
@@ -71,15 +63,14 @@
phiIt.remove();
for (Value operand : phi.getOperands()) {
operand.removePhiUser(phi);
- updateWorklist(worklist, operand, color);
+ updateWorklist(worklist, operand);
}
}
}
}
private static void removeDeadInstructions(
- Queue<BasicBlock> worklist, IRCode code, BasicBlock block, InternalOptions options,
- int color) {
+ Queue<BasicBlock> worklist, IRCode code, BasicBlock block, InternalOptions options) {
InstructionListIterator iterator = block.listIterator(block.getInstructions().size());
while (iterator.hasPrevious()) {
Instruction current = iterator.previous();
@@ -99,7 +90,7 @@
if (!outValue.isDead(options)) {
continue;
}
- updateWorklist(worklist, current, color);
+ updateWorklist(worklist, current);
// All users will be removed for this instruction. Eagerly clear them so further inspection
// of this instruction during dead code elimination will terminate here.
outValue.clearUsers();
@@ -107,22 +98,15 @@
}
}
- private static void removeUnneededCatchHandlers(Queue<BasicBlock> worklist, BasicBlock block,
- IRCode code, int color) {
- if (block.hasCatchHandlers() && !block.canThrow()) {
- CatchHandlers<BasicBlock> handlers = block.getCatchHandlers();
- for (BasicBlock target : handlers.getUniqueTargets()) {
- DominatorTree dominatorTree = new DominatorTree(code);
- for (BasicBlock unlinked : block.unlink(target, dominatorTree)) {
- if (!unlinked.isMarked(color)) {
- Iterator<Instruction> iterator = unlinked.iterator();
- while (iterator.hasNext()) {
- updateWorklist(worklist, iterator.next(), color);
- }
- unlinked.mark(color);
- }
+ private static void removeUnneededCatchHandlers(IRCode code) {
+ for (BasicBlock block : code.blocks) {
+ if (block.hasCatchHandlers() && !block.canThrow()) {
+ CatchHandlers<BasicBlock> handlers = block.getCatchHandlers();
+ for (BasicBlock target : handlers.getUniqueTargets()) {
+ target.unlinkCatchHandler();
}
}
}
+ code.removeUnreachableBlocks();
}
}
