Fix incorrect cache key for new method signature cache
Bug: b/241426917
Change-Id: I96a3e206f4fba54922a7546879b1ddc6147ce64c
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 ce22d5c..ba712ee 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -189,6 +189,10 @@
return accessFlags;
}
+ public int getArgumentIndexFromParameterIndex(int parameterIndex) {
+ return parameterIndex + getFirstNonReceiverArgumentIndex();
+ }
+
public DexType getArgumentType(int argumentIndex) {
return getReference().getArgumentType(argumentIndex, isStatic());
}
@@ -324,6 +328,11 @@
return getReference().getParameters();
}
+ public int getParameterIndexFromArgumentIndex(int argumentIndex) {
+ assert argumentIndex >= getFirstNonReceiverArgumentIndex();
+ return argumentIndex - getFirstNonReceiverArgumentIndex();
+ }
+
public DexType getReturnType() {
return getReference().getReturnType();
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexTypeList.java b/src/main/java/com/android/tools/r8/graph/DexTypeList.java
index 880b51d..7be2dce 100644
--- a/src/main/java/com/android/tools/r8/graph/DexTypeList.java
+++ b/src/main/java/com/android/tools/r8/graph/DexTypeList.java
@@ -8,6 +8,7 @@
import com.android.tools.r8.dex.IndexedItemCollection;
import com.android.tools.r8.dex.MixedSectionCollection;
import com.android.tools.r8.utils.ArrayUtils;
+import com.android.tools.r8.utils.IntObjConsumer;
import com.android.tools.r8.utils.structural.StructuralItem;
import com.android.tools.r8.utils.structural.StructuralMapping;
import com.android.tools.r8.utils.structural.StructuralSpecification;
@@ -104,6 +105,13 @@
}
}
+ public void forEach(IntObjConsumer<DexType> consumer) {
+ for (int parameterIndex = 0; parameterIndex < values.length; parameterIndex++) {
+ DexType parameter = values[parameterIndex];
+ consumer.accept(parameterIndex, parameter);
+ }
+ }
+
public void forEachReverse(Consumer<? super DexType> consumer) {
for (int i = values.length - 1; i >= 0; i--) {
consumer.accept(values[i]);
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorProgramOptimizer.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorProgramOptimizer.java
index f968d56..bf1afad 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorProgramOptimizer.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorProgramOptimizer.java
@@ -42,6 +42,7 @@
import com.android.tools.r8.utils.AccessUtils;
import com.android.tools.r8.utils.AndroidApiLevelUtils;
import com.android.tools.r8.utils.BooleanBox;
+import com.android.tools.r8.utils.BooleanUtils;
import com.android.tools.r8.utils.IntBox;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.InternalOptions.CallSiteOptimizationOptions;
@@ -84,45 +85,63 @@
static class AllowedPrototypeChanges {
private static final AllowedPrototypeChanges EMPTY =
- new AllowedPrototypeChanges(null, Int2ReferenceMaps.emptyMap(), IntSets.EMPTY_SET);
+ new AllowedPrototypeChanges(false, null, Int2ReferenceMaps.emptyMap(), IntSets.EMPTY_SET);
+ boolean canBeConvertedToStaticMethod;
DexType newReturnType;
Int2ReferenceMap<DexType> newParameterTypes;
IntSet removableParameterIndices;
AllowedPrototypeChanges(
+ boolean canBeConvertedToStaticMethod,
DexType newReturnType,
Int2ReferenceMap<DexType> newParameterTypes,
IntSet removableParameterIndices) {
+ this.canBeConvertedToStaticMethod = canBeConvertedToStaticMethod;
this.newReturnType = newReturnType;
this.newParameterTypes = newParameterTypes;
this.removableParameterIndices = removableParameterIndices;
}
- public static AllowedPrototypeChanges create(RewrittenPrototypeDescription prototypeChanges) {
+ public static AllowedPrototypeChanges create(
+ ProgramMethod method, RewrittenPrototypeDescription prototypeChanges) {
if (prototypeChanges.isEmpty()) {
return empty();
}
+ ArgumentInfoCollection argumentInfoCollection = prototypeChanges.getArgumentInfoCollection();
+ boolean canBeConvertedToStaticMethod = argumentInfoCollection.isConvertedToStaticMethod();
+ assert !canBeConvertedToStaticMethod
+ || (method.getDefinition().isInstance()
+ && argumentInfoCollection.getArgumentInfo(0).isRemovedReceiverInfo());
DexType newReturnType =
prototypeChanges.hasRewrittenReturnInfo()
? prototypeChanges.getRewrittenReturnInfo().getNewType()
: null;
Int2ReferenceMap<DexType> newParameterTypes = new Int2ReferenceOpenHashMap<>();
IntSet removableParameterIndices = new IntOpenHashSet();
- prototypeChanges
- .getArgumentInfoCollection()
- .forEach(
- (argumentIndex, argumentInfo) -> {
- if (argumentInfo.isRemovedArgumentInfo()) {
- removableParameterIndices.add(argumentIndex);
- } else {
- assert argumentInfo.isRewrittenTypeInfo();
- RewrittenTypeInfo rewrittenTypeInfo = argumentInfo.asRewrittenTypeInfo();
- newParameterTypes.put(argumentIndex, rewrittenTypeInfo.getNewType());
- }
- });
+ argumentInfoCollection.forEach(
+ (argumentIndex, argumentInfo) -> {
+ if (argumentInfo.isRemovedReceiverInfo()) {
+ // Skip as the receiver is not in the proto.
+ assert canBeConvertedToStaticMethod;
+ } else {
+ int parameterIndex =
+ method.getDefinition().getParameterIndexFromArgumentIndex(argumentIndex);
+ assert parameterIndex >= 0;
+ if (argumentInfo.isRemovedArgumentInfo()) {
+ removableParameterIndices.add(parameterIndex);
+ } else {
+ assert argumentInfo.isRewrittenTypeInfo();
+ RewrittenTypeInfo rewrittenTypeInfo = argumentInfo.asRewrittenTypeInfo();
+ newParameterTypes.put(parameterIndex, rewrittenTypeInfo.getNewType());
+ }
+ }
+ });
return new AllowedPrototypeChanges(
- newReturnType, newParameterTypes, removableParameterIndices);
+ canBeConvertedToStaticMethod,
+ newReturnType,
+ newParameterTypes,
+ removableParameterIndices);
}
public static AllowedPrototypeChanges empty() {
@@ -131,7 +150,11 @@
@Override
public int hashCode() {
- return Objects.hash(newReturnType, newParameterTypes, removableParameterIndices);
+ return Objects.hash(
+ canBeConvertedToStaticMethod,
+ newReturnType,
+ newParameterTypes,
+ removableParameterIndices);
}
@Override
@@ -140,7 +163,8 @@
return false;
}
AllowedPrototypeChanges other = (AllowedPrototypeChanges) obj;
- return newReturnType == other.newReturnType
+ return canBeConvertedToStaticMethod == other.canBeConvertedToStaticMethod
+ && newReturnType == other.newReturnType
&& newParameterTypes.equals(other.newParameterTypes)
&& removableParameterIndices.equals(other.removableParameterIndices);
}
@@ -359,10 +383,11 @@
// Find the parameters that are either (i) the same constant, (ii) all unused, or (iii)
// all possible to strengthen to the same stronger type, in all methods.
+ boolean canBeConvertedToStaticMethod = canRemoveReceiverFromVirtualMethods(methods);
Int2ReferenceMap<DexType> newParameterTypes = new Int2ReferenceOpenHashMap<>();
IntSet removableVirtualMethodParametersInAllMethods = new IntArraySet();
for (int parameterIndex = 0;
- parameterIndex < signature.getProto().getArity() + 1;
+ parameterIndex < signature.getParameters().size();
parameterIndex++) {
if (canRemoveParameterFromVirtualMethods(methods, parameterIndex)) {
removableVirtualMethodParametersInAllMethods.add(parameterIndex);
@@ -380,12 +405,14 @@
getReturnValueForVirtualMethods(methods, signature);
DexType newReturnType =
getNewReturnTypeForVirtualMethods(methods, returnValueForVirtualMethods);
- if (newReturnType != null
+ if (canBeConvertedToStaticMethod
+ || newReturnType != null
|| !newParameterTypes.isEmpty()
|| !removableVirtualMethodParametersInAllMethods.isEmpty()) {
allowedPrototypeChangesForVirtualMethods.put(
signature,
new AllowedPrototypeChanges(
+ canBeConvertedToStaticMethod,
newReturnType,
newParameterTypes,
removableVirtualMethodParametersInAllMethods));
@@ -467,28 +494,30 @@
return false;
}
+ private boolean canRemoveReceiverFromVirtualMethods(ProgramMethodSet methods) {
+ if (methods.size() > 1) {
+ // Method staticizing would break dynamic dispatch.
+ return false;
+ }
+ ProgramMethod method = methods.getFirst();
+ return method.getOptimizationInfo().hasUnusedArguments()
+ && method.getOptimizationInfo().getUnusedArguments().get(0)
+ && ParameterRemovalUtils.canRemoveUnusedParametersFrom(appView, method)
+ && ParameterRemovalUtils.canRemoveUnusedParameter(appView, method, 0);
+ }
+
private boolean canRemoveParameterFromVirtualMethods(
ProgramMethodSet methods, int parameterIndex) {
- if (parameterIndex == 0) {
- if (methods.size() > 1) {
- // Method staticizing would break dynamic dispatch.
- return false;
- }
- ProgramMethod method = methods.getFirst();
- return method.getOptimizationInfo().hasUnusedArguments()
- && method.getOptimizationInfo().getUnusedArguments().get(parameterIndex)
- && ParameterRemovalUtils.canRemoveUnusedParametersFrom(appView, method)
- && ParameterRemovalUtils.canRemoveUnusedParameter(appView, method, parameterIndex);
- }
+ int argumentIndex = parameterIndex + 1;
for (ProgramMethod method : methods) {
if (method.getDefinition().isAbstract()) {
// OK, this parameter can be removed.
continue;
}
if (method.getOptimizationInfo().hasUnusedArguments()
- && method.getOptimizationInfo().getUnusedArguments().get(parameterIndex)
+ && method.getOptimizationInfo().getUnusedArguments().get(argumentIndex)
&& ParameterRemovalUtils.canRemoveUnusedParametersFrom(appView, method)
- && ParameterRemovalUtils.canRemoveUnusedParameter(appView, method, parameterIndex)) {
+ && ParameterRemovalUtils.canRemoveUnusedParameter(appView, method, argumentIndex)) {
// OK, this parameter is unused.
continue;
}
@@ -497,7 +526,7 @@
ConcreteCallSiteOptimizationInfo concreteOptimizationInfo =
optimizationInfo.asConcreteCallSiteOptimizationInfo();
AbstractValue abstractValue =
- concreteOptimizationInfo.getAbstractArgumentValue(parameterIndex);
+ concreteOptimizationInfo.getAbstractArgumentValue(argumentIndex);
if (abstractValue.isSingleValue()
&& abstractValue.asSingleValue().isMaterializableInContext(appView, method)) {
// OK, this parameter has a constant value and can be removed.
@@ -544,9 +573,6 @@
private DexType getNewParameterTypeForVirtualMethods(
ProgramMethodSet methods, int parameterIndex) {
- if (parameterIndex == 0) {
- return null;
- }
DexType newParameterType = null;
for (ProgramMethod method : methods) {
if (method.getAccessFlags().isAbstract()) {
@@ -561,7 +587,7 @@
newParameterType = newParameterTypeForMethod;
}
assert newParameterType == null
- || newParameterType != methods.getFirst().getArgumentType(parameterIndex);
+ || newParameterType != methods.getFirst().getParameter(parameterIndex);
return newParameterType;
}
@@ -734,7 +760,7 @@
ProgramMethod method, RewrittenPrototypeDescription prototypeChanges) {
DexMethodSignature methodSignatureWithoutPrototypeChanges = method.getMethodSignature();
AllowedPrototypeChanges allowedPrototypeChanges =
- AllowedPrototypeChanges.create(prototypeChanges);
+ AllowedPrototypeChanges.create(method, prototypeChanges);
// Check if there is a reserved signature for this already.
DexMethodSignature reservedSignature =
@@ -742,6 +768,9 @@
.getOrDefault(methodSignatureWithoutPrototypeChanges, Collections.emptyMap())
.get(allowedPrototypeChanges);
if (reservedSignature != null) {
+ assert reservedSignature
+ .getProto()
+ .equals(prototypeChanges.rewriteMethod(method, dexItemFactory).getProto());
return reservedSignature.withHolder(method.getHolderType(), dexItemFactory);
}
@@ -882,14 +911,16 @@
removableParameterIndices);
}
+ boolean canBeConvertedToStaticMethod = allowedPrototypeChanges.canBeConvertedToStaticMethod;
RewrittenPrototypeDescription prototypeChanges =
computePrototypeChangesForMethod(
method,
+ canBeConvertedToStaticMethod,
allowedPrototypeChanges.newReturnType,
newParameterTypes::get,
removableParameterIndices::contains);
assert prototypeChanges.getArgumentInfoCollection().numberOfRemovedArguments()
- == removableParameterIndices.size();
+ == BooleanUtils.intValue(canBeConvertedToStaticMethod) + removableParameterIndices.size();
return prototypeChanges;
}
@@ -902,24 +933,26 @@
ArgumentInfoCollection.Builder argumentInfoCollectionBuilder =
ArgumentInfoCollection.builder()
.setArgumentInfosSize(method.getDefinition().getNumberOfArguments());
- for (int argumentIndex = 0;
- argumentIndex < method.getDefinition().getNumberOfArguments();
- argumentIndex++) {
- if (removableParameterIndices.contains(argumentIndex)) {
- argumentInfoCollectionBuilder.addArgumentInfo(
- argumentIndex,
- RemovedArgumentInfo.builder().setType(method.getArgumentType(argumentIndex)).build());
- } else if (newParameterTypes.containsKey(argumentIndex)) {
- DexType newParameterType = newParameterTypes.get(argumentIndex);
- argumentInfoCollectionBuilder.addArgumentInfo(
- argumentIndex,
- RewrittenTypeInfo.builder()
- .setCastType(newParameterType)
- .setOldType(method.getArgumentType(argumentIndex))
- .setNewType(newParameterType)
- .build());
- }
- }
+ method
+ .getParameters()
+ .forEach(
+ (parameterIndex, parameter) -> {
+ int argumentIndex =
+ method.getDefinition().getArgumentIndexFromParameterIndex(parameterIndex);
+ if (removableParameterIndices.contains(parameterIndex)) {
+ argumentInfoCollectionBuilder.addArgumentInfo(
+ argumentIndex, RemovedArgumentInfo.builder().setType(parameter).build());
+ } else if (newParameterTypes.containsKey(parameterIndex)) {
+ DexType newParameterType = newParameterTypes.get(parameterIndex);
+ argumentInfoCollectionBuilder.addArgumentInfo(
+ argumentIndex,
+ RewrittenTypeInfo.builder()
+ .setCastType(newParameterType)
+ .setOldType(parameter)
+ .setNewType(newParameterType)
+ .build());
+ }
+ });
return RewrittenPrototypeDescription.create(
Collections.emptyList(),
computeReturnChangesForMethod(method, newReturnType),
@@ -932,7 +965,11 @@
? parameterIndex -> getNewParameterType(method, parameterIndex)
: parameterIndex -> null;
return computePrototypeChangesForMethod(
- method, getNewReturnType(method), parameterIndexToParameterType, parameterIndex -> true);
+ method,
+ true,
+ getNewReturnType(method),
+ parameterIndexToParameterType,
+ parameterIndex -> true);
}
private DexType getNewReturnType(ProgramMethod method) {
@@ -996,7 +1033,6 @@
} else {
returnValue = method.getOptimizationInfo().getAbstractReturnValue();
}
-
return returnValue.isSingleValue()
&& returnValue.asSingleValue().isMaterializableInAllContexts(appView)
? returnValue.asSingleValue()
@@ -1007,12 +1043,13 @@
if (!appView.getKeepInfo(method).isParameterTypeStrengtheningAllowed(options)) {
return null;
}
- DexType staticType = method.getArgumentType(parameterIndex);
+ DexType staticType = method.getParameter(parameterIndex);
if (!staticType.isClassType()) {
return null;
}
+ int argumentIndex = method.getDefinition().getArgumentIndexFromParameterIndex(parameterIndex);
DynamicType dynamicType =
- method.getOptimizationInfo().getArgumentInfos().getDynamicType(parameterIndex);
+ method.getOptimizationInfo().getArgumentInfos().getDynamicType(argumentIndex);
if (dynamicType == null || dynamicType.isUnknown()) {
return null;
}
@@ -1046,24 +1083,27 @@
private RewrittenPrototypeDescription computePrototypeChangesForMethod(
ProgramMethod method,
+ boolean canBeConvertedToStaticMethod,
DexType newReturnType,
IntFunction<DexType> newParameterTypes,
IntPredicate removableParameterIndices) {
return RewrittenPrototypeDescription.create(
Collections.emptyList(),
computeReturnChangesForMethod(method, newReturnType),
- computeParameterChangesForMethod(method, newParameterTypes, removableParameterIndices));
+ computeParameterChangesForMethod(
+ method, canBeConvertedToStaticMethod, newParameterTypes, removableParameterIndices));
}
private ArgumentInfoCollection computeParameterChangesForMethod(
ProgramMethod method,
+ boolean canBeConvertedToStaticMethod,
IntFunction<DexType> newParameterTypes,
IntPredicate removableParameterIndices) {
ArgumentInfoCollection.Builder parameterChangesBuilder =
ArgumentInfoCollection.builder()
.setArgumentInfosSize(method.getDefinition().getNumberOfArguments());
if (method.getDefinition().isInstance()
- && removableParameterIndices.test(0)
+ && canBeConvertedToStaticMethod
&& method.getOptimizationInfo().hasUnusedArguments()
&& method.getOptimizationInfo().getUnusedArguments().get(0)
&& ParameterRemovalUtils.canRemoveUnusedParametersFrom(appView, method)
@@ -1075,19 +1115,19 @@
}
CallSiteOptimizationInfo optimizationInfo = method.getOptimizationInfo().getArgumentInfos();
- for (int argumentIndex = method.getDefinition().getFirstNonReceiverArgumentIndex();
- argumentIndex < method.getDefinition().getNumberOfArguments();
- argumentIndex++) {
- if (removableParameterIndices.test(argumentIndex)) {
+ for (int parameterIndex = 0;
+ parameterIndex < method.getParameters().size();
+ parameterIndex++) {
+ int argumentIndex =
+ parameterIndex + method.getDefinition().getFirstNonReceiverArgumentIndex();
+ if (removableParameterIndices.test(parameterIndex)) {
if (method.getOptimizationInfo().hasUnusedArguments()
&& method.getOptimizationInfo().getUnusedArguments().get(argumentIndex)
&& ParameterRemovalUtils.canRemoveUnusedParametersFrom(appView, method)
&& ParameterRemovalUtils.canRemoveUnusedParameter(appView, method, argumentIndex)) {
parameterChangesBuilder.addArgumentInfo(
argumentIndex,
- RemovedArgumentInfo.builder()
- .setType(method.getArgumentType(argumentIndex))
- .build());
+ RemovedArgumentInfo.builder().setType(method.getParameter(parameterIndex)).build());
continue;
}
@@ -1098,15 +1138,15 @@
argumentIndex,
RemovedArgumentInfo.builder()
.setSingleValue(abstractValue.asSingleValue())
- .setType(method.getArgumentType(argumentIndex))
+ .setType(method.getParameter(parameterIndex))
.build());
continue;
}
}
- DexType dynamicType = newParameterTypes.apply(argumentIndex);
+ DexType dynamicType = newParameterTypes.apply(parameterIndex);
if (dynamicType != null) {
- DexType staticType = method.getArgumentType(argumentIndex);
+ DexType staticType = method.getParameter(parameterIndex);
assert dynamicType != staticType;
parameterChangesBuilder.addArgumentInfo(
argumentIndex,
diff --git a/src/test/java/com/android/tools/r8/optimize/argumentpropagation/ConsistentMethodRenamingWithArgumentRemovalAndStaticizingTest.java b/src/test/java/com/android/tools/r8/optimize/argumentpropagation/ConsistentMethodRenamingWithArgumentRemovalAndStaticizingTest.java
index 0929298..e8daac4 100644
--- a/src/test/java/com/android/tools/r8/optimize/argumentpropagation/ConsistentMethodRenamingWithArgumentRemovalAndStaticizingTest.java
+++ b/src/test/java/com/android/tools/r8/optimize/argumentpropagation/ConsistentMethodRenamingWithArgumentRemovalAndStaticizingTest.java
@@ -10,7 +10,6 @@
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.AssertUtils;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -31,17 +30,15 @@
@Test
public void test() throws Exception {
- AssertUtils.assertFailsCompilation(
- () ->
- testForR8(parameters.getBackend())
- .addInnerClasses(getClass())
- .addKeepMainRule(Main.class)
- .enableInliningAnnotations()
- .enableNeverClassInliningAnnotations()
- .enableNoHorizontalClassMergingAnnotations()
- .setMinApi(parameters.getApiLevel())
- .run(parameters.getRuntime(), Main.class)
- .assertSuccessWithOutputLines("A.m()", "B.m()"));
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepMainRule(Main.class)
+ .enableInliningAnnotations()
+ .enableNeverClassInliningAnnotations()
+ .enableNoHorizontalClassMergingAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines("A.m()", "B.m()");
}
static class Main {
