Rewrite appInfo.forceinline and appInfo.neverinline using GraphLense
Prior to this CL the two sets were never updated, meaning they would not have any effect if the signature of a method was changed (e.g., due to vertical class merging).
Change-Id: Icc2e828e69472ab710ca82dd4cc2d023b722a7c5
diff --git a/src/main/java/com/android/tools/r8/graph/GraphLense.java b/src/main/java/com/android/tools/r8/graph/GraphLense.java
index 84ceac3..d264b87 100644
--- a/src/main/java/com/android/tools/r8/graph/GraphLense.java
+++ b/src/main/java/com/android/tools/r8/graph/GraphLense.java
@@ -93,6 +93,10 @@
public abstract DexMethod getOriginalMethodSignature(DexMethod method);
+ public abstract DexField getRenamedFieldSignature(DexField originalField);
+
+ public abstract DexMethod getRenamedMethodSignature(DexMethod originalMethod);
+
public abstract DexType lookupType(DexType type);
// This overload can be used when the graph lense is known to be context insensitive.
@@ -140,7 +144,7 @@
return this instanceof IdentityGraphLense;
}
- public boolean assertNotModified(Iterable<DexItem> items) {
+ public <T extends DexItem> boolean assertNotModified(Iterable<T> items) {
for (DexItem item : items) {
if (item instanceof DexClass) {
DexType type = ((DexClass) item).type;
@@ -172,6 +176,16 @@
}
@Override
+ public DexField getRenamedFieldSignature(DexField originalField) {
+ return originalField;
+ }
+
+ @Override
+ public DexMethod getRenamedMethodSignature(DexMethod originalMethod) {
+ return originalMethod;
+ }
+
+ @Override
public DexType lookupType(DexType type) {
return type;
}
@@ -254,6 +268,24 @@
}
@Override
+ public DexField getRenamedFieldSignature(DexField originalField) {
+ DexField renamedField =
+ originalFieldSignatures != null
+ ? originalFieldSignatures.inverse().getOrDefault(originalField, originalField)
+ : originalField;
+ return previousLense.getRenamedFieldSignature(renamedField);
+ }
+
+ @Override
+ public DexMethod getRenamedMethodSignature(DexMethod originalMethod) {
+ DexMethod renamedMethod =
+ originalMethodSignatures != null
+ ? originalMethodSignatures.inverse().getOrDefault(originalMethod, originalMethod)
+ : originalMethod;
+ return previousLense.getRenamedMethodSignature(renamedMethod);
+ }
+
+ @Override
public DexType lookupType(DexType type) {
if (type.isArrayType()) {
synchronized (this) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
index 631f234..998ebe9 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
@@ -93,11 +93,11 @@
private Reason computeInliningReason(DexEncodedMethod target) {
if (target.getOptimizationInfo().forceInline()
|| (inliner.appInfo.hasLiveness()
- && inliner.appInfo.withLiveness().forceInline.contains(target))) {
+ && inliner.appInfo.withLiveness().forceInline.contains(target.method))) {
return Reason.FORCE;
}
if (inliner.appInfo.hasLiveness()
- && inliner.appInfo.withLiveness().alwaysInline.contains(target)) {
+ && inliner.appInfo.withLiveness().alwaysInline.contains(target.method)) {
return Reason.ALWAYS;
}
if (callSiteInformation.hasSingleCallSite(target)) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
index 12baa2c..1cbd2d9 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
@@ -71,7 +71,7 @@
}
public boolean isBlackListed(DexEncodedMethod method) {
- return blackList.contains(method.method) || appInfo.neverInline.contains(method);
+ return blackList.contains(method.method) || appInfo.neverInline.contains(method.method);
}
private ConstraintWithTarget instructionAllowedForInlining(
diff --git a/src/main/java/com/android/tools/r8/naming/ClassNameMapper.java b/src/main/java/com/android/tools/r8/naming/ClassNameMapper.java
index 6414e13..e405bfb 100644
--- a/src/main/java/com/android/tools/r8/naming/ClassNameMapper.java
+++ b/src/main/java/com/android/tools/r8/naming/ClassNameMapper.java
@@ -38,7 +38,7 @@
private final ImmutableMap.Builder<String, ClassNamingForNameMapper.Builder> mapBuilder;
private Builder() {
- mapBuilder = ImmutableMap.builder();
+ this.mapBuilder = ImmutableMap.builder();
}
@Override
diff --git a/src/main/java/com/android/tools/r8/optimize/BridgeMethodAnalysis.java b/src/main/java/com/android/tools/r8/optimize/BridgeMethodAnalysis.java
index 6ba18ac..46fdc82 100644
--- a/src/main/java/com/android/tools/r8/optimize/BridgeMethodAnalysis.java
+++ b/src/main/java/com/android/tools/r8/optimize/BridgeMethodAnalysis.java
@@ -100,6 +100,17 @@
}
@Override
+ public DexField getRenamedFieldSignature(DexField originalField) {
+ return previousLense.getRenamedFieldSignature(originalField);
+ }
+
+ @Override
+ public DexMethod getRenamedMethodSignature(DexMethod originalMethod) {
+ // TODO(b/79143143): implement this when re-enabling bridge analysis.
+ throw new Unimplemented("BridgeLense.getRenamedMethodSignature() not implemented");
+ }
+
+ @Override
public DexType lookupType(DexType type) {
return previousLense.lookupType(type);
}
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 bcd489e..19a116f 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -1681,15 +1681,15 @@
/**
* All methods that should be inlined if possible due to a configuration directive.
*/
- public final Set<DexItem> alwaysInline;
+ public final Set<DexMethod> alwaysInline;
/**
* All methods that *must* be inlined due to a configuration directive (testing only).
*/
- public final Set<DexItem> forceInline;
+ public final Set<DexMethod> forceInline;
/**
* All methods that *must* never be inlined due to a configuration directive (testing only).
*/
- public final Set<DexItem> neverInline;
+ public final Set<DexMethod> neverInline;
/**
* All items with -identifiernamestring rule.
*/
@@ -1836,8 +1836,8 @@
this.assumedValues = previous.assumedValues;
assert lense.assertNotModified(previous.alwaysInline);
this.alwaysInline = previous.alwaysInline;
- this.forceInline = previous.forceInline;
- this.neverInline = previous.neverInline;
+ this.forceInline = rewriteMethodsWithRenamedSignature(previous.forceInline, lense);
+ this.neverInline = rewriteMethodsWithRenamedSignature(previous.neverInline, lense);
this.identifierNameStrings =
rewriteMixedItemsConservatively(previous.identifierNameStrings, lense);
// Switchmap classes should never be affected by renaming.
@@ -1949,6 +1949,16 @@
return builder.build();
}
+ private static ImmutableSortedSet<DexMethod> rewriteMethodsWithRenamedSignature(
+ Set<DexMethod> methods, GraphLense lense) {
+ ImmutableSortedSet.Builder<DexMethod> builder =
+ new ImmutableSortedSet.Builder<>(PresortedComparable::slowCompare);
+ for (DexMethod method : methods) {
+ builder.add(lense.getRenamedMethodSignature(method));
+ }
+ return builder.build();
+ }
+
private static ImmutableSortedSet<DexMethod> rewriteMethodsConservatively(
Set<DexMethod> original, GraphLense lense) {
ImmutableSortedSet.Builder<DexMethod> builder =
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 c95cfe9..473c990 100644
--- a/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
+++ b/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
@@ -59,9 +59,9 @@
private final Set<ProguardConfigurationRule> rulesThatUseExtendsOrImplementsWrong =
Sets.newIdentityHashSet();
private final Set<DexItem> checkDiscarded = Sets.newIdentityHashSet();
- private final Set<DexItem> alwaysInline = Sets.newIdentityHashSet();
- private final Set<DexItem> forceInline = Sets.newIdentityHashSet();
- private final Set<DexItem> neverInline = Sets.newIdentityHashSet();
+ private final Set<DexMethod> alwaysInline = Sets.newIdentityHashSet();
+ private final Set<DexMethod> forceInline = Sets.newIdentityHashSet();
+ private final Set<DexMethod> neverInline = Sets.newIdentityHashSet();
private final Map<DexItem, Map<DexItem, ProguardKeepRule>> dependentNoShrinking =
new IdentityHashMap<>();
private final Map<DexItem, ProguardMemberRule> noSideEffects = new IdentityHashMap<>();
@@ -727,13 +727,19 @@
} else if (context instanceof InlineRule) {
switch (((InlineRule) context).getType()) {
case ALWAYS:
- alwaysInline.add(item);
+ if (item instanceof DexEncodedMethod) {
+ alwaysInline.add(((DexEncodedMethod) item).method);
+ }
break;
case FORCE:
- forceInline.add(item);
+ if (item instanceof DexEncodedMethod) {
+ forceInline.add(((DexEncodedMethod) item).method);
+ }
break;
case NEVER:
- neverInline.add(item);
+ if (item instanceof DexEncodedMethod) {
+ neverInline.add(((DexEncodedMethod) item).method);
+ }
break;
default:
throw new Unreachable();
@@ -755,9 +761,9 @@
public final Set<DexItem> reasonAsked;
public final Set<DexItem> keepPackageName;
public final Set<DexItem> checkDiscarded;
- public final Set<DexItem> alwaysInline;
- public final Set<DexItem> forceInline;
- public final Set<DexItem> neverInline;
+ public final Set<DexMethod> alwaysInline;
+ public final Set<DexMethod> forceInline;
+ public final Set<DexMethod> neverInline;
public final Map<DexItem, ProguardMemberRule> noSideEffects;
public final Map<DexItem, ProguardMemberRule> assumedValues;
private final Map<DexItem, Map<DexItem, ProguardKeepRule>> dependentNoShrinking;
@@ -792,9 +798,9 @@
Set<DexItem> reasonAsked,
Set<DexItem> keepPackageName,
Set<DexItem> checkDiscarded,
- Set<DexItem> alwaysInline,
- Set<DexItem> forceInline,
- Set<DexItem> neverInline,
+ Set<DexMethod> alwaysInline,
+ Set<DexMethod> forceInline,
+ Set<DexMethod> neverInline,
Map<DexItem, ProguardMemberRule> noSideEffects,
Map<DexItem, ProguardMemberRule> assumedValues,
Map<DexItem, Map<DexItem, ProguardKeepRule>> dependentNoShrinking,
diff --git a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
index ee596f3..a2dd383 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -252,7 +252,7 @@
}
}
- private void extractPinnedItems(Iterable<DexItem> items, AbortReason reason) {
+ private <T extends DexItem> void extractPinnedItems(Iterable<T> items, AbortReason reason) {
for (DexItem item : items) {
if (item instanceof DexType || item instanceof DexClass) {
DexType type = item instanceof DexType ? (DexType) item : ((DexClass) item).type;
@@ -1524,6 +1524,16 @@
}
@Override
+ public DexField getRenamedFieldSignature(DexField originalField) {
+ throw new Unreachable();
+ }
+
+ @Override
+ public DexMethod getRenamedMethodSignature(DexMethod originalMethod) {
+ throw new Unreachable();
+ }
+
+ @Override
public DexType lookupType(DexType type) {
return type == source ? target : mergedClasses.getOrDefault(type, type);
}
diff --git a/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java b/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
index 18e8c83..d9d62c3 100644
--- a/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
@@ -49,7 +49,6 @@
import java.util.concurrent.ExecutionException;
import java.util.function.Consumer;
import java.util.function.Predicate;
-import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -479,9 +478,6 @@
});
}
- // TODO(christofferqa): The sets appInfo.forceInline and appInfo.neverInline must be rewritten
- // with the graph lense after vertical class merging.
- @Ignore
@Test
public void testProguardMethodMapAfterInlining() throws Throwable {
String main = "classmerging.ProguardMethodMapTest";
