Do not duplicate resolution in lookupDispatchTarget
Change-Id: I2efc657c7e65715747b3c780566b22a275db7f35
diff --git a/src/main/java/com/android/tools/r8/graph/MethodResolutionResult.java b/src/main/java/com/android/tools/r8/graph/MethodResolutionResult.java
index e8290ab..6dbdc35 100644
--- a/src/main/java/com/android/tools/r8/graph/MethodResolutionResult.java
+++ b/src/main/java/com/android/tools/r8/graph/MethodResolutionResult.java
@@ -411,6 +411,7 @@
.lookupSingleVirtualTarget(
appViewWithLiveness,
invokedMethod,
+ this,
context,
invoke.getInterfaceBit(),
appView,
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/callgraph/InvokeExtractor.java b/src/main/java/com/android/tools/r8/ir/conversion/callgraph/InvokeExtractor.java
index 1f0e197..eb09ac7 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/callgraph/InvokeExtractor.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/callgraph/InvokeExtractor.java
@@ -4,6 +4,8 @@
package com.android.tools.r8.ir.conversion.callgraph;
+import static com.android.tools.r8.graph.DexClassAndMethod.asProgramMethodOrNull;
+
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexCallSite;
import com.android.tools.r8.graph.DexClass;
@@ -77,20 +79,28 @@
.lookupMethod(originalMethod, context.getReference(), originalType, getCodeLens());
DexMethod method = result.getReference();
InvokeType type = result.getType();
- if (type == InvokeType.INTERFACE || type == InvokeType.VIRTUAL) {
+ MethodResolutionResult resolutionResult =
+ type.isInterface() || type.isVirtual()
+ ? appViewWithLiveness.appInfo().resolveMethodLegacy(method, type.isInterface())
+ : appViewWithLiveness.appInfo().unsafeResolveMethodDueToDexFormatLegacy(method);
+ if (!resolutionResult.isSingleResolution()) {
+ return;
+ }
+ if (type.isInterface() || type.isVirtual()) {
// For virtual and interface calls add all potential targets that could be called.
- MethodResolutionResult resolutionResult =
- appViewWithLiveness.appInfo().resolveMethodLegacy(method, type == InvokeType.INTERFACE);
- DexClassAndMethod target = resolutionResult.getResolutionPair();
- if (target != null) {
- processInvokeWithDynamicDispatch(type, target, context);
- }
+ processInvokeWithDynamicDispatch(type, resolutionResult.getResolutionPair(), context);
} else {
ProgramMethod singleTarget =
- appViewWithLiveness
- .appInfo()
- .lookupSingleProgramTarget(
- appViewWithLiveness, type, method, context, appViewWithLiveness);
+ asProgramMethodOrNull(
+ appViewWithLiveness
+ .appInfo()
+ .lookupSingleTarget(
+ appViewWithLiveness,
+ type,
+ method,
+ resolutionResult.asSingleResolution(),
+ context,
+ appViewWithLiveness));
if (singleTarget != null) {
processSingleTarget(singleTarget, context);
}
diff --git a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
index 3bdf1c0..56c1ea4 100644
--- a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
+++ b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
@@ -1289,18 +1289,24 @@
AppView<AppInfoWithLiveness> appView,
InvokeType type,
DexMethod target,
+ SingleResolutionResult<?> resolutionResult,
ProgramMethod context,
LibraryModeledPredicate modeledPredicate) {
assert checkIfObsolete();
- DexType holder = target.holder;
- if (!holder.isClassType()) {
+ if (!target.getHolderType().isClassType()) {
return null;
}
switch (type) {
- case VIRTUAL:
- return lookupSingleVirtualTarget(appView, target, context, false, modeledPredicate);
case INTERFACE:
- return lookupSingleVirtualTarget(appView, target, context, true, modeledPredicate);
+ case VIRTUAL:
+ return lookupSingleVirtualTarget(
+ appView,
+ target,
+ resolutionResult,
+ context,
+ type.isInterface(),
+ modeledPredicate,
+ DynamicType.unknown());
case DIRECT:
return lookupDirectTarget(target, context, appView);
case STATIC:
@@ -1312,42 +1318,34 @@
}
}
- public ProgramMethod lookupSingleProgramTarget(
- AppView<AppInfoWithLiveness> appView,
- InvokeType type,
- DexMethod target,
- ProgramMethod context,
- LibraryModeledPredicate modeledPredicate) {
- return DexClassAndMethod.asProgramMethodOrNull(
- lookupSingleTarget(appView, type, target, context, modeledPredicate));
- }
-
/** For mapping invoke virtual instruction to single target method. */
- public DexClassAndMethod lookupSingleVirtualTarget(
- AppView<AppInfoWithLiveness> appView,
- DexMethod method,
- ProgramMethod context,
- boolean isInterface) {
- assert checkIfObsolete();
- return lookupSingleVirtualTarget(appView, method, context, isInterface, type -> false);
- }
-
- /** For mapping invoke virtual instruction to single target method. */
- public DexClassAndMethod lookupSingleVirtualTarget(
+ public DexClassAndMethod lookupSingleVirtualTargetForTesting(
AppView<AppInfoWithLiveness> appView,
DexMethod method,
ProgramMethod context,
boolean isInterface,
- LibraryModeledPredicate modeledPredicate) {
+ LibraryModeledPredicate modeledPredicate,
+ DynamicType dynamicReceiverType) {
assert checkIfObsolete();
- return lookupSingleVirtualTarget(
- appView, method, context, isInterface, modeledPredicate, DynamicType.unknown());
+ SingleResolutionResult<?> resolutionResult =
+ appView.appInfo().resolveMethodLegacy(method, isInterface).asSingleResolution();
+ if (resolutionResult != null) {
+ return lookupSingleVirtualTarget(
+ appView,
+ method,
+ resolutionResult,
+ context,
+ isInterface,
+ modeledPredicate,
+ dynamicReceiverType);
+ }
+ return null;
}
- @SuppressWarnings("ReferenceEquality")
public DexClassAndMethod lookupSingleVirtualTarget(
AppView<AppInfoWithLiveness> appView,
DexMethod method,
+ SingleResolutionResult<?> resolutionResult,
ProgramMethod context,
boolean isInterface,
LibraryModeledPredicate modeledPredicate,
@@ -1382,16 +1380,15 @@
return null;
}
}
- SingleResolutionResult<?> resolution =
- resolveMethodOnLegacy(initialResolutionHolder, method).asSingleResolution();
- if (resolution == null
- || resolution.isAccessibleForVirtualDispatchFrom(context.getHolder(), appView).isFalse()) {
+ if (resolutionResult
+ .isAccessibleForVirtualDispatchFrom(context.getHolder(), appView)
+ .isFalse()) {
return null;
}
// If the method is modeled, return the resolution.
- DexClassAndMethod resolvedMethod = resolution.getResolutionPair();
- if (modeledPredicate.isModeled(resolution.getResolvedHolder().getType())) {
- if (resolution.getResolvedHolder().isFinal()
+ DexClassAndMethod resolvedMethod = resolutionResult.getResolutionPair();
+ if (modeledPredicate.isModeled(resolutionResult.getResolvedHolder().getType())) {
+ if (resolutionResult.getResolvedHolder().isFinal()
|| (resolvedMethod.getAccessFlags().isFinal()
&& resolvedMethod.getAccessFlags().isPublic())) {
singleTargetLookupCache.addToCache(refinedReceiverType, method, resolvedMethod);
@@ -1402,7 +1399,7 @@
getMethodTargetFromExactRuntimeInformation(
refinedReceiverType,
dynamicReceiverType.getDynamicLowerBoundType(),
- resolution,
+ resolutionResult,
refinedReceiverClass);
if (exactTarget != null) {
// We are not caching single targets here because the cache does not include the
@@ -1416,7 +1413,7 @@
singleTargetLookupCache.addNoSingleTargetToCache(refinedReceiverType, method);
return null;
}
- DexClass resolvedHolder = resolution.getResolvedHolder();
+ DexClass resolvedHolder = resolutionResult.getResolvedHolder();
// TODO(b/148769279): Disable lookup single target on lambda's for now.
if (resolvedHolder.isInterface()
&& resolvedHolder.isProgramClass()
@@ -1439,7 +1436,7 @@
}
}
LookupResultSuccess lookupResult =
- resolution
+ resolutionResult
.lookupVirtualDispatchTargets(
context.getHolder(),
appView,
diff --git a/src/main/java/com/android/tools/r8/shaking/LibraryModeledPredicate.java b/src/main/java/com/android/tools/r8/shaking/LibraryModeledPredicate.java
index 39ee67c..00be009 100644
--- a/src/main/java/com/android/tools/r8/shaking/LibraryModeledPredicate.java
+++ b/src/main/java/com/android/tools/r8/shaking/LibraryModeledPredicate.java
@@ -10,4 +10,8 @@
public interface LibraryModeledPredicate {
boolean isModeled(DexType type);
+
+ static LibraryModeledPredicate alwaysFalse() {
+ return type -> false;
+ }
}
diff --git a/src/test/java/com/android/tools/r8/resolution/SingleTargetLookupTest.java b/src/test/java/com/android/tools/r8/resolution/SingleTargetLookupTest.java
index d32849b..afe93c0 100644
--- a/src/test/java/com/android/tools/r8/resolution/SingleTargetLookupTest.java
+++ b/src/test/java/com/android/tools/r8/resolution/SingleTargetLookupTest.java
@@ -17,6 +17,7 @@
import com.android.tools.r8.graph.LookupResult;
import com.android.tools.r8.graph.MethodResolutionResult;
import com.android.tools.r8.graph.ProgramMethod;
+import com.android.tools.r8.ir.analysis.type.DynamicType;
import com.android.tools.r8.resolution.singletarget.Main;
import com.android.tools.r8.resolution.singletarget.one.AbstractSubClass;
import com.android.tools.r8.resolution.singletarget.one.AbstractTopClass;
@@ -34,6 +35,7 @@
import com.android.tools.r8.resolution.singletarget.two.OtherSubSubClassOne;
import com.android.tools.r8.resolution.singletarget.two.OtherSubSubClassTwo;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
+import com.android.tools.r8.shaking.LibraryModeledPredicate;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Sets;
import java.util.Collections;
@@ -196,7 +198,13 @@
appInfo.definitionForProgramType(reference.holder).getProgramDefaultInitializer();
Assert.assertNotNull(appInfo.resolveMethodOnClassHolderLegacy(reference).getSingleTarget());
DexClassAndMethod singleVirtualTarget =
- appInfo.lookupSingleVirtualTarget(appView, reference, context, false);
+ appInfo.lookupSingleVirtualTargetForTesting(
+ appView,
+ reference,
+ context,
+ false,
+ LibraryModeledPredicate.alwaysFalse(),
+ DynamicType.unknown());
if (singleTargetHolderOrNull == null) {
Assert.assertNull(singleVirtualTarget);
} else {
diff --git a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentInterfaceTest.java b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentInterfaceTest.java
index e15e664..3cbd047 100644
--- a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentInterfaceTest.java
+++ b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentInterfaceTest.java
@@ -17,7 +17,9 @@
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.MethodResolutionResult;
import com.android.tools.r8.graph.ProgramMethod;
+import com.android.tools.r8.ir.analysis.type.DynamicType;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
+import com.android.tools.r8.shaking.LibraryModeledPredicate;
import com.google.common.collect.ImmutableList;
import java.util.List;
import org.junit.Assert;
@@ -134,7 +136,13 @@
assertEquals(methodOnBReference, resolved.getReference());
assertFalse(resolutionResult.isVirtualTarget());
DexClassAndMethod singleVirtualTarget =
- appInfo.lookupSingleVirtualTarget(appView, methodOnBReference, methodOnB, false);
+ appInfo.lookupSingleVirtualTargetForTesting(
+ appView,
+ methodOnBReference,
+ methodOnB,
+ false,
+ LibraryModeledPredicate.alwaysFalse(),
+ DynamicType.unknown());
Assert.assertNull(singleVirtualTarget);
}
diff --git a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentTest.java b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentTest.java
index d166b97..15aa8e9 100644
--- a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentTest.java
+++ b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentTest.java
@@ -19,7 +19,9 @@
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.MethodResolutionResult;
+import com.android.tools.r8.ir.analysis.type.DynamicType;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
+import com.android.tools.r8.shaking.LibraryModeledPredicate;
import com.google.common.collect.ImmutableList;
import java.util.List;
import org.junit.Assert;
@@ -181,8 +183,13 @@
assertEquals(methodOnA, resolved.getReference());
assertFalse(resolutionResult.isVirtualTarget());
DexClassAndMethod singleVirtualTarget =
- appInfo.lookupSingleVirtualTarget(
- appView, methodOnB, bClass.getProgramDefaultInitializer(), false);
+ appInfo.lookupSingleVirtualTargetForTesting(
+ appView,
+ methodOnB,
+ bClass.getProgramDefaultInitializer(),
+ false,
+ LibraryModeledPredicate.alwaysFalse(),
+ DynamicType.unknown());
Assert.assertNull(singleVirtualTarget);
}
diff --git a/src/test/java/com/android/tools/r8/resolution/singletarget/InstantiatedLowerBoundTest.java b/src/test/java/com/android/tools/r8/resolution/singletarget/InstantiatedLowerBoundTest.java
index 7b9b80c..4c11c91 100644
--- a/src/test/java/com/android/tools/r8/resolution/singletarget/InstantiatedLowerBoundTest.java
+++ b/src/test/java/com/android/tools/r8/resolution/singletarget/InstantiatedLowerBoundTest.java
@@ -25,6 +25,7 @@
import com.android.tools.r8.ir.analysis.type.DynamicTypeWithLowerBound;
import com.android.tools.r8.ir.analysis.type.TypeElement;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
+import com.android.tools.r8.shaking.LibraryModeledPredicate;
import com.google.common.collect.Sets;
import java.util.Set;
import org.junit.Test;
@@ -65,12 +66,12 @@
TypeElement latticeA = typeA.toTypeElement(appView);
ClassTypeElement latticeB = typeB.toTypeElement(appView).asClassType();
DexClassAndMethod singleTarget =
- appInfo.lookupSingleVirtualTarget(
+ appInfo.lookupSingleVirtualTargetForTesting(
appView,
fooA,
mainMethod,
false,
- t -> false,
+ LibraryModeledPredicate.alwaysFalse(),
DynamicTypeWithLowerBound.create(appView, latticeA, latticeB));
assertNotNull(singleTarget);
DexMethod fooB = buildNullaryVoidMethod(B.class, "foo", appInfo.dexItemFactory());
@@ -98,12 +99,12 @@
TypeElement latticeA = typeA.toTypeElement(appView);
ClassTypeElement latticeB = typeB.toTypeElement(appView).asClassType();
DexClassAndMethod singleTarget =
- appInfo.lookupSingleVirtualTarget(
+ appInfo.lookupSingleVirtualTargetForTesting(
appView,
fooA,
mainMethod,
false,
- t -> false,
+ LibraryModeledPredicate.alwaysFalse(),
DynamicTypeWithLowerBound.create(appView, latticeA, latticeB));
assertNotNull(singleTarget);
DexMethod fooB = buildNullaryVoidMethod(B.class, "foo", appInfo.dexItemFactory());
@@ -154,12 +155,12 @@
TypeElement latticeA = typeA.toTypeElement(appView);
ClassTypeElement latticeC = typeC.toTypeElement(appView).asClassType();
DexClassAndMethod singleTarget =
- appInfo.lookupSingleVirtualTarget(
+ appInfo.lookupSingleVirtualTargetForTesting(
appView,
fooA,
mainMethod,
false,
- t -> false,
+ LibraryModeledPredicate.alwaysFalse(),
DynamicTypeWithLowerBound.create(appView, latticeA, latticeC));
assertNull(singleTarget);
}
diff --git a/src/test/java/com/android/tools/r8/resolution/singletarget/SuccessAndInvalidLookupTest.java b/src/test/java/com/android/tools/r8/resolution/singletarget/SuccessAndInvalidLookupTest.java
index ec3815d..0a94601 100644
--- a/src/test/java/com/android/tools/r8/resolution/singletarget/SuccessAndInvalidLookupTest.java
+++ b/src/test/java/com/android/tools/r8/resolution/singletarget/SuccessAndInvalidLookupTest.java
@@ -21,6 +21,7 @@
import com.android.tools.r8.ir.analysis.type.DynamicTypeWithUpperBound;
import com.android.tools.r8.optimize.interfaces.collection.NonEmptyOpenClosedInterfacesCollection;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
+import com.android.tools.r8.shaking.LibraryModeledPredicate;
import java.util.Collections;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -59,13 +60,13 @@
DynamicTypeWithUpperBound.create(appView, typeA.toTypeElement(appView));
DexMethod fooA = buildNullaryVoidMethod(A.class, "foo", appInfo.dexItemFactory());
DexClassAndMethod singleTarget =
- appInfo.lookupSingleVirtualTarget(
- appView, fooA, mainMethod, false, t -> false, dynamicTypeA);
+ appInfo.lookupSingleVirtualTargetForTesting(
+ appView, fooA, mainMethod, false, LibraryModeledPredicate.alwaysFalse(), dynamicTypeA);
assertNotNull(singleTarget);
assertEquals(fooA, singleTarget.getReference());
DexClassAndMethod invalidSingleTarget =
- appInfo.lookupSingleVirtualTarget(
- appView, fooA, mainMethod, true, t -> false, dynamicTypeA);
+ appInfo.lookupSingleVirtualTargetForTesting(
+ appView, fooA, mainMethod, true, LibraryModeledPredicate.alwaysFalse(), dynamicTypeA);
assertNull(invalidSingleTarget);
}
@@ -92,13 +93,13 @@
DexMethod fooI = buildNullaryVoidMethod(I.class, "foo", appInfo.dexItemFactory());
DexMethod fooA = buildNullaryVoidMethod(A.class, "foo", appInfo.dexItemFactory());
DexClassAndMethod singleTarget =
- appInfo.lookupSingleVirtualTarget(
- appView, fooI, mainMethod, true, t -> false, dynamicTypeA);
+ appInfo.lookupSingleVirtualTargetForTesting(
+ appView, fooI, mainMethod, true, LibraryModeledPredicate.alwaysFalse(), dynamicTypeA);
assertNotNull(singleTarget);
assertEquals(fooA, singleTarget.getReference());
DexClassAndMethod invalidSingleTarget =
- appInfo.lookupSingleVirtualTarget(
- appView, fooI, mainMethod, false, t -> false, dynamicTypeA);
+ appInfo.lookupSingleVirtualTargetForTesting(
+ appView, fooI, mainMethod, false, LibraryModeledPredicate.alwaysFalse(), dynamicTypeA);
assertNull(invalidSingleTarget);
}
