Relax the inlining constraints for virtual invokes.
We checked the access flags of the containing class of all the
potential targets. That is not necessary. We need to check the
access flags of the containing class of the resolution target.
For the actual targets we only have to check the access flags
on the target method itself, not of the containing class.
BUG: 73738623
Change-Id: I5b499abc3e3feaa03dd964b8b7ff8d2681eb8f66
diff --git a/src/main/java/com/android/tools/r8/ir/code/InvokeMethod.java b/src/main/java/com/android/tools/r8/ir/code/InvokeMethod.java
index eb58c4d..17ebfb5 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InvokeMethod.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InvokeMethod.java
@@ -5,6 +5,7 @@
import com.android.tools.r8.cf.LoadStoreHelper;
import com.android.tools.r8.cf.TypeVerificationHelper;
+import com.android.tools.r8.graph.AppInfo.ResolutionResult;
import com.android.tools.r8.graph.AppInfoWithSubtyping;
import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexEncodedMethod;
@@ -118,23 +119,49 @@
if (targets == null || targets.isEmpty()) {
return Constraint.NEVER;
}
+
Constraint result = Constraint.ALWAYS;
+
+ // Perform resolution and derive inlining constraints based on the accessibility of the
+ // resolution result.
+ ResolutionResult resolutionResult = info.resolveMethod(method.holder, method);
+ DexEncodedMethod resolutionTarget = resolutionResult.asResultOfResolve();
+ if (resolutionTarget == null) {
+ // This will fail at runtime.
+ return Constraint.NEVER;
+ }
+ DexType methodHolder = resolutionTarget.method.holder;
+ DexClass methodClass = info.definitionFor(methodHolder);
+ assert methodClass != null;
+ Constraint methodConstraint = Constraint
+ .deriveConstraint(invocationContext, methodHolder, resolutionTarget.accessFlags, info);
+ result = Constraint.min(result, methodConstraint);
+ // We also have to take the constraint of the enclosing class of the resolution result
+ // into account. We do not allow inlining this method if it is calling something that
+ // is inaccessible. Inlining in that case could move the code to another package making a
+ // call succeed that should not succeed. Conversely, if the resolution result is accessible,
+ // we have to make sure that inlining cannot make it inaccessible.
+ Constraint classConstraint = Constraint
+ .deriveConstraint(invocationContext, methodHolder, methodClass.accessFlags, info);
+ result = Constraint.min(result, classConstraint);
+ if (result == Constraint.NEVER) {
+ return result;
+ }
+
+ // For each of the actual potential targets, derive constraints based on the accessibility
+ // of the method itself.
for (DexEncodedMethod target : targets) {
- DexType methodHolder = target.method.holder;
- DexClass methodClass = info.definitionFor(methodHolder);
- if ((methodClass != null)) {
- Constraint methodConstraint = Constraint
- .deriveConstraint(invocationContext, methodHolder, target.accessFlags, info);
- result = Constraint.min(result, methodConstraint);
- // We also have to take the constraint of the enclosing class into account.
- Constraint classConstraint = Constraint
- .deriveConstraint(invocationContext, methodHolder, methodClass.accessFlags, info);
- result = Constraint.min(result, classConstraint);
- }
+ methodHolder = target.method.holder;
+ methodClass = info.definitionFor(methodHolder);
+ assert methodClass != null;
+ methodConstraint = Constraint
+ .deriveConstraint(invocationContext, methodHolder, target.accessFlags, info);
+ result = Constraint.min(result, methodConstraint);
if (result == Constraint.NEVER) {
- break;
+ return result;
}
}
+
return result;
}
diff --git a/src/test/examples/inlining/IFace.java b/src/test/examples/inlining/IFace.java
new file mode 100644
index 0000000..d0402f2
--- /dev/null
+++ b/src/test/examples/inlining/IFace.java
@@ -0,0 +1,8 @@
+// Copyright (c) 2018, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package inlining;
+
+public interface IFace {
+ int foo();
+}
diff --git a/src/test/examples/inlining/Inlining.java b/src/test/examples/inlining/Inlining.java
index 77b2979..0705587 100644
--- a/src/test/examples/inlining/Inlining.java
+++ b/src/test/examples/inlining/Inlining.java
@@ -4,6 +4,7 @@
package inlining;
import inlining.Nullability.Factor;
+import inlining.pkg.InterfaceImplementationContainer;
import inlining.pkg.OtherPublicClass;
import inlining.pkg.PublicClass;
import inlining.pkg.Subclass;
@@ -211,6 +212,8 @@
} catch (Throwable unexpected) {
System.out.println("Unexpected exception for notInlinableOnThrow");
}
+
+ System.out.println(callInterfaceMethod(InterfaceImplementationContainer.getIFace()));
}
private static boolean intCmpExpression(A a, A b) {
@@ -218,6 +221,11 @@
}
@CheckDiscarded
+ private static int callInterfaceMethod(IFace i) {
+ return i.foo();
+ }
+
+ @CheckDiscarded
private static int intConstantInline() {
return 42;
}
diff --git a/src/test/examples/inlining/pkg/InterfaceImplementationContainer.java b/src/test/examples/inlining/pkg/InterfaceImplementationContainer.java
new file mode 100644
index 0000000..5135fa2
--- /dev/null
+++ b/src/test/examples/inlining/pkg/InterfaceImplementationContainer.java
@@ -0,0 +1,19 @@
+// Copyright (c) 2018, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package inlining.pkg;
+
+import inlining.IFace;
+
+public class InterfaceImplementationContainer {
+ private static class IFaceImplementation implements IFace {
+ @Override
+ public int foo() {
+ return 42;
+ }
+ }
+
+ public static IFace getIFace() {
+ return new IFaceImplementation();
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/R8InliningTest.java b/src/test/java/com/android/tools/r8/ir/optimize/R8InliningTest.java
index 9c03ee7..1c72986 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/R8InliningTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/R8InliningTest.java
@@ -39,6 +39,7 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
+import java.util.List;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
@@ -111,9 +112,13 @@
assertEquals(javaResult.stdout, artOutput);
}
- private void checkAbsent(ClassSubject clazz, String name) {
+ private void checkAbsentBooleanMethod(ClassSubject clazz, String name) {
+ checkAbsent(clazz, "boolean", name, Collections.emptyList());
+ }
+
+ private void checkAbsent(ClassSubject clazz, String returnType, String name, List<String> args) {
assertTrue(clazz.isPresent());
- MethodSubject method = clazz.method("boolean", name, Collections.emptyList());
+ MethodSubject method = clazz.method(returnType, name, args);
assertFalse(method.isPresent());
}
@@ -134,20 +139,24 @@
DexInspector inspector = new DexInspector(getGeneratedDexFile().toAbsolutePath(),
getGeneratedProguardMap());
ClassSubject clazz = inspector.clazz("inlining.Inlining");
+
// Simple constant inlining.
- checkAbsent(clazz, "longExpression");
- checkAbsent(clazz, "intExpression");
- checkAbsent(clazz, "doubleExpression");
- checkAbsent(clazz, "floatExpression");
+ checkAbsentBooleanMethod(clazz, "longExpression");
+ checkAbsentBooleanMethod(clazz, "intExpression");
+ checkAbsentBooleanMethod(clazz, "doubleExpression");
+ checkAbsentBooleanMethod(clazz, "floatExpression");
// Simple return argument inlining.
- checkAbsent(clazz, "longArgumentExpression");
- checkAbsent(clazz, "intArgumentExpression");
- checkAbsent(clazz, "doubleArgumentExpression");
- checkAbsent(clazz, "floatArgumentExpression");
+ checkAbsentBooleanMethod(clazz, "longArgumentExpression");
+ checkAbsentBooleanMethod(clazz, "intArgumentExpression");
+ checkAbsentBooleanMethod(clazz, "doubleArgumentExpression");
+ checkAbsentBooleanMethod(clazz, "floatArgumentExpression");
+ // Static method calling interface method. The interface method implementation is in
+ // a private class in another package.
+ checkAbsent(clazz, "int", "callInterfaceMethod", ImmutableList.of("inlining.IFace"));
clazz = inspector.clazz("inlining.Nullability");
- checkAbsent(clazz, "inlinableWithPublicField");
- checkAbsent(clazz, "inlinableWithControlFlow");
+ checkAbsentBooleanMethod(clazz, "inlinableWithPublicField");
+ checkAbsentBooleanMethod(clazz, "inlinableWithControlFlow");
}
@Test
