Reproduce b/139432507: double-inliner misses simple inlining candidates.
If a certain method is double-inlining candidate (by calling a
double-inlining target) but itself is an inlining candidate as a single
caller, that method is inlined into the caller while its body still has
potentially inlinable method call.
Double-inliner, later, reprocesses those double callers, which has been
already inlined into the single caller. It should either reprocess that
single caller or allow double-inlining target to be inlined in the
beginning.
Bug: 139432507
Change-Id: I4005bb5bc499f9e0444c00e7af3b63c3fb722307
diff --git a/src/test/java/com/android/tools/r8/TestBase.java b/src/test/java/com/android/tools/r8/TestBase.java
index f0dd1c1..8123f4f 100644
--- a/src/test/java/com/android/tools/r8/TestBase.java
+++ b/src/test/java/com/android/tools/r8/TestBase.java
@@ -1094,14 +1094,24 @@
}
protected long countCall(MethodSubject method, String className, String methodName) {
- return Streams.stream(method.iterateInstructions(instructionSubject -> {
+ return method.streamInstructions().filter(instructionSubject -> {
if (instructionSubject.isInvoke()) {
DexMethod invokedMethod = instructionSubject.getMethod();
return invokedMethod.holder.toString().contains(className)
&& invokedMethod.name.toString().contains(methodName);
}
return false;
- })).count();
+ }).count();
+ }
+
+ protected long countCall(MethodSubject method, String methodName) {
+ return method.streamInstructions().filter(instructionSubject -> {
+ if (instructionSubject.isInvoke()) {
+ DexMethod invokedMethod = instructionSubject.getMethod();
+ return invokedMethod.name.toString().contains(methodName);
+ }
+ return false;
+ }).count();
}
public enum MinifyMode {
diff --git a/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java b/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
index c974e78..99cd060 100644
--- a/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
+++ b/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
@@ -157,6 +157,13 @@
return self();
}
+ public T allowAccessModification(boolean allowAccessModification) {
+ if (allowAccessModification) {
+ return addKeepRules("-allowaccessmodification");
+ }
+ return self();
+ }
+
public T addKeepAttributes(String... attributes) {
return addKeepRules("-keepattributes " + String.join(",", attributes));
}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/inliner/DoubleInliningNullCheckTest.java b/src/test/java/com/android/tools/r8/ir/optimize/inliner/DoubleInliningNullCheckTest.java
new file mode 100644
index 0000000..5c9eb22
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/inliner/DoubleInliningNullCheckTest.java
@@ -0,0 +1,89 @@
+// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package com.android.tools.r8.ir.optimize.inliner;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class DoubleInliningNullCheckTest extends TestBase {
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection params() {
+ return getTestParameters().withAllRuntimes().build();
+ }
+
+ private final TestParameters parameters;
+
+ public DoubleInliningNullCheckTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(DoubleInliningNullCheckTest.class)
+ .addKeepMainRule(TestClass.class)
+ .noMinification()
+ .setMinApi(parameters.getRuntime())
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("true")
+ .inspect(codeInspector -> {
+ ClassSubject main = codeInspector.clazz(TestClass.class);
+ assertThat(main, isPresent());
+ MethodSubject mainMethod = main.mainMethod();
+ assertThat(mainMethod, isPresent());
+ // TODO(b/139432507): can be 0 if it's inlined to contains and indexOf.
+ assertEquals(2, countCall(mainMethod, "checkParameterIsNotNull"));
+ });
+ }
+
+ static class TestClass {
+ public static void main(String... args) {
+ String[] array = new String[] { "1", "2", "3" };
+ System.out.println(ArrayUtil.contains(array, "3"));
+ }
+ }
+
+ static class ArrayUtil {
+ public static void throwParameterIsNull(String paramName) {
+ throw new NullPointerException(paramName);
+ }
+
+ // Has double callers, indexOf and contains
+ public static void checkParameterIsNotNull(Object param, String paramName) {
+ if (param == null) {
+ throwParameterIsNull(paramName);
+ }
+ }
+
+ // Has single caller, contains
+ public static int indexOf(Object[] array, Object subject) {
+ checkParameterIsNotNull(array, "array");
+ for (int i = 0; i < array.length; i++) {
+ if (array[i].equals(subject)) {
+ return i;
+ }
+ }
+ return -1;
+ }
+
+ // Has single caller, TestClass#main
+ public static boolean contains(Object[] array, Object subject) {
+ checkParameterIsNotNull(array, "array");
+ return indexOf(array, subject) != -1;
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/kotlin/KotlinIntrinsicsInlineTest.java b/src/test/java/com/android/tools/r8/kotlin/KotlinIntrinsicsInlineTest.java
new file mode 100644
index 0000000..c055493
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/kotlin/KotlinIntrinsicsInlineTest.java
@@ -0,0 +1,107 @@
+// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package com.android.tools.r8.kotlin;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.ToolHelper.KotlinTargetVersion;
+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.MethodSubject;
+import java.util.Collection;
+import org.junit.Ignore;
+import org.junit.Test;
+import org.junit.runners.Parameterized;
+
+public class KotlinIntrinsicsInlineTest extends AbstractR8KotlinTestBase {
+ private static final String FOLDER = "intrinsics";
+ private static final String MAIN = FOLDER + ".InlineKt";
+
+ @Parameterized.Parameters(name = "{0} target: {1}, allowAccessModification: {2}")
+ public static Collection<Object[]> data() {
+ return buildParameters(
+ getTestParameters().withAllRuntimes().build(),
+ KotlinTargetVersion.values(),
+ BooleanUtils.values());
+ }
+
+ private final TestParameters parameters;
+
+ public KotlinIntrinsicsInlineTest(
+ TestParameters parameters,
+ KotlinTargetVersion targetVersion,
+ boolean allowAccessModification) {
+ super(targetVersion, allowAccessModification);
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void b139432507() throws Exception {
+ testForR8(parameters.getBackend())
+ .addProgramFiles(getKotlinJarFile(FOLDER))
+ .addKeepRules(StringUtils.lines(
+ "-keepclasseswithmembers class " + MAIN + "{",
+ " public static *** *(...);",
+ "}"))
+ .allowAccessModification(allowAccessModification)
+ .noMinification()
+ .setMinApi(parameters.getRuntime())
+ .compile()
+ .inspect(inspector -> {
+ ClassSubject main = inspector.clazz(MAIN);
+ assertThat(main, isPresent());
+
+ // Note that isSupported itself has a parameter whose null check would be inlined
+ // with -allowaccessmodification.
+ MethodSubject isSupported = main.uniqueMethodWithName("isSupported");
+ assertThat(isSupported, isPresent());
+ assertEquals(
+ allowAccessModification ? 0 : 1,
+ countCall(isSupported, "checkParameterIsNotNull"));
+
+ // In general cases, null check won't be invoked only once or twice, hence no subtle
+ // situation in double inlining.
+ MethodSubject containsArray = main.uniqueMethodWithName("containsArray");
+ assertThat(containsArray, isPresent());
+ assertEquals(0, countCall(containsArray, "checkParameterIsNotNull"));
+ });
+ }
+
+ @Test
+ public void b139432507_isSupported() throws Exception {
+ testSingle("isSupported");
+ }
+
+ @Ignore("b/139432507: due to double inlining, checkParameterIsNotNull is not fully inlined.")
+ @Test
+ public void b139432507_containsArray() throws Exception {
+ testSingle("containsArray");
+ }
+
+ private void testSingle(String methodName) throws Exception {
+ testForR8(parameters.getBackend())
+ .addProgramFiles(getKotlinJarFile(FOLDER))
+ .addKeepRules(StringUtils.lines(
+ "-keepclasseswithmembers class " + MAIN + "{",
+ " public static *** " + methodName + "(...);",
+ "}"))
+ .allowAccessModification(allowAccessModification)
+ .noMinification()
+ .setMinApi(parameters.getRuntime())
+ .compile()
+ .inspect(inspector -> {
+ ClassSubject main = inspector.clazz(MAIN);
+ assertThat(main, isPresent());
+
+ MethodSubject method = main.uniqueMethodWithName(methodName);
+ assertThat(method, isPresent());
+ assertEquals(
+ allowAccessModification, countCall(method, "checkParameterIsNotNull") == 0);
+ });
+ }
+}
diff --git a/src/test/kotlinR8TestResources/intrinsics/inline.kt b/src/test/kotlinR8TestResources/intrinsics/inline.kt
new file mode 100644
index 0000000..3234dea
--- /dev/null
+++ b/src/test/kotlinR8TestResources/intrinsics/inline.kt
@@ -0,0 +1,12 @@
+// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package intrinsics
+
+fun isSupported(flag: String): Boolean {
+ return flag in arrayOf("--foo", "--bar", "--baz")
+}
+
+fun containsArray() {
+ println("1" in arrayOf("1", "2", "3"))
+}
