Move pinnedItems to descriptors.
Bug:
Change-Id: I07ca870f9b641413215dc447d0d58d0a71587a93
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index 0ed71b8..9977151 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -224,7 +224,7 @@
if (options.proguardConfiguration.isPrintSeeds()) {
ByteArrayOutputStream bytes = new ByteArrayOutputStream();
PrintStream out = new PrintStream(bytes);
- RootSetBuilder.writeSeeds(appInfo.withLiveness().getPinnedItems(), out);
+ RootSetBuilder.writeSeeds(appInfo.withLiveness(), out);
out.flush();
proguardSeedsData = bytes.toByteArray();
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/CallGraph.java b/src/main/java/com/android/tools/r8/ir/conversion/CallGraph.java
index 5704e0f..fdaa541 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/CallGraph.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/CallGraph.java
@@ -179,7 +179,7 @@
}
for (Node value : nodes.values()) {
// For non-pinned methods we know the exact number of call sites.
- if (!appInfo.withLiveness().isPinned(value.method)) {
+ if (!appInfo.withLiveness().isPinned(value.method.method)) {
if (value.invokeCount == 1) {
singleCallSite.add(value.method);
} else if (value.invokeCount == 2) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java b/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
index de0bab0..d71febe 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
@@ -239,7 +239,7 @@
// Without live set information we cannot tell and assume true.
if (liveSet == null
|| liveSet.fieldsRead.contains(field.field)
- || liveSet.isPinned(field)) {
+ || liveSet.isPinned(field.field)) {
return true;
}
// For library classes we don't know whether a field is read.
diff --git a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
index 31a81f9..ed4d34d 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -5,6 +5,7 @@
import com.android.tools.r8.dex.IndexedItemCollection;
import com.android.tools.r8.errors.CompilationError;
+import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AppInfoWithSubtyping;
import com.android.tools.r8.graph.Descriptor;
import com.android.tools.r8.graph.DexAnnotation;
@@ -1149,7 +1150,7 @@
this.staticFieldWrites = enqueuer.collectStaticFieldsWritten();
this.fieldsRead = enqueuer.collectFieldsRead();
this.fieldsWritten = enqueuer.collectFieldsWritten();
- this.pinnedItems = ImmutableSet.copyOf(enqueuer.pinnedItems);
+ this.pinnedItems = rewritePinnedItemsToDescriptors(enqueuer.pinnedItems);
this.virtualInvokes = joinInvokedMethods(enqueuer.virtualInvokes);
this.superInvokes = joinInvokedMethods(enqueuer.superInvokes);
this.directInvokes = joinInvokedMethods(enqueuer.directInvokes);
@@ -1208,8 +1209,7 @@
this.staticFieldWrites = rewriteItems(previous.staticFieldWrites, lense::lookupField);
this.fieldsRead = rewriteItems(previous.fieldsRead, lense::lookupField);
this.fieldsWritten = rewriteItems(previous.fieldsWritten, lense::lookupField);
- assert assertNotModifiedByLense(previous.pinnedItems, lense);
- this.pinnedItems = previous.pinnedItems;
+ this.pinnedItems = rewriteMixedItems(previous.pinnedItems, lense);
this.virtualInvokes = rewriteItems(previous.virtualInvokes, lense::lookupMethod);
this.superInvokes = rewriteItems(previous.superInvokes, lense::lookupMethod);
this.directInvokes = rewriteItems(previous.directInvokes, lense::lookupMethod);
@@ -1231,12 +1231,12 @@
private boolean assertNoItemRemoved(Collection<DexItem> items, Collection<DexType> types) {
Set<DexType> typeSet = ImmutableSet.copyOf(types);
for (DexItem item : items) {
- if (item instanceof DexClass) {
- assert !typeSet.contains(((DexClass) item).type);
- } else if (item instanceof DexEncodedMethod) {
- assert !typeSet.contains(((DexEncodedMethod) item).method.getHolder());
- } else if (item instanceof DexEncodedField) {
- assert !typeSet.contains(((DexEncodedField) item).field.getHolder());
+ if (item instanceof DexType) {
+ assert !typeSet.contains(item);
+ } else if (item instanceof DexMethod) {
+ assert !typeSet.contains(((DexMethod) item).getHolder());
+ } else if (item instanceof DexField) {
+ assert !typeSet.contains(((DexField) item).getHolder());
} else {
assert false;
}
@@ -1282,6 +1282,23 @@
return builder.build();
}
+ private ImmutableSet<DexItem> rewritePinnedItemsToDescriptors(Collection<DexItem> source) {
+ ImmutableSet.Builder<DexItem> builder = ImmutableSet.builder();
+ for (DexItem item : source) {
+ // TODO(67934123) There should be a common interface to extract this information.
+ if (item instanceof DexClass) {
+ builder.add(((DexClass) item).type);
+ } else if (item instanceof DexEncodedMethod) {
+ builder.add(((DexEncodedMethod) item).method);
+ } else if (item instanceof DexEncodedField) {
+ builder.add(((DexEncodedField) item).field);
+ } else {
+ throw new Unreachable();
+ }
+ }
+ return builder.build();
+ }
+
private static <T extends PresortedComparable<T>> ImmutableSortedSet<T> rewriteItems(
Set<T> original, BiFunction<T, DexEncodedMethod, T> rewrite) {
ImmutableSortedSet.Builder<T> builder =
@@ -1292,6 +1309,24 @@
return builder.build();
}
+ private static ImmutableSet<DexItem> rewriteMixedItems(
+ Set<DexItem> original, GraphLense lense) {
+ ImmutableSet.Builder<DexItem> builder = ImmutableSet.builder();
+ for (DexItem item : original) {
+ // TODO(67934123) There should be a common interface to perform the dispatch.
+ if (item instanceof DexType) {
+ builder.add(lense.lookupType((DexType) item, null));
+ } else if (item instanceof DexMethod) {
+ builder.add(lense.lookupMethod((DexMethod) item, null));
+ } else if (item instanceof DexField) {
+ builder.add(lense.lookupField((DexField) item, null));
+ } else {
+ throw new Unreachable();
+ }
+ }
+ return builder.build();
+ }
+
private static <T> Set<T> mergeSets(Collection<T> first, Collection<T> second) {
ImmutableSet.Builder<T> builder = ImmutableSet.builder();
builder.addAll(first);
@@ -1323,10 +1358,22 @@
return this;
}
- public boolean isPinned(DexItem item) {
+ // TODO(67934123) Unify into one method,
+ public boolean isPinned(DexType item) {
return pinnedItems.contains(item);
}
+ // TODO(67934123) Unify into one method,
+ public boolean isPinned(DexMethod item) {
+ return pinnedItems.contains(item);
+ }
+
+ // TODO(67934123) Unify into one method,
+ public boolean isPinned(DexField item) {
+ return pinnedItems.contains(item);
+ }
+
+
public Iterable<DexItem> getPinnedItems() {
return pinnedItems;
}
diff --git a/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java b/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
index b55a7a0..177493a 100644
--- a/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
+++ b/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
@@ -3,6 +3,8 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.shaking;
+import com.android.tools.r8.dex.Constants;
+import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.DexAnnotation;
import com.android.tools.r8.graph.DexAnnotationSet;
@@ -19,6 +21,7 @@
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.DirectMappedDexApplication;
import com.android.tools.r8.logging.Log;
+import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.MethodSignatureEquivalence;
import com.android.tools.r8.utils.StringDiagnostic;
@@ -284,32 +287,34 @@
clazz.forEachField(field -> markField(field, memberKeepRules, rule, onlyIfClassKept));
}
- public static void writeSeeds(Iterable<DexItem> seeds, PrintStream out) {
- for (DexItem seed : seeds) {
- if (seed instanceof DexClass) {
- out.println(((DexClass) seed).type.toSourceString());
- } else if (seed instanceof DexEncodedField) {
- DexField field = ((DexEncodedField) seed).field;
+ // TODO(67934426): Test this code.
+ public static void writeSeeds(AppInfoWithLiveness appInfo, PrintStream out) {
+ for (DexItem seed : appInfo.getPinnedItems()) {
+ if (seed instanceof DexType) {
+ out.println(seed.toSourceString());
+ } else if (seed instanceof DexField) {
+ DexField field = ((DexField) seed);
out.println(
field.clazz.toSourceString() + ": " + field.type.toSourceString() + " " + field.name
.toSourceString());
- } else if (seed instanceof DexEncodedMethod) {
- DexEncodedMethod encodedMethod = (DexEncodedMethod) seed;
- DexMethod method = encodedMethod.method;
+ } else if (seed instanceof DexMethod) {
+ DexMethod method = (DexMethod) seed;
out.print(method.holder.toSourceString() + ": ");
+ DexEncodedMethod encodedMethod = appInfo.definitionFor(method);
if (encodedMethod.accessFlags.isConstructor()) {
if (encodedMethod.accessFlags.isStatic()) {
- out.print("<clinit>(");
+ out.print(Constants.CLASS_INITIALIZER_NAME);
} else {
String holderName = method.holder.toSourceString();
String constrName = holderName.substring(holderName.lastIndexOf('.') + 1);
- out.print(constrName + "(");
+ out.print(constrName);
}
} else {
out.print(
- method.proto.returnType.toSourceString() + " " + method.name.toSourceString() + "(");
+ method.proto.returnType.toSourceString() + " " + method.name.toSourceString());
}
boolean first = true;
+ out.print("(");
for (DexType param : method.proto.parameters.values) {
if (!first) {
out.print(",");
@@ -318,6 +323,8 @@
out.print(param.toSourceString());
}
out.println(")");
+ } else {
+ throw new Unreachable();
}
}
out.close();
diff --git a/src/main/java/com/android/tools/r8/shaking/SimpleClassMerger.java b/src/main/java/com/android/tools/r8/shaking/SimpleClassMerger.java
index bf2b761..9affabf 100644
--- a/src/main/java/com/android/tools/r8/shaking/SimpleClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/SimpleClassMerger.java
@@ -139,7 +139,7 @@
for (DexProgramClass clazz : application.classes()) {
if (isMergeCandidate(clazz)) {
DexClass targetClass = appInfo.definitionFor(clazz.type.getSingleSubtype());
- if (appInfo.isPinned(targetClass)) {
+ if (appInfo.isPinned(targetClass.type)) {
// We have to keep the target class intact, so we cannot merge it.
continue;
}
