Test mixed argument removal and enum unboxing
Change-Id: Ic9741cccedd20570bee1d35488b93feb2a3e6aa0
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
index c68adac..b6c7d02 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -88,6 +88,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Map;
+import java.util.function.BiConsumer;
import java.util.function.BiFunction;
import java.util.function.Consumer;
import java.util.function.Function;
@@ -1627,12 +1628,19 @@
public Builder adjustOptimizationInfoAfterRemovingThisParameter(
AppView<AppInfoWithLiveness> appView) {
- if (optimizationInfo.isMutableOptimizationInfo()) {
- optimizationInfo
- .asMutableMethodOptimizationInfo()
- .adjustOptimizationInfoAfterRemovingThisParameter(appView);
- }
- return this;
+ return modifyOptimizationInfo(
+ (newMethod, optimizationInfo) ->
+ optimizationInfo.adjustOptimizationInfoAfterRemovingThisParameter(appView));
+ }
+
+ public Builder modifyOptimizationInfo(
+ BiConsumer<DexEncodedMethod, MutableMethodOptimizationInfo> consumer) {
+ return addBuildConsumer(
+ newMethod -> {
+ if (optimizationInfo.isMutableOptimizationInfo()) {
+ consumer.accept(newMethod, optimizationInfo.asMutableMethodOptimizationInfo());
+ }
+ });
}
public Builder setCode(Code code) {
diff --git a/src/main/java/com/android/tools/r8/graph/RewrittenPrototypeDescription.java b/src/main/java/com/android/tools/r8/graph/RewrittenPrototypeDescription.java
index 4304fdd..bd96228 100644
--- a/src/main/java/com/android/tools/r8/graph/RewrittenPrototypeDescription.java
+++ b/src/main/java/com/android/tools/r8/graph/RewrittenPrototypeDescription.java
@@ -15,12 +15,14 @@
import com.android.tools.r8.utils.BooleanUtils;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Ordering;
+import it.unimi.dsi.fastutil.ints.Int2ReferenceMap.Entry;
import it.unimi.dsi.fastutil.ints.Int2ReferenceRBTreeMap;
import it.unimi.dsi.fastutil.ints.Int2ReferenceSortedMap;
import it.unimi.dsi.fastutil.ints.IntBidirectionalIterator;
import it.unimi.dsi.fastutil.ints.IntSortedSet;
import java.util.ArrayList;
import java.util.Collections;
+import java.util.Iterator;
import java.util.List;
import java.util.function.Consumer;
@@ -222,6 +224,10 @@
return this == EMPTY;
}
+ public Iterator<Entry<ArgumentInfo>> iterator() {
+ return argumentInfos.int2ReferenceEntrySet().iterator();
+ }
+
public boolean hasRemovedArguments() {
for (ArgumentInfo value : argumentInfos.values()) {
if (value.isRemovedArgumentInfo()) {
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/MethodOptimizationFeedback.java b/src/main/java/com/android/tools/r8/ir/conversion/MethodOptimizationFeedback.java
index 13b7140..53bb277 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/MethodOptimizationFeedback.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/MethodOptimizationFeedback.java
@@ -66,6 +66,8 @@
void setEnumUnboxerMethodClassification(
ProgramMethod method, EnumUnboxerMethodClassification enumUnboxerMethodClassification);
+ void unsetEnumUnboxerMethodClassification(ProgramMethod method);
+
void setInstanceInitializerInfoCollection(
DexEncodedMethod method, InstanceInitializerInfoCollection instanceInitializerInfoCollection);
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/classification/CheckNotNullEnumUnboxerMethodClassification.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/classification/CheckNotNullEnumUnboxerMethodClassification.java
index fd892c1..77d6178 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/classification/CheckNotNullEnumUnboxerMethodClassification.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/classification/CheckNotNullEnumUnboxerMethodClassification.java
@@ -4,8 +4,13 @@
package com.android.tools.r8.ir.optimize.enums.classification;
+import com.android.tools.r8.graph.RewrittenPrototypeDescription.ArgumentInfo;
+import com.android.tools.r8.graph.RewrittenPrototypeDescription.ArgumentInfoCollection;
import com.android.tools.r8.ir.code.InvokeStatic;
import com.android.tools.r8.ir.code.Value;
+import com.android.tools.r8.utils.IteratorUtils;
+import it.unimi.dsi.fastutil.ints.Int2ReferenceMap.Entry;
+import java.util.Iterator;
public final class CheckNotNullEnumUnboxerMethodClassification
extends EnumUnboxerMethodClassification {
@@ -31,6 +36,47 @@
}
@Override
+ public EnumUnboxerMethodClassification fixupAfterParameterRemoval(
+ ArgumentInfoCollection removedParameters) {
+ if (removedParameters.getArgumentInfo(argumentIndex).isRemovedArgumentInfo()) {
+ // If the null-checked argument is removed from the parameters of the method, then we can no
+ // longer classify this method as a check-not-null method. This is OK in terms of enum
+ // unboxing, since after the parameter removal enums at the call site will no longer have the
+ // check-not-null invoke instruction as a user.
+ //
+ // Note that when we materialize the enum instance in the check-not-null method, it is
+ // important that this method is reprocessed by enum unboxing (or that materialized instance
+ // would not be unboxed). This is guaranteed by argument removal: Since we have removed a
+ // parameter from the method, we will need to reprocess its code in the second optimization
+ // pass.
+ return unknown();
+ }
+
+ int numberOfArgumentsRemovedBeforeThis = 0;
+
+ Iterator<Entry<ArgumentInfo>> iterator = removedParameters.iterator();
+ while (iterator.hasNext()) {
+ Entry<ArgumentInfo> entry = iterator.next();
+ int argumentIndexForInfo = entry.getIntKey();
+ if (argumentIndexForInfo >= getArgumentIndex()) {
+ break;
+ }
+ ArgumentInfo argumentInfo = entry.getValue();
+ if (argumentInfo.isRemovedArgumentInfo()) {
+ numberOfArgumentsRemovedBeforeThis++;
+ }
+ }
+
+ assert IteratorUtils.allRemainingMatchDestructive(
+ iterator, entry -> entry.getIntKey() >= getArgumentIndex());
+
+ return numberOfArgumentsRemovedBeforeThis > 0
+ ? new CheckNotNullEnumUnboxerMethodClassification(
+ getArgumentIndex() - numberOfArgumentsRemovedBeforeThis)
+ : this;
+ }
+
+ @Override
public boolean isCheckNotNullClassification() {
return true;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/classification/EnumUnboxerMethodClassification.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/classification/EnumUnboxerMethodClassification.java
index 2b34291..5c93c36 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/classification/EnumUnboxerMethodClassification.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/classification/EnumUnboxerMethodClassification.java
@@ -4,12 +4,17 @@
package com.android.tools.r8.ir.optimize.enums.classification;
+import com.android.tools.r8.graph.RewrittenPrototypeDescription.ArgumentInfoCollection;
+
public abstract class EnumUnboxerMethodClassification {
public static UnknownEnumUnboxerMethodClassification unknown() {
return UnknownEnumUnboxerMethodClassification.getInstance();
}
+ public abstract EnumUnboxerMethodClassification fixupAfterParameterRemoval(
+ ArgumentInfoCollection removedParameters);
+
public EnumUnboxerMethodClassification fixupAfterRemovingThisParameter() {
// Only static methods are currently classified by the enum unboxer.
assert isUnknownClassification();
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/classification/UnknownEnumUnboxerMethodClassification.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/classification/UnknownEnumUnboxerMethodClassification.java
index 9f51d0a..e88239b 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/classification/UnknownEnumUnboxerMethodClassification.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/classification/UnknownEnumUnboxerMethodClassification.java
@@ -4,6 +4,8 @@
package com.android.tools.r8.ir.optimize.enums.classification;
+import com.android.tools.r8.graph.RewrittenPrototypeDescription.ArgumentInfoCollection;
+
public final class UnknownEnumUnboxerMethodClassification extends EnumUnboxerMethodClassification {
private static final UnknownEnumUnboxerMethodClassification INSTANCE =
@@ -16,6 +18,12 @@
}
@Override
+ public EnumUnboxerMethodClassification fixupAfterParameterRemoval(
+ ArgumentInfoCollection removedParameters) {
+ return this;
+ }
+
+ @Override
public boolean isUnknownClassification() {
return true;
}
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 3efe7f6..4263b35 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
@@ -261,6 +261,10 @@
this.enumUnboxerMethodClassification = enumUnboxerMethodClassification;
}
+ void unsetEnumUnboxerMethodClassification() {
+ this.enumUnboxerMethodClassification = EnumUnboxerMethodClassification.unknown();
+ }
+
@Override
public TypeElement getDynamicUpperBoundType() {
return returnsObjectWithUpperBoundType;
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackDelayed.java b/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackDelayed.java
index c9390d7..bd275db 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackDelayed.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackDelayed.java
@@ -272,6 +272,11 @@
}
@Override
+ public synchronized void unsetEnumUnboxerMethodClassification(ProgramMethod method) {
+ getMethodOptimizationInfoForUpdating(method).unsetEnumUnboxerMethodClassification();
+ }
+
+ @Override
public synchronized void setInstanceInitializerInfoCollection(
DexEncodedMethod method,
InstanceInitializerInfoCollection instanceInitializerInfoCollection) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackIgnore.java b/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackIgnore.java
index 623d8d2..1894532 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackIgnore.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackIgnore.java
@@ -122,6 +122,9 @@
ProgramMethod method, EnumUnboxerMethodClassification enumUnboxerMethodClassification) {}
@Override
+ public void unsetEnumUnboxerMethodClassification(ProgramMethod method) {}
+
+ @Override
public void setInstanceInitializerInfoCollection(
DexEncodedMethod method,
InstanceInitializerInfoCollection instanceInitializerInfoCollection) {}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackSimple.java b/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackSimple.java
index fe00440..10649fb 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackSimple.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackSimple.java
@@ -187,7 +187,15 @@
@Override
public void setEnumUnboxerMethodClassification(
ProgramMethod method, EnumUnboxerMethodClassification enumUnboxerMethodClassification) {
- // Ignored.
+ method
+ .getDefinition()
+ .getMutableOptimizationInfo()
+ .setEnumUnboxerMethodClassification(enumUnboxerMethodClassification);
+ }
+
+ @Override
+ public void unsetEnumUnboxerMethodClassification(ProgramMethod method) {
+ method.getDefinition().getMutableOptimizationInfo().unsetEnumUnboxerMethodClassification();
}
@Override
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorApplicationFixer.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorApplicationFixer.java
index 346e3fc..fc74192 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorApplicationFixer.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorApplicationFixer.java
@@ -8,6 +8,11 @@
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.MethodCollection;
+import com.android.tools.r8.graph.ProgramMethod;
+import com.android.tools.r8.graph.RewrittenPrototypeDescription.ArgumentInfoCollection;
+import com.android.tools.r8.ir.optimize.enums.classification.EnumUnboxerMethodClassification;
+import com.android.tools.r8.ir.optimize.info.OptimizationFeedback;
+import com.android.tools.r8.ir.optimize.info.OptimizationFeedbackSimple;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.ThreadUtils;
import java.util.concurrent.ExecutionException;
@@ -55,6 +60,25 @@
methodReferenceAfterParameterRemoval,
builder -> {
// TODO(b/190154391): fixup parameter annotations, if any.
+ ArgumentInfoCollection removedParameters =
+ graphLens.getRemovedParameters(methodReferenceAfterParameterRemoval);
+ builder.modifyOptimizationInfo(
+ (newMethod, optimizationInfo) -> {
+ OptimizationFeedback feedback = OptimizationFeedbackSimple.getInstance();
+ ProgramMethod programMethod = new ProgramMethod(clazz, newMethod);
+ // TODO(b/190154391): test this.
+ EnumUnboxerMethodClassification rewrittenEnumUnboxerMethodClassification =
+ optimizationInfo
+ .getEnumUnboxerMethodClassification()
+ .fixupAfterParameterRemoval(removedParameters);
+ if (rewrittenEnumUnboxerMethodClassification.isCheckNotNullClassification()) {
+ feedback.setEnumUnboxerMethodClassification(
+ programMethod, rewrittenEnumUnboxerMethodClassification);
+ } else {
+ // Bypass monotonicity check.
+ feedback.unsetEnumUnboxerMethodClassification(programMethod);
+ }
+ });
});
});
}
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorGraphLens.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorGraphLens.java
index c60640c..77ec495 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorGraphLens.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorGraphLens.java
@@ -32,6 +32,11 @@
return new Builder(appView);
}
+ public ArgumentInfoCollection getRemovedParameters(DexMethod method) {
+ assert method != internalGetPreviousMethodSignature(method);
+ return removedParameters.getOrDefault(method, ArgumentInfoCollection.empty());
+ }
+
@Override
protected RewrittenPrototypeDescription internalDescribePrototypeChanges(
RewrittenPrototypeDescription prototypeChanges, DexMethod method) {
@@ -40,8 +45,7 @@
assert !removedParameters.containsKey(method);
return prototypeChanges;
}
- return prototypeChanges.withRemovedArguments(
- removedParameters.getOrDefault(method, ArgumentInfoCollection.empty()));
+ return prototypeChanges.withRemovedArguments(getRemovedParameters(method));
}
@Override
diff --git a/src/main/java/com/android/tools/r8/utils/IntObjToObjFunction.java b/src/main/java/com/android/tools/r8/utils/IntObjToObjFunction.java
index 9c07bed..b0ed4ee 100644
--- a/src/main/java/com/android/tools/r8/utils/IntObjToObjFunction.java
+++ b/src/main/java/com/android/tools/r8/utils/IntObjToObjFunction.java
@@ -6,5 +6,5 @@
public interface IntObjToObjFunction<S, T> {
- S apply(int i, T obj);
+ T apply(int i, S obj);
}
diff --git a/src/main/java/com/android/tools/r8/utils/IteratorUtils.java b/src/main/java/com/android/tools/r8/utils/IteratorUtils.java
index 9420440..a34ec96 100644
--- a/src/main/java/com/android/tools/r8/utils/IteratorUtils.java
+++ b/src/main/java/com/android/tools/r8/utils/IteratorUtils.java
@@ -165,6 +165,16 @@
return !anyRemainingMatch(iterator, remaining -> !predicate.test(remaining));
}
+ public static <T> boolean allRemainingMatchDestructive(
+ Iterator<T> iterator, Predicate<T> predicate) {
+ while (iterator.hasNext()) {
+ if (!predicate.test(iterator.next())) {
+ return false;
+ }
+ }
+ return true;
+ }
+
public static <T> boolean anyRemainingMatch(ListIterator<T> iterator, Predicate<T> predicate) {
T state = peekNext(iterator);
boolean result = false;
diff --git a/src/test/java/com/android/tools/r8/optimize/argumentpropagation/MixedArgumentRemovalAndEnumUnboxingTest.java b/src/test/java/com/android/tools/r8/optimize/argumentpropagation/MixedArgumentRemovalAndEnumUnboxingTest.java
new file mode 100644
index 0000000..ad4c08a
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/optimize/argumentpropagation/MixedArgumentRemovalAndEnumUnboxingTest.java
@@ -0,0 +1,99 @@
+// Copyright (c) 2021, 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.optimize.argumentpropagation;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+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 org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class MixedArgumentRemovalAndEnumUnboxingTest extends TestBase {
+
+ @Parameter(0)
+ public TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection parameters() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepMainRule(Main.class)
+ .addOptionsModification(
+ options ->
+ options
+ .callSiteOptimizationOptions()
+ .setEnableExperimentalArgumentPropagation(true))
+ .addEnumUnboxingInspector(inspector -> inspector.assertUnboxed(MyEnum.class))
+ .enableInliningAnnotations()
+ // TODO(b/173398086): uniqueMethodWithName() does not work with argument removal.
+ .noMinification()
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .inspect(
+ inspector -> {
+ ClassSubject mainClassSubject = inspector.clazz(Main.class);
+ assertThat(mainClassSubject, isPresent());
+
+ MethodSubject mainMethodSubject = mainClassSubject.mainMethod();
+ assertThat(mainMethodSubject, isPresent());
+ assertTrue(
+ mainMethodSubject
+ .streamInstructions()
+ .noneMatch(InstructionSubject::isConstNull));
+
+ MethodSubject testMethodSubject = mainClassSubject.uniqueMethodWithName("test");
+ assertThat(testMethodSubject, isPresent());
+ assertEquals(2, testMethodSubject.getProgramMethod().getReference().getArity());
+ assertEquals(
+ "int", testMethodSubject.getProgramMethod().getParameter(0).getTypeName());
+ assertEquals(
+ "int", testMethodSubject.getProgramMethod().getParameter(1).getTypeName());
+ assertTrue(
+ testMethodSubject.streamInstructions().noneMatch(InstructionSubject::isIf));
+ })
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines("A", "B");
+ }
+
+ static class Main {
+
+ public static void main(String[] args) {
+ MyEnum alwaysA = System.currentTimeMillis() >= 1 ? MyEnum.A : MyEnum.B;
+ MyEnum alwaysB = System.currentTimeMillis() >= 1 ? MyEnum.B : MyEnum.A;
+ test(null, alwaysA, null, alwaysB);
+ }
+
+ @NeverInline
+ static void test(Object alwaysNull, MyEnum alwaysA, Object alsoAlwaysNull, MyEnum alwaysB) {
+ if (alwaysNull == null && alsoAlwaysNull == null) {
+ System.out.println(alwaysA.name());
+ System.out.println(alwaysB.name());
+ }
+ }
+ }
+
+ enum MyEnum {
+ A,
+ B
+ }
+}
