Only generate class id fields for disambiguation when needed
Change-Id: I4a23e8da59567148a59ff9db08cf8756eda72d08
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 23cb57c..a7bae5d 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassInstanceFieldsMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassInstanceFieldsMerger.java
@@ -128,8 +128,9 @@
public DexEncodedField[] merge() {
List<DexEncodedField> newFields = new ArrayList<>();
- assert classIdField != null;
- newFields.add(classIdField);
+ if (classIdField != null) {
+ newFields.add(classIdField);
+ }
fieldMappings.forEach(
(targetField, oldFields) ->
newFields.add(mergeSourceFieldsToTargetField(targetField, oldFields)));
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 ad47d8d..220d19e 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java
@@ -17,6 +17,7 @@
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.graph.DexMethodSignature;
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexProto;
import com.android.tools.r8.graph.DexType;
@@ -33,8 +34,6 @@
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.shaking.KeepClassInfo;
import com.android.tools.r8.utils.IterableUtils;
-import com.android.tools.r8.utils.MethodSignatureEquivalence;
-import com.google.common.base.Equivalence.Wrapper;
import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
import it.unimi.dsi.fastutil.objects.Reference2IntMap;
@@ -263,7 +262,10 @@
public void mergeGroup(SyntheticArgumentClass syntheticArgumentClass) {
fixAccessFlags();
- appendClassIdField();
+
+ if (group.hasClassIdField()) {
+ appendClassIdField();
+ }
mergeAnnotations();
mergeInterfaces();
@@ -279,13 +281,6 @@
public static class Builder {
private final AppView<AppInfoWithLiveness> appView;
private final MergeGroup group;
- private final ClassInitializerSynthesizedCode.Builder classInitializerSynthesizedCodeBuilder =
- new ClassInitializerSynthesizedCode.Builder();
- private final Map<DexProto, ConstructorMerger.Builder> constructorMergerBuilders =
- new LinkedHashMap<>();
- private final List<ConstructorMerger.Builder> unmergedConstructorBuilders = new ArrayList<>();
- private final Map<Wrapper<DexMethod>, VirtualMethodMerger.Builder> virtualMethodMergerBuilders =
- new LinkedHashMap<>();
public Builder(AppView<AppInfoWithLiveness> appView, MergeGroup group) {
this.appView = appView;
@@ -314,84 +309,101 @@
group.setTarget(appView.testing().horizontalClassMergingTarget.apply(candidates, target));
}
- private Builder setup() {
- DexItemFactory dexItemFactory = appView.dexItemFactory();
- // TODO(b/165498187): ensure the name for the field is fresh
- group.setClassIdField(
- dexItemFactory.createField(
- group.getTarget().getType(), dexItemFactory.intType, CLASS_ID_FIELD_NAME));
- group.forEach(this::setupForMethodMerging);
- return this;
+ private ClassInitializerSynthesizedCode createClassInitializerMerger() {
+ ClassInitializerSynthesizedCode.Builder builder =
+ new ClassInitializerSynthesizedCode.Builder();
+ group.forEach(
+ clazz -> {
+ if (clazz.hasClassInitializer()) {
+ builder.add(clazz.getClassInitializer());
+ }
+ });
+ return builder.build();
}
- private void setupForMethodMerging(DexProgramClass toMerge) {
- if (toMerge.hasClassInitializer()) {
- classInitializerSynthesizedCodeBuilder.add(toMerge.getClassInitializer());
- }
- toMerge.forEachProgramDirectMethodMatching(
- DexEncodedMethod::isInstanceInitializer, this::addConstructor);
- toMerge.forEachProgramVirtualMethod(this::addVirtualMethod);
- }
-
- private void addConstructor(ProgramMethod method) {
- assert method.getDefinition().isInstanceInitializer();
+ private List<ConstructorMerger> createInstanceInitializerMergers() {
+ List<ConstructorMerger> constructorMergers = new ArrayList<>();
if (appView.options().horizontalClassMergerOptions().isConstructorMergingEnabled()) {
- constructorMergerBuilders
- .computeIfAbsent(
- method.getDefinition().getProto(), ignore -> new ConstructorMerger.Builder(appView))
- .add(method.getDefinition());
+ Map<DexProto, ConstructorMerger.Builder> buildersByProto = new LinkedHashMap<>();
+ group.forEach(
+ clazz ->
+ clazz.forEachProgramDirectMethodMatching(
+ DexEncodedMethod::isInstanceInitializer,
+ method ->
+ buildersByProto
+ .computeIfAbsent(
+ method.getDefinition().getProto(),
+ ignore -> new ConstructorMerger.Builder(appView))
+ .add(method.getDefinition())));
+ for (ConstructorMerger.Builder builder : buildersByProto.values()) {
+ constructorMergers.addAll(builder.build(group));
+ }
} else {
- unmergedConstructorBuilders.add(
- new ConstructorMerger.Builder(appView).add(method.getDefinition()));
+ group.forEach(
+ clazz ->
+ clazz.forEachProgramDirectMethodMatching(
+ DexEncodedMethod::isInstanceInitializer,
+ method ->
+ constructorMergers.addAll(
+ new ConstructorMerger.Builder(appView)
+ .add(method.getDefinition())
+ .build(group))));
}
+
+ // Try and merge the constructors with the most arguments first, to avoid using synthetic
+ // arguments if possible.
+ constructorMergers.sort(Comparator.comparing(ConstructorMerger::getArity).reversed());
+ return constructorMergers;
}
- private void addVirtualMethod(ProgramMethod method) {
- assert method.getDefinition().isNonPrivateVirtualMethod();
- virtualMethodMergerBuilders
- .computeIfAbsent(
- MethodSignatureEquivalence.get().wrap(method.getReference()),
- ignore -> new VirtualMethodMerger.Builder())
- .add(method);
- }
-
- private Collection<ConstructorMerger.Builder> getConstructorMergerBuilders() {
- return appView.options().horizontalClassMergerOptions().isConstructorMergingEnabled()
- ? constructorMergerBuilders.values()
- : unmergedConstructorBuilders;
- }
-
- public ClassMerger build(
- HorizontalClassMergerGraphLens.Builder lensBuilder) {
- selectTarget();
- setup();
+ private List<VirtualMethodMerger> createVirtualMethodMergers() {
+ Map<DexMethodSignature, VirtualMethodMerger.Builder> virtualMethodMergerBuilders =
+ new LinkedHashMap<>();
+ group.forEach(
+ clazz ->
+ clazz.forEachProgramVirtualMethod(
+ virtualMethod ->
+ virtualMethodMergerBuilders
+ .computeIfAbsent(
+ virtualMethod.getReference().getSignature(),
+ ignore -> new VirtualMethodMerger.Builder())
+ .add(virtualMethod)));
List<VirtualMethodMerger> virtualMethodMergers =
new ArrayList<>(virtualMethodMergerBuilders.size());
for (VirtualMethodMerger.Builder builder : virtualMethodMergerBuilders.values()) {
virtualMethodMergers.add(builder.build(appView, group));
}
- // Try and merge the functions with the most arguments first, to avoid using synthetic
- // arguments if possible.
- virtualMethodMergers.sort(Comparator.comparing(VirtualMethodMerger::getArity).reversed());
+ return virtualMethodMergers;
+ }
- List<ConstructorMerger> constructorMergers =
- new ArrayList<>(constructorMergerBuilders.size());
- for (ConstructorMerger.Builder builder : getConstructorMergerBuilders()) {
- constructorMergers.addAll(builder.build(appView, group));
+ private void createClassIdField() {
+ // TODO(b/165498187): ensure the name for the field is fresh
+ DexItemFactory dexItemFactory = appView.dexItemFactory();
+ group.setClassIdField(
+ dexItemFactory.createField(
+ group.getTarget().getType(), dexItemFactory.intType, CLASS_ID_FIELD_NAME));
+ }
+
+ public ClassMerger build(
+ HorizontalClassMergerGraphLens.Builder lensBuilder) {
+ selectTarget();
+
+ List<VirtualMethodMerger> virtualMethodMergers = createVirtualMethodMergers();
+
+ boolean requiresClassIdField =
+ virtualMethodMergers.stream()
+ .anyMatch(virtualMethodMerger -> !virtualMethodMerger.isNopOrTrivial());
+ if (requiresClassIdField) {
+ createClassIdField();
}
- // Try and merge the functions with the most arguments first, to avoid using synthetic
- // arguments if possible.
- virtualMethodMergers.sort(Comparator.comparing(VirtualMethodMerger::getArity).reversed());
- constructorMergers.sort(Comparator.comparing(ConstructorMerger::getArity).reversed());
-
return new ClassMerger(
appView,
lensBuilder,
group,
virtualMethodMergers,
- constructorMergers,
- classInitializerSynthesizedCodeBuilder.build());
+ createInstanceInitializerMergers(),
+ createClassInitializerMerger());
}
}
}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/ConstructorEntryPoint.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/ConstructorEntryPoint.java
index 732a5d2..d650b17 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/ConstructorEntryPoint.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/ConstructorEntryPoint.java
@@ -49,6 +49,10 @@
this.classIdField = classIdField;
}
+ private boolean hasClassIdField() {
+ return classIdField != null;
+ }
+
void addConstructorInvoke(DexMethod typeConstructor) {
add(
builder -> {
@@ -66,11 +70,13 @@
/** Assign the given register to the class id field. */
void addRegisterClassIdAssignment(int idRegister) {
+ assert hasClassIdField();
add(builder -> builder.addInstancePut(idRegister, getReceiverRegister(), classIdField));
}
/** Assign the given constant integer value to the class id field. */
void addConstantRegisterClassIdAssignment(int classId) {
+ assert hasClassIdField();
int idRegister = nextRegister(ValueType.INT);
add(builder -> builder.addIntConst(idRegister, classId));
addRegisterClassIdAssignment(idRegister);
@@ -82,7 +88,9 @@
// The class id register is always the first synthetic argument.
int idRegister = getParamRegister(exampleTargetConstructor.getArity());
- addRegisterClassIdAssignment(idRegister);
+ if (hasClassIdField()) {
+ addRegisterClassIdAssignment(idRegister);
+ }
int[] keys = new int[typeConstructorCount - 1];
int[] offsets = new int[typeConstructorCount - 1];
@@ -115,7 +123,9 @@
protected void prepareSingleConstructorInstructions() {
Entry<DexMethod> entry = typeConstructors.int2ReferenceEntrySet().first();
- addConstantRegisterClassIdAssignment(entry.getIntKey());
+ if (hasClassIdField()) {
+ addConstantRegisterClassIdAssignment(entry.getIntKey());
+ }
addConstructorInvoke(entry.getValue());
add(IRBuilder::addReturn, endsBlock);
}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/ConstructorMerger.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/ConstructorMerger.java
index 13673fa..ecb4295 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/ConstructorMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/ConstructorMerger.java
@@ -96,7 +96,7 @@
return this;
}
- public List<ConstructorMerger> build(AppView<?> appView, MergeGroup group) {
+ public List<ConstructorMerger> build(MergeGroup group) {
assert constructorGroups.stream().noneMatch(List::isEmpty);
return ListUtils.map(
constructorGroups, constructors -> new ConstructorMerger(appView, group, constructors));
@@ -185,7 +185,7 @@
new ConstructorEntryPointSynthesizedCode(
typeConstructorClassMap,
newConstructorReference,
- group.getClassIdField(),
+ group.hasClassIdField() ? group.getClassIdField() : null,
bridgeConstructorReference);
DexEncodedMethod newConstructor =
new DexEncodedMethod(
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 7085879..557164c 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java
@@ -98,13 +98,16 @@
FieldAccessInfoCollectionModifier.Builder builder =
new FieldAccessInfoCollectionModifier.Builder();
for (MergeGroup group : groups) {
- DexProgramClass target = group.getTarget();
- target.forEachProgramInstanceInitializerMatching(
- definition -> definition.getCode().isHorizontalClassMergingCode(),
- method -> builder.recordFieldWrittenInContext(group.getClassIdField(), method));
- target.forEachProgramVirtualMethodMatching(
- definition -> definition.hasCode() && definition.getCode().isHorizontalClassMergingCode(),
- method -> builder.recordFieldReadInContext(group.getClassIdField(), method));
+ if (group.hasClassIdField()) {
+ DexProgramClass target = group.getTarget();
+ target.forEachProgramInstanceInitializerMatching(
+ definition -> definition.getCode().isHorizontalClassMergingCode(),
+ method -> builder.recordFieldWrittenInContext(group.getClassIdField(), method));
+ target.forEachProgramVirtualMethodMatching(
+ definition ->
+ definition.hasCode() && definition.getCode().isHorizontalClassMergingCode(),
+ method -> builder.recordFieldReadInContext(group.getClassIdField(), method));
+ }
}
return builder.build();
}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/VirtualMethodMerger.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/VirtualMethodMerger.java
index c144128..91e6cc0 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/VirtualMethodMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/VirtualMethodMerger.java
@@ -171,6 +171,10 @@
return numberOfNonAbstractMethods <= 1;
}
+ boolean isNopOrTrivial() {
+ return isNop() || isTrivial();
+ }
+
/**
* If there is only a single method that does not override anything then it is safe to just move
* it to the target type if it is not already in it.
@@ -221,12 +225,11 @@
assert !methods.isEmpty();
// Handle trivial merges.
- if (isNop() || isTrivial()) {
+ if (isNopOrTrivial()) {
mergeTrivial(classMethodsBuilder, lensBuilder);
return;
}
-
Int2ReferenceSortedMap<DexMethod> classIdToMethodMap = new Int2ReferenceAVLTreeMap<>();
CfVersion classFileVersion = null;
