Share logic for analyzing invokes in class inliner
With the new class inliner constraint analysis, there is no longer a need to distinguish methods calls where the instance that is subject to class inlining is being used as the receiver of a virtual invoke, or a non-receiver argument.
This CL therefore unifies the handling of virtual method calls and "extra method calls" in the class inliner.
Additionally, this CL changes the uses of Reason.SIMPLE in the class inliner to Reason.ALWAYS. The motivation for this change is that there is an explicit cost analysis in the ClassInliner, which is responsible for checking if the cost of the inlining does not exceed a given threshold. Therefore, we should not restrict each of the individual called methods to satisfy the 'SIMPLE' predicate.
Bug: 153773246
Change-Id: Ifbe9a918665c32498d0f719e790d989a31a90179
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
index 37b3796..4857999 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
@@ -210,8 +210,7 @@
// Inline the class instance.
Set<Value> affectedValues = Sets.newIdentityHashSet();
try {
- anyInlinedMethods |=
- processor.processInlining(code, affectedValues, defaultOracle, inliningIRProvider);
+ anyInlinedMethods |= processor.processInlining(code, affectedValues, inliningIRProvider);
} catch (IllegalClassInlinerStateException e) {
// We introduced a user that we cannot handle in the class inliner as a result of force
// inlining. Abort gracefully from class inlining without removing the instance.
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
index 7b97eb6..854f6e9 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
@@ -64,7 +64,6 @@
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.OptionalBool;
import com.android.tools.r8.utils.SetUtils;
-import com.android.tools.r8.utils.StringUtils;
import com.android.tools.r8.utils.Timing;
import com.android.tools.r8.utils.WorkList;
import com.android.tools.r8.utils.collections.ProgramMethodSet;
@@ -96,12 +95,9 @@
private DexProgramClass eligibleClass;
private ObjectState objectState;
- private final Map<InvokeMethodWithReceiver, InliningInfo> methodCallsOnInstance =
- new IdentityHashMap<>();
+ private final Map<InvokeMethod, InliningInfo> directMethodCalls = new IdentityHashMap<>();
private final ProgramMethodSet indirectMethodCallsOnInstance = ProgramMethodSet.create();
- private final Map<InvokeMethod, InliningInfo> extraMethodCalls
- = new IdentityHashMap<>();
private final Map<InvokeMethod, ProgramMethod> directInlinees = new IdentityHashMap<>();
private final List<ProgramMethod> indirectInlinees = new ArrayList<>();
@@ -292,11 +288,11 @@
}
if (user.isInvokeMethod()) {
- InvokeMethod invokeMethod = user.asInvokeMethod();
+ InvokeMethod invoke = user.asInvokeMethod();
SingleResolutionResult resolutionResult =
appView
.appInfo()
- .resolveMethod(invokeMethod.getInvokedMethod(), invokeMethod.getInterfaceBit())
+ .resolveMethod(invoke.getInvokedMethod(), invoke.getInterfaceBit())
.asSingleResolution();
if (resolutionResult == null
|| resolutionResult.isAccessibleFrom(method, appView).isPossiblyFalse()) {
@@ -304,13 +300,13 @@
}
// TODO(b/156853206): Avoid duplicating resolution.
- DexClassAndMethod singleTarget = invokeMethod.lookupSingleTarget(appView, method);
+ DexClassAndMethod singleTarget = invoke.lookupSingleTarget(appView, method);
if (singleTarget == null) {
return user; // Not eligible.
}
if (singleTarget.isLibraryMethod()
- && isEligibleLibraryMethodCall(invokeMethod, singleTarget.asLibraryMethod())) {
+ && isEligibleLibraryMethodCall(invoke, singleTarget.asLibraryMethod())) {
continue;
}
@@ -326,39 +322,24 @@
// Eligible constructor call (for new instance roots only).
if (user.isInvokeConstructor(dexItemFactory)) {
- InvokeDirect invoke = user.asInvokeDirect();
+ InvokeDirect invokeDirect = user.asInvokeDirect();
boolean isCorrespondingConstructorCall =
- root.isNewInstance()
- && !invoke.inValues().isEmpty()
- && root.outValue() == invoke.getReceiver();
+ root.isNewInstance() && root.outValue() == invokeDirect.getReceiver();
if (isCorrespondingConstructorCall) {
- InliningInfo inliningInfo = isEligibleConstructorCall(invoke, singleProgramTarget);
+ InliningInfo inliningInfo =
+ isEligibleConstructorCall(invokeDirect, singleProgramTarget);
if (inliningInfo != null) {
- methodCallsOnInstance.put(invoke, inliningInfo);
+ directMethodCalls.put(invoke, inliningInfo);
continue;
}
}
return user; // Not eligible.
}
- // Eligible virtual method call on the instance as a receiver.
- if (user.isInvokeVirtual() || user.isInvokeInterface()) {
- InvokeMethodWithReceiver invoke = user.asInvokeMethodWithReceiver();
- InliningInfo inliningInfo =
- isEligibleDirectVirtualMethodCall(
- invoke, resolutionResult, singleProgramTarget, indirectUsers, defaultOracle);
- if (inliningInfo != null) {
- methodCallsOnInstance.put(invoke, inliningInfo);
- continue;
- }
- }
-
- // Eligible usage as an invocation argument.
- if (isExtraMethodCall(invokeMethod)) {
- if (isExtraMethodCallEligible(
- invokeMethod, resolutionResult, singleProgramTarget, defaultOracle)) {
- continue;
- }
+ // Eligible non-constructor method call.
+ if (isEligibleDirectMethodCall(
+ invoke, resolutionResult, singleProgramTarget, defaultOracle, indirectUsers)) {
+ continue;
}
return user; // Not eligible.
@@ -395,27 +376,12 @@
boolean processInlining(
IRCode code,
Set<Value> affectedValues,
- Supplier<InliningOracle> defaultOracle,
InliningIRProvider inliningIRProvider)
throws IllegalClassInlinerStateException {
// Verify that `eligibleInstance` is not aliased.
assert eligibleInstance == eligibleInstance.getAliasedValue();
- boolean anyInlinedMethods = forceInlineExtraMethodInvocations(code, inliningIRProvider);
- if (anyInlinedMethods) {
- // Reset the collections.
- clear();
-
- // Repeat user analysis
- InstructionOrPhi ineligibleUser = areInstanceUsersEligible(defaultOracle);
- if (ineligibleUser != null) {
- throw new IllegalClassInlinerStateException();
- }
- assert extraMethodCalls.isEmpty()
- : "Remaining extra method calls: " + StringUtils.join(extraMethodCalls.entrySet(), ", ");
- }
-
- anyInlinedMethods |= forceInlineDirectMethodInvocations(code, inliningIRProvider);
+ boolean anyInlinedMethods = forceInlineDirectMethodInvocations(code, inliningIRProvider);
anyInlinedMethods |= forceInlineIndirectMethodInvocations(code, inliningIRProvider);
rebindIndirectEligibleInstanceUsersFromPhis();
@@ -426,43 +392,21 @@
return anyInlinedMethods;
}
- private void clear() {
- methodCallsOnInstance.clear();
- indirectMethodCallsOnInstance.clear();
- extraMethodCalls.clear();
- receivers.reset();
- }
-
- private boolean forceInlineExtraMethodInvocations(
- IRCode code, InliningIRProvider inliningIRProvider) {
- if (extraMethodCalls.isEmpty()) {
- return false;
- }
- // Inline extra methods.
- inliner.performForcedInlining(
- method, code, extraMethodCalls, inliningIRProvider, Timing.empty());
- return true;
- }
-
private boolean forceInlineDirectMethodInvocations(
IRCode code, InliningIRProvider inliningIRProvider) throws IllegalClassInlinerStateException {
- if (methodCallsOnInstance.isEmpty()) {
+ if (directMethodCalls.isEmpty()) {
return false;
}
- assert methodCallsOnInstance.keySet().stream()
- .map(InvokeMethodWithReceiver::getReceiver)
- .allMatch(receivers::isReceiverAlias);
-
inliner.performForcedInlining(
- method, code, methodCallsOnInstance, inliningIRProvider, Timing.empty());
+ method, code, directMethodCalls, inliningIRProvider, Timing.empty());
// In case we are class inlining an object allocation that does not inherit directly from
// java.lang.Object, we need keep force inlining the constructor until we reach
// java.lang.Object.<init>().
if (root.isNewInstance()) {
do {
- methodCallsOnInstance.clear();
+ directMethodCalls.clear();
for (Instruction instruction : eligibleInstance.uniqueUsers()) {
if (instruction.isInvokeDirect()) {
InvokeDirect invoke = instruction.asInvokeDirect();
@@ -492,21 +436,21 @@
.getDefinition()
.isInliningCandidate(
method,
- Reason.SIMPLE,
+ Reason.ALWAYS,
appView.appInfo(),
NopWhyAreYouNotInliningReporter.getInstance())) {
throw new IllegalClassInlinerStateException();
}
- methodCallsOnInstance.put(invoke, new InliningInfo(singleTarget, eligibleClass.type));
+ directMethodCalls.put(invoke, new InliningInfo(singleTarget, eligibleClass.type));
break;
}
}
- if (!methodCallsOnInstance.isEmpty()) {
+ if (!directMethodCalls.isEmpty()) {
inliner.performForcedInlining(
- method, code, methodCallsOnInstance, inliningIRProvider, Timing.empty());
+ method, code, directMethodCalls, inliningIRProvider, Timing.empty());
}
- } while (!methodCallsOnInstance.isEmpty());
+ } while (!directMethodCalls.isEmpty());
}
return true;
@@ -945,7 +889,7 @@
DexEncodedMethod encodedParentMethod = encodedParent.getDefinition();
if (!encodedParentMethod.isInliningCandidate(
method,
- Reason.SIMPLE,
+ Reason.ALWAYS,
appView.appInfo(),
NopWhyAreYouNotInliningReporter.getInstance())) {
return null;
@@ -967,11 +911,29 @@
// - if it is a regular chaining pattern where the only users of the out value are receivers to
// other invocations. In that case, we should add all indirect users of the out value to ensure
// they can also be inlined.
- private boolean isEligibleInvokeWithAllUsersAsReceivers(
- InvokeMethodWithReceiver invoke,
- OptionalBool returnsReceiver,
+ private boolean scheduleNewUsersForAnalysis(
+ InvokeMethod invoke,
+ ProgramMethod singleTarget,
+ int parameter,
Set<Instruction> indirectUsers) {
- if (returnsReceiver.isFalse()) {
+ ClassInlinerMethodConstraint classInlinerMethodConstraint =
+ singleTarget.getDefinition().getOptimizationInfo().getClassInlinerMethodConstraint();
+ ParameterUsage usage = classInlinerMethodConstraint.getParameterUsage(parameter);
+
+ OptionalBool returnsParameter;
+ if (usage.isParameterReturned()) {
+ if (singleTarget.getDefinition().getOptimizationInfo().returnsArgument()) {
+ assert singleTarget.getDefinition().getOptimizationInfo().getReturnedArgument()
+ == parameter;
+ returnsParameter = OptionalBool.TRUE;
+ } else {
+ returnsParameter = OptionalBool.UNKNOWN;
+ }
+ } else {
+ returnsParameter = OptionalBool.FALSE;
+ }
+
+ if (returnsParameter.isFalse()) {
return true;
}
@@ -989,149 +951,22 @@
// We cannot guarantee the invoke returns the receiver or another instance and since the
// return value is used we have to bail out.
- if (returnsReceiver.isUnknown()) {
+ if (returnsParameter.isUnknown()) {
return false;
}
// Add the out-value as a definite-alias if the invoke instruction is guaranteed to return the
// receiver. Otherwise, the out-value may be an alias of the receiver, and it is added to the
// may-alias set.
- assert returnsReceiver.isTrue();
+ assert returnsParameter.isTrue();
if (!receivers.addReceiverAlias(outValue)) {
return false;
}
- Set<Instruction> currentUsers = outValue.uniqueUsers();
- while (!currentUsers.isEmpty()) {
- Set<Instruction> indirectOutValueUsers = Sets.newIdentityHashSet();
- for (Instruction instruction : currentUsers) {
- if (instruction.isAssume() || instruction.isCheckCast()) {
- if (instruction.isCheckCast()) {
- CheckCast checkCast = instruction.asCheckCast();
- if (!appView.appInfo().isSubtype(eligibleClass.type, checkCast.getType())) {
- return false; // Unsafe cast.
- }
- }
- Value outValueAlias = instruction.outValue();
- if (outValueAlias.hasPhiUsers() || outValueAlias.hasDebugUsers()) {
- return false;
- }
- if (!receivers.addReceiverAlias(outValueAlias)) {
- return false;
- }
- indirectOutValueUsers.addAll(outValueAlias.uniqueUsers());
- continue;
- }
-
- if (instruction.isInvokeMethodWithReceiver()) {
- InvokeMethodWithReceiver user = instruction.asInvokeMethodWithReceiver();
- if (user.getReceiver().getAliasedValue(aliasesThroughAssumeAndCheckCasts) != outValue) {
- return false;
- }
- for (int i = 1; i < user.inValues().size(); i++) {
- Value inValue = user.inValues().get(i);
- if (inValue.getAliasedValue(aliasesThroughAssumeAndCheckCasts) == outValue) {
- return false;
- }
- }
- indirectUsers.add(user);
- continue;
- }
-
- return false;
- }
- currentUsers = indirectOutValueUsers;
- }
-
+ indirectUsers.addAll(outValue.uniqueUsers());
return true;
}
- private InliningInfo isEligibleDirectVirtualMethodCall(
- InvokeMethodWithReceiver invoke,
- SingleResolutionResult resolutionResult,
- ProgramMethod singleTarget,
- Set<Instruction> indirectUsers,
- Supplier<InliningOracle> defaultOracle) {
- // None of the none-receiver arguments may be an alias of the receiver.
- for (Value nonReceiverArgument : invoke.getNonReceiverArguments()) {
- if (receivers.isReceiverAlias(nonReceiverArgument)) {
- return null;
- }
- }
-
- // TODO(b/141719453): Should not constrain library overrides if all instantiations are inlined.
- if (singleTarget.getDefinition().isLibraryMethodOverride().isTrue()) {
- InliningOracle inliningOracle = defaultOracle.get();
- if (!inliningOracle.passesInliningConstraints(
- invoke,
- resolutionResult,
- singleTarget,
- Reason.SIMPLE,
- NopWhyAreYouNotInliningReporter.getInstance())) {
- return null;
- }
- }
-
- if (!isEligibleVirtualMethodCall(invoke.getInvokedMethod(), singleTarget)) {
- return null;
- }
-
- ParameterUsage usage =
- singleTarget
- .getDefinition()
- .getOptimizationInfo()
- .getClassInlinerMethodConstraint()
- .getParameterUsage(0);
- assert !usage.isTop();
-
- // If the method returns receiver and the return value is actually used in the code we need to
- // make some additional checks.
- OptionalBool isReceiverReturned;
- if (usage.isParameterReturned()) {
- if (singleTarget.getDefinition().getOptimizationInfo().returnsArgument()) {
- assert singleTarget.getDefinition().getOptimizationInfo().getReturnedArgument() == 0;
- isReceiverReturned = OptionalBool.TRUE;
- } else {
- isReceiverReturned = OptionalBool.UNKNOWN;
- }
- } else {
- isReceiverReturned = OptionalBool.FALSE;
- }
-
- if (!isEligibleInvokeWithAllUsersAsReceivers(invoke, isReceiverReturned, indirectUsers)) {
- return null;
- }
-
- if (!usage.isBottom()) {
- NonEmptyParameterUsage nonEmptyUsage = usage.asNonEmpty();
- if (nonEmptyUsage.getMethodCallsWithParameterAsReceiver().size() > 1) {
- return null;
- }
- if (!nonEmptyUsage.getMethodCallsWithParameterAsReceiver().isEmpty()) {
- DexMethod indirectlyInvokedMethod =
- nonEmptyUsage.getMethodCallsWithParameterAsReceiver().iterator().next();
- SingleResolutionResult indirectResolutionResult =
- appView
- .appInfo()
- .resolveMethodOn(eligibleClass, indirectlyInvokedMethod)
- .asSingleResolution();
- if (indirectResolutionResult == null) {
- return null;
- }
- ProgramMethod indirectSingleTarget =
- indirectResolutionResult.getResolutionPair().asProgramMethod();
- if (!isEligibleIndirectVirtualMethodCall(indirectlyInvokedMethod, indirectSingleTarget)) {
- return null;
- }
- markSizeOfIndirectTargetForInlining(indirectSingleTarget);
- indirectMethodCallsOnInstance.add(indirectSingleTarget);
- }
- }
-
- invoke.getNonReceiverArguments().forEach(receivers::addIllegalReceiverAlias);
- markSizeOfDirectTargetForInlining(invoke, singleTarget);
- return new InliningInfo(singleTarget, eligibleClass.type);
- }
private boolean isEligibleIndirectVirtualMethodCall(
DexMethod invokedMethod, ProgramMethod singleTarget) {
@@ -1194,25 +1029,6 @@
appView, parameter, objectState, method);
}
- private boolean isExtraMethodCall(InvokeMethod invoke) {
- if (invoke.isInvokeDirect() && dexItemFactory.isConstructor(invoke.getInvokedMethod())) {
- return false;
- }
- if (invoke.isInvokeMethodWithReceiver()) {
- Value receiver = invoke.asInvokeMethodWithReceiver().getReceiver();
- if (!receivers.addIllegalReceiverAlias(receiver)) {
- return false;
- }
- }
- if (invoke.isInvokeSuper()) {
- return false;
- }
- if (invoke.isInvokePolymorphic()) {
- return false;
- }
- return true;
- }
-
// Analyzes if a method invoke the eligible instance is passed to is eligible. In short,
// it can be eligible if:
//
@@ -1227,21 +1043,24 @@
//
// -- method itself can be inlined
//
- private boolean isExtraMethodCallEligible(
+ private boolean isEligibleDirectMethodCall(
InvokeMethod invoke,
SingleResolutionResult resolutionResult,
ProgramMethod singleTarget,
- Supplier<InliningOracle> defaultOracle) {
+ Supplier<InliningOracle> defaultOracle,
+ Set<Instruction> indirectUsers) {
+ if (!((invoke.isInvokeDirect() && !invoke.isInvokeConstructor(dexItemFactory))
+ || invoke.isInvokeInterface()
+ || invoke.isInvokeStatic()
+ || invoke.isInvokeVirtual())) {
+ return false;
+ }
+
// If we got here with invocation on receiver the user is ineligible.
if (invoke.isInvokeMethodWithReceiver()) {
- InvokeMethodWithReceiver invokeMethodWithReceiver = invoke.asInvokeMethodWithReceiver();
- Value receiver = invokeMethodWithReceiver.getReceiver();
- if (!receivers.addIllegalReceiverAlias(receiver)) {
- return false;
- }
-
// A definitely null receiver will throw an error on call site.
- if (receiver.getType().nullability().isDefinitelyNull()) {
+ Value receiver = invoke.asInvokeMethodWithReceiver().getReceiver();
+ if (receiver.getType().isDefinitelyNull()) {
return false;
}
}
@@ -1252,17 +1071,17 @@
invoke,
resolutionResult,
singleTarget,
- Reason.SIMPLE,
+ Reason.ALWAYS,
NopWhyAreYouNotInliningReporter.getInstance())) {
return false;
}
// Go through all arguments, see if all usages of eligibleInstance are good.
- if (!isEligibleParameterUsages(invoke, singleTarget.getDefinition())) {
+ if (!isEligibleParameterUsages(invoke, singleTarget, indirectUsers)) {
return false;
}
- extraMethodCalls.put(invoke, new InliningInfo(singleTarget, null));
+ directMethodCalls.put(invoke, new InliningInfo(singleTarget, null));
// Looks good.
markSizeOfDirectTargetForInlining(invoke, singleTarget);
@@ -1281,69 +1100,70 @@
return false;
}
- private boolean isEligibleParameterUsages(InvokeMethod invoke, DexEncodedMethod singleTarget) {
+ private boolean isEligibleParameterUsages(
+ InvokeMethod invoke, ProgramMethod singleTarget, Set<Instruction> indirectUsers) {
// Go through all arguments, see if all usages of eligibleInstance are good.
for (int parameter = 0; parameter < invoke.arguments().size(); parameter++) {
- MethodOptimizationInfo optimizationInfo = singleTarget.getOptimizationInfo();
- ParameterUsage usage =
- optimizationInfo.getClassInlinerMethodConstraint().getParameterUsage(parameter);
-
Value argument = invoke.getArgument(parameter);
if (receivers.isReceiverAlias(argument)) {
// Have parameter usage info?
- if (!isEligibleParameterUsage(usage, invoke)) {
+ if (!isEligibleParameterUsage(invoke, singleTarget, parameter, indirectUsers)) {
return false;
}
} else {
// Nothing to worry about, unless `argument` becomes an alias of the receiver later.
+ int finalParameter = parameter;
receivers.addDeferredAliasValidityCheck(
- argument, () -> isEligibleParameterUsage(usage, invoke));
+ argument,
+ () -> isEligibleParameterUsage(invoke, singleTarget, finalParameter, indirectUsers));
}
}
return true;
}
- private boolean isEligibleParameterUsage(ParameterUsage usage, InvokeMethod invoke) {
- if (usage.isBottom()) {
- return true;
+ private boolean isEligibleParameterUsage(
+ InvokeMethod invoke,
+ ProgramMethod singleTarget,
+ int parameter,
+ Set<Instruction> indirectUsers) {
+ ClassInlinerMethodConstraint classInlinerMethodConstraint =
+ singleTarget.getDefinition().getOptimizationInfo().getClassInlinerMethodConstraint();
+ if (root.isNewInstance()) {
+ if (!classInlinerMethodConstraint.isEligibleForNewInstanceClassInlining(
+ singleTarget, parameter)) {
+ return false;
+ }
+ } else {
+ assert root.isStaticGet();
+ if (!classInlinerMethodConstraint.isEligibleForStaticGetClassInlining(
+ appView, parameter, objectState, method)) {
+ return false;
+ }
}
- if (usage.isTop()) {
+ ParameterUsage usage = classInlinerMethodConstraint.getParameterUsage(parameter);
+ if (!scheduleNewUsersForAnalysis(invoke, singleTarget, parameter, indirectUsers)) {
return false;
}
- NonEmptyParameterUsage nonEmptyUsage = usage.asNonEmpty();
+ if (!usage.isBottom()) {
+ NonEmptyParameterUsage nonEmptyUsage = usage.asNonEmpty();
+ for (DexMethod invokedMethod : nonEmptyUsage.getMethodCallsWithParameterAsReceiver()) {
+ SingleResolutionResult resolutionResult =
+ appView.appInfo().resolveMethodOn(eligibleClass, invokedMethod).asSingleResolution();
+ if (resolutionResult == null || !resolutionResult.getResolvedHolder().isProgramClass()) {
+ return false;
+ }
- if (root.isStaticGet()) {
- // If we are class inlining a singleton instance from a static-get, then we don't know
- // the value of the fields, and we also can't optimize away instance-field assignments, as
- // they have global side effects.
- if (nonEmptyUsage.isParameterMutated()
- || usage.isParameterUsedAsLock()
- || nonEmptyUsage.hasFieldsReadFromParameter()) {
- return false;
+ // Is the method called indirectly still eligible?
+ ProgramMethod indirectSingleTarget = resolutionResult.getResolutionPair().asProgramMethod();
+ if (!isEligibleIndirectVirtualMethodCall(invokedMethod, indirectSingleTarget)) {
+ return false;
+ }
+ markSizeOfIndirectTargetForInlining(indirectSingleTarget);
}
}
- if (usage.isParameterReturned() && invoke.hasUsedOutValue()) {
- // Used as return value which is not ignored.
- return false;
- }
-
- for (DexMethod invokedMethod : nonEmptyUsage.getMethodCallsWithParameterAsReceiver()) {
- SingleResolutionResult resolutionResult =
- appView.appInfo().resolveMethodOn(eligibleClass, invokedMethod).asSingleResolution();
- if (resolutionResult == null || !resolutionResult.getResolvedHolder().isProgramClass()) {
- return false;
- }
-
- // Is the method called indirectly still eligible?
- ProgramMethod singleTarget = resolutionResult.getResolutionPair().asProgramMethod();
- if (!isEligibleIndirectVirtualMethodCall(invokedMethod, singleTarget)) {
- return false;
- }
- markSizeOfIndirectTargetForInlining(singleTarget);
- }
return true;
}
@@ -1357,6 +1177,7 @@
if (!exemptFromInstructionLimit(inlinee)) {
indirectInlinees.add(inlinee);
}
+ indirectMethodCallsOnInstance.add(inlinee);
}
private void markSizeOfDirectTargetForInlining(InvokeMethod invoke, ProgramMethod inlinee) {
@@ -1378,7 +1199,7 @@
.getDefinition()
.isInliningCandidate(
method,
- Reason.SIMPLE,
+ Reason.ALWAYS,
appView.appInfo(),
NopWhyAreYouNotInliningReporter.getInstance())) {
// If `singleTarget` is not an inlining candidate, we won't be able to inline it here.
diff --git a/src/test/java/com/android/tools/r8/R8RunExamplesAndroidOTest.java b/src/test/java/com/android/tools/r8/R8RunExamplesAndroidOTest.java
index 3ba2c7b..7f2c924 100644
--- a/src/test/java/com/android/tools/r8/R8RunExamplesAndroidOTest.java
+++ b/src/test/java/com/android/tools/r8/R8RunExamplesAndroidOTest.java
@@ -115,7 +115,7 @@
.withMinApiLevel(ToolHelper.getMinApiLevelForDexVmNoHigherThan(AndroidApiLevel.K))
.withBuilderTransformation(
b -> b.addProguardConfiguration(PROGUARD_OPTIONS, Origin.unknown()))
- .withDexCheck(inspector -> checkLambdaCount(inspector, 7, "lambdadesugaring"))
+ .withDexCheck(inspector -> checkLambdaCount(inspector, 5, "lambdadesugaring"))
.run();
}
@@ -154,7 +154,7 @@
.withMinApiLevel(AndroidApiLevel.N)
.withBuilderTransformation(
b -> b.addProguardConfiguration(PROGUARD_OPTIONS, Origin.unknown()))
- .withDexCheck(inspector -> checkLambdaCount(inspector, 7, "lambdadesugaring"))
+ .withDexCheck(inspector -> checkLambdaCount(inspector, 5, "lambdadesugaring"))
.run();
}
@@ -177,7 +177,7 @@
.withBuilderTransformation(ToolHelper::allowTestProguardOptions)
.withBuilderTransformation(
b -> b.addProguardConfiguration(PROGUARD_OPTIONS_N_PLUS, Origin.unknown()))
- .withDexCheck(inspector -> checkLambdaCount(inspector, 6, "lambdadesugaringnplus"))
+ .withDexCheck(inspector -> checkLambdaCount(inspector, 4, "lambdadesugaringnplus"))
.run();
}
@@ -200,7 +200,7 @@
.withBuilderTransformation(ToolHelper::allowTestProguardOptions)
.withBuilderTransformation(
b -> b.addProguardConfiguration(PROGUARD_OPTIONS_N_PLUS, Origin.unknown()))
- .withDexCheck(inspector -> checkLambdaCount(inspector, 2, "lambdadesugaringnplus"))
+ .withDexCheck(inspector -> checkLambdaCount(inspector, 0, "lambdadesugaringnplus"))
.run();
}
diff --git a/src/test/java/com/android/tools/r8/ir/analysis/sideeffect/SingletonClassInitializerPatternCanBePostponedTest.java b/src/test/java/com/android/tools/r8/ir/analysis/sideeffect/SingletonClassInitializerPatternCanBePostponedTest.java
index 9461c6b..1950e53 100644
--- a/src/test/java/com/android/tools/r8/ir/analysis/sideeffect/SingletonClassInitializerPatternCanBePostponedTest.java
+++ b/src/test/java/com/android/tools/r8/ir/analysis/sideeffect/SingletonClassInitializerPatternCanBePostponedTest.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.ir.analysis.sideeffect;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isAbsent;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
@@ -48,7 +49,7 @@
private void inspect(CodeInspector inspector) {
ClassSubject classSubject = inspector.clazz(A.class);
- assertThat(classSubject, isPresent());
+ assertThat(classSubject, isAbsent());
// A.inlineable() should be inlined because we should be able to determine that A.<clinit>() can
// safely be postponed.
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerPhiDirectUserAfterInlineTest.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerPhiDirectUserAfterInlineTest.java
index 1740c8a..6001549 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerPhiDirectUserAfterInlineTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerPhiDirectUserAfterInlineTest.java
@@ -73,7 +73,7 @@
.addKeepMainRule(Main.class)
.addOptionsModification(
options -> {
- options.testing.validInliningReasons = ImmutableSet.of(Reason.FORCE);
+ options.testing.validInliningReasons = ImmutableSet.of(Reason.ALWAYS);
options.testing.inlineeIrModifier = this::modifyIr;
})
.run(parameters.getRuntime(), Main.class)
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerSimplePairBuilderTest.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerSimplePairBuilderTest.java
index 1d1f00e..b1e3cfc 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerSimplePairBuilderTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerSimplePairBuilderTest.java
@@ -77,24 +77,12 @@
private void inspect(CodeInspector inspector) {
ClassSubject clazz = inspector.clazz(TestClass.class);
- if (parameters.isCfRuntime()) {
- assertThat(inspector.clazz(PairBuilder.class), isPresent());
+ assertThat(inspector.clazz(PairBuilder.class), not(isPresent()));
- // const-string canonicalization is disabled in CF, which helps ClassInliner identify
- // PairBuilder as candidate.
- Set<String> expected =
- ImmutableSet.of(StringBuilder.class.getTypeName(), PairBuilder.class.getTypeName());
- assertEquals(expected, collectTypes(clazz.uniqueMethodWithName("testSimpleBuilder1")));
- assertEquals(expected, collectTypes(clazz.uniqueMethodWithName("testSimpleBuilder2")));
- assertEquals(expected, collectTypes(clazz.uniqueMethodWithName("testSimpleBuilder3")));
- } else {
- assertThat(inspector.clazz(PairBuilder.class), not(isPresent()));
-
- Set<String> expected = ImmutableSet.of(StringBuilder.class.getTypeName());
- assertEquals(expected, collectTypes(clazz.uniqueMethodWithName("testSimpleBuilder1")));
- assertEquals(expected, collectTypes(clazz.uniqueMethodWithName("testSimpleBuilder2")));
- assertEquals(expected, collectTypes(clazz.uniqueMethodWithName("testSimpleBuilder3")));
- }
+ Set<String> expected = ImmutableSet.of(StringBuilder.class.getTypeName());
+ assertEquals(expected, collectTypes(clazz.uniqueMethodWithName("testSimpleBuilder1")));
+ assertEquals(expected, collectTypes(clazz.uniqueMethodWithName("testSimpleBuilder2")));
+ assertEquals(expected, collectTypes(clazz.uniqueMethodWithName("testSimpleBuilder3")));
}
static class TestClass {
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/LibraryOverrideClassInliningTest.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/LibraryOverrideClassInliningTest.java
index 4f020fd..24c1c8b 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/LibraryOverrideClassInliningTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/LibraryOverrideClassInliningTest.java
@@ -6,6 +6,7 @@
import static com.android.tools.r8.references.Reference.methodFromMethod;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import com.android.tools.r8.NeverInline;
@@ -113,8 +114,9 @@
.holder
.toString()
.equals(SimpleLibraryOverride.class.getTypeName())));
- // Check the non-simple run is not inlined.
- assertTrue(
+ // TODO(b/181942160): The non-simple run should ideally not inlined by the class inliner, since
+ // there is an instance of NonSimpleLibraryOverride that is not eligible for class inlining.
+ assertFalse(
"Expected NonSimple.run invoke in:\n" + main.getMethod().codeToString(),
main.streamInstructions()
.anyMatch(
diff --git a/src/test/java/com/android/tools/r8/kotlin/KotlinClassInlinerTest.java b/src/test/java/com/android/tools/r8/kotlin/KotlinClassInlinerTest.java
index b329f9f..f6b24da 100644
--- a/src/test/java/com/android/tools/r8/kotlin/KotlinClassInlinerTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/KotlinClassInlinerTest.java
@@ -24,8 +24,8 @@
import com.android.tools.r8.utils.codeinspector.InstructionSubject;
import com.android.tools.r8.utils.codeinspector.MethodSubject;
import com.google.common.collect.Lists;
-import com.google.common.collect.Sets;
import com.google.common.collect.Streams;
+import java.util.Collections;
import java.util.List;
import java.util.Set;
import java.util.function.Predicate;
@@ -200,10 +200,8 @@
.inspect(
inspector -> {
ClassSubject clazz = inspector.clazz(mainClassName);
-
- // TODO(b/141719453): Data class should maybe be class inlined.
assertEquals(
- Sets.newHashSet("class_inliner_data_class.Alpha"),
+ Collections.emptySet(),
collectAccessedTypes(
type -> !type.toSourceString().startsWith("java."),
clazz,
