Merge "Check incoming stack when removing identical predecessor blocks."
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.
