Take first library boundary for devirtualizing super invoke
Bug: 209060415
Change-Id: Ibb6939e51d91cba50f69fa3ed03b89300f27a788
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Devirtualizer.java b/src/main/java/com/android/tools/r8/ir/optimize/Devirtualizer.java
index 7dae8cd..72f4852 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/Devirtualizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/Devirtualizer.java
@@ -125,8 +125,8 @@
if (current.isInvokeSuper()) {
InvokeSuper invoke = current.asInvokeSuper();
- // Check if the instruction can be rewritten to invoke-super. This allows inlining of the
- // enclosing method into contexts outside the current class.
+ // Check if the instruction can be rewritten to invoke-virtual. This allows inlining of
+ // the enclosing method into contexts outside the current class.
if (options.testing.enableInvokeSuperToInvokeVirtualRewriting) {
DexClassAndMethod singleTarget = invoke.lookupSingleTarget(appView, context);
if (singleTarget != null) {
@@ -149,16 +149,19 @@
// Rebind the invoke to the most specific target.
DexMethod invokedMethod = invoke.getInvokedMethod();
- DexClassAndMethod reboundTarget = rebindSuperInvokeToMostSpecific(invokedMethod, context);
- if (reboundTarget != null
- && reboundTarget.getReference() != invokedMethod
- && !isRebindingNewClassIntoMainDex(context, reboundTarget.getReference())) {
- it.replaceCurrentInstruction(
- new InvokeSuper(
- reboundTarget.getReference(),
- invoke.outValue(),
- invoke.arguments(),
- reboundTarget.getHolder().isInterface()));
+ DexClass reboundTargetClass = rebindSuperInvokeToMostSpecific(invokedMethod, context);
+ if (reboundTargetClass != null) {
+ DexMethod reboundMethod =
+ invokedMethod.withHolder(reboundTargetClass, appView.dexItemFactory());
+ if (reboundMethod != invokedMethod
+ && !isRebindingNewClassIntoMainDex(context, reboundMethod)) {
+ it.replaceCurrentInstruction(
+ new InvokeSuper(
+ reboundMethod,
+ invoke.outValue(),
+ invoke.arguments(),
+ reboundTargetClass.isInterface()));
+ }
}
continue;
}
@@ -316,8 +319,7 @@
}
/** This rebinds invoke-super instructions to their most specific target. */
- private DexClassAndMethod rebindSuperInvokeToMostSpecific(
- DexMethod target, ProgramMethod context) {
+ private DexClass rebindSuperInvokeToMostSpecific(DexMethod target, ProgramMethod context) {
DexClassAndMethod method = appView.appInfo().lookupSuperTarget(target, context);
if (method == null) {
return null;
@@ -334,7 +336,20 @@
return null;
}
- return method;
+ if (method.getHolder().isLibraryClass()) {
+ // We've found a library class as the new holder of the method. Since the library can only
+ // rebind to the library class boundary. Search from the target upwards until we find a
+ // library class.
+ DexClass lowerBound = appView.definitionFor(target.getHolderType(), context);
+ while (lowerBound != null
+ && lowerBound.isProgramClass()
+ && lowerBound != method.getHolder()) {
+ lowerBound = appView.definitionFor(lowerBound.superType, lowerBound.asProgramClass());
+ }
+ return lowerBound;
+ }
+
+ return method.getHolder();
}
/**
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/devirtualize/DevirtualizeLibrarySuperTest.java b/src/test/java/com/android/tools/r8/ir/optimize/devirtualize/DevirtualizeLibrarySuperTest.java
index f68ba28..e82bd6e 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/devirtualize/DevirtualizeLibrarySuperTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/devirtualize/DevirtualizeLibrarySuperTest.java
@@ -6,6 +6,7 @@
import static com.android.tools.r8.utils.codeinspector.CodeMatchers.invokesMethodWithHolderAndName;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
import com.android.tools.r8.NeverInline;
@@ -45,15 +46,11 @@
.compile()
.inspect(
inspector -> {
- if (parameters.isCfRuntime()) {
- return;
- }
MethodSubject fooMethod = inspector.clazz(Main.class).uniqueMethodWithName("foo");
assertThat(fooMethod, isPresent());
- // TODO(b/209060415): Should not have a static holder of `LibraryOverride`.
assertThat(
fooMethod,
- invokesMethodWithHolderAndName(LibraryOverride.class.getTypeName(), "foo"));
+ not(invokesMethodWithHolderAndName(LibraryOverride.class.getTypeName(), "foo")));
})
.applyIf(
hasNewLibraryHierarchyOnClassPath,
@@ -70,15 +67,7 @@
.transform()))
.run(parameters.getRuntime(), Main.class)
.assertSuccessWithOutputLinesIf(hasNewLibraryHierarchyOnClassPath, "LibraryOverride::foo")
- // TODO(b/209060415): Should not fail.
- .applyIf(
- !hasNewLibraryHierarchyOnClassPath,
- result -> {
- result.assertFailureWithErrorThatThrows(
- (parameters.getDexRuntimeVersion().isDalvik()
- ? NoClassDefFoundError.class
- : VerifyError.class));
- });
+ .assertSuccessWithOutputLinesIf(!hasNewLibraryHierarchyOnClassPath, "Library::foo");
}
public static class Library {