Ensure horizontal class merger only merges fields with the same flags
Fixes: 175855245
Fixes: 175849452
Change-Id: I48b9297355247c7d613b1eb857d3f47f25287743
diff --git a/src/main/java/com/android/tools/r8/graph/AccessFlags.java b/src/main/java/com/android/tools/r8/graph/AccessFlags.java
index e47b428..98806f6 100644
--- a/src/main/java/com/android/tools/r8/graph/AccessFlags.java
+++ b/src/main/java/com/android/tools/r8/graph/AccessFlags.java
@@ -204,8 +204,9 @@
set(Constants.ACC_FINAL);
}
- public void unsetFinal() {
+ public T unsetFinal() {
unset(Constants.ACC_FINAL);
+ return self();
}
public boolean isSynthetic() {
@@ -216,8 +217,9 @@
set(Constants.ACC_SYNTHETIC);
}
- public void unsetSynthetic() {
+ public T unsetSynthetic() {
unset(Constants.ACC_SYNTHETIC);
+ return self();
}
public void demoteFromSynthetic() {
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedField.java b/src/main/java/com/android/tools/r8/graph/DexEncodedField.java
index e90a784..fc0eeb0 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedField.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedField.java
@@ -220,6 +220,10 @@
return isStatic();
}
+ public boolean isSynthetic() {
+ return accessFlags.isSynthetic();
+ }
+
public boolean isVolatile() {
return accessFlags.isVolatile();
}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassInstanceFieldsMerger.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassInstanceFieldsMerger.java
index 488e33e..23cb57c 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassInstanceFieldsMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassInstanceFieldsMerger.java
@@ -9,9 +9,9 @@
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.horizontalclassmerging.HorizontalClassMergerGraphLens.Builder;
+import com.android.tools.r8.horizontalclassmerging.policies.SameInstanceFields.InstanceFieldInfo;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.IterableUtils;
-import com.android.tools.r8.utils.ListUtils;
import com.google.common.collect.Iterables;
import java.util.ArrayList;
import java.util.Collection;
@@ -19,7 +19,6 @@
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
-import java.util.Map.Entry;
public class ClassInstanceFieldsMerger {
@@ -54,62 +53,70 @@
* type.
*/
public void addFields(DexProgramClass clazz) {
- Map<DexType, LinkedList<DexEncodedField>> availableFieldsPerFieldType =
- computeAvailableFieldsPerFieldType();
+ Map<InstanceFieldInfo, LinkedList<DexEncodedField>> availableFieldsByExactInfo =
+ getAvailableFieldsByExactInfo();
List<DexEncodedField> needsMerge = new ArrayList<>();
// Pass 1: Match fields that have the exact same type.
for (DexEncodedField oldField : clazz.instanceFields()) {
- DexEncodedField newField =
- removeFirstCompatibleField(oldField, availableFieldsPerFieldType.get(oldField.getType()));
- if (newField != null) {
- fieldMappings.get(newField).add(oldField);
- } else {
+ InstanceFieldInfo info = InstanceFieldInfo.createExact(oldField);
+ LinkedList<DexEncodedField> availableFieldsWithExactSameInfo =
+ availableFieldsByExactInfo.get(info);
+ if (availableFieldsWithExactSameInfo == null || availableFieldsWithExactSameInfo.isEmpty()) {
needsMerge.add(oldField);
+ } else {
+ DexEncodedField newField = availableFieldsWithExactSameInfo.removeFirst();
+ fieldMappings.get(newField).add(oldField);
+ if (availableFieldsWithExactSameInfo.isEmpty()) {
+ availableFieldsByExactInfo.remove(info);
+ }
}
}
// Pass 2: Match fields that do not have the same reference type.
+ Map<InstanceFieldInfo, LinkedList<DexEncodedField>> availableFieldsByRelaxedInfo =
+ getAvailableFieldsByRelaxedInfo(availableFieldsByExactInfo);
for (DexEncodedField oldField : needsMerge) {
assert oldField.getType().isReferenceType();
- DexEncodedField newField = null;
- for (Entry<DexType, LinkedList<DexEncodedField>> availableFieldsForType :
- availableFieldsPerFieldType.entrySet()) {
- assert availableFieldsForType.getKey().isReferenceType()
- || availableFieldsForType.getValue().isEmpty();
- newField = removeFirstCompatibleField(oldField, availableFieldsForType.getValue());
- if (newField != null) {
- break;
- }
- }
+ DexEncodedField newField =
+ availableFieldsByRelaxedInfo
+ .get(InstanceFieldInfo.createRelaxed(oldField, appView.dexItemFactory()))
+ .removeFirst();
assert newField != null;
assert newField.getType().isReferenceType();
fieldMappings.get(newField).add(oldField);
}
}
- private Map<DexType, LinkedList<DexEncodedField>> computeAvailableFieldsPerFieldType() {
- Map<DexType, LinkedList<DexEncodedField>> availableFieldsPerFieldType = new LinkedHashMap<>();
+ private Map<InstanceFieldInfo, LinkedList<DexEncodedField>> getAvailableFieldsByExactInfo() {
+ Map<InstanceFieldInfo, LinkedList<DexEncodedField>> availableFieldsByInfo =
+ new LinkedHashMap<>();
for (DexEncodedField field : fieldMappings.keySet()) {
- availableFieldsPerFieldType
- .computeIfAbsent(field.type(), ignore -> new LinkedList<>())
+ availableFieldsByInfo
+ .computeIfAbsent(InstanceFieldInfo.createExact(field), ignore -> new LinkedList<>())
.add(field);
}
- return availableFieldsPerFieldType;
+ return availableFieldsByInfo;
}
- public DexEncodedField removeFirstCompatibleField(
- DexEncodedField oldField, LinkedList<DexEncodedField> availableFields) {
- if (availableFields == null) {
- return null;
- }
- return ListUtils.removeFirstMatch(
- availableFields,
- field -> field.getAccessFlags().isSameVisibility(oldField.getAccessFlags()))
- .orElse(null);
+ private Map<InstanceFieldInfo, LinkedList<DexEncodedField>> getAvailableFieldsByRelaxedInfo(
+ Map<InstanceFieldInfo, LinkedList<DexEncodedField>> availableFieldsByExactInfo) {
+ Map<InstanceFieldInfo, LinkedList<DexEncodedField>> availableFieldsByRelaxedInfo =
+ new LinkedHashMap<>();
+ availableFieldsByExactInfo.forEach(
+ (info, fields) ->
+ availableFieldsByRelaxedInfo
+ .computeIfAbsent(
+ info.toInfoWithRelaxedType(appView.dexItemFactory()),
+ ignore -> new LinkedList<>())
+ .addAll(fields));
+ return availableFieldsByRelaxedInfo;
}
private void fixAccessFlags(DexEncodedField newField, Collection<DexEncodedField> oldFields) {
+ if (newField.isSynthetic() && Iterables.any(oldFields, oldField -> !oldField.isSynthetic())) {
+ newField.getAccessFlags().demoteFromSynthetic();
+ }
if (newField.isFinal() && Iterables.any(oldFields, oldField -> !oldField.isFinal())) {
newField.getAccessFlags().demoteFromFinal();
}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/FieldMultiset.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/FieldMultiset.java
deleted file mode 100644
index 65127b5..0000000
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/FieldMultiset.java
+++ /dev/null
@@ -1,85 +0,0 @@
-// Copyright (c) 2020, 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.horizontalclassmerging;
-
-import com.android.tools.r8.graph.AccessFlags;
-import com.android.tools.r8.graph.DexEncodedField;
-import com.android.tools.r8.graph.DexProgramClass;
-import com.android.tools.r8.graph.DexType;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.Objects;
-
-public class FieldMultiset {
-
- private static class VisibilitySignature {
- private int publicVisible = 0;
- private int protectedVisible = 0;
- private int privateVisible = 0;
- private int packagePrivateVisible = 0;
-
- public void addAccessModifier(AccessFlags accessFlags) {
- if (accessFlags.isPublic()) {
- publicVisible++;
- } else if (accessFlags.isPrivate()) {
- privateVisible++;
- } else if (accessFlags.isPackagePrivate()) {
- packagePrivateVisible++;
- } else if (accessFlags.isProtected()) {
- protectedVisible++;
- }
- }
-
- @Override
- public boolean equals(Object o) {
- if (this == o) return true;
- if (o == null || getClass() != o.getClass()) return false;
- VisibilitySignature that = (VisibilitySignature) o;
- return publicVisible == that.publicVisible
- && protectedVisible == that.protectedVisible
- && privateVisible == that.privateVisible
- && packagePrivateVisible == that.packagePrivateVisible;
- }
-
- @Override
- public int hashCode() {
- return Objects.hash(publicVisible, protectedVisible, privateVisible, packagePrivateVisible);
- }
- }
-
- // This *must* not be an IdentityHashMap, because hash equality does not work for the values.
- private final Map<DexType, VisibilitySignature> fields = new HashMap<>();
-
- public FieldMultiset(DexProgramClass clazz) {
- for (DexEncodedField field : clazz.instanceFields()) {
- fields
- .computeIfAbsent(field.type(), ignore -> new VisibilitySignature())
- .addAccessModifier(field.getAccessFlags());
- }
- }
-
- public FieldMultiset(Iterable<DexEncodedField> instanceFields) {
- for (DexEncodedField field : instanceFields) {
- fields
- .computeIfAbsent(field.type(), ignore -> new VisibilitySignature())
- .addAccessModifier(field.getAccessFlags());
- }
- }
-
- @Override
- public int hashCode() {
- return fields.hashCode();
- }
-
- @Override
- public boolean equals(Object object) {
- if (object instanceof FieldMultiset) {
- FieldMultiset other = (FieldMultiset) object;
- return fields.equals(other.fields);
- } else {
- return false;
- }
- }
-}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java
index ed6ddce..6115e7e 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java
@@ -13,6 +13,7 @@
import com.android.tools.r8.horizontalclassmerging.policies.DontMergeSynchronizedClasses;
import com.android.tools.r8.horizontalclassmerging.policies.IgnoreSynthetics;
import com.android.tools.r8.horizontalclassmerging.policies.LimitGroups;
+import com.android.tools.r8.horizontalclassmerging.policies.MinimizeFieldCasts;
import com.android.tools.r8.horizontalclassmerging.policies.NoAnnotations;
import com.android.tools.r8.horizontalclassmerging.policies.NoClassInitializerWithObservableSideEffects;
import com.android.tools.r8.horizontalclassmerging.policies.NoClassesOrMembersWithAnnotations;
@@ -33,7 +34,7 @@
import com.android.tools.r8.horizontalclassmerging.policies.PreventMethodImplementation;
import com.android.tools.r8.horizontalclassmerging.policies.RespectPackageBoundaries;
import com.android.tools.r8.horizontalclassmerging.policies.SameFeatureSplit;
-import com.android.tools.r8.horizontalclassmerging.policies.SameFields;
+import com.android.tools.r8.horizontalclassmerging.policies.SameInstanceFields;
import com.android.tools.r8.horizontalclassmerging.policies.SameNestHost;
import com.android.tools.r8.horizontalclassmerging.policies.SameParentClass;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
@@ -105,7 +106,7 @@
RuntimeTypeCheckInfo runtimeTypeCheckInfo) {
return ImmutableList.of(
new NotMatchedByNoHorizontalClassMerging(appView),
- new SameFields(appView),
+ new SameInstanceFields(appView),
new NoInterfaces(),
new NoAnnotations(),
new NoEnums(appView),
@@ -132,6 +133,7 @@
new SameFeatureSplit(appView),
new RespectPackageBoundaries(appView),
new DontMergeSynchronizedClasses(appView),
+ new MinimizeFieldCasts(),
new LimitGroups(appView));
}
@@ -175,15 +177,8 @@
HorizontalClassMergerGraphLens.Builder lensBuilder,
FieldAccessInfoCollectionModifier.Builder fieldAccessChangesBuilder,
SyntheticArgumentClass syntheticArgumentClass) {
-
- HorizontalClassMergerGraphLens lens =
- new TreeFixer(
- appView,
- mergedClasses,
- lensBuilder,
- fieldAccessChangesBuilder,
- syntheticArgumentClass)
- .fixupTypeReferences();
- return lens;
+ return new TreeFixer(
+ appView, mergedClasses, lensBuilder, fieldAccessChangesBuilder, syntheticArgumentClass)
+ .fixupTypeReferences();
}
}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/MergeGroup.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/MergeGroup.java
index 02e4684..9e4ebf5 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/MergeGroup.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/MergeGroup.java
@@ -45,6 +45,10 @@
classes.add(clazz);
}
+ public void add(MergeGroup group) {
+ classes.addAll(group.getClasses());
+ }
+
public void addAll(Collection<DexProgramClass> classes) {
this.classes.addAll(classes);
}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/MinimizeFieldCasts.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/MinimizeFieldCasts.java
new file mode 100644
index 0000000..73e0786
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/MinimizeFieldCasts.java
@@ -0,0 +1,73 @@
+// Copyright (c) 2020, 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.horizontalclassmerging.policies;
+
+import com.android.tools.r8.graph.DexEncodedField;
+import com.android.tools.r8.graph.DexProgramClass;
+import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.horizontalclassmerging.MergeGroup;
+import com.android.tools.r8.horizontalclassmerging.MultiClassPolicy;
+import com.google.common.collect.HashMultiset;
+import com.google.common.collect.Multiset;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+public class MinimizeFieldCasts extends MultiClassPolicy {
+
+ @Override
+ public final Collection<MergeGroup> apply(MergeGroup group) {
+ // First find all classes that can be merged without changing field types.
+ Map<Multiset<DexType>, MergeGroup> newGroups = new LinkedHashMap<>();
+ group.forEach(clazz -> addExact(clazz, newGroups));
+
+ // Create a single group from all trivial groups.
+ MergeGroup pendingGroup = new MergeGroup();
+ newGroups
+ .values()
+ .removeIf(
+ newGroup -> {
+ if (newGroup.isTrivial()) {
+ pendingGroup.add(newGroup);
+ return true;
+ }
+ return false;
+ });
+
+ if (pendingGroup.isEmpty()) {
+ return newGroups.values();
+ }
+
+ if (!pendingGroup.isTrivial()) {
+ List<MergeGroup> newGroupsIncludingRelaxedGroup = new ArrayList<>(newGroups.values());
+ newGroupsIncludingRelaxedGroup.add(pendingGroup);
+ return newGroupsIncludingRelaxedGroup;
+ }
+
+ MergeGroup smallestNewGroup = null;
+ for (MergeGroup newGroup : newGroups.values()) {
+ if (smallestNewGroup == null || newGroup.size() < smallestNewGroup.size()) {
+ smallestNewGroup = newGroup;
+ }
+ }
+ assert smallestNewGroup != null;
+ smallestNewGroup.add(pendingGroup);
+ return newGroups.values();
+ }
+
+ private void addExact(DexProgramClass clazz, Map<Multiset<DexType>, MergeGroup> groups) {
+ groups.computeIfAbsent(getExactMergeKey(clazz), ignore -> new MergeGroup()).add(clazz);
+ }
+
+ private Multiset<DexType> getExactMergeKey(DexProgramClass clazz) {
+ Multiset<DexType> fieldTypes = HashMultiset.create();
+ for (DexEncodedField field : clazz.instanceFields()) {
+ fieldTypes.add(field.getType());
+ }
+ return fieldTypes;
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/SameFields.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/SameFields.java
deleted file mode 100644
index adeb3ce..0000000
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/SameFields.java
+++ /dev/null
@@ -1,78 +0,0 @@
-// Copyright (c) 2020, 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.horizontalclassmerging.policies;
-
-import com.android.tools.r8.graph.AppView;
-import com.android.tools.r8.graph.DexEncodedField;
-import com.android.tools.r8.graph.DexProgramClass;
-import com.android.tools.r8.horizontalclassmerging.FieldMultiset;
-import com.android.tools.r8.horizontalclassmerging.MergeGroup;
-import com.android.tools.r8.horizontalclassmerging.MultiClassPolicy;
-import com.android.tools.r8.shaking.AppInfoWithLiveness;
-import com.google.common.collect.Iterables;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Iterator;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
-
-public class SameFields extends MultiClassPolicy {
-
- private final AppView<AppInfoWithLiveness> appView;
-
- public SameFields(AppView<AppInfoWithLiveness> appView) {
- this.appView = appView;
- }
-
- public void addTo(FieldMultiset key, DexProgramClass clazz, Map<FieldMultiset, MergeGroup> map) {
- map.computeIfAbsent(key, ignore -> new MergeGroup()).add(clazz);
- }
-
- @Override
- public final Collection<MergeGroup> apply(MergeGroup group) {
- // First find all classes that can be merged without changing field types.
- Map<FieldMultiset, MergeGroup> groups = new LinkedHashMap<>();
- group.getClasses().forEach(clazz -> addTo(getMergeKey(clazz), clazz, groups));
-
- // For each trivial group, try to generalise it and then group it.
- Map<FieldMultiset, MergeGroup> generalizedGroups = new LinkedHashMap<>();
- Iterator<MergeGroup> iterator = groups.values().iterator();
- while (iterator.hasNext()) {
- MergeGroup newGroup = iterator.next();
- if (newGroup.isTrivial()) {
- newGroup
- .getClasses()
- .forEach(clazz -> addTo(getGeneralizedMergeKey(clazz), clazz, generalizedGroups));
- iterator.remove();
- }
- }
-
- removeTrivialGroups(generalizedGroups.values());
-
- List<MergeGroup> newGroups = new ArrayList<>();
- newGroups.addAll(groups.values());
- newGroups.addAll(generalizedGroups.values());
- return newGroups;
- }
-
- public FieldMultiset getMergeKey(DexProgramClass clazz) {
- return new FieldMultiset(clazz);
- }
-
- public DexEncodedField generalizeField(DexEncodedField field) {
- if (!field.type().isReferenceType()) {
- return field;
- }
- return field.toTypeSubstitutedField(
- field
- .getReference()
- .withType(appView.dexItemFactory().objectType, appView.dexItemFactory()));
- }
-
- public FieldMultiset getGeneralizedMergeKey(DexProgramClass clazz) {
- return new FieldMultiset(Iterables.transform(clazz.instanceFields(), this::generalizeField));
- }
-}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/SameInstanceFields.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/SameInstanceFields.java
new file mode 100644
index 0000000..13e0388
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/SameInstanceFields.java
@@ -0,0 +1,84 @@
+// Copyright (c) 2020, 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.horizontalclassmerging.policies;
+
+import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.DexEncodedField;
+import com.android.tools.r8.graph.DexItemFactory;
+import com.android.tools.r8.graph.DexProgramClass;
+import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.FieldAccessFlags;
+import com.android.tools.r8.horizontalclassmerging.MultiClassSameReferencePolicy;
+import com.android.tools.r8.horizontalclassmerging.policies.SameInstanceFields.InstanceFieldInfo;
+import com.android.tools.r8.shaking.AppInfoWithLiveness;
+import com.google.common.collect.HashMultiset;
+import com.google.common.collect.Multiset;
+import java.util.Objects;
+
+public class SameInstanceFields extends MultiClassSameReferencePolicy<Multiset<InstanceFieldInfo>> {
+
+ private final DexItemFactory dexItemFactory;
+
+ public SameInstanceFields(AppView<AppInfoWithLiveness> appView) {
+ this.dexItemFactory = appView.dexItemFactory();
+ }
+
+ @Override
+ public Multiset<InstanceFieldInfo> getMergeKey(DexProgramClass clazz) {
+ Multiset<InstanceFieldInfo> fields = HashMultiset.create();
+ for (DexEncodedField field : clazz.instanceFields()) {
+ fields.add(InstanceFieldInfo.createRelaxed(field, dexItemFactory));
+ }
+ return fields;
+ }
+
+ public static class InstanceFieldInfo {
+
+ private final FieldAccessFlags accessFlags;
+ private final DexType type;
+
+ private InstanceFieldInfo(FieldAccessFlags accessFlags, DexType type) {
+ this.accessFlags =
+ FieldAccessFlags.fromSharedAccessFlags(accessFlags.materialize())
+ .unsetFinal()
+ .unsetSynthetic();
+ this.type = type;
+ }
+
+ public static InstanceFieldInfo createExact(DexEncodedField field) {
+ return new InstanceFieldInfo(field.getAccessFlags(), field.getType());
+ }
+
+ public static InstanceFieldInfo createRelaxed(
+ DexEncodedField field, DexItemFactory dexItemFactory) {
+ return new InstanceFieldInfo(
+ field.getAccessFlags(),
+ field.getType().isReferenceType() ? dexItemFactory.objectType : field.getType());
+ }
+
+ public FieldAccessFlags getAccessFlags() {
+ return accessFlags;
+ }
+
+ public InstanceFieldInfo toInfoWithRelaxedType(DexItemFactory dexItemFactory) {
+ return new InstanceFieldInfo(
+ accessFlags, type.isReferenceType() ? dexItemFactory.objectType : type);
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (obj == null || getClass() != obj.getClass()) {
+ return false;
+ }
+ InstanceFieldInfo info = (InstanceFieldInfo) obj;
+ return accessFlags.materialize() == info.accessFlags.materialize() && type == info.type;
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(accessFlags, type);
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/FieldsWithDifferentAccessFlagsTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/FieldsWithDifferentAccessFlagsTest.java
new file mode 100644
index 0000000..58acdbf
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/FieldsWithDifferentAccessFlagsTest.java
@@ -0,0 +1,89 @@
+/*
+ * // Copyright (c) 2020, 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.classmerging.horizontal;
+
+import com.android.tools.r8.NeverClassInline;
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.utils.codeinspector.HorizontallyMergedClassesInspector;
+import org.junit.Test;
+
+public class FieldsWithDifferentAccessFlagsTest extends HorizontalClassMergingTestBase {
+
+ public FieldsWithDifferentAccessFlagsTest(
+ TestParameters parameters, boolean enableHorizontalClassMerging) {
+ super(parameters, enableHorizontalClassMerging);
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepMainRule(Main.class)
+ .addOptionsModification(
+ options ->
+ options.horizontalClassMergerOptions().enableIf(enableHorizontalClassMerging))
+ .addHorizontallyMergedClassesInspectorIf(
+ enableHorizontalClassMerging, HorizontallyMergedClassesInspector::assertNoClassesMerged)
+ .enableInliningAnnotations()
+ .enableNeverClassInliningAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines("A", "B", "C");
+ }
+
+ @NeverClassInline
+ public static class A {
+ public volatile String msg;
+
+ public A(String msg) {
+ this.msg = msg;
+ }
+
+ @NeverInline
+ public void foo() {
+ System.out.println(msg);
+ }
+ }
+
+ @NeverClassInline
+ public static class B {
+ public transient String msg;
+
+ public B(String msg) {
+ this.msg = msg;
+ }
+
+ @NeverInline
+ public void foo() {
+ System.out.println(msg);
+ }
+ }
+
+ @NeverClassInline
+ public static class C {
+ public String msg;
+
+ public C(String msg) {
+ this.msg = msg;
+ }
+
+ @NeverInline
+ public void foo() {
+ System.out.println(msg);
+ }
+ }
+
+ public static class Main {
+ public static void main(String[] args) {
+ new A(System.currentTimeMillis() > 0 ? "A" : null).foo();
+ new B(System.currentTimeMillis() > 0 ? "B" : null).foo();
+ new C(System.currentTimeMillis() > 0 ? "C" : null).foo();
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/MinimizeFieldCastsTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/MinimizeFieldCastsTest.java
new file mode 100644
index 0000000..90ae326
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/MinimizeFieldCastsTest.java
@@ -0,0 +1,121 @@
+// Copyright (c) 2020, 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.classmerging.horizontal;
+
+import com.android.tools.r8.NeverClassInline;
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.NoHorizontalClassMerging;
+import com.android.tools.r8.TestParameters;
+import org.junit.Test;
+
+public class MinimizeFieldCastsTest extends HorizontalClassMergingTestBase {
+
+ public MinimizeFieldCastsTest(TestParameters parameters, boolean enableHorizontalClassMerging) {
+ super(parameters, enableHorizontalClassMerging);
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepMainRule(Main.class)
+ .addOptionsModification(
+ options ->
+ options.horizontalClassMergerOptions().enableIf(enableHorizontalClassMerging))
+ .enableInliningAnnotations()
+ .enableNeverClassInliningAnnotations()
+ .enableNoHorizontalClassMergingAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ .addHorizontallyMergedClassesInspectorIf(
+ enableHorizontalClassMerging,
+ inspector ->
+ // Two merge groups are expected since we attempt to merge classes in a way that
+ // avoids merging fields with different types unless strictly required for merging.
+ inspector.assertMergedInto(B.class, A.class).assertMergedInto(D.class, C.class))
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines("Foo", "Foo", "Bar", "Bar");
+ }
+
+ @NeverClassInline
+ public static class A {
+ FooGreeter greeter;
+
+ A(FooGreeter greeter) {
+ this.greeter = greeter;
+ }
+
+ void greet() {
+ greeter.greet();
+ }
+ }
+
+ @NeverClassInline
+ public static class B {
+ FooGreeter greeter;
+
+ B(FooGreeter greeter) {
+ this.greeter = greeter;
+ }
+
+ void greet() {
+ greeter.greet();
+ }
+ }
+
+ @NeverClassInline
+ public static class C {
+ BarGreeter greeter;
+
+ C(BarGreeter greeter) {
+ this.greeter = greeter;
+ }
+
+ void greet() {
+ greeter.greet();
+ }
+ }
+
+ @NeverClassInline
+ public static class D {
+ BarGreeter greeter;
+
+ D(BarGreeter greeter) {
+ this.greeter = greeter;
+ }
+
+ void greet() {
+ greeter.greet();
+ }
+ }
+
+ @NeverClassInline
+ @NoHorizontalClassMerging
+ static class FooGreeter {
+
+ @NeverInline
+ void greet() {
+ System.out.println("Foo");
+ }
+ }
+
+ @NeverClassInline
+ @NoHorizontalClassMerging
+ static class BarGreeter {
+
+ @NeverInline
+ void greet() {
+ System.out.println("Bar");
+ }
+ }
+
+ static class Main {
+ public static void main(String[] args) {
+ new A(new FooGreeter()).greet();
+ new B(new FooGreeter()).greet();
+ new C(new BarGreeter()).greet();
+ new D(new BarGreeter()).greet();
+ }
+ }
+}
