Version 1.4.38
Cherry pick: Reland: Fix bug making us not inline when a simple class without clinit had static fields with nothing setting them
CL: https://r8-review.googlesource.com/c/r8/+/33843
Bug: 122683370, 123358657
Change-Id: If6631a44765a449d2e7b7a5e2f56609324a29002
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index b64a3c7..f8b366e 100644
--- a/src/main/java/com/android/tools/r8/Version.java
+++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@
// This field is accessed from release scripts using simple pattern matching.
// Therefore, changing this field could break our release scripts.
- public static final String LABEL = "1.4.37";
+ public static final String LABEL = "1.4.38";
private Version() {
}
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);
