Mark methods obsolete after optimizations
Change-Id: If57dbdf8afe7254d0912b706f4a109d6d742bf32
diff --git a/src/main/java/com/android/tools/r8/graph/AppView.java b/src/main/java/com/android/tools/r8/graph/AppView.java
index cc0041b..83ea812 100644
--- a/src/main/java/com/android/tools/r8/graph/AppView.java
+++ b/src/main/java/com/android/tools/r8/graph/AppView.java
@@ -936,7 +936,10 @@
timing.begin("Rewrite AppView");
boolean changed = appView.setGraphLens(lens);
- assert changed;
+
+ // Verify that the lens changed, except in the horizontal class merger case, where we install
+ // the lens prior to lens rewriting AppView.
+ assert changed || lens.isHorizontalClassMergerGraphLens();
assert application.verifyWithLens(appView.appInfo().app().asDirect(), lens);
// The application has already been rewritten with the given applied lens. Therefore, we
@@ -1051,7 +1054,10 @@
private static void rewriteWithD8Lens(
NonIdentityGraphLens lens, Timing timing, AppView<AppInfo> appView) {
boolean changed = appView.setGraphLens(lens);
- assert changed;
+
+ // Verify that the lens changed, except in the horizontal class merger case, where we install
+ // the lens prior to lens rewriting AppView.
+ assert changed || lens.isHorizontalClassMergerGraphLens();
appView.setArtProfileCollection(
appView.getArtProfileCollection().rewrittenWithLens(appView, lens, timing));
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 459cf9a..efbfad4 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java
@@ -114,6 +114,10 @@
profileCollectionAdditions, syntheticArgumentClass, syntheticInitializerConverterBuilder);
mergeStaticClassInitializers(syntheticInitializerConverterBuilder);
group.forEach(this::mergeDirectMethods);
+ if (!classInitializerMerger.isEmpty() && classInitializerMerger.isTrivialMerge()) {
+ classInitializerMerger.setObsolete();
+ }
+ instanceInitializerMergers.setObsolete();
}
void mergeStaticClassInitializers(
@@ -165,7 +169,10 @@
if (!classMethodsBuilder.isFresh(newMethod)) {
newMethod = renameDirectMethod(method);
}
- classMethodsBuilder.addDirectMethod(definition.toTypeSubstitutedMethod(newMethod));
+ classMethodsBuilder.addDirectMethod(
+ newMethod != method.getReference()
+ ? definition.toTypeSubstitutedMethod(newMethod)
+ : method.getDefinition());
if (definition.getReference() != newMethod) {
lensBuilder.moveMethod(definition.getReference(), newMethod);
}
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 746dbd5..3fa5ee2 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java
@@ -162,6 +162,21 @@
assert verifyNoCyclesInInterfaceHierarchies(appView, groups);
+ FieldAccessInfoCollectionModifier fieldAccessInfoCollectionModifier = null;
+ if (mode.isInitial()) {
+ fieldAccessInfoCollectionModifier = createFieldAccessInfoCollectionModifier(groups);
+ } else {
+ assert groups.stream().noneMatch(MergeGroup::hasClassIdField);
+ }
+
+ // Set the new graph lens before finalizing any synthetic code.
+ appView.setGraphLens(horizontalClassMergerGraphLens);
+ codeProvider.setGraphLens(horizontalClassMergerGraphLens);
+
+ // Finalize synthetic code.
+ transformIncompleteCode(groups, horizontalClassMergerGraphLens, executorService);
+ syntheticInitializerConverter.convertInstanceInitializers(executorService);
+
// Must rewrite AppInfoWithLiveness before pruning the merged classes, to ensure that allocation
// sites, fields accesses, etc. are correctly transferred to the target classes.
DexApplication newApplication = getNewApplication(mergedClasses);
@@ -188,7 +203,6 @@
.rewrittenWithLens(syntheticItems, horizontalClassMergerGraphLens, timing)));
appView.rewriteWithD8Lens(horizontalClassMergerGraphLens, timing);
}
- codeProvider.setGraphLens(horizontalClassMergerGraphLens);
// Amend art profile collection.
profileCollectionAdditions
@@ -197,15 +211,10 @@
.commit(appView);
// Record where the synthesized $r8$classId fields are read and written.
- if (mode.isInitial()) {
- createFieldAccessInfoCollectionModifier(groups).modify(appView.withLiveness());
- } else {
- assert groups.stream().noneMatch(MergeGroup::hasClassIdField);
+ if (fieldAccessInfoCollectionModifier != null) {
+ fieldAccessInfoCollectionModifier.modify(appView.withLiveness());
}
- transformIncompleteCode(groups, horizontalClassMergerGraphLens, executorService);
- syntheticInitializerConverter.convertInstanceInitializers(executorService);
-
appView.pruneItems(
prunedItems.toBuilder().setPrunedApp(appView.app()).build(), executorService);
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMerger.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMerger.java
index 0cf3a94..62def22 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMerger.java
@@ -412,6 +412,13 @@
}
}
+ void setObsolete() {
+ if (hasInstanceInitializerDescription() || !useSyntheticMethod()) {
+ instanceInitializers.forEach(
+ instanceInitializer -> instanceInitializer.getDefinition().setObsolete());
+ }
+ }
+
private boolean useSyntheticMethod() {
return !isSingleton() || group.hasClassIdField();
}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMergerCollection.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMergerCollection.java
index 952421e..4ce1f2f 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMergerCollection.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMergerCollection.java
@@ -141,4 +141,8 @@
instanceInitializerMergers.forEach(consumer);
equivalentInstanceInitializerMergers.values().forEach(consumer);
}
+
+ public void setObsolete() {
+ forEach(InstanceInitializerMerger::setObsolete);
+ }
}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/code/ClassInitializerMerger.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/code/ClassInitializerMerger.java
index 6a030d7..09e9911 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/code/ClassInitializerMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/code/ClassInitializerMerger.java
@@ -75,9 +75,8 @@
}
public Code getCode(DexMethod syntheticMethodReference) {
- assert !classInitializers.isEmpty();
- ProgramMethod firstClassInitializer = ListUtils.first(classInitializers);
- if (firstClassInitializer.getDefinition().getCode().isCfCode()) {
+ assert !isEmpty();
+ if (isTrivialMerge()) {
assert IterableUtils.allIdentical(
classInitializers,
classInitializer -> classInitializer.getDefinition().getCode().isCfCode());
@@ -100,6 +99,11 @@
return null;
}
+ public boolean isTrivialMerge() {
+ ProgramMethod firstClassInitializer = ListUtils.first(classInitializers);
+ return firstClassInitializer.getDefinition().getCode().isCfCode();
+ }
+
public ComputedApiLevel getApiReferenceLevel(AppView<?> appView) {
assert !classInitializers.isEmpty();
return ListUtils.fold(
@@ -108,6 +112,10 @@
(accApiLevel, method) -> accApiLevel.max(method.getDefinition().getApiLevel()));
}
+ public void setObsolete() {
+ classInitializers.forEach(classInitializer -> classInitializer.getDefinition().setObsolete());
+ }
+
public static class Builder {
private final ImmutableList.Builder<ProgramMethod> classInitializers = ImmutableList.builder();
@@ -182,7 +190,7 @@
* Provides a piece of {@link IRCode} that is the concatenation of a collection of class
* initializers.
*/
- private static class IRProvider extends Code {
+ static class IRProvider extends Code {
private final ImmutableList<ProgramMethod> classInitializers;
private final DexMethod syntheticMethodReference;
@@ -261,6 +269,7 @@
callerPosition,
classInitializer.getOrigin(),
RewrittenPrototypeDescription.none());
+ classInitializer.getDefinition().setObsolete();
DexProgramClass downcast = null;
instructionIterator.previous();
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingTreeFixer.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingTreeFixer.java
index 940b639..eaa1d40 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingTreeFixer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingTreeFixer.java
@@ -583,8 +583,10 @@
&& enumDataMap.representativeType(method.getHolderType()) != method.getHolderType()) {
assert method.getDefinition().getCode().isEmptyVoidMethod();
prunedItemsBuilder.addRemovedMethod(method.getReference());
+ method.getDefinition().setObsolete();
} else if (method.getDefinition().isInstanceInitializer()) {
prunedItemsBuilder.addRemovedMethod(method.getReference());
+ method.getDefinition().setObsolete();
} else if (method.getDefinition().isNonPrivateVirtualMethod()) {
nonPrivateVirtualMethods.add(method.getReference());
} else {
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorApplicationFixer.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorApplicationFixer.java
index 4ad22f3..4441690 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorApplicationFixer.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorApplicationFixer.java
@@ -100,23 +100,26 @@
return method;
}
- return method.toTypeSubstitutedMethod(
- methodReferenceAfterParameterRemoval,
- builder -> {
- if (graphLens.hasPrototypeChanges(methodReferenceAfterParameterRemoval)) {
- RewrittenPrototypeDescription prototypeChanges =
- graphLens.getPrototypeChanges(methodReferenceAfterParameterRemoval);
- builder
- .apply(prototypeChanges.createParameterAnnotationsRemover(method))
- .setGenericSignature(MethodTypeSignature.noSignature());
- if (method.isInstance()
- && prototypeChanges.getArgumentInfoCollection().isArgumentRemoved(0)) {
- builder
- .modifyAccessFlags(flags -> flags.demoteFromFinal().promoteToStatic())
- .unsetIsLibraryMethodOverride();
- }
- }
- });
+ DexEncodedMethod replacement =
+ method.toTypeSubstitutedMethod(
+ methodReferenceAfterParameterRemoval,
+ builder -> {
+ if (graphLens.hasPrototypeChanges(methodReferenceAfterParameterRemoval)) {
+ RewrittenPrototypeDescription prototypeChanges =
+ graphLens.getPrototypeChanges(methodReferenceAfterParameterRemoval);
+ builder
+ .apply(prototypeChanges.createParameterAnnotationsRemover(method))
+ .setGenericSignature(MethodTypeSignature.noSignature());
+ if (method.isInstance()
+ && prototypeChanges.getArgumentInfoCollection().isArgumentRemoved(0)) {
+ builder
+ .modifyAccessFlags(flags -> flags.demoteFromFinal().promoteToStatic())
+ .unsetIsLibraryMethodOverride();
+ }
+ }
+ });
+ method.setObsolete();
+ return replacement;
});
}
