Use nop inlining constraint for dead code elimination
Bug: 191131830
Bug: 132600418
Bug: 139276374
Change-Id: Ia5f5fc8c1541ff98210700c4d4eddefd3ed977f9
diff --git a/src/main/java/com/android/tools/r8/graph/DexClassAndField.java b/src/main/java/com/android/tools/r8/graph/DexClassAndField.java
index e2deec9..2c78b93 100644
--- a/src/main/java/com/android/tools/r8/graph/DexClassAndField.java
+++ b/src/main/java/com/android/tools/r8/graph/DexClassAndField.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.graph;
+import com.android.tools.r8.ir.optimize.info.FieldOptimizationInfo;
import com.android.tools.r8.references.FieldReference;
public abstract class DexClassAndField extends DexClassAndMember<DexEncodedField, DexField> {
@@ -35,6 +36,11 @@
return getReference().asFieldReference();
}
+ @Override
+ public FieldOptimizationInfo getOptimizationInfo() {
+ return getDefinition().getOptimizationInfo();
+ }
+
public DexType getType() {
return getReference().getType();
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexClassAndMember.java b/src/main/java/com/android/tools/r8/graph/DexClassAndMember.java
index 2ac7ced..4aebe97 100644
--- a/src/main/java/com/android/tools/r8/graph/DexClassAndMember.java
+++ b/src/main/java/com/android/tools/r8/graph/DexClassAndMember.java
@@ -5,6 +5,7 @@
package com.android.tools.r8.graph;
import com.android.tools.r8.errors.Unreachable;
+import com.android.tools.r8.ir.optimize.info.MemberOptimizationInfo;
import com.android.tools.r8.origin.Origin;
public abstract class DexClassAndMember<D extends DexEncodedMember<D, R>, R extends DexMember<D, R>>
@@ -49,6 +50,10 @@
return definition.getReference();
}
+ public MemberOptimizationInfo<?> getOptimizationInfo() {
+ return getDefinition().getOptimizationInfo();
+ }
+
@Override
public Origin getOrigin() {
return holder.origin;
diff --git a/src/main/java/com/android/tools/r8/graph/DexClassAndMethod.java b/src/main/java/com/android/tools/r8/graph/DexClassAndMethod.java
index ccc7dbc..53cbfe8 100644
--- a/src/main/java/com/android/tools/r8/graph/DexClassAndMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexClassAndMethod.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.graph;
+import com.android.tools.r8.ir.optimize.info.MethodOptimizationInfo;
import com.android.tools.r8.references.MethodReference;
public abstract class DexClassAndMethod extends DexClassAndMember<DexEncodedMethod, DexMethod>
@@ -52,6 +53,11 @@
return getReference().getSignature();
}
+ @Override
+ public MethodOptimizationInfo getOptimizationInfo() {
+ return getDefinition().getOptimizationInfo();
+ }
+
public DexType getParameter(int index) {
return getReference().getParameter(index);
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/InvokeMethodWithReceiver.java b/src/main/java/com/android/tools/r8/ir/code/InvokeMethodWithReceiver.java
index 39f52cb..6a83961 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InvokeMethodWithReceiver.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InvokeMethodWithReceiver.java
@@ -269,6 +269,6 @@
}
}
- return optimizationInfo.mayHaveSideEffects();
+ return optimizationInfo.mayHaveSideEffects(this, appView.options());
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/InvokeStatic.java b/src/main/java/com/android/tools/r8/ir/code/InvokeStatic.java
index 1b3aef3..9313b59 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InvokeStatic.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InvokeStatic.java
@@ -216,11 +216,11 @@
}
// Verify that the target method does not have side-effects.
- if (appViewWithLiveness.appInfo().noSideEffects.containsKey(singleTarget.getReference())) {
+ if (appViewWithLiveness.appInfo().isAssumeNoSideEffectsMethod(singleTarget)) {
return false;
}
- if (singleTarget.getDefinition().getOptimizationInfo().mayHaveSideEffects()) {
+ if (singleTarget.getOptimizationInfo().mayHaveSideEffects(this, appView.options())) {
return true;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/IdempotentFunctionCallCanonicalizer.java b/src/main/java/com/android/tools/r8/ir/optimize/IdempotentFunctionCallCanonicalizer.java
index 50443c6..b3dd6b6 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/IdempotentFunctionCallCanonicalizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/IdempotentFunctionCallCanonicalizer.java
@@ -160,7 +160,7 @@
}
MethodOptimizationInfo optimizationInfo = target.getDefinition().getOptimizationInfo();
- if (optimizationInfo.mayHaveSideEffects()
+ if (optimizationInfo.mayHaveSideEffects(invoke, appViewWithLiveness.options())
|| !optimizationInfo.returnValueOnlyDependsOnArguments()) {
continue;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/DefaultMethodOptimizationInfo.java b/src/main/java/com/android/tools/r8/ir/optimize/info/DefaultMethodOptimizationInfo.java
index 0aed133..a98bc38 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/info/DefaultMethodOptimizationInfo.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/info/DefaultMethodOptimizationInfo.java
@@ -12,11 +12,13 @@
import com.android.tools.r8.ir.analysis.value.AbstractValue;
import com.android.tools.r8.ir.analysis.value.UnknownValue;
import com.android.tools.r8.ir.code.InvokeDirect;
+import com.android.tools.r8.ir.code.InvokeMethod;
import com.android.tools.r8.ir.optimize.classinliner.constraint.ClassInlinerMethodConstraint;
import com.android.tools.r8.ir.optimize.info.bridge.BridgeInfo;
import com.android.tools.r8.ir.optimize.info.initializer.DefaultInstanceInitializerInfo;
import com.android.tools.r8.ir.optimize.info.initializer.InstanceInitializerInfo;
import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.InternalOptions;
import com.google.common.collect.ImmutableSet;
import java.util.BitSet;
import java.util.Set;
@@ -134,6 +136,11 @@
}
@Override
+ public SimpleInliningConstraint getNopInliningConstraint(InternalOptions options) {
+ return NeverSimpleInliningConstraint.getInstance();
+ }
+
+ @Override
public SimpleInliningConstraint getSimpleInliningConstraint() {
return NeverSimpleInliningConstraint.getInstance();
}
@@ -169,6 +176,11 @@
}
@Override
+ public boolean mayHaveSideEffects(InvokeMethod invoke, InternalOptions options) {
+ return UNKNOWN_MAY_HAVE_SIDE_EFFECTS;
+ }
+
+ @Override
public boolean returnValueOnlyDependsOnArguments() {
return UNKNOWN_RETURN_VALUE_ONLY_DEPENDS_ON_ARGUMENTS;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfo.java b/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfo.java
index 748d82d..a25eabb 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfo.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfo.java
@@ -12,6 +12,7 @@
import com.android.tools.r8.ir.analysis.type.TypeElement;
import com.android.tools.r8.ir.analysis.value.AbstractValue;
import com.android.tools.r8.ir.code.InvokeDirect;
+import com.android.tools.r8.ir.code.InvokeMethod;
import com.android.tools.r8.ir.optimize.classinliner.constraint.ClassInlinerMethodConstraint;
import com.android.tools.r8.ir.optimize.info.bridge.BridgeInfo;
import com.android.tools.r8.ir.optimize.info.initializer.InstanceInitializerInfo;
@@ -78,6 +79,8 @@
public abstract AbstractValue getAbstractReturnValue();
+ public abstract SimpleInliningConstraint getNopInliningConstraint(InternalOptions options);
+
public abstract SimpleInliningConstraint getSimpleInliningConstraint();
public abstract boolean forceInline();
@@ -90,6 +93,9 @@
public abstract boolean mayHaveSideEffects();
+ /** Context sensitive version of {@link #mayHaveSideEffects()}. */
+ public abstract boolean mayHaveSideEffects(InvokeMethod invoke, InternalOptions options);
+
public abstract boolean returnValueOnlyDependsOnArguments();
public abstract boolean returnValueHasBeenPropagated();
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/MutableMethodOptimizationInfo.java b/src/main/java/com/android/tools/r8/ir/optimize/info/MutableMethodOptimizationInfo.java
index 460b2a2..5b99126 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/info/MutableMethodOptimizationInfo.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/info/MutableMethodOptimizationInfo.java
@@ -17,6 +17,7 @@
import com.android.tools.r8.ir.analysis.value.AbstractValue;
import com.android.tools.r8.ir.analysis.value.UnknownValue;
import com.android.tools.r8.ir.code.InvokeDirect;
+import com.android.tools.r8.ir.code.InvokeMethod;
import com.android.tools.r8.ir.optimize.classinliner.constraint.ClassInlinerMethodConstraint;
import com.android.tools.r8.ir.optimize.info.bridge.BridgeInfo;
import com.android.tools.r8.ir.optimize.info.initializer.InstanceInitializerInfo;
@@ -24,6 +25,7 @@
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.AndroidApiLevel;
import com.android.tools.r8.utils.BooleanUtils;
+import com.android.tools.r8.utils.InternalOptions;
import java.util.BitSet;
import java.util.Optional;
import java.util.Set;
@@ -322,6 +324,15 @@
}
@Override
+ public SimpleInliningConstraint getNopInliningConstraint(InternalOptions options) {
+ // We currently require that having a simple inlining constraint implies that the method becomes
+ // empty after inlining. Therefore, an invoke is a nop if the simple inlining constraint is
+ // satisfied (if the invoke does not trigger other side effects, such as class initialization).
+ assert options.simpleInliningConstraintThreshold == 0;
+ return getSimpleInliningConstraint();
+ }
+
+ @Override
public SimpleInliningConstraint getSimpleInliningConstraint() {
return simpleInliningConstraint;
}
@@ -357,6 +368,17 @@
}
@Override
+ public boolean mayHaveSideEffects(InvokeMethod invoke, InternalOptions options) {
+ if (!mayHaveSideEffects()) {
+ return false;
+ }
+ if (getNopInliningConstraint(options).isSatisfied(invoke)) {
+ return false;
+ }
+ return true;
+ }
+
+ @Override
public boolean returnValueOnlyDependsOnArguments() {
return isFlagSet(RETURN_VALUE_ONLY_DEPENDS_ON_ARGUMENTS_FLAG);
}
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index b75ac5a..0a1fcc7 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -263,7 +263,7 @@
public boolean enableInliningOfInvokesWithNullableReceivers = true;
public boolean disableInliningOfLibraryMethodOverrides = true;
public boolean enableSimpleInliningConstraints = true;
- public int simpleInliningConstraintThreshold = 0;
+ public final int simpleInliningConstraintThreshold = 0;
public boolean enableClassInlining = true;
public boolean enableClassStaticizer = true;
public boolean enableInitializedClassesAnalysis = true;
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/inliner/conditionalsimpleinlining/NopInliningConstraintTest.java b/src/test/java/com/android/tools/r8/ir/optimize/inliner/conditionalsimpleinlining/NopInliningConstraintTest.java
index c6cbbe4..3295203 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/inliner/conditionalsimpleinlining/NopInliningConstraintTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/inliner/conditionalsimpleinlining/NopInliningConstraintTest.java
@@ -55,10 +55,10 @@
mainClassSubject.uniqueMethodWithName("checkNotNull");
assertThat(checkNotNullMethodSubject, isPresent());
- // There are two calls to checkNotNull() in main().
- // TODO(b/191131830): Should only be one.
+ // There is a single call to checkNotNull() in main(), as checkNotNull(newObject())
+ // is dead code eliminated.
assertEquals(
- 2,
+ 1,
mainClassSubject
.mainMethod()
.streamInstructions()
diff --git a/src/test/java/com/android/tools/r8/kotlin/KotlinIntrinsicsInlineTest.java b/src/test/java/com/android/tools/r8/kotlin/KotlinIntrinsicsInlineTest.java
index 872b5f0..f6cc385 100644
--- a/src/test/java/com/android/tools/r8/kotlin/KotlinIntrinsicsInlineTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/KotlinIntrinsicsInlineTest.java
@@ -130,10 +130,10 @@
MethodSubject method = main.uniqueMethodWithName(methodName);
assertThat(method, isPresent());
int arity = method.getMethod().getReference().getArity();
- // One from the method's own argument, if any, and
- // Two from Array utils, `contains` and `indexOf`, if inlined with access relaxation.
+ // One for each of the method's own arguments, unless building with
+ // -allowaccessmodification.
assertEquals(
- allowAccessModification ? 0 : arity + 2,
+ allowAccessModification ? 0 : arity,
countCall(method, "checkParameterIsNotNull"));
});
}
diff --git a/src/test/java/com/android/tools/r8/kotlin/SimplifyIfNotNullKotlinTest.java b/src/test/java/com/android/tools/r8/kotlin/SimplifyIfNotNullKotlinTest.java
index 4769427..e83626b 100644
--- a/src/test/java/com/android/tools/r8/kotlin/SimplifyIfNotNullKotlinTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/SimplifyIfNotNullKotlinTest.java
@@ -91,7 +91,7 @@
// ?: in aOrDefault
// TODO(b/179951729): Not the same amount of ifz on CF and DEX.
assertEquals(testParameters.isCfRuntime() ? 0 : 1, ifzCount);
- assertEquals(allowAccessModification ? 0 : 1, paramNullCheckCount);
+ assertEquals(0, paramNullCheckCount);
});
}
