Combine the removal of unneeded catch handlers with dead code removal
Dead code removal runs over all the code anyway, and can also result in
additional unneeded catch handlers when throwing instructions are removed.
Specialized unneeded catch handler removal have been removed, as dead code
removal always run one last time as the last step before register allocation.
R=ager@google.com
Change-Id: Ibc87d0a87bc5be0535e38b6b599ffc49a63472bd
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
index a787025..c94d55f 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -449,7 +449,6 @@
Log.debug(getClass(), "Intermediate (SSA) flow graph for %s:\n%s",
method.toSourceString(), code);
}
- codeRewriter.removeUnneededCatchHandlers(code);
// Dead code removal. Performed after simplifications to remove code that becomes dead
// as a result of those simplifications. The following optimizations could reveal more
// dead code which is removed right before register allocation in performRegisterAllocation.
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
index c280a23..f26cacd 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
@@ -1149,25 +1149,6 @@
assert code.isConsistentSSA();
}
- public void removeUnneededCatchHandlers(IRCode code) {
- DominatorTree dominator = new DominatorTree(code);
- code.clearMarks();
- for (BasicBlock block : code.blocks) {
- if (block.hasCatchHandlers() && !block.canThrow()) {
- CatchHandlers<BasicBlock> handlers = block.getCatchHandlers();
- for (BasicBlock target : handlers.getUniqueTargets()) {
- for (BasicBlock unlinked : block.unlink(target, dominator)) {
- if (!unlinked.isMarked()) {
- unlinked.mark();
- }
- }
- }
- }
- }
- code.removeMarkedBlocks();
- assert code.isConsistentSSA();
- }
-
public void rewriteLongCompareAndRequireNonNull(IRCode code, boolean canUseObjectsNonNull) {
InstructionIterator iterator = code.instructionIterator();
@@ -1204,7 +1185,6 @@
// Note that addSuppressed() and getSuppressed() methods are final in
// Throwable, so these changes don't have to worry about overrides.
public void rewriteThrowableAddAndGetSuppressed(IRCode code) {
- boolean removeUnneededCatchHandlers = false;
DexItemFactory.ThrowableMethods throwableMethods = dexItemFactory.throwableMethods;
for (BasicBlock block : code.blocks) {
@@ -1217,15 +1197,12 @@
if (matchesMethodOfThrowable(invokedMethod, throwableMethods.addSuppressed)) {
// Remove Throwable::addSuppressed(Throwable) call.
iterator.remove();
- removeUnneededCatchHandlers = true;
-
} else if (matchesMethodOfThrowable(invokedMethod, throwableMethods.getSuppressed)) {
Value destValue = current.outValue();
if (destValue == null) {
// If the result of the call was not used we don't create
// an empty array and just remove the call.
iterator.remove();
- removeUnneededCatchHandlers = true;
continue;
}
@@ -1243,21 +1220,10 @@
NewArrayEmpty newArray = new NewArrayEmpty(destValue, zero,
dexItemFactory.createType(dexItemFactory.throwableArrayDescriptor));
iterator.replaceCurrentInstruction(newArray);
-
- // NOTE: nothing needs to be changed in catch handlers since we replace
- // one throwable instruction with another.
}
}
}
}
-
- // If at least one addSuppressed(...) call was removed, or we were able
- // to remove getSuppressed() call without replacing it with a new empty array,
- // we need to deal with possible unreachable catch handlers.
- if (removeUnneededCatchHandlers) {
- removeUnneededCatchHandlers(code);
- }
-
assert code.isConsistentSSA();
}
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 d6023ed..0c60642 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
@@ -4,6 +4,8 @@
package com.android.tools.r8.ir.optimize;
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;
@@ -11,6 +13,7 @@
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.utils.InternalOptions;
import java.util.ArrayList;
+import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.Queue;
@@ -20,14 +23,48 @@
public static void removeDeadCode(
IRCode code, CodeRewriter codeRewriter, InternalOptions options) {
Queue<BasicBlock> worklist = new LinkedList<>();
+ DominatorTree dominator = new DominatorTree(code);
+ code.clearMarks();
worklist.addAll(code.blocks);
for (BasicBlock block = worklist.poll(); block != null; block = worklist.poll()) {
+ if (block.isMarked()) {
+ // Ignore marked blocks, as they are scheduled for removal.
+ continue;
+ }
removeDeadInstructions(worklist, block, options);
removeDeadPhis(worklist, block, options);
+ removeUnneededCatchHandlers(worklist, block, dominator);
}
+ code.removeMarkedBlocks();
+ 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) {
+ BasicBlock block;
+ if (value.isPhi()) {
+ block = value.asPhi().getBlock();
+ } else {
+ block = value.definition.getBlock();
+ }
+ if (!block.isMarked()) {
+ worklist.add(block);
+ }
+ }
+
+ // Add all blocks from where the in-values to the instruction originates.
+ private static void updateWorklist(Queue<BasicBlock> worklist, Instruction instruction) {
+ for (Value inValue : instruction.inValues()) {
+ updateWorklist(worklist, inValue);
+ }
+
+ Value previousLocalValue = instruction.getPreviousLocalValue();
+ if (previousLocalValue != null) {
+ updateWorklist(worklist, previousLocalValue);
+ }
+ }
+
private static void removeDeadPhis(
Queue<BasicBlock> worklist, BasicBlock block, InternalOptions options) {
List<Phi> toRemove = new ArrayList<>();
@@ -36,11 +73,7 @@
toRemove.add(phi);
for (Value operand : phi.getOperands()) {
operand.removePhiUser(phi);
- if (operand.isPhi()) {
- worklist.add(operand.asPhi().getBlock());
- } else {
- worklist.add(operand.definition.getBlock());
- }
+ updateWorklist(worklist, operand);
}
}
}
@@ -83,26 +116,31 @@
assert outValue != null;
if (!outValue.isDead(options)) {
continue;
+
}
- for (Value inValue : current.inValues()) {
- if (inValue.isPhi()) {
- worklist.add(inValue.asPhi().getBlock());
- } else {
- worklist.add(inValue.definition.getBlock());
- }
- }
- Value previousLocalValue = current.getPreviousLocalValue();
- if (previousLocalValue != null) {
- if (previousLocalValue.isPhi()) {
- worklist.add(previousLocalValue.asPhi().getBlock());
- } else {
- worklist.add(previousLocalValue.definition.getBlock());
- }
- }
+ 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();
iterator.remove();
}
}
+
+ private static void removeUnneededCatchHandlers(
+ Queue<BasicBlock> worklist, BasicBlock block, DominatorTree dominator) {
+ if (block.hasCatchHandlers() && !block.canThrow()) {
+ CatchHandlers<BasicBlock> handlers = block.getCatchHandlers();
+ for (BasicBlock target : handlers.getUniqueTargets()) {
+ for (BasicBlock unlinked : block.unlink(target, dominator)) {
+ if (!unlinked.isMarked()) {
+ Iterator<Instruction> iterator = unlinked.iterator();
+ while (iterator.hasNext()) {
+ updateWorklist(worklist, iterator.next());
+ }
+ unlinked.mark();
+ }
+ }
+ }
+ }
+ }
}
