Lookup -assumevalues when trying to replace a call with a constant.
We overweigh -assumenosideeffects. But, if the matched rule doesn't have
a return value, while there is another rule to specify assumed value,
each rule interfere each other.
While keeping the precedence as-is, we need to switch to use -assumevalues
if -assumenosideeffects without return value is about to be used for
attempting to find a replacement constant.
Bug: 141127471
Change-Id: Icd73958acf5c25c6caaf2fffecbcfc34f8ec09f8
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java b/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
index 86a4de5..6644b26 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
@@ -277,21 +277,32 @@
}
boolean invokeReplaced = false;
if (lookup != null) {
- boolean outValueNullOrNotUsed = current.outValue() == null || !current.outValue().isUsed();
- if (lookup.type == RuleType.ASSUME_NO_SIDE_EFFECTS && outValueNullOrNotUsed) {
- // Remove invoke if marked as having no side effects and the return value is not used.
- iterator.removeOrReplaceByDebugLocalRead();
+ boolean hasUsedOutValue = current.hasOutValue() && current.outValue().isUsed();
+ if (!hasUsedOutValue) {
+ if (lookup.type == RuleType.ASSUME_NO_SIDE_EFFECTS) {
+ // Remove invoke if marked as having no side effects and the return value is not used.
+ iterator.removeOrReplaceByDebugLocalRead();
+ }
return;
}
- if (!outValueNullOrNotUsed) {
- // Check to see if a constant value can be assumed.
- invokeReplaced =
- tryConstantReplacementFromProguard(
- code, affectedValues, blocks, iterator, current, lookup);
+ // Check to see if a constant value can be assumed.
+ // But, if the current matched rule is -assumenosideeffects without the return value, it won't
+ // be transformed into a replacement instruction. Check if there is -assumevalues rule bound
+ // to the target.
+ if (target != null
+ && lookup.type == RuleType.ASSUME_NO_SIDE_EFFECTS
+ && !lookup.rule.hasReturnValue()) {
+ ProguardMemberRule rule = appView.appInfo().assumedValues.get(target.toReference());
+ if (rule != null) {
+ lookup = new ProguardMemberRuleLookup(RuleType.ASSUME_VALUES, rule);
+ }
}
+ invokeReplaced =
+ tryConstantReplacementFromProguard(
+ code, affectedValues, blocks, iterator, current, lookup);
}
- if (invokeReplaced || current.outValue() == null) {
+ if (invokeReplaced || !current.hasOutValue()) {
return;
}
// No Proguard rule could replace the instruction check for knowledge about the return value.
diff --git a/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsWithoutReturnValueTest.java b/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsWithoutReturnValueTest.java
new file mode 100644
index 0000000..3596b42
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsWithoutReturnValueTest.java
@@ -0,0 +1,105 @@
+// 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.shaking.assumenosideeffects;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.errors.Unreachable;
+import com.android.tools.r8.utils.StringUtils;
+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 java.util.Collection;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class AssumenosideeffectsWithoutReturnValueTest extends TestBase {
+ private static final Class<?> MAIN = TestClass.class;
+
+ enum TestConfig {
+ RULE_WITH_RETURN_VALUE,
+ SEPARATE_RULES;
+
+ public String getKeepRules() {
+ switch (this) {
+ case RULE_WITH_RETURN_VALUE:
+ return StringUtils.lines(
+ "-assumenosideeffects class ** {",
+ " *** debug(...) return false;",
+ "}"
+ );
+ case SEPARATE_RULES:
+ return StringUtils.lines(
+ "-assumenosideeffects class ** {",
+ " *** debug(...);",
+ "}",
+ "-assumevalues class ** {",
+ " *** debug(...) return false;",
+ "}"
+ );
+ default:
+ throw new Unreachable();
+ }
+ }
+ }
+
+ private final TestParameters parameters;
+ private final TestConfig config;
+
+ @Parameterized.Parameters(name = "{0} {1}")
+ public static Collection<Object[]> data() {
+ return buildParameters(getTestParameters().withAllRuntimes().build(), TestConfig.values());
+ }
+
+ public AssumenosideeffectsWithoutReturnValueTest(
+ TestParameters parameters, TestConfig config) {
+ this.parameters = parameters;
+ this.config = config;
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(AssumenosideeffectsWithoutReturnValueTest.class)
+ .enableInliningAnnotations()
+ .addKeepMainRule(MAIN)
+ .addKeepRules(config.getKeepRules())
+ .noMinification()
+ .setMinApi(parameters.getRuntime())
+ .compile()
+ .inspect(this::verifyDebugMethodIsRemoved)
+ .run(parameters.getRuntime(), MAIN)
+ .assertSuccessWithOutputLines("The end");
+ }
+
+ private void verifyDebugMethodIsRemoved(CodeInspector inspector) {
+ ClassSubject main = inspector.clazz(MAIN);
+ assertThat(main, isPresent());
+ MethodSubject debug = main.uniqueMethodWithName("debug");
+ assertThat(debug, not(isPresent()));
+ }
+
+ static class TestClass {
+ @NeverInline
+ boolean debug() {
+ return System.currentTimeMillis() > 0;
+ }
+
+ public static void main(String... args) {
+ TestClass instance = new TestClass();
+ if (instance.debug()) {
+ System.out.println("message");
+ }
+ System.out.println("The end");
+ }
+ }
+}
