Handle final fields that are assigned more than once in constructors
Change-Id: I85486a04dedd4765ba8aa136930eb37b37cdbec3
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/fieldvalueanalysis/FieldValueAnalysis.java b/src/main/java/com/android/tools/r8/ir/analysis/fieldvalueanalysis/FieldValueAnalysis.java
index e4bf2a9..fd8dbf7 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/fieldvalueanalysis/FieldValueAnalysis.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/fieldvalueanalysis/FieldValueAnalysis.java
@@ -15,20 +15,36 @@
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.InstructionIterator;
+import com.android.tools.r8.ir.code.InvokeDirect;
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.ir.optimize.ClassInitializerDefaultsOptimization.ClassInitializerDefaultsResult;
import com.android.tools.r8.ir.optimize.info.OptimizationFeedback;
+import com.android.tools.r8.ir.optimize.info.field.InstanceFieldInitializationInfo;
+import com.android.tools.r8.ir.optimize.info.field.UnknownInstanceFieldInitializationInfo;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.DequeUtils;
+import com.android.tools.r8.utils.ListUtils;
+import java.util.ArrayList;
import java.util.Deque;
import java.util.IdentityHashMap;
-import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
public abstract class FieldValueAnalysis {
+ static class FieldInitializationInfo {
+
+ private final Instruction instruction;
+ private final InstanceFieldInitializationInfo instanceFieldInitializationInfo;
+
+ FieldInitializationInfo(
+ Instruction instruction, InstanceFieldInitializationInfo instanceFieldInitializationInfo) {
+ this.instruction = instruction;
+ this.instanceFieldInitializationInfo = instanceFieldInitializationInfo;
+ }
+ }
+
final AppView<AppInfoWithLiveness> appView;
final IRCode code;
final ProgramMethod context;
@@ -37,7 +53,7 @@
private DominatorTree dominatorTree;
private Map<BasicBlock, AbstractFieldSet> fieldsMaybeReadBeforeBlockInclusiveCache;
- final Map<DexEncodedField, LinkedList<FieldInstruction>> putsPerField = new IdentityHashMap<>();
+ final Map<DexEncodedField, List<FieldInitializationInfo>> putsPerField = new IdentityHashMap<>();
FieldValueAnalysis(
AppView<AppInfoWithLiveness> appView, IRCode code, OptimizationFeedback feedback) {
@@ -61,8 +77,27 @@
return fieldsMaybeReadBeforeBlockInclusiveCache;
}
+ boolean isInstanceFieldValueAnalysis() {
+ return false;
+ }
+
+ InstanceFieldValueAnalysis asInstanceFieldValueAnalysis() {
+ return null;
+ }
+
abstract boolean isSubjectToOptimization(DexEncodedField field);
+ void recordFieldPut(DexEncodedField field, Instruction instruction) {
+ recordFieldPut(field, instruction, UnknownInstanceFieldInitializationInfo.getInstance());
+ }
+
+ void recordFieldPut(
+ DexEncodedField field, Instruction instruction, InstanceFieldInitializationInfo info) {
+ putsPerField
+ .computeIfAbsent(field, ignore -> new ArrayList<>())
+ .add(new FieldInitializationInfo(instruction, info));
+ }
+
/** This method analyzes initializers with the purpose of computing field optimization info. */
void computeFieldOptimizationInfo(ClassInitializerDefaultsResult classInitializerDefaultsResult) {
AppInfoWithLiveness appInfo = appView.appInfo();
@@ -80,31 +115,42 @@
DexField field = fieldPut.getField();
DexEncodedField encodedField = appInfo.resolveField(field).getResolvedField();
if (encodedField != null && isSubjectToOptimization(encodedField)) {
- putsPerField.computeIfAbsent(encodedField, ignore -> new LinkedList<>()).add(fieldPut);
+ recordFieldPut(encodedField, fieldPut);
}
+ } else if (isInstanceFieldValueAnalysis()
+ && instruction.isInvokeConstructor(appView.dexItemFactory())) {
+ InvokeDirect invoke = instruction.asInvokeDirect();
+ asInstanceFieldValueAnalysis().analyzeForwardingConstructorCall(invoke, code.getThis());
}
}
}
List<BasicBlock> normalExitBlocks = code.computeNormalExitBlocks();
- for (Entry<DexEncodedField, LinkedList<FieldInstruction>> entry : putsPerField.entrySet()) {
- DexEncodedField encodedField = entry.getKey();
- LinkedList<FieldInstruction> fieldPuts = entry.getValue();
+ for (Entry<DexEncodedField, List<FieldInitializationInfo>> entry : putsPerField.entrySet()) {
+ DexEncodedField field = entry.getKey();
+ List<FieldInitializationInfo> fieldPuts = entry.getValue();
if (fieldPuts.size() > 1) {
continue;
}
- FieldInstruction fieldPut = fieldPuts.getFirst();
+ FieldInitializationInfo info = ListUtils.first(fieldPuts);
+ Instruction instruction = info.instruction;
+ if (instruction.isInvokeDirect()) {
+ asInstanceFieldValueAnalysis()
+ .recordInstanceFieldIsInitializedWithInfo(field, info.instanceFieldInitializationInfo);
+ continue;
+ }
+ FieldInstruction fieldPut = instruction.asFieldInstruction();
if (!isStraightLineCode) {
if (!getOrCreateDominatorTree().dominatesAllOf(fieldPut.getBlock(), normalExitBlocks)) {
continue;
}
}
boolean priorReadsWillReadSameValue =
- !classInitializerDefaultsResult.hasStaticValue(encodedField) && fieldPut.value().isZero();
- if (!priorReadsWillReadSameValue && fieldMaybeReadBeforeInstruction(encodedField, fieldPut)) {
+ !classInitializerDefaultsResult.hasStaticValue(field) && fieldPut.value().isZero();
+ if (!priorReadsWillReadSameValue && fieldMaybeReadBeforeInstruction(field, fieldPut)) {
continue;
}
- updateFieldOptimizationInfo(encodedField, fieldPut, fieldPut.value());
+ updateFieldOptimizationInfo(field, fieldPut, fieldPut.value());
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/fieldvalueanalysis/InstanceFieldValueAnalysis.java b/src/main/java/com/android/tools/r8/ir/analysis/fieldvalueanalysis/InstanceFieldValueAnalysis.java
index b818cc2..f508479 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/fieldvalueanalysis/InstanceFieldValueAnalysis.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/fieldvalueanalysis/InstanceFieldValueAnalysis.java
@@ -11,6 +11,7 @@
import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.ir.analysis.type.ClassTypeElement;
import com.android.tools.r8.ir.analysis.type.TypeElement;
import com.android.tools.r8.ir.analysis.value.AbstractValue;
@@ -26,8 +27,10 @@
import com.android.tools.r8.ir.optimize.ClassInitializerDefaultsOptimization.ClassInitializerDefaultsResult;
import com.android.tools.r8.ir.optimize.info.OptimizationFeedback;
import com.android.tools.r8.ir.optimize.info.field.EmptyInstanceFieldInitializationInfoCollection;
+import com.android.tools.r8.ir.optimize.info.field.InstanceFieldInitializationInfo;
import com.android.tools.r8.ir.optimize.info.field.InstanceFieldInitializationInfoCollection;
import com.android.tools.r8.ir.optimize.info.field.InstanceFieldInitializationInfoFactory;
+import com.android.tools.r8.ir.optimize.info.field.UnknownInstanceFieldInitializationInfo;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.Timing;
@@ -106,6 +109,16 @@
}
@Override
+ boolean isInstanceFieldValueAnalysis() {
+ return true;
+ }
+
+ @Override
+ InstanceFieldValueAnalysis asInstanceFieldValueAnalysis() {
+ return this;
+ }
+
+ @Override
boolean isSubjectToOptimization(DexEncodedField field) {
return !field.isStatic() && field.holder() == context.getHolderType();
}
@@ -113,11 +126,45 @@
@Override
void updateFieldOptimizationInfo(DexEncodedField field, FieldInstruction fieldPut, Value value) {
if (fieldNeverWrittenBetweenInstancePutAndMethodExit(field, fieldPut.asInstancePut())) {
- recordFieldIsInitializedWithValue(field, value);
+ recordInstanceFieldIsInitializedWithValue(field, value);
+ }
+ }
+
+ void analyzeForwardingConstructorCall(InvokeDirect invoke, Value thisValue) {
+ if (invoke.getReceiver() != thisValue
+ || invoke.getInvokedMethod().getHolderType() != context.getHolderType()) {
+ // Not a forwarding constructor call.
+ return;
+ }
+
+ ProgramMethod singleTarget = invoke.lookupSingleProgramTarget(appView, context);
+ if (singleTarget == null) {
+ // Failure, should generally not happen.
+ return;
+ }
+
+ InstanceFieldInitializationInfoCollection infos =
+ singleTarget
+ .getDefinition()
+ .getOptimizationInfo()
+ .getInstanceInitializerInfo()
+ .fieldInitializationInfos();
+ for (DexEncodedField field : singleTarget.getHolder().instanceFields()) {
+ assert isSubjectToOptimization(field);
+ InstanceFieldInitializationInfo info = infos.get(field);
+ if (info.isArgumentInitializationInfo()) {
+ int argumentIndex = info.asArgumentInitializationInfo().getArgumentIndex();
+ info = getInstanceFieldInitializationInfo(field, invoke.getArgument(argumentIndex));
+ }
+ recordFieldPut(field, invoke, info);
}
}
private void analyzeParentConstructorCall() {
+ if (parentConstructor.getHolderType() == context.getHolderType()) {
+ // Forwarding constructor calls are handled similar to instance-put instructions.
+ return;
+ }
InstanceFieldInitializationInfoCollection infos =
parentConstructor
.getOptimizationInfo()
@@ -129,8 +176,8 @@
if (fieldNeverWrittenBetweenParentConstructorCallAndMethodExit(field)) {
if (info.isArgumentInitializationInfo()) {
int argumentIndex = info.asArgumentInitializationInfo().getArgumentIndex();
- recordFieldIsInitializedWithValue(
- field, parentConstructorCall.arguments().get(argumentIndex));
+ recordInstanceFieldIsInitializedWithValue(
+ field, parentConstructorCall.getArgument(argumentIndex));
} else {
assert info.isSingleValue() || info.isTypeInitializationInfo();
builder.recordInitializationInfo(field, info);
@@ -139,32 +186,39 @@
});
}
- private void recordFieldIsInitializedWithValue(DexEncodedField field, Value value) {
+ private InstanceFieldInitializationInfo getInstanceFieldInitializationInfo(
+ DexEncodedField field, Value value) {
Value root = value.getAliasedValue();
if (root.isDefinedByInstructionSatisfying(Instruction::isArgument)) {
Argument argument = root.definition.asArgument();
- builder.recordInitializationInfo(
- field, factory.createArgumentInitializationInfo(argument.getIndex()));
- return;
+ return factory.createArgumentInitializationInfo(argument.getIndex());
}
-
AbstractValue abstractValue = value.getAbstractValue(appView, context);
if (abstractValue.isSingleValue()) {
- builder.recordInitializationInfo(field, abstractValue.asSingleValue());
- return;
+ return abstractValue.asSingleValue();
}
-
- DexType fieldType = field.field.type;
+ DexType fieldType = field.type();
if (fieldType.isClassType()) {
ClassTypeElement dynamicLowerBoundType = value.getDynamicLowerBoundType(appView);
TypeElement dynamicUpperBoundType = value.getDynamicUpperBoundType(appView);
TypeElement staticFieldType = TypeElement.fromDexType(fieldType, maybeNull(), appView);
if (dynamicLowerBoundType != null || !dynamicUpperBoundType.equals(staticFieldType)) {
- builder.recordInitializationInfo(
- field,
- factory.createTypeInitializationInfo(dynamicLowerBoundType, dynamicUpperBoundType));
+ return factory.createTypeInitializationInfo(dynamicLowerBoundType, dynamicUpperBoundType);
}
}
+ return UnknownInstanceFieldInitializationInfo.getInstance();
+ }
+
+ void recordInstanceFieldIsInitializedWithInfo(
+ DexEncodedField field, InstanceFieldInitializationInfo info) {
+ if (!info.isUnknown()) {
+ builder.recordInitializationInfo(field, info);
+ }
+ }
+
+ void recordInstanceFieldIsInitializedWithValue(DexEncodedField field, Value value) {
+ recordInstanceFieldIsInitializedWithInfo(
+ field, getInstanceFieldInitializationInfo(field, value));
}
private boolean fieldNeverWrittenBetweenInstancePutAndMethodExit(
diff --git a/src/main/java/com/android/tools/r8/ir/code/Instruction.java b/src/main/java/com/android/tools/r8/ir/code/Instruction.java
index 6a42c80..62c94b8 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Instruction.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Instruction.java
@@ -9,6 +9,7 @@
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DebugLocalInfo;
+import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.ir.analysis.ClassInitializationAnalysis.AnalysisAssumption;
@@ -1240,6 +1241,10 @@
return null;
}
+ public boolean isInvokeConstructor(DexItemFactory dexItemFactory) {
+ return false;
+ }
+
public boolean isInvokeDirect() {
return false;
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/InvokeDirect.java b/src/main/java/com/android/tools/r8/ir/code/InvokeDirect.java
index 6d452e1..7d488a0 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InvokeDirect.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InvokeDirect.java
@@ -8,6 +8,7 @@
import com.android.tools.r8.dex.Constants;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexEncodedMethod;
+import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.ProgramMethod;
@@ -109,6 +110,11 @@
}
@Override
+ public boolean isInvokeConstructor(DexItemFactory dexItemFactory) {
+ return getInvokedMethod().isInstanceInitializer(dexItemFactory);
+ }
+
+ @Override
public boolean isInvokeDirect() {
return true;
}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/fields/NonFinalFinalFieldTest.java b/src/test/java/com/android/tools/r8/ir/optimize/fields/NonFinalFinalFieldTest.java
new file mode 100644
index 0000000..292f74e
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/fields/NonFinalFinalFieldTest.java
@@ -0,0 +1,170 @@
+// Copyright (c) 2020, 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.fields;
+
+import static org.junit.Assume.assumeTrue;
+
+import com.android.tools.r8.NeverClassInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.graph.AccessFlags;
+import com.android.tools.r8.transformers.MethodTransformer;
+import com.google.common.collect.ImmutableList;
+import java.util.List;
+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 NonFinalFinalFieldTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public NonFinalFinalFieldTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testJvm() throws Exception {
+ assumeTrue(parameters.isCfRuntime());
+ testForJvm()
+ .addProgramClasses(TestClass.class)
+ .addProgramClassFileData(getProgramClassFileData())
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("2", "2", "2");
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ testForR8(parameters.getBackend())
+ .addProgramClasses(TestClass.class)
+ .addProgramClassFileData(getProgramClassFileData())
+ .addKeepMainRule(TestClass.class)
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("2", "2", "2");
+ }
+
+ private List<byte[]> getProgramClassFileData() throws Exception {
+ return ImmutableList.of(
+ transformer(A.class)
+ .setAccessFlags(A.class.getDeclaredField("f"), AccessFlags::setFinal)
+ .transform(),
+ transformer(B.class)
+ .setAccessFlags(B.class.getDeclaredField("f"), AccessFlags::setFinal)
+ .addMethodTransformer(createRemoveObjectInitMethodTransformer())
+ .transform(),
+ transformer(C.class)
+ .setAccessFlags(C.class.getDeclaredField("f"), AccessFlags::setFinal)
+ .addMethodTransformer(createRemoveObjectInitMethodTransformer())
+ .transform());
+ }
+
+ private MethodTransformer createRemoveObjectInitMethodTransformer() {
+ return new MethodTransformer() {
+
+ private boolean seenInit = false;
+
+ @Override
+ public void visitMethodInsn(
+ int opcode, String owner, String name, String descriptor, boolean isInterface) {
+ if (isDefaultConstructor()) {
+ if (name.equals("<init>")) {
+ seenInit = true;
+ return;
+ }
+ super.visitMethodInsn(
+ opcode, owner, name.equals("init") ? "<init>" : name, descriptor, isInterface);
+ } else {
+ super.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
+ }
+ }
+
+ @Override
+ public void visitVarInsn(int opcode, int var) {
+ if (isDefaultConstructor()) {
+ if (seenInit) {
+ super.visitVarInsn(opcode, var);
+ }
+ } else {
+ super.visitVarInsn(opcode, var);
+ }
+ }
+
+ private boolean isDefaultConstructor() {
+ return getContext().getReference().getMethodName().equals("<init>")
+ && getContext().getReference().getFormalTypes().isEmpty();
+ }
+ };
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ System.out.println(new A().f);
+ System.out.println(new B().f);
+ System.out.println(new C().f);
+ }
+ }
+
+ @NeverClassInline
+ static class A {
+
+ /*final*/ int f;
+
+ A() {
+ this(1);
+ this.f = 2;
+ }
+
+ A(int f) {
+ this.f = f;
+ }
+ }
+
+ @NeverClassInline
+ static class B {
+
+ /*final*/ int f;
+
+ B() {
+ this.f = 1;
+ init(2); // Rewritten to B(2)
+ }
+
+ B(int f) {
+ this.f = f;
+ }
+
+ private void init(int f) {}
+ }
+
+ @NeverClassInline
+ static class C {
+
+ /*final*/ int f;
+
+ C() {
+ this.f = 1;
+ init(2, true); // Rewritten to B(2, true)
+ }
+
+ C(int f, boolean b) {
+ if (b) {
+ this.f = f;
+ }
+ }
+
+ private void init(int f, boolean b) {}
+ }
+}
