Reapply inlining to GeneratedMessageLite bridge methods after inlining
Change-Id: Ie5c953eaf7e3036c5a51fd6ebaaaa9426c8f4021
Bug: b/244451885
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/proto/GeneratedMessageLiteShrinker.java b/src/main/java/com/android/tools/r8/ir/analysis/proto/GeneratedMessageLiteShrinker.java
index 4d9b9a0..74a3fb5 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/proto/GeneratedMessageLiteShrinker.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/proto/GeneratedMessageLiteShrinker.java
@@ -42,6 +42,7 @@
import com.android.tools.r8.ir.optimize.info.OptimizationFeedbackIgnore;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.shaking.DependentMinimumKeepInfoCollection;
+import com.android.tools.r8.shaking.KeepMethodInfo;
import com.android.tools.r8.utils.Timing;
import com.android.tools.r8.utils.collections.ProgramMethodSet;
import com.google.common.collect.Sets;
@@ -87,9 +88,10 @@
ProgramMethod dynamicMethod =
generatedMessageLiteClass.lookupProgramMethod(references.dynamicMethod);
if (dynamicMethod != null) {
- dependentMinimumKeepInfo
- .getOrCreateUnconditionalMinimumKeepInfoFor(dynamicMethod.getReference())
- .disallowOptimization();
+ disallowSignatureOptimizations(
+ dependentMinimumKeepInfo
+ .getOrCreateUnconditionalMinimumKeepInfoFor(dynamicMethod.getReference())
+ .asMethodJoiner());
}
references.forEachMethodReference(
@@ -98,14 +100,27 @@
asProgramClassOrNull(appView.definitionFor(reference.getHolderType()));
ProgramMethod method = reference.lookupOnProgramClass(holder);
if (method != null) {
- dependentMinimumKeepInfo
- .getOrCreateUnconditionalMinimumKeepInfoFor(method.getReference())
- .disallowOptimization();
+ disallowSignatureOptimizations(
+ dependentMinimumKeepInfo
+ .getOrCreateUnconditionalMinimumKeepInfoFor(method.getReference())
+ .asMethodJoiner());
}
});
}
}
+ private void disallowSignatureOptimizations(KeepMethodInfo.Joiner methodJoiner) {
+ methodJoiner
+ .disallowConstantArgumentOptimization()
+ .disallowMethodStaticizing()
+ .disallowParameterRemoval()
+ .disallowParameterReordering()
+ .disallowParameterTypeStrengthening()
+ .disallowReturnTypeStrengthening()
+ .disallowUnusedArgumentOptimization()
+ .disallowUnusedReturnValueOptimization();
+ }
+
public void run(IRCode code) {
ProgramMethod method = code.context();
if (references.isDynamicMethod(method.getReference())) {
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/proto/ProtoShrinker.java b/src/main/java/com/android/tools/r8/ir/analysis/proto/ProtoShrinker.java
index c83f118..dfc9eb3 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/proto/ProtoShrinker.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/proto/ProtoShrinker.java
@@ -78,6 +78,10 @@
return deadProtoTypes;
}
+ public ProtoReferences getProtoReferences() {
+ return references;
+ }
+
public void setDeadProtoTypes(Set<DexType> deadProtoTypes) {
// We should only need to keep track of the dead proto types for assertion purposes.
InternalOptions.checkAssertionsEnabled();
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 9490667..64c4725 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
@@ -452,26 +452,6 @@
}
@Override
- public boolean allowInliningOfInvokeInInlinee(
- InlineAction action,
- int inliningDepth,
- WhyAreYouNotInliningReporter whyAreYouNotInliningReporter) {
- assert inliningDepth > 0;
-
- if (action.reason.mustBeInlined()) {
- return true;
- }
-
- int threshold = inlinerOptions.applyInliningToInlineeMaxDepth;
- if (inliningDepth <= threshold) {
- return true;
- }
-
- whyAreYouNotInliningReporter.reportWillExceedMaxInliningDepth(inliningDepth, threshold);
- return false;
- }
-
- @Override
public boolean canInlineInstanceInitializer(
IRCode code,
InvokeDirect invoke,
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/ForcedInliningOracle.java b/src/main/java/com/android/tools/r8/ir/optimize/ForcedInliningOracle.java
index 493b052..bdf8eda 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/ForcedInliningOracle.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/ForcedInliningOracle.java
@@ -101,15 +101,6 @@
}
@Override
- public boolean allowInliningOfInvokeInInlinee(
- InlineAction action,
- int inliningDepth,
- WhyAreYouNotInliningReporter whyAreYouNotInliningReporter) {
- // The purpose of force inlining is generally to inline a given invoke-instruction in the IR.
- return false;
- }
-
- @Override
public boolean canInlineInstanceInitializer(
IRCode code,
InvokeDirect invoke,
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 577dbf2..bf8914f 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
@@ -1013,13 +1013,6 @@
continue;
}
- if (!inlineeStack.isEmpty()
- && !strategy.allowInliningOfInvokeInInlinee(
- action, inlineeStack.size(), whyAreYouNotInliningReporter)) {
- assert whyAreYouNotInliningReporter.unsetReasonHasBeenReportedFlag();
- continue;
- }
-
if (!strategy.stillHasBudget(action, whyAreYouNotInliningReporter)) {
assert whyAreYouNotInliningReporter.unsetReasonHasBeenReportedFlag();
continue;
@@ -1083,17 +1076,16 @@
context.getDefinition().copyMetadata(appView, singleTargetMethod);
- if (inlineeMayHaveInvokeMethod && options.applyInliningToInlinee) {
- if (inlineeStack.size() + 1 > options.applyInliningToInlineeMaxDepth
- && appView.appInfo().hasNoAlwaysInlineMethods()) {
- continue;
+ if (inlineeMayHaveInvokeMethod) {
+ int inliningDepth = inlineeStack.size() + 1;
+ if (options.shouldApplyInliningToInlinee(appView, singleTarget, inliningDepth)) {
+ // Record that we will be inside the inlinee until the next block.
+ BasicBlock inlineeEnd = IteratorUtils.peekNext(blockIterator);
+ inlineeStack.push(inlineeEnd);
+ // Move the cursor back to where the first inlinee block was added.
+ IteratorUtils.previousUntil(blockIterator, previous -> previous == block);
+ blockIterator.next();
}
- // Record that we will be inside the inlinee until the next block.
- BasicBlock inlineeEnd = IteratorUtils.peekNext(blockIterator);
- inlineeStack.push(inlineeEnd);
- // Move the cursor back to where the first inlinee block was added.
- IteratorUtils.previousUntil(blockIterator, previous -> previous == block);
- blockIterator.next();
}
} else if (current.isAssume()) {
assumeRemover.removeIfMarked(current.asAssume(), iterator);
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/InliningStrategy.java b/src/main/java/com/android/tools/r8/ir/optimize/InliningStrategy.java
index 564fcf8..b76544b 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/InliningStrategy.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/InliningStrategy.java
@@ -24,11 +24,6 @@
AppView<AppInfoWithLiveness> appView();
- boolean allowInliningOfInvokeInInlinee(
- InlineAction action,
- int inliningDepth,
- WhyAreYouNotInliningReporter whyAreYouNotInliningReporter);
-
boolean canInlineInstanceInitializer(
IRCode code,
InvokeDirect invoke,
diff --git a/src/main/java/com/android/tools/r8/shaking/KeepMethodInfo.java b/src/main/java/com/android/tools/r8/shaking/KeepMethodInfo.java
index 0173fb2..41ec85a 100644
--- a/src/main/java/com/android/tools/r8/shaking/KeepMethodInfo.java
+++ b/src/main/java/com/android/tools/r8/shaking/KeepMethodInfo.java
@@ -29,6 +29,7 @@
private final boolean allowConstantArgumentOptimization;
private final boolean allowInlining;
private final boolean allowMethodStaticizing;
+ private final boolean allowParameterRemoval;
private final boolean allowParameterReordering;
private final boolean allowParameterTypeStrengthening;
private final boolean allowReturnTypeStrengthening;
@@ -42,6 +43,7 @@
this.allowConstantArgumentOptimization = builder.isConstantArgumentOptimizationAllowed();
this.allowInlining = builder.isInliningAllowed();
this.allowMethodStaticizing = builder.isMethodStaticizingAllowed();
+ this.allowParameterRemoval = builder.isParameterRemovalAllowed();
this.allowParameterReordering = builder.isParameterReorderingAllowed();
this.allowParameterTypeStrengthening = builder.isParameterTypeStrengtheningAllowed();
this.allowReturnTypeStrengthening = builder.isReturnTypeStrengtheningAllowed();
@@ -60,13 +62,6 @@
return isParameterRemovalAllowed(configuration);
}
- public boolean isParameterRemovalAllowed(GlobalKeepInfoConfiguration configuration) {
- return isClosedWorldReasoningAllowed(configuration)
- && isOptimizationAllowed(configuration)
- && isShrinkingAllowed(configuration)
- && !isCheckDiscardedEnabled(configuration);
- }
-
public boolean isClassInliningAllowed(GlobalKeepInfoConfiguration configuration) {
return isOptimizationAllowed(configuration) && internalIsClassInliningAllowed();
}
@@ -111,6 +106,18 @@
return allowMethodStaticizing;
}
+ public boolean isParameterRemovalAllowed(GlobalKeepInfoConfiguration configuration) {
+ return isClosedWorldReasoningAllowed(configuration)
+ && isOptimizationAllowed(configuration)
+ && isShrinkingAllowed(configuration)
+ && !isCheckDiscardedEnabled(configuration)
+ && internalIsParameterRemovalAllowed();
+ }
+
+ boolean internalIsParameterRemovalAllowed() {
+ return allowParameterRemoval;
+ }
+
public boolean isParameterReorderingAllowed(GlobalKeepInfoConfiguration configuration) {
return isClosedWorldReasoningAllowed(configuration)
&& isOptimizationAllowed(configuration)
@@ -188,6 +195,7 @@
private boolean allowConstantArgumentOptimization;
private boolean allowInlining;
private boolean allowMethodStaticizing;
+ private boolean allowParameterRemoval;
private boolean allowParameterReordering;
private boolean allowParameterTypeStrengthening;
private boolean allowReturnTypeStrengthening;
@@ -205,6 +213,7 @@
allowConstantArgumentOptimization = original.internalIsConstantArgumentOptimizationAllowed();
allowInlining = original.internalIsInliningAllowed();
allowMethodStaticizing = original.internalIsMethodStaticizingAllowed();
+ allowParameterRemoval = original.internalIsParameterRemovalAllowed();
allowParameterReordering = original.internalIsParameterReorderingAllowed();
allowParameterTypeStrengthening = original.internalIsParameterTypeStrengtheningAllowed();
allowReturnTypeStrengthening = original.internalIsReturnTypeStrengtheningAllowed();
@@ -308,6 +317,25 @@
return setAllowMethodStaticizing(false);
}
+ // Parameter removal.
+
+ public boolean isParameterRemovalAllowed() {
+ return allowParameterRemoval;
+ }
+
+ public Builder setAllowParameterRemoval(boolean allowParameterRemoval) {
+ this.allowParameterRemoval = allowParameterRemoval;
+ return self();
+ }
+
+ public Builder allowParameterRemoval() {
+ return setAllowParameterRemoval(true);
+ }
+
+ public Builder disallowParameterRemoval() {
+ return setAllowParameterRemoval(false);
+ }
+
// Parameter reordering.
public boolean isParameterReorderingAllowed() {
@@ -433,6 +461,7 @@
== other.internalIsConstantArgumentOptimizationAllowed()
&& isInliningAllowed() == other.internalIsInliningAllowed()
&& isMethodStaticizingAllowed() == other.internalIsMethodStaticizingAllowed()
+ && isParameterRemovalAllowed() == other.internalIsParameterRemovalAllowed()
&& isParameterReorderingAllowed() == other.internalIsParameterReorderingAllowed()
&& isParameterTypeStrengtheningAllowed()
== other.internalIsParameterTypeStrengtheningAllowed()
@@ -456,6 +485,7 @@
.disallowConstantArgumentOptimization()
.disallowInlining()
.disallowMethodStaticizing()
+ .disallowParameterRemoval()
.disallowParameterReordering()
.disallowParameterTypeStrengthening()
.disallowReturnTypeStrengthening()
@@ -471,6 +501,7 @@
.allowConstantArgumentOptimization()
.allowInlining()
.allowMethodStaticizing()
+ .allowParameterRemoval()
.allowParameterReordering()
.allowParameterTypeStrengthening()
.allowReturnTypeStrengthening()
@@ -510,6 +541,11 @@
return self();
}
+ public Joiner disallowParameterRemoval() {
+ builder.disallowParameterRemoval();
+ return self();
+ }
+
public Joiner disallowParameterReordering() {
builder.disallowParameterReordering();
return self();
@@ -552,6 +588,7 @@
Joiner::disallowConstantArgumentOptimization)
.applyIf(!joiner.builder.isInliningAllowed(), Joiner::disallowInlining)
.applyIf(!joiner.builder.isMethodStaticizingAllowed(), Joiner::disallowMethodStaticizing)
+ .applyIf(!joiner.builder.isParameterRemovalAllowed(), Joiner::disallowParameterRemoval)
.applyIf(
!joiner.builder.isParameterReorderingAllowed(), Joiner::disallowParameterReordering)
.applyIf(
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index d616716..84386dd 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -54,6 +54,7 @@
import com.android.tools.r8.horizontalclassmerging.HorizontallyMergedClasses;
import com.android.tools.r8.horizontalclassmerging.Policy;
import com.android.tools.r8.inspector.internal.InspectorImpl;
+import com.android.tools.r8.ir.analysis.proto.ProtoReferences;
import com.android.tools.r8.ir.analysis.type.TypeElement;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.desugar.TypeRewriter;
@@ -231,7 +232,6 @@
}
void enableProtoShrinking() {
- inlinerOptions.applyInliningToInlinee = true;
enableFieldBitAccessAnalysis = true;
protoShrinking.enableGeneratedMessageLiteShrinking = true;
protoShrinking.enableGeneratedMessageLiteBuilderShrinking = true;
@@ -1391,6 +1391,11 @@
}
}
+ public interface ApplyInliningToInlineePredicate {
+
+ boolean test(AppView<?> appView, ProgramMethod method, int inliningDepth);
+ }
+
public class InlinerOptions {
public boolean enableInlining =
@@ -1416,15 +1421,12 @@
// GMS Core.
public int inliningControlFlowResolutionBlocksThreshold = 15;
- // TODO(b/141451716): Evaluate the effect of allowing inlining in the inlinee.
- public boolean applyInliningToInlinee =
- System.getProperty("com.android.tools.r8.applyInliningToInlinee") != null;
- public int applyInliningToInlineeMaxDepth = 0;
-
public boolean enableInliningOfInvokesWithClassInitializationSideEffects = true;
public boolean enableInliningOfInvokesWithNullableReceivers = true;
public boolean disableInliningOfLibraryMethodOverrides = true;
+ public ApplyInliningToInlineePredicate applyInliningToInlineePredicateForTesting = null;
+
public int getSimpleInliningInstructionLimit() {
// If a custom simple inlining instruction limit is set, then use that.
if (simpleInliningInstructionLimit >= 0) {
@@ -1438,6 +1440,17 @@
assert isGeneratingDex();
return 5;
}
+
+ public boolean shouldApplyInliningToInlinee(
+ AppView<?> appView, ProgramMethod inlinee, int inliningDepth) {
+ if (applyInliningToInlineePredicateForTesting != null) {
+ return applyInliningToInlineePredicateForTesting.test(appView, inlinee, inliningDepth);
+ }
+ if (protoShrinking.shouldApplyInliningToInlinee(appView, inlinee, inliningDepth)) {
+ return true;
+ }
+ return false;
+ }
}
public class HorizontalClassMergerOptions {
@@ -1700,6 +1713,15 @@
public boolean isEnumLiteProtoShrinkingEnabled() {
return enableEnumLiteProtoShrinking;
}
+
+ public boolean shouldApplyInliningToInlinee(
+ AppView<?> appView, ProgramMethod inlinee, int inliningDepth) {
+ if (isProtoShrinkingEnabled() && inliningDepth == 1) {
+ ProtoReferences protoReferences = appView.protoShrinker().getProtoReferences();
+ return inlinee.getHolderType() == protoReferences.generatedMessageLiteType;
+ }
+ return false;
+ }
}
public static class TestingOptions {
diff --git a/src/test/java/com/android/tools/r8/internal/Chrome200430Test.java b/src/test/java/com/android/tools/r8/internal/Chrome200430Test.java
index 6314424..4e7c6e3 100644
--- a/src/test/java/com/android/tools/r8/internal/Chrome200430Test.java
+++ b/src/test/java/com/android/tools/r8/internal/Chrome200430Test.java
@@ -34,6 +34,10 @@
.addProgramFiles(getProgramFiles())
.addLibraryFiles(getLibraryFiles())
.addKeepRuleFiles(getKeepRuleFiles())
+ .addOptionsModification(
+ options -> options.getOpenClosedInterfacesOptions().suppressAllOpenInterfaces())
+ .allowUnnecessaryDontWarnWildcards()
+ .allowUnusedDontWarnPatterns()
.allowUnusedProguardConfigurationRules()
.setMinApi(AndroidApiLevel.N)
.compile();
diff --git a/src/test/java/com/android/tools/r8/internal/proto/Proto2BuilderShrinkingTest.java b/src/test/java/com/android/tools/r8/internal/proto/Proto2BuilderShrinkingTest.java
index 9ea0db9..262f164 100644
--- a/src/test/java/com/android/tools/r8/internal/proto/Proto2BuilderShrinkingTest.java
+++ b/src/test/java/com/android/tools/r8/internal/proto/Proto2BuilderShrinkingTest.java
@@ -15,6 +15,7 @@
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import com.android.tools.r8.utils.codeinspector.InstructionSubject;
import com.android.tools.r8.utils.codeinspector.MethodSubject;
@@ -24,9 +25,9 @@
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
-// TODO(b/112437944): Strengthen test to ensure that builder inlining succeeds even without single-
-// and double-caller inlining.
@RunWith(Parameterized.class)
public class Proto2BuilderShrinkingTest extends ProtoShrinkingTestBase {
@@ -36,10 +37,13 @@
private static List<Path> PROGRAM_FILES =
ImmutableList.of(PROTO2_EXAMPLES_JAR, PROTO2_PROTO_JAR, PROTOBUF_LITE_JAR);
- private final List<String> mains;
- private final TestParameters parameters;
+ @Parameter(0)
+ public List<String> mains;
- @Parameterized.Parameters(name = "{1}, {0}")
+ @Parameter(1)
+ public TestParameters parameters;
+
+ @Parameters(name = "{1}, {0}")
public static List<Object[]> data() {
return buildParameters(
ImmutableList.of(
@@ -59,11 +63,6 @@
getTestParameters().withDefaultDexRuntime().withAllApiLevels().build());
}
- public Proto2BuilderShrinkingTest(List<String> mains, TestParameters parameters) {
- this.mains = mains;
- this.parameters = parameters;
- }
-
@Test
public void test() throws Exception {
R8TestCompileResult result =
@@ -187,18 +186,46 @@
}
private void verifyMethodToInvokeValuesAreAbsent(CodeInspector outputInspector) {
+ ClassSubject generatedMessageLiteClassSubject =
+ outputInspector.clazz("com.google.protobuf.GeneratedMessageLite");
+ assertThat(generatedMessageLiteClassSubject, isPresent());
+
+ MethodSubject isInitializedMethodSubject =
+ generatedMessageLiteClassSubject.uniqueMethodWithName("isInitialized");
+
DexType methodToInvokeType =
- outputInspector.clazz(METHOD_TO_INVOKE_ENUM).getDexProgramClass().type;
+ outputInspector.clazz(METHOD_TO_INVOKE_ENUM).getDexProgramClass().getType();
for (String main : mains) {
MethodSubject mainMethodSubject = outputInspector.clazz(main).mainMethod();
assertThat(mainMethodSubject, isPresent());
+
+ // Verify that the calls to GeneratedMessageLite.createBuilder() have been inlined.
+ assertTrue(
+ mainMethodSubject
+ .streamInstructions()
+ .filter(InstructionSubject::isInvoke)
+ .map(InstructionSubject::getMethod)
+ .allMatch(
+ method ->
+ method.getHolderType()
+ != generatedMessageLiteClassSubject.getDexProgramClass().getType()
+ || (isInitializedMethodSubject.isPresent()
+ && method
+ == isInitializedMethodSubject
+ .getProgramMethod()
+ .getReference())));
+
+ // Verify that there are no accesses to MethodToInvoke after inlining createBuilder() -- and
+ // specifically no accesses to MethodToInvoke.NEW_BUILDER.
assertTrue(
main,
mainMethodSubject
.streamInstructions()
.filter(InstructionSubject::isStaticGet)
- .map(instruction -> instruction.getField().type)
+ .map(instruction -> instruction.getField().getType())
.noneMatch(methodToInvokeType::equals));
+
+ // Verify that there is no switches on the ordinal of a MethodToInvoke instance.
assertTrue(mainMethodSubject.streamInstructions().noneMatch(InstructionSubject::isSwitch));
}
}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/inliner/SingleTargetAfterInliningTest.java b/src/test/java/com/android/tools/r8/ir/optimize/inliner/SingleTargetAfterInliningTest.java
index 1f51c2f..e0a1d16 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/inliner/SingleTargetAfterInliningTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/inliner/SingleTargetAfterInliningTest.java
@@ -46,10 +46,9 @@
.addInnerClasses(SingleTargetAfterInliningTest.class)
.addKeepMainRule(TestClass.class)
.addOptionsModification(
- options -> {
- options.inlinerOptions().applyInliningToInlinee = true;
- options.inlinerOptions().applyInliningToInlineeMaxDepth = maxInliningDepth;
- })
+ options ->
+ options.inlinerOptions().applyInliningToInlineePredicateForTesting =
+ (appView, inlinee, inliningDepth) -> true)
.enableAlwaysInliningAnnotations()
.enableNeverClassInliningAnnotations()
.enableNoHorizontalClassMergingAnnotations()
