Extend unused argument removal to private methods
Bug: 65810338
Change-Id: I4be35f2719afa97234985fb1948726ffa4bca7ab
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/UnusedArgumentsCollector.java b/src/main/java/com/android/tools/r8/ir/optimize/UnusedArgumentsCollector.java
index e06cdbef..573ffb5 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/UnusedArgumentsCollector.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/UnusedArgumentsCollector.java
@@ -104,14 +104,22 @@
private final MethodSignatureEquivalence equivalence = MethodSignatureEquivalence.get();
private final Set<Wrapper<DexMethod>> usedSignatures = new HashSet<>();
- private DexProto protoWithRemovedArguments(DexMethod method, RemovedArgumentsInfo unused) {
- DexType[] parameters =
- new DexType[method.proto.parameters.size() - unused.numberOfRemovedArguments()];
- if (parameters.length > 0) {
+ private DexProto protoWithRemovedArguments(
+ DexEncodedMethod encodedMethod, RemovedArgumentsInfo unused) {
+ DexMethod method = encodedMethod.method;
+
+ int firstArgumentIndex = encodedMethod.isStatic() ? 0 : 1;
+ int numberOfParameters = method.proto.parameters.size() - unused.numberOfRemovedArguments();
+ if (!encodedMethod.isStatic() && unused.isArgumentRemoved(0)) {
+ numberOfParameters++;
+ }
+
+ DexType[] parameters = new DexType[numberOfParameters];
+ if (numberOfParameters > 0) {
int newIndex = 0;
- for (int j = 0; j < method.proto.parameters.size(); j++) {
- if (!unused.isArgumentRemoved(j)) {
- parameters[newIndex++] = method.proto.parameters.values[j];
+ for (int oldIndex = 0; oldIndex < method.proto.parameters.size(); oldIndex++) {
+ if (!unused.isArgumentRemoved(oldIndex + firstArgumentIndex)) {
+ parameters[newIndex++] = method.proto.parameters.values[oldIndex];
}
}
assert newIndex == parameters.length;
@@ -128,19 +136,26 @@
}
DexEncodedMethod removeArguments(DexEncodedMethod method, RemovedArgumentsInfo unused) {
+ if (unused == null) {
+ return null;
+ }
+
boolean removed = usedSignatures.remove(equivalence.wrap(method.method));
assert removed;
- DexProto newProto = protoWithRemovedArguments(method.method, unused);
+ DexProto newProto = protoWithRemovedArguments(method, unused);
DexMethod newSignature;
int count = 0;
DexString newName = null;
do {
- newName =
- newName == null
- ? method.method.name
- : appView
- .dexItemFactory()
- .createString(method.method.name.toSourceString() + count);
+ if (newName == null) {
+ newName = method.method.name;
+ } else if (method.method.name != appView.dexItemFactory().initMethodName) {
+ newName =
+ appView.dexItemFactory().createString(method.method.name.toSourceString() + count);
+ } else {
+ // Constructors must be named `<init>`.
+ return null;
+ }
newSignature =
appView.dexItemFactory().createMethod(method.method.holder, newProto, newName);
count++;
@@ -158,8 +173,8 @@
for (int i = 0; i < clazz.directMethods().length; i++) {
DexEncodedMethod method = clazz.directMethods()[i];
RemovedArgumentsInfo unused = collectUnusedArguments(method);
- if (unused != null) {
- DexEncodedMethod newMethod = signatures.removeArguments(method, unused);
+ DexEncodedMethod newMethod = signatures.removeArguments(method, unused);
+ if (newMethod != null) {
clazz.directMethods()[i] = newMethod;
methodMapping.put(method.method, newMethod.method);
removedArguments.put(newMethod.method, unused);
@@ -178,21 +193,25 @@
assert method.getCode().getOwner() == method;
int argumentCount =
method.method.proto.parameters.size() + (method.accessFlags.isStatic() ? 0 : 1);
- // TODO(65810338): Implement for private and virtual as well.
- if (!method.accessFlags.isStatic()) {
- return null;
- }
- CollectUsedArguments collector = new CollectUsedArguments();
- method.getCode().registerArgumentReferences(collector);
- BitSet used = collector.getUsedArguments();
- if (used.cardinality() < argumentCount) {
- List<RemovedArgumentInfo> unused = new ArrayList<>();
- for (int i = 0; i < argumentCount; i++) {
- if (!used.get(i)) {
- unused.add(RemovedArgumentInfo.builder().setArgumentIndex(i).build());
- }
+ // TODO(65810338): Implement for virtual methods as well.
+ if (method.accessFlags.isPrivate() || method.accessFlags.isStatic()) {
+ CollectUsedArguments collector = new CollectUsedArguments();
+ if (!method.accessFlags.isStatic()) {
+ // TODO(65810338): The receiver cannot be removed without transforming the method to being
+ // static.
+ collector.register(0);
}
- return new RemovedArgumentsInfo(unused);
+ method.getCode().registerArgumentReferences(collector);
+ BitSet used = collector.getUsedArguments();
+ if (used.cardinality() < argumentCount) {
+ List<RemovedArgumentInfo> unused = new ArrayList<>();
+ for (int i = 0; i < argumentCount; i++) {
+ if (!used.get(i)) {
+ unused.add(RemovedArgumentInfo.builder().setArgumentIndex(i).build());
+ }
+ }
+ return new RemovedArgumentsInfo(unused);
+ }
}
return null;
}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/unusedarguments/UnusedArgumentsObjectTest.java b/src/test/java/com/android/tools/r8/ir/optimize/unusedarguments/UnusedArgumentsObjectTest.java
index 10bf5df..2f7e437 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/unusedarguments/UnusedArgumentsObjectTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/unusedarguments/UnusedArgumentsObjectTest.java
@@ -6,6 +6,7 @@
import static org.junit.Assert.assertEquals;
+import com.android.tools.r8.NeverClassInline;
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.google.common.collect.ImmutableList;
@@ -40,6 +41,7 @@
}
}
+ @NeverClassInline
static class TestClass {
@NeverInline
@@ -57,10 +59,29 @@
return a;
}
+ @NeverInline
+ private Object b(Object a) {
+ return a;
+ }
+
+ @NeverInline
+ private Object b(Object a, Object b) {
+ return a;
+ }
+
+ @NeverInline
+ private Object b(Object a, Object b, Object c) {
+ return a;
+ }
+
public static void main(String[] args) {
+ TestClass instance = new TestClass();
System.out.print(a(new TestObject("1")));
System.out.print(a(new TestObject("2"), new TestObject("3")));
System.out.print(a(new TestObject("4"), new TestObject("5"), new TestObject("6")));
+ System.out.print(instance.b(new TestObject("1")));
+ System.out.print(instance.b(new TestObject("2"), new TestObject("3")));
+ System.out.print(instance.b(new TestObject("4"), new TestObject("5"), new TestObject("6")));
}
}
@@ -76,18 +97,18 @@
@Override
public String getExpectedResult() {
- return "124";
+ return "124124";
}
@Override
public void inspectTestClass(ClassSubject clazz) {
- assertEquals(4, clazz.allMethods().size());
+ assertEquals(8, clazz.allMethods().size());
clazz.forAllMethods(
- method -> {
- Assert.assertTrue(
- method.getFinalName().equals("main")
- || (method.getFinalSignature().parameters.length == 1
- && method.getFinalSignature().parameters[0].equals("java.lang.Object")));
- });
+ method ->
+ Assert.assertTrue(
+ method.getFinalName().equals("main")
+ || method.getFinalName().equals("<init>")
+ || (method.getFinalSignature().parameters.length == 1
+ && method.getFinalSignature().parameters[0].equals("java.lang.Object"))));
}
}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/unusedarguments/UnusedArgumentsTestBase.java b/src/test/java/com/android/tools/r8/ir/optimize/unusedarguments/UnusedArgumentsTestBase.java
index d20de91..0a5d8e9 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/unusedarguments/UnusedArgumentsTestBase.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/unusedarguments/UnusedArgumentsTestBase.java
@@ -55,8 +55,9 @@
.addProgramClasses(getTestClass())
.addProgramClasses(getAdditionalClasses())
.addKeepMainRule(getTestClass())
- .addKeepRules(minification ? "" : "-dontobfuscate")
+ .minification(minification)
.enableInliningAnnotations()
+ .enableClassInliningAnnotations()
.compile()
.run(getTestClass())
.inspect(this::inspect)
