Reland: Fix bug making us not inline when a simple class without clinit had static fields with nothing setting them
The conditional was reversed, so if there was fields, we would only
allow inlining if there was a field that had an allocation side effect.
Fix MethodHandle inlining. We simply newer inline methods that has const method handles in them (javac will not produce these)
This gives 50K on chrome
Bug: 122683370, 123358657
Change-Id: Iecb187b54cee4930eb4a96ca4fc57fefc8bfd285
diff --git a/src/main/java/com/android/tools/r8/graph/DexClass.java b/src/main/java/com/android/tools/r8/graph/DexClass.java
index d9940f3..b229074 100644
--- a/src/main/java/com/android/tools/r8/graph/DexClass.java
+++ b/src/main/java/com/android/tools/r8/graph/DexClass.java
@@ -474,7 +474,7 @@
public boolean defaultValuesForStaticFieldsMayTriggerAllocation() {
return Arrays.stream(staticFields())
- .anyMatch(field -> !field.getStaticValue().mayTriggerAllocation());
+ .anyMatch(field -> field.getStaticValue().mayTriggerAllocation());
}
public List<InnerClassAttribute> getInnerClasses() {
diff --git a/src/main/java/com/android/tools/r8/ir/code/ConstMethodHandle.java b/src/main/java/com/android/tools/r8/ir/code/ConstMethodHandle.java
index e0b57f1..64740ba 100644
--- a/src/main/java/com/android/tools/r8/ir/code/ConstMethodHandle.java
+++ b/src/main/java/com/android/tools/r8/ir/code/ConstMethodHandle.java
@@ -13,6 +13,8 @@
import com.android.tools.r8.ir.analysis.type.TypeLatticeElement;
import com.android.tools.r8.ir.conversion.CfBuilder;
import com.android.tools.r8.ir.conversion.DexBuilder;
+import com.android.tools.r8.ir.optimize.Inliner.ConstraintWithTarget;
+import com.android.tools.r8.ir.optimize.InliningConstraints;
public class ConstMethodHandle extends ConstInstruction {
@@ -68,6 +70,12 @@
}
@Override
+ public ConstraintWithTarget inliningConstraint(
+ InliningConstraints inliningConstraints, DexType invocationContext) {
+ return inliningConstraints.forConstMethodHandle();
+ }
+
+ @Override
public int maxOutValueRegister() {
return Constants.U8BIT_MAX;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java b/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java
index 3292704..873eb20 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java
@@ -259,6 +259,10 @@
return ConstraintWithTarget.ALWAYS;
}
+ public ConstraintWithTarget forConstMethodHandle() {
+ return ConstraintWithTarget.NEVER;
+ }
+
private ConstraintWithTarget forFieldInstruction(
DexField field, DexEncodedField target, DexType invocationContext) {
// Resolve the field if possible and decide whether the instruction can inlined.
diff --git a/src/main/java/com/android/tools/r8/jar/InliningConstraintVisitor.java b/src/main/java/com/android/tools/r8/jar/InliningConstraintVisitor.java
index 18998c8..4bfb5ce 100644
--- a/src/main/java/com/android/tools/r8/jar/InliningConstraintVisitor.java
+++ b/src/main/java/com/android/tools/r8/jar/InliningConstraintVisitor.java
@@ -21,6 +21,7 @@
import com.android.tools.r8.ir.optimize.Inliner.ConstraintWithTarget;
import com.android.tools.r8.ir.optimize.InliningConstraints;
import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
+import org.objectweb.asm.Handle;
import org.objectweb.asm.MethodVisitor;
import org.objectweb.asm.Opcodes;
import org.objectweb.asm.Type;
@@ -116,6 +117,8 @@
if (cst instanceof Type && ((Type) cst).getSort() != Type.METHOD) {
DexType type = application.getType((Type) cst);
updateConstraint(inliningConstraints.forConstClass(type, invocationContext));
+ } else if (cst instanceof Handle) {
+ updateConstraint(inliningConstraints.forConstMethodHandle());
} else {
updateConstraint(inliningConstraints.forConstInstruction());
}
diff --git a/src/test/java/com/android/tools/r8/cf/MethodHandleTestRunner.java b/src/test/java/com/android/tools/r8/cf/MethodHandleTestRunner.java
index 46ed96b..da70125 100644
--- a/src/test/java/com/android/tools/r8/cf/MethodHandleTestRunner.java
+++ b/src/test/java/com/android/tools/r8/cf/MethodHandleTestRunner.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.cf;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
import com.android.tools.r8.ByteDataView;
import com.android.tools.r8.ClassFileConsumer;
@@ -23,10 +24,14 @@
import com.android.tools.r8.utils.AndroidApiLevel;
import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.Reporter;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import java.io.IOException;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
+import java.util.concurrent.ExecutionException;
import org.junit.Assume;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -133,6 +138,16 @@
ProcessResult runCf = ToolHelper.runJava(outCf, CLASS.getCanonicalName(), expected);
assertEquals(runCf.stderr, 0, runCf.exitCode);
assertEquals(runInput.toString(), runCf.toString());
+ // Ensure that we did not inline the const method handle
+ ensureConstHandleNotInlined(outCf);
+ }
+
+ private void ensureConstHandleNotInlined(Path file) throws IOException, ExecutionException {
+ CodeInspector inspector = new CodeInspector(file);
+ MethodSubject subject = inspector.clazz(MethodHandleTest.D.class).method(
+ "java.lang.MethodHandle", "vcviSpecialMethod");
+ assertTrue(inspector.clazz(MethodHandleTest.D.class)
+ .method("java.lang.invoke.MethodHandle", "vcviSpecialMethod").isPresent());
}
private void runDex() throws Exception {
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineWithSimpleFieldNoValue.java b/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineWithSimpleFieldNoValue.java
new file mode 100644
index 0000000..eac5881
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineWithSimpleFieldNoValue.java
@@ -0,0 +1,43 @@
+// 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 junit.framework.TestCase.assertTrue;
+
+import com.android.tools.r8.R8TestRunResult;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.utils.StringUtils;
+import org.junit.Test;
+
+class TestClass {
+ public static void main(String[] args) {
+ System.out.println(InlineFrom.getValue());
+ InlineFrom.value = 43;
+ System.out.println(InlineFrom.value);
+ }
+}
+
+// Simple class, with no clinit and no static field initialization.
+// We should always inline getValue()
+// We ensure that we use value.
+class InlineFrom {
+ public static int value;
+
+ public static int getValue() {
+ return 42;
+ }
+}
+
+public class InlineWithSimpleFieldNoValue extends TestBase {
+ @Test
+ public void test() throws Exception {
+ R8TestRunResult result = testForR8(Backend.DEX)
+ .addKeepMainRule(TestClass.class)
+ .addProgramClasses(TestClass.class, InlineFrom.class)
+ .run(TestClass.class)
+ .assertSuccessWithOutput(StringUtils.lines("42", "43"));
+ assertTrue(result.inspector().clazz(InlineFrom.class).allMethods().isEmpty());
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/kotlin/R8KotlinPropertiesTest.java b/src/test/java/com/android/tools/r8/kotlin/R8KotlinPropertiesTest.java
index ac75230..bc23757 100644
--- a/src/test/java/com/android/tools/r8/kotlin/R8KotlinPropertiesTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/R8KotlinPropertiesTest.java
@@ -910,11 +910,10 @@
MemberNaming.MethodSignature getter = testedClass.getGetterForProperty(propertyName);
MemberNaming.MethodSignature setter = testedClass.getSetterForProperty(propertyName);
- // Field is public and getter is small so we expect to always inline it.
+ // Field is public and getter/setter is only called from one place so we expect to always
+ // inline it.
checkMethodIsRemoved(objectClass, getter);
-
- // Setter has null check of new value, thus may not be inlined.
- checkMethodIsKept(objectClass, setter);
+ checkMethodIsRemoved(objectClass, setter);
});
}
@@ -934,7 +933,7 @@
MemberNaming.MethodSignature setter = testedClass.getSetterForProperty(propertyName);
checkMethodIsRemoved(objectClass, getter);
- checkMethodIsKept(objectClass, setter);
+ checkMethodIsRemoved(objectClass, setter);
assertTrue(fieldSubject.getField().accessFlags.isPublic());
});
}
diff --git a/src/test/java/com/android/tools/r8/shaking/testrules/C.java b/src/test/java/com/android/tools/r8/shaking/testrules/C.java
index 79370a4..7a30a65 100644
--- a/src/test/java/com/android/tools/r8/shaking/testrules/C.java
+++ b/src/test/java/com/android/tools/r8/shaking/testrules/C.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.shaking.testrules;
+import com.android.tools.r8.NeverInline;
import com.android.tools.r8.NeverMerge;
@NeverMerge
@@ -11,6 +12,7 @@
private static int i;
+ @NeverInline
public static int x() {
return i;
}
diff --git a/src/test/java/com/android/tools/r8/shaking/testrules/ForceInlineTest.java b/src/test/java/com/android/tools/r8/shaking/testrules/ForceInlineTest.java
index 432711f..4bf8c92 100644
--- a/src/test/java/com/android/tools/r8/shaking/testrules/ForceInlineTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/testrules/ForceInlineTest.java
@@ -69,6 +69,7 @@
ImmutableList.of(
"-keep class **.Main { *; }",
"-nevermerge @com.android.tools.r8.NeverMerge class *",
+ "-neverinline class *{ @com.android.tools.r8.NeverInline <methods>;}",
"-dontobfuscate"));
ClassSubject classA = inspector.clazz(A.class);
@@ -99,6 +100,7 @@
"-neverinline class **.B { method(); }",
"-keep class **.Main { *; }",
"-nevermerge @com.android.tools.r8.NeverMerge class *",
+ "-neverinline class *{ @com.android.tools.r8.NeverInline <methods>;}",
"-dontobfuscate"));
ClassSubject classA = inspector.clazz(A.class);
@@ -126,6 +128,7 @@
"-forceinline class **.B { int m(int, int); }",
"-keep class **.Main { *; }",
"-nevermerge @com.android.tools.r8.NeverMerge class *",
+ "-neverinline class *{ @com.android.tools.r8.NeverInline <methods>;}",
"-dontobfuscate"));
ClassSubject classA = inspector.clazz(A.class);
