Lookup invocation type using lense in inlining constraint visitor
Change-Id: Ifcd2698ebb5cf7b3115377430d764f794dcabcb1
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java b/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java
index 1e9566a..073d6ae 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.ir.optimize;
+import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AppInfo.ResolutionResult;
import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexEncodedField;
@@ -12,6 +13,7 @@
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.GraphLense;
+import com.android.tools.r8.ir.code.Invoke.Type;
import com.android.tools.r8.ir.optimize.Inliner.Constraint;
import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
import java.util.Collection;
@@ -101,16 +103,37 @@
return Constraint.classIsVisible(invocationContext, type, appInfo);
}
- public Constraint forInvokeCustom() {
- return Constraint.NEVER;
- }
-
public Constraint forInstancePut(DexField field, DexType invocationContext) {
DexField lookup = graphLense.lookupField(field);
return forFieldInstruction(
lookup, appInfo.lookupInstanceTarget(lookup.clazz, lookup), invocationContext);
}
+ public Constraint forInvoke(DexMethod method, Type type, DexType invocationContext) {
+ switch (type) {
+ case DIRECT:
+ return forInvokeDirect(method, invocationContext);
+ case INTERFACE:
+ return forInvokeInterface(method, invocationContext);
+ case STATIC:
+ return forInvokeStatic(method, invocationContext);
+ case SUPER:
+ return forInvokeSuper(method, invocationContext);
+ case VIRTUAL:
+ return forInvokeVirtual(method, invocationContext);
+ case CUSTOM:
+ return forInvokeCustom();
+ case POLYMORPHIC:
+ return forInvokePolymorphic(method, invocationContext);
+ default:
+ throw new Unreachable("Unexpected type: " + type);
+ }
+ }
+
+ public Constraint forInvokeCustom() {
+ return Constraint.NEVER;
+ }
+
public Constraint forInvokeDirect(DexMethod method, DexType invocationContext) {
DexMethod lookup = graphLense.lookupMethod(method);
return forSingleTargetInvoke(lookup, appInfo.lookupDirectTarget(lookup), invocationContext);
diff --git a/src/main/java/com/android/tools/r8/jar/InliningConstraintVisitor.java b/src/main/java/com/android/tools/r8/jar/InliningConstraintVisitor.java
index 4807b89..459d7de 100644
--- a/src/main/java/com/android/tools/r8/jar/InliningConstraintVisitor.java
+++ b/src/main/java/com/android/tools/r8/jar/InliningConstraintVisitor.java
@@ -15,6 +15,7 @@
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.GraphLense;
import com.android.tools.r8.graph.JarApplicationReader;
+import com.android.tools.r8.ir.code.Invoke;
import com.android.tools.r8.ir.conversion.JarSourceCode;
import com.android.tools.r8.ir.optimize.Inliner.Constraint;
import com.android.tools.r8.ir.optimize.InliningConstraints;
@@ -34,6 +35,7 @@
private final JarApplicationReader application;
private final AppInfoWithLiveness appInfo;
+ private final GraphLense graphLense;
private final InliningConstraints inliningConstraints;
private final DexEncodedMethod method;
private final DexType invocationContext;
@@ -47,8 +49,10 @@
DexEncodedMethod method,
DexType invocationContext) {
super(ASM6);
+ assert graphLense.isContextFreeForMethods();
this.application = application;
this.appInfo = appInfo;
+ this.graphLense = graphLense;
this.inliningConstraints = new InliningConstraints(appInfo, graphLense);
this.method = method;
this.invocationContext = invocationContext;
@@ -116,48 +120,73 @@
public void visitMethodInsn(int opcode, String owner, String name, String desc, boolean itf) {
DexType ownerType = application.getTypeFromName(owner);
DexMethod target = application.getMethod(ownerType, name, desc);
+
+ // Find the DEX invocation type.
+ Invoke.Type type;
switch (opcode) {
case Opcodes.INVOKEDYNAMIC:
- if (JarSourceCode.isCallToPolymorphicSignatureMethod(owner, name)) {
- updateConstraint(inliningConstraints.forInvokePolymorphic(target, invocationContext));
- } else {
- updateConstraint(inliningConstraints.forInvokeCustom());
- }
+ type =
+ JarSourceCode.isCallToPolymorphicSignatureMethod(owner, name)
+ ? Invoke.Type.POLYMORPHIC
+ : Invoke.Type.CUSTOM;
+ assert noNeedToUseGraphLense(target, type);
break;
case Opcodes.INVOKEINTERFACE:
- updateConstraint(inliningConstraints.forInvokeInterface(target, invocationContext));
+ // Could have changed to an invoke-virtual instruction due to vertical class merging
+ // (if an interface is merged into a class).
+ type = graphLense.lookupMethod(target, null, Invoke.Type.INTERFACE).getType();
+ assert type == Invoke.Type.INTERFACE || type == Invoke.Type.VIRTUAL;
break;
case Opcodes.INVOKESPECIAL:
- if (name.equals(Constants.INSTANCE_INITIALIZER_NAME) || ownerType == invocationContext) {
- updateConstraint(inliningConstraints.forInvokeDirect(target, invocationContext));
+ if (name.equals(Constants.INSTANCE_INITIALIZER_NAME)) {
+ type = Invoke.Type.DIRECT;
+ assert noNeedToUseGraphLense(target, type);
+ } else if (ownerType == invocationContext) {
+ // The method could have been publicized.
+ type = graphLense.lookupMethod(target, null, Invoke.Type.DIRECT).getType();
+ assert type == Invoke.Type.DIRECT || type == Invoke.Type.VIRTUAL;
} else {
- updateConstraint(inliningConstraints.forInvokeSuper(target, invocationContext));
+ // This is a super call. Note that the vertical class merger translates some invoke-super
+ // instructions to invoke-direct. However, when that happens, the invoke instruction and
+ // the target method end up being in the same class, and therefore, we will allow inlining
+ // it. The result of using type=SUPER below will be the same, since it leads to the
+ // inlining constraint SAMECLASS.
+ // TODO(christofferqa): Consider using graphLense.lookupMethod (to do this, we need the
+ // context for the graph lense, though).
+ type = Invoke.Type.SUPER;
+ assert noNeedToUseGraphLense(target, type);
}
break;
case Opcodes.INVOKESTATIC:
- updateConstraint(inliningConstraints.forInvokeStatic(target, invocationContext));
+ type = Invoke.Type.STATIC;
+ assert noNeedToUseGraphLense(target, type);
break;
case Opcodes.INVOKEVIRTUAL:
- // Instructions that target a private method in the same class are translated to
- // invoke-direct.
+ type = Invoke.Type.VIRTUAL;
+ // Instructions that target a private method in the same class translates to invoke-direct.
if (target.holder == method.method.holder) {
DexClass clazz = appInfo.definitionFor(target.holder);
if (clazz != null && clazz.lookupDirectMethod(target) != null) {
- updateConstraint(inliningConstraints.forInvokeDirect(target, invocationContext));
- break;
+ type = Invoke.Type.DIRECT;
}
}
-
- updateConstraint(inliningConstraints.forInvokeVirtual(target, invocationContext));
+ assert noNeedToUseGraphLense(target, type);
break;
default:
throw new Unreachable("Unexpected opcode " + opcode);
}
+
+ updateConstraint(inliningConstraints.forInvoke(target, type, invocationContext));
+ }
+
+ private boolean noNeedToUseGraphLense(DexMethod method, Invoke.Type type) {
+ assert graphLense.lookupMethod(method, null, type).getType() == type;
+ return true;
}
@Override
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 12a2257..0840d91 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -1406,14 +1406,32 @@
@Override
public DexType lookupType(DexType type) {
- return type == source ? target : type;
+ return type == source ? target : mergedClasses.getOrDefault(type, type);
}
@Override
public GraphLenseLookupResult lookupMethod(
DexMethod method, DexEncodedMethod context, Type type) {
- return new GraphLenseLookupResult(
- renamedMembersLense.methodMap.getOrDefault(method, method), type);
+ // First look up the method using the existing graph lense (for example, the type will have
+ // changed if the method was publicized by ClassAndMemberPublicizer).
+ GraphLenseLookupResult lookup = graphLense.lookupMethod(method, context, type);
+ DexMethod previousMethod = lookup.getMethod();
+ Type previousType = lookup.getType();
+ // Then check if there is a renaming due to the vertical class merger.
+ DexMethod newMethod = renamedMembersLense.methodMap.get(previousMethod);
+ if (newMethod != null) {
+ if (previousType == Type.INTERFACE) {
+ // If an interface has been merged into a class, invoke-interface needs to be translated
+ // to invoke-virtual.
+ DexClass clazz = appInfo.definitionFor(newMethod.holder);
+ if (clazz != null && !clazz.accessFlags.isInterface()) {
+ assert appInfo.definitionFor(method.holder).accessFlags.isInterface();
+ return new GraphLenseLookupResult(newMethod, Type.VIRTUAL);
+ }
+ }
+ return new GraphLenseLookupResult(newMethod, previousType);
+ }
+ return new GraphLenseLookupResult(previousMethod, previousType);
}
@Override
