Extend redundant block removal to blocks with non-jump instructions
Change-Id: Ida0e3a5c91990a50488727df934efe500a51ae6e
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 50e4f53..3f61db3 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
@@ -886,7 +886,7 @@
private boolean noRedundantBlocks() {
for (BasicBlock block : blocks) {
- assert !isRedundantBlock(block);
+ assert !isBlockWithRedundantSuccessorBlock(block);
}
return true;
}
@@ -1315,7 +1315,30 @@
return true;
}
- private boolean isRedundantBlock(BasicBlock block) {
+ private boolean isBlockWithRedundantSuccessorBlock(BasicBlock block) {
+ if (options.debug) {
+ return isBlockWithRedundantSuccessorBlockInDebug(block);
+ } else {
+ return isBlockWithRedundantSuccessorBlockInRelease(block);
+ }
+ }
+
+ private boolean isBlockWithRedundantSuccessorBlockInRelease(BasicBlock block) {
+ if (!block.hasUniqueSuccessorWithUniquePredecessor()
+ || !block.exit().isGoto()
+ || !block.exit().getDebugValues().isEmpty()) {
+ return false;
+ }
+ BasicBlock successor = block.getUniqueSuccessor();
+ if (successor.hasCatchHandlers()) {
+ if (block.isEntry() || block.canThrow()) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ private boolean isBlockWithRedundantSuccessorBlockInDebug(BasicBlock block) {
return block.hasUniqueSuccessorWithUniquePredecessor()
&& block.getInstructions().size() == 1
&& block.exit().isGoto()
@@ -1324,31 +1347,59 @@
}
public void removeRedundantBlocks() {
- List<BasicBlock> blocksToRemove = new ArrayList<>();
-
+ // See b/237567012.
+ assert verifyNoStackInstructions();
+ Set<BasicBlock> blocksToRemove = Sets.newIdentityHashSet();
+ // Run over all blocks while merging successor blocks into the current block.
for (BasicBlock block : blocks) {
- // Check that there are no redundant blocks.
- assert !blocksToRemove.contains(block);
- if (isRedundantBlock(block)) {
- assert block.getUniqueSuccessor().getMutablePredecessors().size() == 1;
- assert block.getUniqueSuccessor().getMutablePredecessors().get(0) == block;
- assert block.getUniqueSuccessor().getPhis().size() == 0;
- // Let the successor consume this block.
+ if (blocksToRemove.contains(block)) {
+ continue;
+ }
+ while (isBlockWithRedundantSuccessorBlock(block)) {
+ // Let the current block consume the successor.;
BasicBlock successor = block.getUniqueSuccessor();
- successor.getMutablePredecessors().clear();
- successor.getMutablePredecessors().addAll(block.getPredecessors());
- successor.getPhis().addAll(block.getPhis());
- successor.getPhis().forEach(phi -> phi.setBlock(block.getUniqueSuccessor()));
- block
- .getPredecessors()
- .forEach(predecessors -> predecessors.replaceSuccessor(block, successor));
- block.getMutablePredecessors().clear();
+ assert !successor.hasCatchHandlers() || !block.canThrow();
+ assert !successor.hasPhis();
+ Instruction instruction = successor.entry();
+ while (instruction != null) {
+ Instruction next = instruction.getNext();
+ if (instruction.isJumpInstruction()) {
+ block.replaceLastInstruction(instruction);
+ } else {
+ block.getInstructions().addBefore(instruction, block.exit());
+ }
+ instruction = next;
+ }
+
+ // Unlink successor block.
block.getMutableSuccessors().clear();
- block.getPhis().clear();
- blocksToRemove.add(block);
+ if (successor.hasCatchHandlers()) {
+ block.moveCatchHandlers(successor);
+ }
+ block.getMutableSuccessors().addAll(successor.getSuccessors());
+ block.forEachNormalSuccessor(
+ successorOfSuccessor -> successorOfSuccessor.replacePredecessor(successor, block));
+
+ // Clean successor block and record for removal.
+ successor.getInstructions().clear();
+ successor.getMutablePredecessors().clear();
+ successor.getMutableSuccessors().clear();
+ blocksToRemove.add(successor);
}
}
blocks.removeAll(blocksToRemove);
+ assert noRedundantBlocks();
+ }
+
+ private boolean verifyNoStackInstructions() {
+ for (BasicBlock block : blocks) {
+ for (Instruction instruction : block.getInstructions()) {
+ assert !instruction.isStore();
+ assert !instruction.isPop();
+ assert !instruction.hasOutValue() || !instruction.outValue().isValueOnStack();
+ }
+ }
+ return true;
}
public boolean removeAllDeadAndTrivialPhis() {
diff --git a/src/test/java/com/android/tools/r8/kotlin/SimplifyIfNotNullKotlinTest.java b/src/test/java/com/android/tools/r8/kotlin/SimplifyIfNotNullKotlinTest.java
index da53eb3..b86c119 100644
--- a/src/test/java/com/android/tools/r8/kotlin/SimplifyIfNotNullKotlinTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/SimplifyIfNotNullKotlinTest.java
@@ -75,7 +75,7 @@
long paramNullCheckCount =
countCall(testMethod, "Intrinsics", "checkParameterIsNotNull");
// One after Iterator#hasNext, and another in the filter predicate: sinceYear != null.
- assertEquals(2, ifzCount);
+ assertEquals(testParameters.isCfRuntime() ? 5 : 2, ifzCount);
assertEquals(0, paramNullCheckCount);
});
}
diff --git a/src/test/java/com/android/tools/r8/kotlin/lambda/KotlinLambdaMergingKeepAttributesKotlinStyleTest.java b/src/test/java/com/android/tools/r8/kotlin/lambda/KotlinLambdaMergingKeepAttributesKotlinStyleTest.java
index 8f359b5..d0faa2b 100644
--- a/src/test/java/com/android/tools/r8/kotlin/lambda/KotlinLambdaMergingKeepAttributesKotlinStyleTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/lambda/KotlinLambdaMergingKeepAttributesKotlinStyleTest.java
@@ -105,12 +105,6 @@
inspector
.assertIsCompleteMergeGroup(
lambdasInInput.getKStyleLambdaReferenceFromTypeName(
- getTestName(), "MainKt$testFirst$1"),
- lambdasInInput.getKStyleLambdaReferenceFromTypeName(
- getTestName(), "MainKt$testFirst$2"),
- lambdasInInput.getKStyleLambdaReferenceFromTypeName(
- getTestName(), "MainKt$testFirst$3"),
- lambdasInInput.getKStyleLambdaReferenceFromTypeName(
getTestName(), "MainKt$testFirst$4"),
lambdasInInput.getKStyleLambdaReferenceFromTypeName(
getTestName(), "MainKt$testFirst$5"),
@@ -123,12 +117,6 @@
lambdasInInput.getKStyleLambdaReferenceFromTypeName(
getTestName(), "MainKt$testFirst$9"),
lambdasInInput.getKStyleLambdaReferenceFromTypeName(
- getTestName(), "MainKt$testSecond$1"),
- lambdasInInput.getKStyleLambdaReferenceFromTypeName(
- getTestName(), "MainKt$testSecond$2"),
- lambdasInInput.getKStyleLambdaReferenceFromTypeName(
- getTestName(), "MainKt$testSecond$3"),
- lambdasInInput.getKStyleLambdaReferenceFromTypeName(
getTestName(), "MainKt$testSecond$4"),
lambdasInInput.getKStyleLambdaReferenceFromTypeName(
getTestName(), "MainKt$testSecond$5"),
diff --git a/src/test/java/com/android/tools/r8/repackage/MappingFileAfterRepackagingTest.java b/src/test/java/com/android/tools/r8/repackage/MappingFileAfterRepackagingTest.java
index 1464cff..2c53eb2 100644
--- a/src/test/java/com/android/tools/r8/repackage/MappingFileAfterRepackagingTest.java
+++ b/src/test/java/com/android/tools/r8/repackage/MappingFileAfterRepackagingTest.java
@@ -59,14 +59,18 @@
line.contains(
"java.lang.String MappingFileAfterRepackagingTest$A.toString()"))
.count();
- assertEquals(repackage ? 2 : 0, syntheticMatches);
+ assertEquals(
+ repackage ? 1 + BooleanUtils.intValue(parameters.isDexRuntime()) : 0,
+ syntheticMatches);
long unqualifiedMatches =
StringUtils.splitLines(runResult.proguardMap()).stream()
.filter(line -> line.contains("java.lang.String toString()"))
.count();
assertEquals(
- (repackage ? 1 : 3) + BooleanUtils.intValue(isPc2pc()), unqualifiedMatches);
+ (repackage ? 1 : 2 + BooleanUtils.intValue(parameters.isDexRuntime()))
+ + BooleanUtils.intValue(isPc2pc()),
+ unqualifiedMatches);
});
}
@@ -78,7 +82,7 @@
static class Main {
public static void main(String[] args) {
- System.out.println((System.currentTimeMillis() > 0 ? new A() : new B()));
+ System.out.println(System.currentTimeMillis() > 0 ? new A() : new B());
}
}
