Unify type-may-have-finalizer checks
Change-Id: If7e74a4f0315c657749476f00a7bcee97d594602
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/fieldaccess/TrivialFieldAccessReprocessor.java b/src/main/java/com/android/tools/r8/ir/analysis/fieldaccess/TrivialFieldAccessReprocessor.java
index e8130e4..b06a015 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/fieldaccess/TrivialFieldAccessReprocessor.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/fieldaccess/TrivialFieldAccessReprocessor.java
@@ -4,7 +4,6 @@
package com.android.tools.r8.ir.analysis.fieldaccess;
-import static com.android.tools.r8.ir.analysis.type.Nullability.maybeNull;
import com.android.tools.r8.dex.Constants;
import com.android.tools.r8.graph.AppView;
@@ -20,8 +19,6 @@
import com.android.tools.r8.graph.FieldAccessInfo;
import com.android.tools.r8.graph.FieldAccessInfoCollection;
import com.android.tools.r8.graph.UseRegistry;
-import com.android.tools.r8.ir.analysis.type.ClassTypeLatticeElement;
-import com.android.tools.r8.ir.analysis.type.TypeLatticeElement;
import com.android.tools.r8.ir.analysis.value.AbstractValue;
import com.android.tools.r8.ir.analysis.value.SingleFieldValue;
import com.android.tools.r8.ir.analysis.value.SingleValue;
@@ -165,16 +162,8 @@
if (singleValue.isSingleFieldValue()) {
SingleFieldValue singleFieldValue = singleValue.asSingleFieldValue();
DexField singleField = singleFieldValue.getField();
- if (singleField == field.field) {
- return false;
- }
- if (singleField.type.isClassType()) {
- ClassTypeLatticeElement fieldType =
- TypeLatticeElement.fromDexType(singleFieldValue.getField().type, maybeNull(), appView)
- .asClassTypeLatticeElement();
- return !appView.appInfo().mayHaveFinalizeMethodDirectlyOrIndirectly(fieldType);
- }
- return true;
+ return singleField != field.field
+ && !singleFieldValue.mayHaveFinalizeMethodDirectlyOrIndirectly(appView);
}
}
return false;
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/value/SingleFieldValue.java b/src/main/java/com/android/tools/r8/ir/analysis/value/SingleFieldValue.java
index 8e48aad..dffd9e0 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/value/SingleFieldValue.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/value/SingleFieldValue.java
@@ -14,6 +14,7 @@
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.GraphLense;
+import com.android.tools.r8.ir.analysis.type.ClassTypeLatticeElement;
import com.android.tools.r8.ir.analysis.type.TypeLatticeElement;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.Instruction;
@@ -35,6 +36,18 @@
return field;
}
+ public boolean mayHaveFinalizeMethodDirectlyOrIndirectly(AppView<AppInfoWithLiveness> appView) {
+ DexType fieldType = field.type;
+ if (fieldType.isClassType()) {
+ ClassTypeLatticeElement fieldClassType =
+ TypeLatticeElement.fromDexType(fieldType, maybeNull(), appView)
+ .asClassTypeLatticeElement();
+ return appView.appInfo().mayHaveFinalizeMethodDirectlyOrIndirectly(fieldClassType);
+ }
+ assert fieldType.isArrayType() || fieldType.isPrimitiveType();
+ return false;
+ }
+
@Override
public boolean isSingleFieldValue() {
return true;
diff --git a/src/main/java/com/android/tools/r8/ir/code/FieldInstruction.java b/src/main/java/com/android/tools/r8/ir/code/FieldInstruction.java
index 42e8e4a..366f0cf 100644
--- a/src/main/java/com/android/tools/r8/ir/code/FieldInstruction.java
+++ b/src/main/java/com/android/tools/r8/ir/code/FieldInstruction.java
@@ -19,6 +19,7 @@
import com.android.tools.r8.ir.analysis.fieldvalueanalysis.UnknownFieldSet;
import com.android.tools.r8.ir.analysis.type.TypeLatticeElement;
import com.android.tools.r8.ir.analysis.value.AbstractValue;
+import com.android.tools.r8.ir.analysis.value.SingleFieldValue;
import com.android.tools.r8.ir.analysis.value.UnknownValue;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.google.common.collect.Sets;
@@ -198,7 +199,8 @@
* default finalize() method in a field. In that case, it is not safe to remove this instruction,
* since that could change the lifetime of the value.
*/
- boolean isStoringObjectWithFinalizer(AppInfoWithLiveness appInfo, DexEncodedField field) {
+ boolean isStoringObjectWithFinalizer(
+ AppView<AppInfoWithLiveness> appView, DexEncodedField field) {
assert isFieldPut();
TypeLatticeElement type = value().getTypeLattice();
@@ -208,25 +210,33 @@
return false;
}
- if (field.getOptimizationInfo().getAbstractValue().isZero()) {
- return false;
+ AbstractValue abstractValue = field.getOptimizationInfo().getAbstractValue();
+ if (abstractValue.isSingleValue()) {
+ if (abstractValue.isZero()) {
+ return false;
+ }
+ if (abstractValue.isSingleFieldValue()) {
+ SingleFieldValue singleFieldValue = abstractValue.asSingleFieldValue();
+ return singleFieldValue.mayHaveFinalizeMethodDirectlyOrIndirectly(appView);
+ }
}
+ AppInfoWithLiveness appInfo = appView.appInfo();
Value root = value().getAliasedValue();
if (!root.isPhi() && root.definition.isNewInstance()) {
- DexClass clazz = appInfo.definitionFor(root.definition.asNewInstance().clazz);
+ DexClass clazz = appView.definitionFor(root.definition.asNewInstance().clazz);
if (clazz == null) {
return true;
}
if (clazz.superType == null) {
return false;
}
- DexItemFactory dexItemFactory = appInfo.dexItemFactory();
+ DexItemFactory dexItemFactory = appView.dexItemFactory();
DexEncodedMethod resolutionResult =
appInfo
.resolveMethod(clazz.type, dexItemFactory.objectMembers.finalize)
.getSingleTarget();
- return resolutionResult != null && resolutionResult.isProgramMethod(appInfo);
+ return resolutionResult != null && resolutionResult.isProgramMethod(appView);
}
return appInfo.mayHaveFinalizeMethodDirectlyOrIndirectly(baseType.asClassTypeLatticeElement());
diff --git a/src/main/java/com/android/tools/r8/ir/code/InstancePut.java b/src/main/java/com/android/tools/r8/ir/code/InstancePut.java
index 12f3d93..d073a90 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InstancePut.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InstancePut.java
@@ -123,7 +123,8 @@
public boolean instructionMayHaveSideEffects(
AppView<?> appView, DexType context, Assumption assumption) {
if (appView.appInfo().hasLiveness()) {
- AppInfoWithLiveness appInfoWithLiveness = appView.appInfo().withLiveness();
+ AppView<AppInfoWithLiveness> appViewWithLiveness = appView.withLiveness();
+ AppInfoWithLiveness appInfoWithLiveness = appViewWithLiveness.appInfo();
if (instructionInstanceCanThrow(appView, context, assumption).isThrowing()) {
return true;
@@ -132,7 +133,7 @@
DexEncodedField encodedField = appInfoWithLiveness.resolveField(getField());
assert encodedField != null : "NoSuchFieldError (resolution failure) should be caught.";
return appInfoWithLiveness.isFieldRead(encodedField)
- || isStoringObjectWithFinalizer(appInfoWithLiveness, encodedField);
+ || isStoringObjectWithFinalizer(appViewWithLiveness, encodedField);
}
// In D8, we always have to assume that the field can be read, and thus have side effects.
diff --git a/src/main/java/com/android/tools/r8/ir/code/StaticPut.java b/src/main/java/com/android/tools/r8/ir/code/StaticPut.java
index 721a7bc..9d25691 100644
--- a/src/main/java/com/android/tools/r8/ir/code/StaticPut.java
+++ b/src/main/java/com/android/tools/r8/ir/code/StaticPut.java
@@ -102,7 +102,8 @@
public boolean instructionMayHaveSideEffects(
AppView<?> appView, DexType context, Assumption assumption) {
if (appView.appInfo().hasLiveness()) {
- AppInfoWithLiveness appInfoWithLiveness = appView.appInfo().withLiveness();
+ AppView<AppInfoWithLiveness> appViewWithLiveness = appView.withLiveness();
+ AppInfoWithLiveness appInfoWithLiveness = appViewWithLiveness.appInfo();
// MemberValuePropagation will replace the field read only if the target field has bound
// -assumevalues rule whose return value is *single*.
//
@@ -128,7 +129,7 @@
}
return appInfoWithLiveness.isFieldRead(encodedField)
- || isStoringObjectWithFinalizer(appInfoWithLiveness, encodedField);
+ || isStoringObjectWithFinalizer(appViewWithLiveness, encodedField);
}
// In D8, we always have to assume that the field can be read, and thus have side effects.
diff --git a/src/main/java/com/android/tools/r8/shaking/TreePruner.java b/src/main/java/com/android/tools/r8/shaking/TreePruner.java
index da42de6..1fb713e 100644
--- a/src/main/java/com/android/tools/r8/shaking/TreePruner.java
+++ b/src/main/java/com/android/tools/r8/shaking/TreePruner.java
@@ -18,13 +18,11 @@
import com.android.tools.r8.graph.EnclosingMethodAttribute;
import com.android.tools.r8.graph.InnerClassAttribute;
import com.android.tools.r8.graph.NestMemberClassAttribute;
-import com.android.tools.r8.ir.optimize.info.FieldOptimizationInfo;
import com.android.tools.r8.logging.Log;
import com.android.tools.r8.utils.ExceptionUtils;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.Timing;
import com.google.common.collect.Sets;
-import com.google.common.collect.Streams;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
@@ -177,9 +175,7 @@
clazz.removeEnclosingMethod(this::isAttributeReferencingPrunedItem);
rewriteNestAttributes(clazz);
usagePrinter.visited();
- assert Streams.stream(clazz.fields())
- .map(DexEncodedField::getOptimizationInfo)
- .noneMatch(FieldOptimizationInfo::isDead);
+ assert verifyNoDeadFields(clazz);
}
private void rewriteNestAttributes(DexProgramClass clazz) {
@@ -360,4 +356,12 @@
public Collection<DexReference> getMethodsToKeepForConfigurationDebugging() {
return Collections.unmodifiableCollection(methodsToKeepForConfigurationDebugging);
}
+
+ private boolean verifyNoDeadFields(DexProgramClass clazz) {
+ for (DexEncodedField field : clazz.fields()) {
+ assert !field.getOptimizationInfo().isDead()
+ : "Expected field `" + field.field.toSourceString() + "` to be absent";
+ }
+ return true;
+ }
}
