Handle the remaining -whyareyounotinlining cases
Bug: 142108662
Change-Id: Iff62762dbd162cdb505f09d24954a1967926c09f
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
index f12c956..f34e2bc 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -384,33 +384,36 @@
if (appInfo.isSubtype(containerType, method.holder)) {
return true;
}
- whyAreYouNotInliningReporter.reportUnknownReason();
+ whyAreYouNotInliningReporter.reportCallerNotSubtype();
return false;
case PROCESSED_INLINING_CANDIDATE_SAME_PACKAGE:
if (containerType.isSamePackage(method.holder)) {
return true;
}
- whyAreYouNotInliningReporter.reportUnknownReason();
+ whyAreYouNotInliningReporter.reportCallerNotSamePackage();
return false;
case PROCESSED_INLINING_CANDIDATE_SAME_NEST:
if (NestUtils.sameNest(containerType, method.holder, appInfo)) {
return true;
}
- whyAreYouNotInliningReporter.reportUnknownReason();
+ whyAreYouNotInliningReporter.reportCallerNotSameNest();
return false;
case PROCESSED_INLINING_CANDIDATE_SAME_CLASS:
if (containerType == method.holder) {
return true;
}
- whyAreYouNotInliningReporter.reportUnknownReason();
+ whyAreYouNotInliningReporter.reportCallerNotSameClass();
return false;
case PROCESSED_NOT_INLINING_CANDIDATE:
+ whyAreYouNotInliningReporter.reportInlineeNotInliningCandidate();
+ return false;
+
case NOT_PROCESSED:
- whyAreYouNotInliningReporter.reportUnknownReason();
+ whyAreYouNotInliningReporter.reportInlineeNotProcessed();
return false;
default:
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
index cd59784..87350e7 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
@@ -209,11 +209,11 @@
private boolean passesInliningConstraints(
InvokeMethod invoke,
- DexEncodedMethod candidate,
+ DexEncodedMethod singleTarget,
Reason reason,
WhyAreYouNotInliningReporter whyAreYouNotInliningReporter) {
- if (candidate.getOptimizationInfo().neverInline()) {
- whyAreYouNotInliningReporter.reportUnknownReason();
+ if (singleTarget.getOptimizationInfo().neverInline()) {
+ whyAreYouNotInliningReporter.reportMarkedAsNeverInline();
return false;
}
@@ -222,15 +222,15 @@
if (method.isInstanceInitializer()
&& appView.options().isGeneratingClassFiles()
&& reason != Reason.FORCE) {
- whyAreYouNotInliningReporter.reportUnknownReason();
+ whyAreYouNotInliningReporter.reportNoInliningIntoConstructorsWhenGeneratingClassFiles();
return false;
}
- if (method == candidate) {
+ if (method == singleTarget) {
// Cannot handle recursive inlining at this point.
// Force inlined method should never be recursive.
- assert !candidate.getOptimizationInfo().forceInline();
- whyAreYouNotInliningReporter.reportUnknownReason();
+ assert !singleTarget.getOptimizationInfo().forceInline();
+ whyAreYouNotInliningReporter.reportRecursiveMethod();
return false;
}
@@ -239,67 +239,64 @@
// or optimized code. Right now this happens for the class class staticizer, as it just
// processes all relevant methods in parallel with the full optimization pipeline enabled.
// TODO(sgjesse): Add this assert "assert !isProcessedConcurrently.test(candidate);"
- if (reason != Reason.FORCE && isProcessedConcurrently.test(candidate)) {
- whyAreYouNotInliningReporter.reportUnknownReason();
+ if (reason != Reason.FORCE && isProcessedConcurrently.test(singleTarget)) {
+ whyAreYouNotInliningReporter.reportProcessedConcurrently();
return false;
}
InternalOptions options = appView.options();
if (options.featureSplitConfiguration != null
&& !options.featureSplitConfiguration.inSameFeatureOrBase(
- candidate.method, method.method)) {
- whyAreYouNotInliningReporter.reportUnknownReason();
+ singleTarget.method, method.method)) {
+ whyAreYouNotInliningReporter.reportInliningAcrossFeatureSplit();
return false;
}
- if (options.testing.validInliningReasons != null
- && !options.testing.validInliningReasons.contains(reason)) {
- whyAreYouNotInliningReporter.reportUnknownReason();
+ Set<Reason> validInliningReasons = options.testing.validInliningReasons;
+ if (validInliningReasons != null && !validInliningReasons.contains(reason)) {
+ whyAreYouNotInliningReporter.reportInvalidInliningReason(reason, validInliningReasons);
return false;
}
// Abort inlining attempt if method -> target access is not right.
- if (!inliner.hasInliningAccess(method, candidate)) {
- whyAreYouNotInliningReporter.reportUnknownReason();
+ if (!inliner.hasInliningAccess(method, singleTarget)) {
+ whyAreYouNotInliningReporter.reportInaccessible();
return false;
}
- DexClass holder = appView.definitionFor(candidate.method.holder);
-
+ DexClass holder = appView.definitionFor(singleTarget.method.holder);
if (holder.isInterface()) {
// Art978_virtual_interfaceTest correctly expects an IncompatibleClassChangeError exception at
// runtime.
- whyAreYouNotInliningReporter.reportUnknownReason();
- return false;
- }
-
- if (holder.isNotProgramClass()) {
- whyAreYouNotInliningReporter.reportUnknownReason();
+ whyAreYouNotInliningReporter.reportIncompatibleClassChangeError();
return false;
}
// Don't inline if target is synchronized.
- if (candidate.accessFlags.isSynchronized()) {
- whyAreYouNotInliningReporter.reportUnknownReason();
+ if (singleTarget.accessFlags.isSynchronized()) {
+ whyAreYouNotInliningReporter.reportSynchronizedMethod();
return false;
}
if (reason == Reason.DUAL_CALLER) {
- if (satisfiesRequirementsForSimpleInlining(invoke, candidate)) {
+ if (satisfiesRequirementsForSimpleInlining(invoke, singleTarget)) {
// When we have a method with two call sites, we simply inline the method as we normally do
// when the method is small. We still need to ensure that the other call site is also
// inlined, though. Therefore, we record here that we have seen one of the two call sites
// as we normally do.
- inliner.recordDoubleInliningCandidate(method, candidate);
- } else {
- if (inliner.satisfiesRequirementsForDoubleInlining(method, candidate)) {
- whyAreYouNotInliningReporter.reportUnknownReason();
+ inliner.recordDoubleInliningCandidate(method, singleTarget);
+ } else if (inliner.isDoubleInliningEnabled()) {
+ if (!inliner.satisfiesRequirementsForDoubleInlining(method, singleTarget)) {
+ whyAreYouNotInliningReporter.reportInvalidDoubleInliningCandidate();
return false;
}
+ } else {
+ // TODO(b/142300882): Should in principle disallow inlining in this case.
+ inliner.recordDoubleInliningCandidate(method, singleTarget);
}
} else if (reason == Reason.SIMPLE
- && !satisfiesRequirementsForSimpleInlining(invoke, candidate)) {
- whyAreYouNotInliningReporter.reportUnknownReason();
+ && !satisfiesRequirementsForSimpleInlining(invoke, singleTarget)) {
+ whyAreYouNotInliningReporter.reportInlineeNotSimple();
return false;
}
@@ -308,8 +305,8 @@
// If we do this it can increase the size of the main dex dependent classes.
if (inliner.mainDexClasses.getRoots().contains(method.method.holder)
&& MainDexDirectReferenceTracer.hasReferencesOutsideFromCode(
- appView.appInfo(), candidate, inliner.mainDexClasses.getRoots())) {
- whyAreYouNotInliningReporter.reportUnknownReason();
+ appView.appInfo(), singleTarget, inliner.mainDexClasses.getRoots())) {
+ whyAreYouNotInliningReporter.reportInlineeRefersToClassesNotInMainDex();
return false;
}
// Allow inlining into the classes in the main dex dependent set without restrictions.
@@ -391,7 +388,7 @@
Value receiver = invoke.getReceiver();
if (receiver.getTypeLattice().isDefinitelyNull()) {
// A definitely null receiver will throw an error on call site.
- whyAreYouNotInliningReporter.reportUnknownReason();
+ whyAreYouNotInliningReporter.reportReceiverDefinitelyNull();
return null;
}
@@ -405,7 +402,7 @@
if (!singleTarget.getOptimizationInfo().checksNullReceiverBeforeAnySideEffect()) {
InternalOptions options = appView.options();
if (!options.enableInliningOfInvokesWithNullableReceivers) {
- whyAreYouNotInliningReporter.reportUnknownReason();
+ whyAreYouNotInliningReporter.reportReceiverMaybeNull();
return null;
}
action.setShouldSynthesizeNullCheckForReceiver();
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
index 18c101c..44c72c5 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
@@ -106,6 +106,10 @@
return false;
}
+ boolean isDoubleInliningEnabled() {
+ return applyDoubleInlining;
+ }
+
private ConstraintWithTarget instructionAllowedForInlining(
Instruction instruction, InliningConstraints inliningConstraints, DexType invocationContext) {
ConstraintWithTarget result =
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/inliner/NopWhyAreYouNotInliningReporter.java b/src/main/java/com/android/tools/r8/ir/optimize/inliner/NopWhyAreYouNotInliningReporter.java
index 81bdb7c..818b8c2 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/inliner/NopWhyAreYouNotInliningReporter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/inliner/NopWhyAreYouNotInliningReporter.java
@@ -7,6 +7,8 @@
import com.android.tools.r8.ir.code.InstancePut;
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.InvokeDirect;
+import com.android.tools.r8.ir.optimize.Inliner.Reason;
+import java.util.Set;
public class NopWhyAreYouNotInliningReporter extends WhyAreYouNotInliningReporter {
@@ -23,18 +25,57 @@
public void reportBlacklisted() {}
@Override
+ public void reportCallerNotSameClass() {}
+
+ @Override
+ public void reportCallerNotSameNest() {}
+
+ @Override
+ public void reportCallerNotSamePackage() {}
+
+ @Override
+ public void reportCallerNotSubtype() {}
+
+ @Override
public void reportClasspathMethod() {}
@Override
+ public void reportInaccessible() {}
+
+ @Override
+ public void reportIncompatibleClassChangeError() {}
+
+ @Override
public void reportIncorrectArity(int numberOfArguments, int arity) {}
@Override
public void reportInlineeDoesNotHaveCode() {}
@Override
+ public void reportInlineeNotInliningCandidate() {}
+
+ @Override
+ public void reportInlineeNotProcessed() {}
+
+ @Override
+ public void reportInlineeNotSimple() {}
+
+ @Override
+ public void reportInlineeRefersToClassesNotInMainDex() {}
+
+ @Override
+ public void reportInliningAcrossFeatureSplit() {}
+
+ @Override
public void reportInstructionBudgetIsExceeded() {}
@Override
+ public void reportInvalidDoubleInliningCandidate() {}
+
+ @Override
+ public void reportInvalidInliningReason(Reason reason, Set<Reason> validInliningReasons) {}
+
+ @Override
public void reportLibraryMethod() {}
@Override
@@ -44,6 +85,9 @@
public void reportMustTriggerClassInitialization() {}
@Override
+ public void reportNoInliningIntoConstructorsWhenGeneratingClassFiles() {}
+
+ @Override
public void reportPinned() {}
@Override
@@ -51,7 +95,19 @@
int estimatedNumberOfControlFlowResolutionBlocks, int threshold) {}
@Override
- public void reportUnknownReason() {}
+ public void reportProcessedConcurrently() {}
+
+ @Override
+ public void reportReceiverDefinitelyNull() {}
+
+ @Override
+ public void reportReceiverMaybeNull() {}
+
+ @Override
+ public void reportRecursiveMethod() {}
+
+ @Override
+ public void reportSynchronizedMethod() {}
@Override
public void reportUnknownTarget() {}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/inliner/WhyAreYouNotInliningReporter.java b/src/main/java/com/android/tools/r8/ir/optimize/inliner/WhyAreYouNotInliningReporter.java
index 9bfd769..c560944 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/inliner/WhyAreYouNotInliningReporter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/inliner/WhyAreYouNotInliningReporter.java
@@ -10,8 +10,10 @@
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.InvokeDirect;
import com.android.tools.r8.ir.code.InvokeMethod;
+import com.android.tools.r8.ir.optimize.Inliner.Reason;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import java.util.Collection;
+import java.util.Set;
public abstract class WhyAreYouNotInliningReporter {
@@ -46,26 +48,62 @@
public abstract void reportBlacklisted();
+ public abstract void reportCallerNotSameClass();
+
+ public abstract void reportCallerNotSameNest();
+
+ public abstract void reportCallerNotSamePackage();
+
+ public abstract void reportCallerNotSubtype();
+
public abstract void reportClasspathMethod();
+ public abstract void reportInaccessible();
+
+ public abstract void reportIncompatibleClassChangeError();
+
public abstract void reportIncorrectArity(int numberOfArguments, int arity);
public abstract void reportInlineeDoesNotHaveCode();
+ public abstract void reportInlineeNotInliningCandidate();
+
+ public abstract void reportInlineeNotProcessed();
+
+ public abstract void reportInlineeNotSimple();
+
+ public abstract void reportInlineeRefersToClassesNotInMainDex();
+
+ public abstract void reportInliningAcrossFeatureSplit();
+
public abstract void reportInstructionBudgetIsExceeded();
+ public abstract void reportInvalidDoubleInliningCandidate();
+
+ public abstract void reportInvalidInliningReason(Reason reason, Set<Reason> validInliningReasons);
+
public abstract void reportLibraryMethod();
public abstract void reportMarkedAsNeverInline();
public abstract void reportMustTriggerClassInitialization();
+ public abstract void reportNoInliningIntoConstructorsWhenGeneratingClassFiles();
+
public abstract void reportPinned();
public abstract void reportPotentialExplosionInExceptionalControlFlowResolutionBlocks(
int estimatedNumberOfControlFlowResolutionBlocks, int threshold);
- public abstract void reportUnknownReason();
+ public abstract void reportProcessedConcurrently();
+
+ public abstract void reportReceiverDefinitelyNull();
+
+ public abstract void reportReceiverMaybeNull();
+
+ public abstract void reportRecursiveMethod();
+
+ public abstract void reportSynchronizedMethod();
abstract void reportUnknownTarget();
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/inliner/WhyAreYouNotInliningReporterImpl.java b/src/main/java/com/android/tools/r8/ir/optimize/inliner/WhyAreYouNotInliningReporterImpl.java
index e6ad63e..6bc09fd 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/inliner/WhyAreYouNotInliningReporterImpl.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/inliner/WhyAreYouNotInliningReporterImpl.java
@@ -8,7 +8,10 @@
import com.android.tools.r8.ir.code.InstancePut;
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.InvokeDirect;
+import com.android.tools.r8.ir.optimize.Inliner.Reason;
+import com.android.tools.r8.utils.StringUtils;
import java.io.PrintStream;
+import java.util.Set;
class WhyAreYouNotInliningReporterImpl extends WhyAreYouNotInliningReporter {
@@ -50,11 +53,46 @@
}
@Override
+ public void reportCallerNotSameClass() {
+ print("inlinee can only be inlined into methods in the same class.");
+ }
+
+ @Override
+ public void reportCallerNotSameNest() {
+ print("inlinee can only be inlined into methods in the same class (and its nest members).");
+ }
+
+ @Override
+ public void reportCallerNotSamePackage() {
+ print(
+ "inlinee can only be inlined into methods in the same package "
+ + "(declared package private or accesses package private type or member).");
+ }
+
+ @Override
+ public void reportCallerNotSubtype() {
+ print(
+ "inlinee can only be inlined into methods in the same package and methods in subtypes of "
+ + "the inlinee's enclosing class"
+ + "(declared protected or accesses protected type or member).");
+ }
+
+ @Override
public void reportClasspathMethod() {
print("inlinee is on the classpath.");
}
@Override
+ public void reportInaccessible() {
+ print("inlinee is not accessible from the caller context.");
+ }
+
+ @Override
+ public void reportIncompatibleClassChangeError() {
+ print("invoke may fail with an IncompatibleClassChangeError.");
+ }
+
+ @Override
public void reportIncorrectArity(int numberOfArguments, int arity) {
print(
"number of arguments ("
@@ -70,11 +108,55 @@
}
@Override
+ public void reportInlineeNotInliningCandidate() {
+ print("unsupported instruction in inlinee.");
+ }
+
+ @Override
+ public void reportInlineeNotProcessed() {
+ print("inlinee not processed yet.");
+ }
+
+ @Override
+ public void reportInlineeNotSimple() {
+ print(
+ "not inlining due to code size heuristic "
+ + "(inlinee may have multiple callers and is not considered trivial).");
+ }
+
+ @Override
+ public void reportInlineeRefersToClassesNotInMainDex() {
+ print(
+ "inlining could increase the main dex size "
+ + "(caller is in main dex and inlinee refers to classes not in main dex).");
+ }
+
+ @Override
+ public void reportInliningAcrossFeatureSplit() {
+ print("cannot inline across feature splits.");
+ }
+
+ @Override
public void reportInstructionBudgetIsExceeded() {
print("caller's instruction budget is exceeded.");
}
@Override
+ public void reportInvalidDoubleInliningCandidate() {
+ print("inlinee is invoked more than once and could not be inlined into all call sites.");
+ }
+
+ @Override
+ public void reportInvalidInliningReason(Reason reason, Set<Reason> validInliningReasons) {
+ print(
+ "not a valid inlining reason (was: "
+ + reason
+ + ", allowed: one of "
+ + StringUtils.join(validInliningReasons, ", ")
+ + ").");
+ }
+
+ @Override
public void reportLibraryMethod() {
print("inlinee is a library method.");
}
@@ -92,6 +174,11 @@
}
@Override
+ public void reportNoInliningIntoConstructorsWhenGeneratingClassFiles() {
+ print("inlining into constructors not supported when generating class files.");
+ }
+
+ @Override
public void reportPinned() {
print("method is kept by a Proguard configuration rule.");
}
@@ -106,10 +193,30 @@
threshold);
}
- // TODO(b/142108662): Always report a meaningful reason.
@Override
- public void reportUnknownReason() {
- print(null);
+ public void reportProcessedConcurrently() {
+ print(
+ "could lead to nondeterministic output since the inlinee is being optimized concurrently.");
+ }
+
+ @Override
+ public void reportReceiverDefinitelyNull() {
+ print("the receiver is always null at the call site.");
+ }
+
+ @Override
+ public void reportReceiverMaybeNull() {
+ print("the receiver may be null at the call site.");
+ }
+
+ @Override
+ public void reportRecursiveMethod() {
+ print("recursive calls are not inlined.");
+ }
+
+ @Override
+ public void reportSynchronizedMethod() {
+ print("synchronized methods are not inlined.");
}
@Override
