Synthesize null-check to enable inlining of invokes with nullable receiver
Bug: 130202534, 113859358
Change-Id: I28d37f8e109fc4080617bf68ee6d3e41c12465bf
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 25dd6e8..b451490 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -144,8 +144,7 @@
public boolean enableNonNullTracking = true;
public boolean enableInlining =
!Version.isDev() || System.getProperty("com.android.tools.r8.disableinlining") == null;
- // TODO(b/130202534): Enable this once code size regressions are fixed.
- public boolean enableInliningOfInvokesWithNullableReceivers = false;
+ public boolean enableInliningOfInvokesWithNullableReceivers = true;
public boolean enableClassInlining = true;
public boolean enableClassStaticizer = true;
public boolean enableInitializedClassesAnalysis = true;
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineInvokeWithNullableReceiverTest.java b/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineInvokeWithNullableReceiverTest.java
index bb42f7c..c00b563 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineInvokeWithNullableReceiverTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineInvokeWithNullableReceiverTest.java
@@ -6,8 +6,9 @@
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
import com.android.tools.r8.R8TestCompileResult;
import com.android.tools.r8.TestBase;
@@ -66,16 +67,14 @@
assertThat(methodSubject, isPresent());
// A `throw` instruction should have been synthesized into main().
- // TODO(b/130202534): Allow inlining.
- assertFalse(methodSubject.streamInstructions().anyMatch(InstructionSubject::isThrow));
+ assertTrue(methodSubject.streamInstructions().anyMatch(InstructionSubject::isThrow));
// Class A is still present because the instance flows into a phi that has a null-check.
ClassSubject otherClassSubject = inspector.clazz(A.class);
assertThat(otherClassSubject, isPresent());
// Method A.m() should no longer be present due to inlining.
- // TODO(b/130202534): Allow inlining.
- assertThat(otherClassSubject.uniqueMethodWithName("m"), isPresent());
+ assertThat(otherClassSubject.uniqueMethodWithName("m"), not(isPresent()));
}
static class TestClass {
diff --git a/src/test/java/com/android/tools/r8/naming/b130791310/B130791310.java b/src/test/java/com/android/tools/r8/naming/b130791310/B130791310.java
index 40482d3..40cd981 100644
--- a/src/test/java/com/android/tools/r8/naming/b130791310/B130791310.java
+++ b/src/test/java/com/android/tools/r8/naming/b130791310/B130791310.java
@@ -7,17 +7,20 @@
import static com.android.tools.r8.utils.codeinspector.Matchers.isRenamed;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assume.assumeFalse;
import com.android.tools.r8.ProguardTestBuilder;
import com.android.tools.r8.R8FullTestBuilder;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.ir.optimize.Inliner.Reason;
import com.android.tools.r8.utils.BooleanUtils;
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.MethodSubject;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import java.util.List;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -86,14 +89,16 @@
);
private final boolean enableClassMerging;
+ private final boolean onlyForceInlining;
- @Parameterized.Parameters(name = "enable class merging: {0}")
- public static Boolean[] data() {
- return BooleanUtils.values();
+ @Parameterized.Parameters(name = "enable class merging: {0}, only force inlining: {1}")
+ public static List<Object[]> data() {
+ return buildParameters(BooleanUtils.values(), BooleanUtils.values());
}
- public B130791310(boolean enableClassMerging) {
+ public B130791310(boolean enableClassMerging, boolean onlyForceInlining) {
this.enableClassMerging = enableClassMerging;
+ this.onlyForceInlining = onlyForceInlining;
}
private void inspect(CodeInspector inspector, boolean isR8) {
@@ -101,18 +106,28 @@
assertThat(holder, isPresent());
assertThat(holder, not(isRenamed()));
MethodSubject someMethod = holder.uniqueMethodWithName("someMethod");
- if (enableClassMerging && !isR8) {
- // Note that the method is not entirely gone, but merged to the implementer, along with some
- // method signature modification.
- assertThat(someMethod, not(isPresent()));
+ if (isR8) {
+ if (onlyForceInlining) {
+ assertThat(someMethod, isPresent());
+ assertThat(someMethod, not(isRenamed()));
+ } else {
+ assertThat(someMethod, not(isPresent()));
+ }
} else {
- assertThat(someMethod, isPresent());
- assertThat(someMethod, not(isRenamed()));
+ if (enableClassMerging) {
+ // Note that the method is not entirely gone, but merged to the implementer, along with some
+ // method signature modification.
+ assertThat(someMethod, not(isPresent()));
+ } else {
+ assertThat(someMethod, isPresent());
+ assertThat(someMethod, not(isRenamed()));
+ }
}
}
@Test
public void testProguard() throws Exception {
+ assumeFalse(onlyForceInlining);
ProguardTestBuilder builder =
testForProguard()
.addProgramClasses(CLASSES)
@@ -138,6 +153,10 @@
if (!enableClassMerging) {
builder.addOptionsModification(o -> o.enableVerticalClassMerging = false);
}
+ if (onlyForceInlining) {
+ builder.addOptionsModification(
+ o -> o.testing.validInliningReasons = ImmutableSet.of(Reason.FORCE));
+ }
builder
.compile()
.inspect(inspector -> inspect(inspector, true));
