Extend the range of assume instructions, part 5.
ClassStaticizer is updated to take into accounts that users of eligible
singleton instances can be aliased via assume instructions.
StaticizingProcessor is also changed to consider aliased users when
searching for references of staticized singleton.
Bug: 120920488
Change-Id: I5b79478d3c1a3cf23efc7421efcf0be15a668e6d
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
index a75ce9e..46f8bf4 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -1317,26 +1317,6 @@
assert code.isConsistentSSA();
}
- if (nonNullTracker != null) {
- // TODO(b/139246447): Once we extend this optimization to, e.g., constants of primitive args,
- // this may not be the right place to collect call site optimization info.
- // Collecting call-site optimization info depends on the existence of non-null IRs.
- // Arguments can be changed during the debug mode.
- if (!isDebugMode && appView.callSiteOptimizationInfoPropagator() != null) {
- appView.callSiteOptimizationInfoPropagator().collectCallSiteOptimizationInfo(code);
- }
- // Computation of non-null parameters on normal exits rely on the existence of non-null IRs.
- nonNullTracker.computeNonNullParamOnNormalExits(feedback, code);
- }
- if (aliasIntroducer != null || nonNullTracker != null || dynamicTypeOptimization != null) {
- codeRewriter.removeAssumeInstructions(code);
- assert code.isConsistentSSA();
- }
- // Assert that we do not have unremoved non-sense code in the output, e.g., v <- non-null NULL.
- assert code.verifyNoNullabilityBottomTypes();
-
- assert code.verifyTypes(appView);
-
previous = printMethod(code, "IR after outline handler (SSA)", previous);
// TODO(mkroghj) Test if shorten live ranges is worth it.
@@ -1362,6 +1342,26 @@
classStaticizer.examineMethodCode(method, code);
}
+ if (nonNullTracker != null) {
+ // TODO(b/139246447): Once we extend this optimization to, e.g., constants of primitive args,
+ // this may not be the right place to collect call site optimization info.
+ // Collecting call-site optimization info depends on the existence of non-null IRs.
+ // Arguments can be changed during the debug mode.
+ if (!isDebugMode && appView.callSiteOptimizationInfoPropagator() != null) {
+ appView.callSiteOptimizationInfoPropagator().collectCallSiteOptimizationInfo(code);
+ }
+ // Computation of non-null parameters on normal exits rely on the existence of non-null IRs.
+ nonNullTracker.computeNonNullParamOnNormalExits(feedback, code);
+ }
+ if (aliasIntroducer != null || nonNullTracker != null || dynamicTypeOptimization != null) {
+ codeRewriter.removeAssumeInstructions(code);
+ assert code.isConsistentSSA();
+ }
+ // Assert that we do not have unremoved non-sense code in the output, e.g., v <- non-null NULL.
+ assert code.verifyNoNullabilityBottomTypes();
+
+ assert code.verifyTypes(appView);
+
if (appView.enableWholeProgramOptimizations()) {
if (libraryMethodOverrideAnalysis != null) {
libraryMethodOverrideAnalysis.analyze(code);
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/staticizer/ClassStaticizer.java b/src/main/java/com/android/tools/r8/ir/optimize/staticizer/ClassStaticizer.java
index 88a20da..1e7d21a 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/staticizer/ClassStaticizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/staticizer/ClassStaticizer.java
@@ -28,6 +28,7 @@
import com.android.tools.r8.ir.conversion.IRConverter;
import com.android.tools.r8.ir.optimize.info.OptimizationFeedback;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
+import com.android.tools.r8.utils.ListUtils;
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import java.util.Arrays;
@@ -43,6 +44,7 @@
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.BiConsumer;
+import java.util.function.Predicate;
public final class ClassStaticizer {
@@ -230,8 +232,8 @@
// We are inside an instance method of candidate class (not an instance initializer
// which we will check later), check if all the references to 'this' are valid
// (the call will invalidate the candidate if some of them are not valid).
- analyzeAllValueUsers(receiverClassCandidateInfo,
- receiverValue, factory.isConstructor(method.method));
+ analyzeAllValueUsers(
+ receiverClassCandidateInfo, receiverValue, factory.isConstructor(method.method));
// If the candidate is still valid, ignore all instructions
// we treat as valid usages on receiver.
@@ -289,7 +291,7 @@
// If the candidate still valid, ignore all usages in further analysis.
Value value = instruction.outValue();
if (value != null) {
- alreadyProcessed.addAll(value.uniqueUsers());
+ alreadyProcessed.addAll(value.aliasedUsers());
}
}
continue;
@@ -433,14 +435,16 @@
appView.appInfo().lookupDirectTarget(invoke.getInvokedMethod());
List<Value> values = invoke.inValues();
- if (values.lastIndexOf(candidateValue) != 0 ||
- methodInvoked == null || methodInvoked.method.holder != candidateType) {
+ if (ListUtils.lastIndexMatching(values, v -> v.getAliasedValue() == candidateValue) != 0
+ || methodInvoked == null
+ || methodInvoked.method.holder != candidateType) {
return false;
}
// Check arguments.
for (int i = 1; i < values.size(); i++) {
- if (!values.get(i).definition.isConstInstruction()) {
+ Value arg = values.get(i).getAliasedValue();
+ if (arg.isPhi() || !arg.definition.isConstInstruction()) {
return false;
}
}
@@ -484,40 +488,54 @@
private CandidateInfo analyzeAllValueUsers(
CandidateInfo candidateInfo, Value value, boolean ignoreSuperClassInitInvoke) {
- assert value != null;
+ assert value != null && value == value.getAliasedValue();
if (value.numberOfPhiUsers() > 0) {
return candidateInfo.invalidate();
}
- for (Instruction user : value.uniqueUsers()) {
- if (user.isInvokeVirtual() || user.isInvokeDirect() /* private methods */) {
- InvokeMethodWithReceiver invoke = user.asInvokeMethodWithReceiver();
- DexMethod methodReferenced = invoke.getInvokedMethod();
- if (factory.isConstructor(methodReferenced)) {
- assert user.isInvokeDirect();
- if (ignoreSuperClassInitInvoke &&
- invoke.inValues().lastIndexOf(value) == 0 &&
- methodReferenced == factory.objectMethods.constructor) {
- // If we are inside candidate constructor and analyzing usages
- // of the receiver, we want to ignore invocations of superclass
- // constructor which will be removed after staticizing.
- continue;
+ Set<Instruction> currentUsers = value.uniqueUsers();
+ while (!currentUsers.isEmpty()) {
+ Set<Instruction> indirectUsers = Sets.newIdentityHashSet();
+ for (Instruction user : currentUsers) {
+ if (user.isAssume()) {
+ if (user.outValue().numberOfPhiUsers() > 0) {
+ return candidateInfo.invalidate();
}
- return candidateInfo.invalidate();
- }
- AppInfo appInfo = appView.appInfo();
- DexEncodedMethod methodInvoked = user.isInvokeDirect()
- ? appInfo.lookupDirectTarget(methodReferenced)
- : appInfo.lookupVirtualTarget(methodReferenced.holder, methodReferenced);
- if (invoke.inValues().lastIndexOf(value) == 0 &&
- methodInvoked != null && methodInvoked.method.holder == candidateInfo.candidate.type) {
+ indirectUsers.addAll(user.outValue().uniqueUsers());
continue;
}
- }
+ if (user.isInvokeVirtual() || user.isInvokeDirect() /* private methods */) {
+ InvokeMethodWithReceiver invoke = user.asInvokeMethodWithReceiver();
+ Predicate<Value> isAliasedValue = v -> v.getAliasedValue() == value;
+ DexMethod methodReferenced = invoke.getInvokedMethod();
+ if (factory.isConstructor(methodReferenced)) {
+ assert user.isInvokeDirect();
+ if (ignoreSuperClassInitInvoke
+ && ListUtils.lastIndexMatching(invoke.inValues(), isAliasedValue) == 0
+ && methodReferenced == factory.objectMethods.constructor) {
+ // If we are inside candidate constructor and analyzing usages
+ // of the receiver, we want to ignore invocations of superclass
+ // constructor which will be removed after staticizing.
+ continue;
+ }
+ return candidateInfo.invalidate();
+ }
+ AppInfo appInfo = appView.appInfo();
+ DexEncodedMethod methodInvoked = user.isInvokeDirect()
+ ? appInfo.lookupDirectTarget(methodReferenced)
+ : appInfo.lookupVirtualTarget(methodReferenced.holder, methodReferenced);
+ if (ListUtils.lastIndexMatching(invoke.inValues(), isAliasedValue) == 0
+ && methodInvoked != null
+ && methodInvoked.method.holder == candidateInfo.candidate.type) {
+ continue;
+ }
+ }
- // All other users are not allowed.
- return candidateInfo.invalidate();
+ // All other users are not allowed.
+ return candidateInfo.invalidate();
+ }
+ currentUsers = indirectUsers;
}
return candidateInfo;
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/staticizer/StaticizingProcessor.java b/src/main/java/com/android/tools/r8/ir/optimize/staticizer/StaticizingProcessor.java
index 1ab806d..3956609 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/staticizer/StaticizingProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/staticizer/StaticizingProcessor.java
@@ -340,10 +340,11 @@
}
Set<Phi> chainedPhis = Sets.newIdentityHashSet();
for (Value operand : phi.getOperands()) {
- if (operand.isPhi()) {
+ Value v = operand.getAliasedValue();
+ if (v.isPhi()) {
chainedPhis.add(operand.asPhi());
} else {
- if (operand != thisValue) {
+ if (v != thisValue) {
return false;
}
}
@@ -360,7 +361,7 @@
// Fixup `this` usages: rewrites all method calls so that they point to static methods.
private void fixupStaticizedThisUsers(IRCode code, Value thisValue) {
- assert thisValue != null;
+ assert thisValue != null && thisValue == thisValue.getAliasedValue();
// Depending on other optimizations, e.g., inlining, `this` can be flown to phis.
Set<Phi> trivialPhis = Sets.newIdentityHashSet();
boolean onlyHasTrivialPhis = testAndCollectPhisComposedOfThis(
@@ -368,10 +369,10 @@
assert thisValue.numberOfPhiUsers() == 0 || onlyHasTrivialPhis;
assert trivialPhis.isEmpty() || onlyHasTrivialPhis;
- Set<Instruction> users = SetUtils.newIdentityHashSet(thisValue.uniqueUsers());
+ Set<Instruction> users = SetUtils.newIdentityHashSet(thisValue.aliasedUsers());
// If that is the case, method calls we want to fix up include users of those phis.
for (Phi phi : trivialPhis) {
- users.addAll(phi.uniqueUsers());
+ users.addAll(phi.aliasedUsers());
}
fixupStaticizedValueUsers(code, users);
@@ -425,13 +426,14 @@
}
Set<Phi> chainedPhis = Sets.newIdentityHashSet();
for (Value operand : phi.getOperands()) {
- if (operand.isPhi()) {
+ Value v = operand.getAliasedValue();
+ if (v.isPhi()) {
chainedPhis.add(operand.asPhi());
} else {
- if (!operand.definition.isStaticGet()) {
+ if (!v.definition.isStaticGet()) {
return false;
}
- if (operand.definition.asStaticGet().getField() != field) {
+ if (v.definition.asStaticGet().getField() != field) {
return false;
}
}
@@ -458,10 +460,10 @@
assert dest.numberOfPhiUsers() == 0 || onlyHasTrivialPhis;
assert trivialPhis.isEmpty() || onlyHasTrivialPhis;
- Set<Instruction> users = SetUtils.newIdentityHashSet(dest.uniqueUsers());
+ Set<Instruction> users = SetUtils.newIdentityHashSet(dest.aliasedUsers());
// If that is the case, method calls we want to fix up include users of those phis.
for (Phi phi : trivialPhis) {
- users.addAll(phi.uniqueUsers());
+ users.addAll(phi.aliasedUsers());
}
fixupStaticizedValueUsers(code, users);
@@ -475,6 +477,9 @@
private void fixupStaticizedValueUsers(IRCode code, Set<Instruction> users) {
for (Instruction user : users) {
+ if (user.isAssume()) {
+ continue;
+ }
assert user.isInvokeVirtual() || user.isInvokeDirect();
InvokeMethodWithReceiver invoke = user.asInvokeMethodWithReceiver();
Value newValue = null;
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/callsites/nullability/WithStaticizerTest.java b/src/test/java/com/android/tools/r8/ir/optimize/callsites/nullability/WithStaticizerTest.java
index 79ea92e..d55484d 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/callsites/nullability/WithStaticizerTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/callsites/nullability/WithStaticizerTest.java
@@ -44,7 +44,6 @@
.addKeepMainRule(MAIN)
.enableInliningAnnotations()
.enableClassInliningAnnotations()
- .noMinification()
.setMinApi(parameters.getRuntime())
.run(parameters.getRuntime(), MAIN)
.assertSuccessWithOutputLines("Input")
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/staticizer/CompanionAsArgumentTest.java b/src/test/java/com/android/tools/r8/ir/optimize/staticizer/CompanionAsArgumentTest.java
new file mode 100644
index 0000000..a93e72b
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/staticizer/CompanionAsArgumentTest.java
@@ -0,0 +1,92 @@
+// 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.ir.optimize.staticizer;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+import com.android.tools.r8.NeverClassInline;
+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.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class CompanionAsArgumentTest extends TestBase {
+ private static final Class<?> MAIN = Main.class;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ // TODO(b/112831361): support for class staticizer in CF backend.
+ return getTestParameters().withDexRuntimes().build();
+ }
+
+ private final TestParameters parameters;
+
+ public CompanionAsArgumentTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(CompanionAsArgumentTest.class)
+ .addKeepMainRule(MAIN)
+ .enableInliningAnnotations()
+ .enableClassInliningAnnotations()
+ .setMinApi(parameters.getRuntime())
+ .run(parameters.getRuntime(), MAIN)
+ .assertSuccessWithOutputLines("Companion#foo(true)")
+ .inspect(this::inspect);
+ }
+
+ private void inspect(CodeInspector inspector) {
+ // Check if the candidate is not staticized.
+ ClassSubject companion = inspector.clazz(Host.Companion.class);
+ assertThat(companion, isPresent());
+ MethodSubject foo = companion.uniqueMethodWithName("foo");
+ assertThat(foo, isPresent());
+ assertTrue(foo.streamInstructions().anyMatch(
+ i -> i.isInvokeVirtual()
+ && i.getMethod().toSourceString().contains("PrintStream.println")));
+
+ // Nothing migrated from Companion to Host.
+ ClassSubject host = inspector.clazz(Host.class);
+ assertThat(host, isPresent());
+ MethodSubject migrated_foo = host.uniqueMethodWithName("foo");
+ assertThat(migrated_foo, not(isPresent()));
+ }
+
+ @NeverClassInline
+ static class Host {
+ private static final Companion companion = new Companion();
+
+ static class Companion {
+ @NeverInline
+ public void foo(Object arg) {
+ System.out.println("Companion#foo(" + (arg != null) + ")");
+ }
+ }
+
+ @NeverInline
+ static void bar() {
+ // The target singleton is used as not only a receiver but also an argument.
+ companion.foo(companion);
+ }
+ }
+
+ static class Main {
+ public static void main(String[] args) {
+ Host.bar();
+ }
+ }
+}
\ No newline at end of file
