Revert "Relax the dead condition of field reads."
This reverts commit 3fc9942045d6dc19d734b8ddc12703c1727df753.
Reason for revert: test status failures on 6.0.1
Bug: 124039115, 70295684
Change-Id: I66403a0fd6958ef8dc729a440186636532d06d72
diff --git a/src/main/java/com/android/tools/r8/ir/code/InstanceGet.java b/src/main/java/com/android/tools/r8/ir/code/InstanceGet.java
index 14d7ffb..40756ae 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InstanceGet.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InstanceGet.java
@@ -4,8 +4,6 @@
package com.android.tools.r8.ir.code;
-import static com.android.tools.r8.optimize.MemberRebindingAnalysis.isVisibleFromOriginalContext;
-
import com.android.tools.r8.cf.LoadStoreHelper;
import com.android.tools.r8.cf.TypeVerificationHelper;
import com.android.tools.r8.cf.code.CfFieldInstruction;
@@ -21,7 +19,6 @@
import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.AppInfoWithSubtyping;
import com.android.tools.r8.graph.AppView;
-import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexType;
@@ -101,24 +98,6 @@
}
@Override
- public boolean canBeDeadCode(AppInfo appInfo, IRCode code) {
- // instance-get can be dead code as long as it cannot have any of the following:
- // * NoSuchFieldError (resolution failure)
- // * IllegalAccessError (not visible from the access context)
- // * NullPointerException (null receiver).
- // TODO(b/123857022): Should be possible to use definitionFor().
- DexEncodedField resolvedField = appInfo.resolveFieldOn(getField().getHolder(), getField());
- if (resolvedField == null) {
- return false;
- }
- if (code == null
- || !isVisibleFromOriginalContext(appInfo, code.method.method.getHolder(), resolvedField)) {
- return false;
- }
- return object().getTypeLattice().nullability().isDefinitelyNotNull();
- }
-
- @Override
public int maxInValueRegister() {
return Constants.U4BIT_MAX;
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/StaticGet.java b/src/main/java/com/android/tools/r8/ir/code/StaticGet.java
index 80e74f3..b79d076 100644
--- a/src/main/java/com/android/tools/r8/ir/code/StaticGet.java
+++ b/src/main/java/com/android/tools/r8/ir/code/StaticGet.java
@@ -3,8 +3,6 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.ir.code;
-import static com.android.tools.r8.optimize.MemberRebindingAnalysis.isVisibleFromOriginalContext;
-
import com.android.tools.r8.cf.LoadStoreHelper;
import com.android.tools.r8.cf.TypeVerificationHelper;
import com.android.tools.r8.cf.code.CfFieldInstruction;
@@ -20,7 +18,6 @@
import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.AppInfoWithSubtyping;
import com.android.tools.r8.graph.AppView;
-import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.ir.analysis.ClassInitializationAnalysis;
@@ -94,25 +91,6 @@
}
@Override
- public boolean canBeDeadCode(AppInfo appInfo, IRCode code) {
- // static-get can be dead as long as it cannot have any of the following:
- // * NoSuchFieldError (resolution failure)
- // * IllegalAccessError (not visible from the access context)
- // * side-effects in <clinit>
- // TODO(b/123857022): Should be possible to use definitionFor().
- DexEncodedField resolvedField = appInfo.resolveFieldOn(getField().getHolder(), getField());
- if (resolvedField == null) {
- return false;
- }
- if (code == null
- || !isVisibleFromOriginalContext(appInfo, code.method.method.getHolder(), resolvedField)) {
- return false;
- }
- return appInfo.hasSubtyping()
- && !getField().getHolder().classInitializationMayHaveSideEffects(appInfo);
- }
-
- @Override
public int maxInValueRegister() {
return Constants.U8BIT_MAX;
}
diff --git a/src/main/java/com/android/tools/r8/optimize/MemberRebindingAnalysis.java b/src/main/java/com/android/tools/r8/optimize/MemberRebindingAnalysis.java
index 60aec09..608f915 100644
--- a/src/main/java/com/android/tools/r8/optimize/MemberRebindingAnalysis.java
+++ b/src/main/java/com/android/tools/r8/optimize/MemberRebindingAnalysis.java
@@ -3,7 +3,6 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.optimize;
-import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexEncodedField;
@@ -292,15 +291,14 @@
Set<DexEncodedMethod> contexts = fieldsWithContexts.get(field);
if (target != null && target.field != field
&& contexts.stream().allMatch(context ->
- isVisibleFromOriginalContext(appInfo, context.method.getHolder(), target))) {
+ isVisibleFromOriginalContext(context.method.getHolder(), target))) {
builder.map(field,
lense.lookupField(validTargetFor(target.field, field, lookupTargetOnClass)));
}
}
}
- public static boolean isVisibleFromOriginalContext(
- AppInfo appInfo, DexType context, DexEncodedField field) {
+ private boolean isVisibleFromOriginalContext(DexType context, DexEncodedField field) {
DexType holderType = field.field.getHolder();
DexClass holder = appInfo.definitionFor(holderType);
if (holder == null) {
diff --git a/src/test/java/com/android/tools/r8/jasmin/MemberResolutionTest.java b/src/test/java/com/android/tools/r8/jasmin/MemberResolutionTest.java
index ea8ed0b..33f28fc 100644
--- a/src/test/java/com/android/tools/r8/jasmin/MemberResolutionTest.java
+++ b/src/test/java/com/android/tools/r8/jasmin/MemberResolutionTest.java
@@ -462,7 +462,7 @@
main.addMainMethod(
".limit stack 2",
".limit locals 1",
- " getstatic Empty/aField I",
+ " getstatic Empty/aMethod I",
" return");
ensureRuntimeException(builder, NoSuchFieldError.class);
diff --git a/src/test/java/com/android/tools/r8/shaking/FieldReadsJasminTest.java b/src/test/java/com/android/tools/r8/shaking/FieldReadsJasminTest.java
deleted file mode 100644
index ac1fd90..0000000
--- a/src/test/java/com/android/tools/r8/shaking/FieldReadsJasminTest.java
+++ /dev/null
@@ -1,180 +0,0 @@
-// 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.shaking;
-
-import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
-
-import com.android.tools.r8.jasmin.JasminBuilder;
-import com.android.tools.r8.jasmin.JasminBuilder.ClassBuilder;
-import com.android.tools.r8.jasmin.JasminTestBase;
-import com.android.tools.r8.naming.MemberNaming.MethodSignature;
-import com.android.tools.r8.utils.codeinspector.ClassSubject;
-import com.android.tools.r8.utils.codeinspector.InstructionSubject;
-import com.android.tools.r8.utils.codeinspector.MethodSubject;
-import com.google.common.collect.ImmutableList;
-import java.util.Iterator;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-
-@RunWith(Parameterized.class)
-public class FieldReadsJasminTest extends JasminTestBase {
- private static final String CLS = "Empty";
- private static final String MAIN = "Main";
- private final Backend backend;
-
- @Parameterized.Parameters(name = "Backend: {0}")
- public static Object[] data() {
- return Backend.values();
- }
-
- public FieldReadsJasminTest(Backend backend) {
- this.backend = backend;
- }
-
- @Test
- public void testInstanceGet_nonNullReceiver() throws Exception {
- JasminBuilder builder = new JasminBuilder();
-
- ClassBuilder empty = builder.addClass(CLS);
- empty.addField("protected", "aField", "I", null);
-
- ClassBuilder main = builder.addClass(MAIN);
- main.addMainMethod(
- ".limit stack 2",
- ".limit locals 1",
- " new Empty",
- " dup",
- " invokespecial Empty/<init>()V",
- " getfield Empty/aField I",
- " return");
-
- ensureNoFields(builder, empty);
- }
-
- @Test
- public void testStaticGet_noSideEffect() throws Exception {
- JasminBuilder builder = new JasminBuilder();
-
- ClassBuilder empty = builder.addClass(CLS);
- empty.addStaticField("sField", "I");
-
- ClassBuilder main = builder.addClass(MAIN);
- main.addMainMethod(
- ".limit stack 2",
- ".limit locals 1",
- " getstatic Empty/sField I",
- " return");
-
- ensureNoFields(builder, empty);
- }
-
- private void ensureNoFields(JasminBuilder app, ClassBuilder clazz) throws Exception {
- testForR8(backend)
- .addProgramClassFileData(app.buildClasses())
- .addKeepRules("-keep class * { <methods>; }")
- .compile()
- .inspect(inspector -> {
- ClassSubject classSubject = inspector.clazz(clazz.name);
- assertThat(classSubject, isPresent());
- classSubject.forAllFields(foundFieldSubject -> {
- fail("Expect not to see any fields.");
- });
- });
- }
-
- @Test
- public void testInstanceGet_nullableReceiver() throws Exception {
- JasminBuilder builder = new JasminBuilder();
-
- ClassBuilder empty = builder.addClass(CLS);
- empty.addDefaultConstructor();
- empty.addField("protected", "aField", "I", null);
- MethodSignature foo = empty.addStaticMethod("foo", ImmutableList.of("L" + CLS + ";"), "V",
- ".limit stack 2",
- ".limit locals 1",
- " aload 0",
- " getfield Empty/aField I",
- " return");
-
- ClassBuilder main = builder.addClass(MAIN);
- main.addMainMethod(
- ".limit stack 2",
- ".limit locals 1",
- " aconst_null",
- " invokestatic Empty/foo(L" + CLS + ";)V",
- " return");
-
- ensureFieldExists(builder, empty, foo, "aField");
- }
-
- @Test
- public void testStaticGet_nonTrivialClinit() throws Exception {
- JasminBuilder builder = new JasminBuilder();
-
- ClassBuilder empty = builder.addClass(CLS);
- empty.addDefaultConstructor();
- empty.addStaticField("sField", "I");
- empty.addClassInitializer(
- ".limit stack 3",
- ".limit locals 1",
- " getstatic java/lang/System/out Ljava/io/PrintStream;",
- " ldc \"hello\"",
- " invokevirtual java/io/PrintStream/print(Ljava/lang/String;)V",
- " return");
-
- ClassBuilder main = builder.addClass(MAIN);
- MethodSignature mainMethod = main.addMainMethod(
- ".limit stack 2",
- ".limit locals 1",
- " getstatic Empty/sField I",
- " return");
-
- ensureFieldExists(builder, main, mainMethod, "sField");
- }
-
- @Test
- public void testStaticGet_allocation() throws Exception {
- JasminBuilder builder = new JasminBuilder();
-
- ClassBuilder empty = builder.addClass(CLS);
- empty.addDefaultConstructor();
- empty.addStaticField("sField", "Ljava/lang/String;", "\"8\"");
-
- ClassBuilder main = builder.addClass(MAIN);
- MethodSignature mainMethod = main.addMainMethod(
- ".limit stack 2",
- ".limit locals 1",
- " getstatic Empty/sField Ljava/lang/String;",
- " return");
-
- ensureFieldExists(builder, main, mainMethod, "sField");
- }
-
- private void ensureFieldExists(
- JasminBuilder app, ClassBuilder clazz, MethodSignature method, String fieldName)
- throws Exception {
- testForR8(backend)
- .addProgramClassFileData(app.buildClasses())
- .addKeepRules("-keep class * { <methods>; }")
- .compile()
- .inspect(inspector -> {
- ClassSubject classSubject = inspector.clazz(clazz.name);
- assertThat(classSubject, isPresent());
- MethodSubject methodSubject = classSubject.uniqueMethodWithName(method.name);
- assertThat(methodSubject, isPresent());
- Iterator<InstructionSubject> it =
- methodSubject.iterateInstructions(InstructionSubject::isFieldAccess);
- assertTrue(it.hasNext());
- assertEquals(
- inspector.clazz(CLS).uniqueFieldWithName(fieldName).getFinalName(),
- it.next().getField().name.toString());
- });
- }
-
-}
