Check incoming stack when removing identical predecessor blocks.
We can only merge predecessor blocks if the blocks stack map table
entries are similar. This change checks the live at entry stack are
similar when considering blocks to be identical with special handling
for MoveException for CF.
Bug: 120121607
Change-Id: If53e7b0cafdfa50633713e8b300238f0ab899d00
diff --git a/src/main/java/com/android/tools/r8/cf/CfRegisterAllocator.java b/src/main/java/com/android/tools/r8/cf/CfRegisterAllocator.java
index 6914bc1..724faf1 100644
--- a/src/main/java/com/android/tools/r8/cf/CfRegisterAllocator.java
+++ b/src/main/java/com/android/tools/r8/cf/CfRegisterAllocator.java
@@ -381,6 +381,37 @@
updateFirstStackByJoiningTheSecond(typesOfKept.stack, typesOfRemoved.stack);
}
+ @Override
+ public boolean hasEqualTypesAtEntry(BasicBlock first, BasicBlock second) {
+ if (!java.util.Objects.equals(first.getLocalsAtEntry(), second.getLocalsAtEntry())) {
+ return false;
+ }
+ // Check that stack at entry is same.
+ List<TypeInfo> firstStack = getTypesAtBlockEntry(first).stack;
+ List<TypeInfo> secondStack = getTypesAtBlockEntry(second).stack;
+ if (firstStack.size() != secondStack.size()) {
+ return false;
+ }
+ for (int i = 0; i < firstStack.size(); i++) {
+ if (firstStack.get(i).getDexType() != secondStack.get(i).getDexType()) {
+ return false;
+ }
+ }
+ // Check if the blocks are catch-handlers, which implies that the exception is transferred
+ // through the stack.
+ if (first.entry().isMoveException() != second.entry().isMoveException()) {
+ return false;
+ }
+ // If both blocks are catch handlers, we require that the exceptions are the same type.
+ if (first.entry().isMoveException()
+ && typeHelper.getTypeInfo(first.entry().outValue()).getDexType()
+ != typeHelper.getTypeInfo(second.entry().outValue()).getDexType()) {
+ return false;
+ }
+
+ return true;
+ }
+
// Return false if this is not an instruction with the type of outvalue dependent on types of
// invalues.
private boolean tryApplyInstructionWithDependentOutType(
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/BasicBlockInstructionsEquivalence.java b/src/main/java/com/android/tools/r8/ir/optimize/BasicBlockInstructionsEquivalence.java
index 73cf07f..17c8d1a 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/BasicBlockInstructionsEquivalence.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/BasicBlockInstructionsEquivalence.java
@@ -10,7 +10,6 @@
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.ir.regalloc.RegisterAllocator;
import com.google.common.base.Equivalence;
-import com.google.common.base.Objects;
import java.util.Arrays;
import java.util.List;
@@ -40,7 +39,7 @@
}
}
- if (!Objects.equal(first.getLocalsAtEntry(), second.getLocalsAtEntry())) {
+ if (!allocator.hasEqualTypesAtEntry(first, second)) {
return false;
}
diff --git a/src/main/java/com/android/tools/r8/ir/regalloc/LinearScanRegisterAllocator.java b/src/main/java/com/android/tools/r8/ir/regalloc/LinearScanRegisterAllocator.java
index c971a14..40d31d2 100644
--- a/src/main/java/com/android/tools/r8/ir/regalloc/LinearScanRegisterAllocator.java
+++ b/src/main/java/com/android/tools/r8/ir/regalloc/LinearScanRegisterAllocator.java
@@ -3172,6 +3172,11 @@
}
@Override
+ public boolean hasEqualTypesAtEntry(BasicBlock first, BasicBlock second) {
+ return java.util.Objects.equals(first.getLocalsAtEntry(), second.getLocalsAtEntry());
+ }
+
+ @Override
public void addNewBlockToShareIdenticalSuffix(
BasicBlock block, int suffixSize, List<BasicBlock> predsBeforeSplit) {
// Intentionally empty, we don't need to track suffix sharing in this allocator.
diff --git a/src/main/java/com/android/tools/r8/ir/regalloc/RegisterAllocator.java b/src/main/java/com/android/tools/r8/ir/regalloc/RegisterAllocator.java
index 1314b3f..43b41e5 100644
--- a/src/main/java/com/android/tools/r8/ir/regalloc/RegisterAllocator.java
+++ b/src/main/java/com/android/tools/r8/ir/regalloc/RegisterAllocator.java
@@ -17,6 +17,8 @@
void mergeBlocks(BasicBlock kept, BasicBlock removed);
+ boolean hasEqualTypesAtEntry(BasicBlock first, BasicBlock second);
+
// Call before removing the suffix from the preds. Block is used only as a key.
void addNewBlockToShareIdenticalSuffix(
BasicBlock block, int suffixSize, List<BasicBlock> predsBeforeSplit);
diff --git a/src/test/java/com/android/tools/r8/cf/NonidenticalCatchHandlerTest.java b/src/test/java/com/android/tools/r8/cf/NonidenticalCatchHandlerTest.java
new file mode 100644
index 0000000..b778c41
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/cf/NonidenticalCatchHandlerTest.java
@@ -0,0 +1,83 @@
+// Copyright (c) 2018, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package com.android.tools.r8.cf;
+
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.ClassFileConsumer;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.cf.code.CfLabel;
+import com.android.tools.r8.cf.code.CfTryCatch;
+import com.android.tools.r8.errors.Unimplemented;
+import com.android.tools.r8.graph.CfCode;
+import com.android.tools.r8.graph.Code;
+import com.android.tools.r8.graph.DexClass;
+import com.android.tools.r8.graph.DexCode;
+import com.android.tools.r8.graph.DexCode.Try;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.utils.AndroidApp;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.google.common.collect.Sets;
+import it.unimi.dsi.fastutil.ints.IntOpenHashSet;
+import it.unimi.dsi.fastutil.ints.IntSet;
+import java.util.Set;
+import org.junit.Test;
+
+public class NonidenticalCatchHandlerTest extends TestBase {
+
+ private static class TestClass {
+ public void foo(Object a) {
+ int x = 0;
+ try {
+ x = ((String)a).length();
+ } catch (ClassCastException e) {
+ x = -1;
+ } catch (NullPointerException e) {
+ x = -1;
+ }
+ System.out.println(x);
+ }
+ }
+
+ @Test
+ public void test() throws Exception {
+ byte[] inputBytes = ToolHelper.getClassAsBytes(TestClass.class);
+ AndroidApp inputApp =
+ AndroidApp.builder()
+ .addClassProgramData(inputBytes, Origin.unknown())
+ .addLibraryFiles(ToolHelper.getJava8RuntimeJar())
+ .build();
+ assertEquals(2, countCatchHandlers(inputApp));
+ AndroidApp outputDexApp = ToolHelper.runR8(inputApp);
+ assertEquals(1, countCatchHandlers(outputDexApp));
+ AndroidApp outputCfApp =
+ ToolHelper.runR8WithProgramConsumer(inputApp, ClassFileConsumer.emptyConsumer());
+ // ClassCastException and NullPointerException will not have same type on stack.
+ assertEquals(2, countCatchHandlers(outputCfApp));
+ }
+
+ private int countCatchHandlers(AndroidApp inputApp) throws Exception {
+ CodeInspector inspector = new CodeInspector(inputApp);
+ DexClass dexClass = inspector.clazz(TestClass.class).getDexClass();
+ Code code = dexClass.virtualMethods()[0].getCode();
+ if (code.isCfCode()) {
+ CfCode cfCode = code.asCfCode();
+ Set<CfLabel> targets = Sets.newIdentityHashSet();
+ for (CfTryCatch tryCatch : cfCode.getTryCatchRanges()) {
+ targets.addAll(tryCatch.targets);
+ }
+ return targets.size();
+ } else if (code.isDexCode()) {
+ DexCode dexCode = code.asDexCode();
+ IntSet targets = new IntOpenHashSet();
+ for (Try aTry : dexCode.tries) {
+ targets.add(aTry.handlerOffset);
+ }
+ return targets.size();
+ } else {
+ throw new Unimplemented(code.getClass().getName());
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/ir/regalloc/IdenticalAfterRegisterAllocationTest.java b/src/test/java/com/android/tools/r8/ir/regalloc/IdenticalAfterRegisterAllocationTest.java
index 5ed3cc5..12d0a78 100644
--- a/src/test/java/com/android/tools/r8/ir/regalloc/IdenticalAfterRegisterAllocationTest.java
+++ b/src/test/java/com/android/tools/r8/ir/regalloc/IdenticalAfterRegisterAllocationTest.java
@@ -51,6 +51,11 @@
}
@Override
+ public boolean hasEqualTypesAtEntry(BasicBlock first, BasicBlock second) {
+ return false;
+ }
+
+ @Override
public void addNewBlockToShareIdenticalSuffix(
BasicBlock block, int suffixSize, List<BasicBlock> predsBeforeSplit) {
// Intentionally empty, we don't need to track suffix sharing in this allocator.
