Allow horizontal field merging of fields with different reference types
Change-Id: I00a4ae618a5e5e32ea341b6da31919f177ef6041
diff --git a/src/main/java/com/android/tools/r8/graph/DexClass.java b/src/main/java/com/android/tools/r8/graph/DexClass.java
index be3a802..82faf34 100644
--- a/src/main/java/com/android/tools/r8/graph/DexClass.java
+++ b/src/main/java/com/android/tools/r8/graph/DexClass.java
@@ -381,6 +381,10 @@
assert verifyNoDuplicateFields();
}
+ public void clearInstanceFields() {
+ instanceFields = DexEncodedField.EMPTY_ARRAY;
+ }
+
private boolean verifyCorrectnessOfFieldHolder(DexEncodedField field) {
assert field.getHolderType() == type
: "Expected field `"
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 6839371..e90a784 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedField.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedField.java
@@ -158,6 +158,10 @@
return field;
}
+ public DexType getType() {
+ return getReference().getType();
+ }
+
@Override
public boolean isDexEncodedField() {
return true;
diff --git a/src/main/java/com/android/tools/r8/graph/DexField.java b/src/main/java/com/android/tools/r8/graph/DexField.java
index 70bb89b..1537fb2 100644
--- a/src/main/java/com/android/tools/r8/graph/DexField.java
+++ b/src/main/java/com/android/tools/r8/graph/DexField.java
@@ -170,6 +170,10 @@
return dexItemFactory.createField(holder, type, name);
}
+ public DexField withType(DexType type, DexItemFactory dexItemFactory) {
+ return dexItemFactory.createField(holder, type, name);
+ }
+
public FieldReference asFieldReference() {
return Reference.field(
Reference.classFromDescriptor(holder.toDescriptorString()),
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 96282ee..488e33e 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassInstanceFieldsMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassInstanceFieldsMerger.java
@@ -4,29 +4,38 @@
package com.android.tools.r8.horizontalclassmerging;
+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.graph.DexType;
import com.android.tools.r8.horizontalclassmerging.HorizontalClassMergerGraphLens.Builder;
+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;
-import java.util.IdentityHashMap;
import java.util.LinkedHashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
+import java.util.Map.Entry;
public class ClassInstanceFieldsMerger {
- // Map from target class field to all fields which should be merged into that field.
- private final Map<DexEncodedField, List<DexEncodedField>> fieldMappings = new LinkedHashMap<>();
+ private final AppView<AppInfoWithLiveness> appView;
private final Builder lensBuilder;
+ private DexEncodedField classIdField;
+
+ // Map from target class field to all fields which should be merged into that field.
+ private final Map<DexEncodedField, List<DexEncodedField>> fieldMappings = new LinkedHashMap<>();
+
public ClassInstanceFieldsMerger(
- HorizontalClassMergerGraphLens.Builder lensBuilder, MergeGroup group) {
+ AppView<AppInfoWithLiveness> appView,
+ HorizontalClassMergerGraphLens.Builder lensBuilder,
+ MergeGroup group) {
+ this.appView = appView;
this.lensBuilder = lensBuilder;
group
.getTarget()
@@ -34,39 +43,114 @@
.forEach(field -> fieldMappings.computeIfAbsent(field, ignore -> new ArrayList<>()));
}
+ /**
+ * Adds all fields from {@param clazz} to the class merger. For each field, we must choose which
+ * field on the target class to merge into.
+ *
+ * <p>A field that stores a reference type can be merged into a field that stores a different
+ * reference type. To avoid that we change fields that store a reference type to have type
+ * java.lang.Object when it is not needed (e.g., class Foo has fields 'A a' and 'B b' and class
+ * Bar has fields 'A b' and 'B a'), we make a prepass that matches fields with the same reference
+ * type.
+ */
public void addFields(DexProgramClass clazz) {
- Map<DexType, List<DexEncodedField>> availableFields = new IdentityHashMap<>();
- for (DexEncodedField field : fieldMappings.keySet()) {
- availableFields.computeIfAbsent(field.type(), ignore -> new LinkedList<>()).add(field);
- }
+ Map<DexType, LinkedList<DexEncodedField>> availableFieldsPerFieldType =
+ computeAvailableFieldsPerFieldType();
+ List<DexEncodedField> needsMerge = new ArrayList<>();
+ // Pass 1: Match fields that have the exact same type.
for (DexEncodedField oldField : clazz.instanceFields()) {
DexEncodedField newField =
- ListUtils.removeFirstMatch(
- availableFields.get(oldField.type()),
- field -> field.getAccessFlags().isSameVisibility(oldField.getAccessFlags()))
- .get();
+ removeFirstCompatibleField(oldField, availableFieldsPerFieldType.get(oldField.getType()));
+ if (newField != null) {
+ fieldMappings.get(newField).add(oldField);
+ } else {
+ needsMerge.add(oldField);
+ }
+ }
+
+ // Pass 2: Match fields that do not have the same reference type.
+ 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;
+ }
+ }
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<>();
+ for (DexEncodedField field : fieldMappings.keySet()) {
+ availableFieldsPerFieldType
+ .computeIfAbsent(field.type(), ignore -> new LinkedList<>())
+ .add(field);
+ }
+ return availableFieldsPerFieldType;
+ }
+
+ 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 void fixAccessFlags(DexEncodedField newField, Collection<DexEncodedField> oldFields) {
if (newField.isFinal() && Iterables.any(oldFields, oldField -> !oldField.isFinal())) {
newField.getAccessFlags().demoteFromFinal();
}
}
- private void mergeFields(DexEncodedField newField, List<DexEncodedField> oldFields) {
- fixAccessFlags(newField, oldFields);
- lensBuilder.recordNewFieldSignature(
- Iterables.transform(
- IterableUtils.append(oldFields, newField), DexEncodedField::getReference),
- newField.getReference(),
- newField.getReference());
+ public void setClassIdField(DexEncodedField classIdField) {
+ this.classIdField = classIdField;
}
- public void merge() {
- fieldMappings.forEach(this::mergeFields);
+ public DexEncodedField[] merge() {
+ List<DexEncodedField> newFields = new ArrayList<>();
+ assert classIdField != null;
+ newFields.add(classIdField);
+ fieldMappings.forEach(
+ (targetField, oldFields) ->
+ newFields.add(mergeSourceFieldsToTargetField(targetField, oldFields)));
+ return newFields.toArray(DexEncodedField.EMPTY_ARRAY);
+ }
+
+ private DexEncodedField mergeSourceFieldsToTargetField(
+ DexEncodedField targetField, List<DexEncodedField> oldFields) {
+ fixAccessFlags(targetField, oldFields);
+
+ DexEncodedField newField;
+ DexType targetFieldType = targetField.type();
+ if (!Iterables.all(oldFields, oldField -> oldField.getType() == targetFieldType)) {
+ newField =
+ targetField.toTypeSubstitutedField(
+ targetField
+ .getReference()
+ .withType(appView.dexItemFactory().objectType, appView.dexItemFactory()));
+ } else {
+ newField = targetField;
+ }
+
+ lensBuilder.recordNewFieldSignature(
+ Iterables.transform(
+ IterableUtils.append(oldFields, targetField), DexEncodedField::getReference),
+ newField.getReference(),
+ targetField.getReference());
+
+ return newField;
}
}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java
index 569669c..b48d8ce 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java
@@ -88,7 +88,7 @@
this.dexItemFactory = appView.dexItemFactory();
this.classInitializerSynthesizedCode = classInitializerSynthesizedCode;
this.classStaticFieldsMerger = new ClassStaticFieldsMerger(appView, lensBuilder, group);
- this.classInstanceFieldsMerger = new ClassInstanceFieldsMerger(lensBuilder, group);
+ this.classInstanceFieldsMerger = new ClassInstanceFieldsMerger(appView, lensBuilder, group);
buildClassIdentifierMap();
}
@@ -201,15 +201,14 @@
}
void appendClassIdField() {
- DexEncodedField encodedField =
+ classInstanceFieldsMerger.setClassIdField(
new DexEncodedField(
group.getClassIdField(),
FieldAccessFlags.fromSharedAccessFlags(
Constants.ACC_PUBLIC + Constants.ACC_FINAL + Constants.ACC_SYNTHETIC),
FieldTypeSignature.noSignature(),
DexAnnotationSet.empty(),
- null);
- group.getTarget().appendInstanceField(encodedField);
+ null));
}
void mergeStaticFields() {
@@ -240,9 +239,9 @@
group.forEachSource(
clazz -> {
classInstanceFieldsMerger.addFields(clazz);
- clazz.setInstanceFields(null);
+ clazz.clearInstanceFields();
});
- classInstanceFieldsMerger.merge();
+ group.getTarget().setInstanceFields(classInstanceFieldsMerger.merge());
}
public void mergeGroup(SyntheticArgumentClass syntheticArgumentClass) {
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/FieldMultiset.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/FieldMultiset.java
index 6cbffcc..65127b5 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/FieldMultiset.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/FieldMultiset.java
@@ -60,6 +60,14 @@
}
}
+ 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();
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 6be6b5b..ed6ddce 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java
@@ -105,7 +105,7 @@
RuntimeTypeCheckInfo runtimeTypeCheckInfo) {
return ImmutableList.of(
new NotMatchedByNoHorizontalClassMerging(appView),
- new SameFields(),
+ new SameFields(appView),
new NoInterfaces(),
new NoAnnotations(),
new NoEnums(appView),
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMergerGraphLens.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMergerGraphLens.java
index b42cb24..f5fdd6f 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMergerGraphLens.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMergerGraphLens.java
@@ -72,6 +72,22 @@
.build();
}
+ @Override
+ protected FieldLookupResult internalDescribeLookupField(FieldLookupResult previous) {
+ FieldLookupResult lookup = super.internalDescribeLookupField(previous);
+ if (lookup.getReference() == previous.getReference()) {
+ return lookup;
+ }
+ return FieldLookupResult.builder(this)
+ .setReference(lookup.getReference())
+ .setReboundReference(lookup.getReboundReference())
+ .setCastType(
+ lookup.getReference().getType() != previous.getReference().getType()
+ ? lookupType(previous.getReference().getType())
+ : null)
+ .build();
+ }
+
public static class Builder {
private final MutableBidirectionalManyToOneRepresentativeMap<DexField, DexField> fieldMap =
@@ -127,12 +143,10 @@
if (originalFieldSignatures.isEmpty()) {
fieldMap.put(oldFieldSignature, newFieldSignature);
} else if (originalFieldSignatures.size() == 1) {
- fieldMap.put(originalFieldSignatures.iterator().next(), newFieldSignature);
+ fieldMap.put(originalFieldSignatures, newFieldSignature);
} else {
assert representative != null;
- for (DexField originalFieldSignature : originalFieldSignatures) {
- fieldMap.put(originalFieldSignature, newFieldSignature);
- }
+ fieldMap.put(originalFieldSignatures, newFieldSignature);
fieldMap.setRepresentative(newFieldSignature, representative);
}
}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/SimplePolicyExecutor.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/SimplePolicyExecutor.java
index 14b249a..3b96a76 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/SimplePolicyExecutor.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/SimplePolicyExecutor.java
@@ -72,6 +72,10 @@
policy.clear();
+ if (linkedGroups.isEmpty()) {
+ break;
+ }
+
// Any policy should not return any trivial groups.
assert linkedGroups.stream().allMatch(group -> group.size() >= 2);
}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/PreserveMethodCharacteristics.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/PreserveMethodCharacteristics.java
index adcde5c..5165bd9 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/PreserveMethodCharacteristics.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/PreserveMethodCharacteristics.java
@@ -9,6 +9,7 @@
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.horizontalclassmerging.MergeGroup;
import com.android.tools.r8.horizontalclassmerging.MultiClassPolicy;
+import com.android.tools.r8.utils.OptionalBool;
import com.google.common.collect.Iterables;
import java.util.ArrayList;
import java.util.Collection;
@@ -25,15 +26,17 @@
static class MethodCharacteristics {
+ private final OptionalBool isLibraryMethodOverride;
private final int visibilityOrdinal;
private MethodCharacteristics(DexEncodedMethod method) {
+ this.isLibraryMethodOverride = method.isLibraryMethodOverride();
this.visibilityOrdinal = method.getAccessFlags().getVisibilityOrdinal();
}
@Override
public int hashCode() {
- return visibilityOrdinal;
+ return (visibilityOrdinal << 2) | isLibraryMethodOverride.ordinal();
}
@Override
@@ -45,7 +48,8 @@
return false;
}
MethodCharacteristics characteristics = (MethodCharacteristics) obj;
- return visibilityOrdinal == characteristics.visibilityOrdinal;
+ return isLibraryMethodOverride == characteristics.isLibraryMethodOverride
+ && visibilityOrdinal == characteristics.visibilityOrdinal;
}
}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/PreventChangingVisibility.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/PreventChangingVisibility.java
deleted file mode 100644
index cd58e10..0000000
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/PreventChangingVisibility.java
+++ /dev/null
@@ -1,77 +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.DexEncodedMethod;
-import com.android.tools.r8.graph.DexMethod;
-import com.android.tools.r8.graph.DexProgramClass;
-import com.android.tools.r8.graph.MethodAccessFlags;
-import com.android.tools.r8.horizontalclassmerging.MergeGroup;
-import com.android.tools.r8.horizontalclassmerging.MultiClassPolicy;
-import com.android.tools.r8.utils.MethodSignatureEquivalence;
-import com.google.common.base.Equivalence.Wrapper;
-import com.google.common.collect.Iterables;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-
-public class PreventChangingVisibility extends MultiClassPolicy {
- public PreventChangingVisibility() {}
-
- public static class TargetGroup {
- private final MergeGroup group = new MergeGroup();
- private final Map<Wrapper<DexMethod>, MethodAccessFlags> methodMap = new HashMap<>();
-
- public MergeGroup getGroup() {
- return group;
- }
-
- public boolean tryAdd(DexProgramClass clazz) {
- Map<Wrapper<DexMethod>, MethodAccessFlags> newMethods = new HashMap<>();
- for (DexEncodedMethod method : clazz.methods()) {
- Wrapper<DexMethod> methodSignature =
- MethodSignatureEquivalence.get().wrap(method.getReference());
- MethodAccessFlags flags = methodMap.get(methodSignature);
-
- if (flags == null) {
- newMethods.put(methodSignature, method.getAccessFlags());
- } else {
- if (!flags.isSameVisibility(method.getAccessFlags())) {
- return false;
- }
- }
- }
-
- methodMap.putAll(newMethods);
- group.add(clazz);
- return true;
- }
- }
-
- @Override
- public Collection<MergeGroup> apply(MergeGroup group) {
- List<TargetGroup> groups = new ArrayList<>();
-
- for (DexProgramClass clazz : group) {
- boolean added = Iterables.any(groups, targetGroup -> targetGroup.tryAdd(clazz));
- if (!added) {
- TargetGroup newGroup = new TargetGroup();
- added = newGroup.tryAdd(clazz);
- assert added;
- groups.add(newGroup);
- }
- }
-
- Collection<MergeGroup> newGroups = new ArrayList<>();
- for (TargetGroup newGroup : groups) {
- if (!newGroup.getGroup().isTrivial()) {
- newGroups.add(newGroup.getGroup());
- }
- }
- return newGroups;
- }
-}
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
index 63134de..adeb3ce 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/SameFields.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/SameFields.java
@@ -4,14 +4,75 @@
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.MultiClassSameReferencePolicy;
+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 MultiClassSameReferencePolicy<FieldMultiset> {
+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/ir/code/BasicBlockInstructionListIterator.java b/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionListIterator.java
index 54354f6..5f56717 100644
--- a/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionListIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionListIterator.java
@@ -123,6 +123,23 @@
metadata.record(instruction);
}
+ @Override
+ public void addThrowingInstructionToPossiblyThrowingBlock(
+ IRCode code,
+ ListIterator<BasicBlock> blockIterator,
+ Instruction instruction,
+ InternalOptions options) {
+ if (block.hasCatchHandlers()) {
+ BasicBlock splitBlock = split(code, blockIterator, false);
+ splitBlock.listIterator(code).add(instruction);
+ assert !block.hasCatchHandlers();
+ assert splitBlock.hasCatchHandlers();
+ block.copyCatchHandlers(code, blockIterator, splitBlock, options);
+ } else {
+ add(instruction);
+ }
+ }
+
/**
* Replaces the last instruction returned by {@link #next} or {@link #previous} with the specified
* instruction.
diff --git a/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionListIterator.java b/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionListIterator.java
index d2b9ffb..a96760f 100644
--- a/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionListIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionListIterator.java
@@ -161,6 +161,16 @@
}
@Override
+ public void addThrowingInstructionToPossiblyThrowingBlock(
+ IRCode code,
+ ListIterator<BasicBlock> blockIterator,
+ Instruction instruction,
+ InternalOptions options) {
+ instructionIterator.addThrowingInstructionToPossiblyThrowingBlock(
+ code, blockIterator, instruction, options);
+ }
+
+ @Override
public void remove() {
instructionIterator.remove();
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/InstructionListIterator.java b/src/main/java/com/android/tools/r8/ir/code/InstructionListIterator.java
index 1acdb98..c4cceea 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InstructionListIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InstructionListIterator.java
@@ -20,6 +20,12 @@
public interface InstructionListIterator
extends InstructionIterator, ListIterator<Instruction>, PreviousUntilIterator<Instruction> {
+ void addThrowingInstructionToPossiblyThrowingBlock(
+ IRCode code,
+ ListIterator<BasicBlock> blockIterator,
+ Instruction instruction,
+ InternalOptions options);
+
default void addBefore(Instruction instruction) {
previous();
add(instruction);
diff --git a/src/main/java/com/android/tools/r8/ir/code/LinearFlowInstructionListIterator.java b/src/main/java/com/android/tools/r8/ir/code/LinearFlowInstructionListIterator.java
index e6009dd..0fc2768 100644
--- a/src/main/java/com/android/tools/r8/ir/code/LinearFlowInstructionListIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/LinearFlowInstructionListIterator.java
@@ -131,6 +131,16 @@
}
@Override
+ public void addThrowingInstructionToPossiblyThrowingBlock(
+ IRCode code,
+ ListIterator<BasicBlock> blockIterator,
+ Instruction instruction,
+ InternalOptions options) {
+ currentBlockIterator.addThrowingInstructionToPossiblyThrowingBlock(
+ code, blockIterator, instruction, options);
+ }
+
+ @Override
public void removeOrReplaceByDebugLocalRead() {
currentBlockIterator.removeOrReplaceByDebugLocalRead();
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java b/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
index 76e3b0f..b6609b8 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
@@ -55,6 +55,7 @@
import com.android.tools.r8.graph.RewrittenPrototypeDescription.RewrittenTypeInfo;
import com.android.tools.r8.graph.classmerging.VerticallyMergedClasses;
import com.android.tools.r8.ir.analysis.type.DestructivePhiTypeUpdater;
+import com.android.tools.r8.ir.analysis.type.Nullability;
import com.android.tools.r8.ir.analysis.type.TypeElement;
import com.android.tools.r8.ir.analysis.value.SingleNumberValue;
import com.android.tools.r8.ir.code.Assume;
@@ -64,6 +65,7 @@
import com.android.tools.r8.ir.code.ConstClass;
import com.android.tools.r8.ir.code.ConstInstruction;
import com.android.tools.r8.ir.code.ConstMethodHandle;
+import com.android.tools.r8.ir.code.FieldInstruction;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.InitClass;
import com.android.tools.r8.ir.code.InstanceGet;
@@ -92,6 +94,7 @@
import com.android.tools.r8.ir.optimize.enums.EnumUnboxer;
import com.android.tools.r8.logging.Log;
import com.android.tools.r8.optimize.MemberRebindingAnalysis;
+import com.android.tools.r8.utils.InternalOptions;
import com.google.common.collect.Sets;
import java.util.ArrayList;
import java.util.HashSet;
@@ -105,14 +108,15 @@
public class LensCodeRewriter {
private final AppView<? extends AppInfoWithClassHierarchy> appView;
-
private final EnumUnboxer enumUnboxer;
private final LensCodeRewriterUtils helper;
+ private final InternalOptions options;
LensCodeRewriter(AppView<? extends AppInfoWithClassHierarchy> appView, EnumUnboxer enumUnboxer) {
this.appView = appView;
this.enumUnboxer = enumUnboxer;
this.helper = new LensCodeRewriterUtils(appView);
+ this.options = appView.options();
}
private Value makeOutValue(Instruction insn, IRCode code) {
@@ -125,6 +129,15 @@
return null;
}
+ private Value makeOutValue(FieldInstruction insn, IRCode code, DexField rewrittenField) {
+ if (insn.hasOutValue()) {
+ Nullability nullability = insn.getOutType().nullability();
+ TypeElement newType = TypeElement.fromDexType(rewrittenField.getType(), nullability, appView);
+ return code.createValue(newType, insn.getLocalInfo());
+ }
+ return null;
+ }
+
/** Replace type appearances, invoke targets and field accesses with actual definitions. */
public void rewrite(IRCode code, ProgramMethod method) {
Set<Phi> affectedPhis =
@@ -137,7 +150,7 @@
boolean mayHaveUnreachableBlocks = false;
while (blocks.hasNext()) {
BasicBlock block = blocks.next();
- if (block.hasCatchHandlers() && appView.options().enableVerticalClassMerging) {
+ if (block.hasCatchHandlers() && options.enableVerticalClassMerging) {
boolean anyGuardsRenamed = block.renameGuardsInCatchHandlers(graphLens);
if (anyGuardsRenamed) {
mayHaveUnreachableBlocks |= unlinkDeadCatchHandlers(block);
@@ -312,9 +325,7 @@
parameter.getTypeElement(appView, type), null));
assert !instruction.instructionTypeCanThrow();
instruction.setPosition(
- appView.options().debug
- ? invoke.getPosition()
- : Position.none());
+ options.debug ? invoke.getPosition() : Position.none());
iterator.add(instruction);
iterator.next();
return instruction.outValue();
@@ -389,11 +400,11 @@
graphLens.lookupGetFieldForMethod(rewrittenField, method.getReference());
Value newOutValue = null;
if (replacementMethod != null) {
- newOutValue = makeOutValue(instanceGet, code);
+ newOutValue = makeOutValue(instanceGet, code, rewrittenField);
iterator.replaceCurrentInstruction(
new InvokeStatic(replacementMethod, newOutValue, instanceGet.inValues()));
} else if (rewrittenField != field) {
- newOutValue = makeOutValue(instanceGet, code);
+ newOutValue = makeOutValue(instanceGet, code, rewrittenField);
iterator.replaceCurrentInstruction(
new InstanceGet(newOutValue, instanceGet.object(), rewrittenField));
}
@@ -402,15 +413,17 @@
TypeElement castType =
TypeElement.fromDexType(
lookup.getCastType(), newOutValue.getType().nullability(), appView);
+ Value castOutValue = code.createValue(castType);
+ newOutValue.replaceUsers(castOutValue);
CheckCast checkCast =
CheckCast.builder()
.setCastType(lookup.getCastType())
- .setFreshOutValue(code, castType)
.setObject(newOutValue)
+ .setOutValue(castOutValue)
.setPosition(instanceGet)
.build();
- iterator.add(checkCast);
- newOutValue.replaceUsers(checkCast.outValue());
+ iterator.addThrowingInstructionToPossiblyThrowingBlock(
+ code, blocks, checkCast, options);
affectedPhis.addAll(checkCast.outValue().uniquePhiUsers());
} else if (newOutValue.getType() != instanceGet.getOutType()) {
affectedPhis.addAll(newOutValue.uniquePhiUsers());
@@ -452,11 +465,11 @@
graphLens.lookupGetFieldForMethod(rewrittenField, method.getReference());
Value newOutValue = null;
if (replacementMethod != null) {
- newOutValue = makeOutValue(staticGet, code);
+ newOutValue = makeOutValue(staticGet, code, rewrittenField);
iterator.replaceCurrentInstruction(
new InvokeStatic(replacementMethod, newOutValue, staticGet.inValues()));
} else if (rewrittenField != field) {
- newOutValue = makeOutValue(staticGet, code);
+ newOutValue = makeOutValue(staticGet, code, rewrittenField);
iterator.replaceCurrentInstruction(new StaticGet(newOutValue, rewrittenField));
}
if (newOutValue != null) {
@@ -464,15 +477,17 @@
TypeElement castType =
TypeElement.fromDexType(
lookup.getCastType(), newOutValue.getType().nullability(), appView);
+ Value castOutValue = code.createValue(castType);
+ newOutValue.replaceUsers(castOutValue);
CheckCast checkCast =
CheckCast.builder()
.setCastType(lookup.getCastType())
- .setFreshOutValue(code, castType)
.setObject(newOutValue)
+ .setOutValue(castOutValue)
.setPosition(staticGet)
.build();
- iterator.add(checkCast);
- newOutValue.replaceUsers(checkCast.outValue());
+ iterator.addThrowingInstructionToPossiblyThrowingBlock(
+ code, blocks, checkCast, options);
affectedPhis.addAll(checkCast.outValue().uniquePhiUsers());
} else if (newOutValue.getType() != staticGet.getOutType()) {
affectedPhis.addAll(newOutValue.uniquePhiUsers());
@@ -554,8 +569,7 @@
MoveException moveException = current.asMoveException();
new InstructionReplacer(code, current, iterator, affectedPhis)
.replaceInstructionIfTypeChanged(
- moveException.getExceptionType(),
- (t, v) -> new MoveException(v, t, appView.options()));
+ moveException.getExceptionType(), (t, v) -> new MoveException(v, t, options));
}
break;
@@ -695,7 +709,7 @@
iterator.previous();
Value rewrittenDefaultValue =
iterator.insertConstNumberInstruction(
- code, appView.options(), 0, defaultValueLatticeElement(newType));
+ code, options, 0, defaultValueLatticeElement(newType));
iterator.next();
return rewrittenDefaultValue;
}
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/ClassesWithDifferentFieldsTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/ClassesWithDifferentFieldsTest.java
new file mode 100644
index 0000000..d322729
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/ClassesWithDifferentFieldsTest.java
@@ -0,0 +1,82 @@
+/*
+ * // 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 static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static com.android.tools.r8.utils.codeinspector.Matchers.notIf;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.android.tools.r8.NeverClassInline;
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestParameters;
+import org.junit.Test;
+
+public class ClassesWithDifferentFieldsTest extends HorizontalClassMergingTestBase {
+ public ClassesWithDifferentFieldsTest(
+ 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))
+ .enableNeverClassInliningAnnotations()
+ .enableInliningAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ .addHorizontallyMergedClassesInspectorIf(
+ enableHorizontalClassMerging, inspector -> inspector.assertMergedInto(B.class, A.class))
+ .compile()
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines("A. v: a", "B. i: 2")
+ .inspect(
+ codeInspector -> {
+ assertThat(codeInspector.clazz(A.class), isPresent());
+ assertThat(
+ codeInspector.clazz(B.class), notIf(isPresent(), enableHorizontalClassMerging));
+ });
+ }
+
+ @NeverClassInline
+ public static class A {
+ public String v;
+
+ public A(String v) {
+ this.v = v;
+ }
+
+ @NeverInline
+ public void foo() {
+ System.out.println("A. v: " + v);
+ }
+ }
+
+ @NeverClassInline
+ public static class B {
+ public Integer i;
+
+ public B(Integer i) {
+ this.i = i;
+ }
+
+ @NeverInline
+ public void foo() {
+ System.out.println("B. i: " + i);
+ }
+ }
+
+ public static class Main {
+ public static void main(String[] args) {
+ new A("a").foo();
+ new B(2).foo();
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/EscapingBuilderTest.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/EscapingBuilderTest.java
index bf5e92a..50201f3 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/EscapingBuilderTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/EscapingBuilderTest.java
@@ -10,6 +10,7 @@
import static org.hamcrest.MatcherAssert.assertThat;
import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.NoHorizontalClassMerging;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import org.junit.Test;
@@ -23,6 +24,7 @@
.addInnerClasses(EscapingBuilderTest.class)
.addKeepMainRule(TestClass.class)
.enableInliningAnnotations()
+ .enableNoHorizontalClassMergingAnnotations()
.compile()
.inspect(this::inspect);
}
@@ -101,6 +103,7 @@
}
// Builder that escapes via field `f` that is assigned in a virtual method.
+ @NoHorizontalClassMerging
static class Builder2 {
public Builder2 f;
@@ -118,6 +121,7 @@
}
// Builder that escapes via field `f` that is assigned in a virtual method.
+ @NoHorizontalClassMerging
static class Builder3 {
public Builder3 f;
@@ -135,6 +139,7 @@
}
// Builder that escapes via field `f` that is assigned in a virtual method.
+ @NoHorizontalClassMerging
static class Builder4 {
public Builder4 f;
@@ -152,6 +157,7 @@
}
// Builder that escapes via field `f` that is assigned in a static method.
+ @NoHorizontalClassMerging
static class Builder5 {
public Builder5 f;
diff --git a/src/test/java/com/android/tools/r8/ir/regalloc/RegisterMoveSchedulerTest.java b/src/test/java/com/android/tools/r8/ir/regalloc/RegisterMoveSchedulerTest.java
index dff06df..c253c45 100644
--- a/src/test/java/com/android/tools/r8/ir/regalloc/RegisterMoveSchedulerTest.java
+++ b/src/test/java/com/android/tools/r8/ir/regalloc/RegisterMoveSchedulerTest.java
@@ -145,6 +145,15 @@
}
@Override
+ public void addThrowingInstructionToPossiblyThrowingBlock(
+ IRCode code,
+ ListIterator<BasicBlock> blockIterator,
+ Instruction instruction,
+ InternalOptions options) {
+ throw new Unimplemented();
+ }
+
+ @Override
public BasicBlock split(
IRCode code, ListIterator<BasicBlock> blockIterator, boolean keepCatchHandlers) {
throw new Unimplemented();
diff --git a/src/test/java/com/android/tools/r8/kotlin/KotlinClassInlinerTest.java b/src/test/java/com/android/tools/r8/kotlin/KotlinClassInlinerTest.java
index a949213..293caa8 100644
--- a/src/test/java/com/android/tools/r8/kotlin/KotlinClassInlinerTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/KotlinClassInlinerTest.java
@@ -205,21 +205,29 @@
Predicate<DexType> lambdaCheck = createLambdaCheck(inspector);
ClassSubject clazz = inspector.clazz(mainClassName);
+ // TODO(b/173337498): Should be empty, but horizontal class merging interferes with
+ // class inlining.
assertEquals(
- Sets.newHashSet(),
+ Sets.newHashSet(
+ "class_inliner_lambda_k_style.MainKt$testKotlinSequencesStateless$1"),
collectAccessedTypes(
lambdaCheck,
clazz,
"testKotlinSequencesStateless",
"kotlin.sequences.Sequence"));
+ // TODO(b/173337498): Should be absent, but horizontal class merging interferes with
+ // class inlining.
assertThat(
inspector.clazz(
"class_inliner_lambda_k_style.MainKt$testKotlinSequencesStateless$1"),
- not(isPresent()));
+ isPresent());
+ // TODO(b/173337498): Should be empty, but horizontal class merging interferes with
+ // class inlining.
assertEquals(
- Sets.newHashSet(),
+ Sets.newHashSet(
+ "class_inliner_lambda_k_style.MainKt$testKotlinSequencesStateful$1"),
collectAccessedTypes(
lambdaCheck,
clazz,
@@ -228,10 +236,12 @@
"int",
"kotlin.sequences.Sequence"));
+ // TODO(b/173337498): Should be absent, but horizontal class merging interferes with
+ // class inlining.
assertThat(
inspector.clazz(
"class_inliner_lambda_k_style.MainKt$testKotlinSequencesStateful$1"),
- not(isPresent()));
+ isPresent());
assertEquals(
Sets.newHashSet(),
