Enable horizontal class merging in disabled tests
Change-Id: I05c752631be63e498c86e6e83735a664cf3d9d49
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Devirtualizer.java b/src/main/java/com/android/tools/r8/ir/optimize/Devirtualizer.java
index 89681f0..4c7570c 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/Devirtualizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/Devirtualizer.java
@@ -6,6 +6,7 @@
import com.android.tools.r8.graph.AccessControl;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexClass;
+import com.android.tools.r8.graph.DexClassAndMethod;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexType;
@@ -116,9 +117,12 @@
}
}
- if (appView.options().testing.enableInvokeSuperToInvokeVirtualRewriting) {
- if (current.isInvokeSuper()) {
- InvokeSuper invoke = current.asInvokeSuper();
+ if (current.isInvokeSuper()) {
+ InvokeSuper invoke = current.asInvokeSuper();
+
+ // Check if the instruction can be rewritten to invoke-super. This allows inlining of the
+ // enclosing method into contexts outside the current class.
+ if (appView.options().testing.enableInvokeSuperToInvokeVirtualRewriting) {
DexEncodedMethod singleTarget = invoke.lookupSingleTarget(appView, context);
if (singleTarget != null) {
DexClass holder = appView.definitionForHolder(singleTarget, context);
@@ -134,10 +138,23 @@
if (newSingleTarget == singleTarget) {
it.replaceCurrentInstruction(
new InvokeVirtual(invokedMethod, invoke.outValue(), invoke.arguments()));
+ continue;
}
}
- continue;
}
+
+ // Rebind the invoke to the most specific target.
+ DexMethod invokedMethod = invoke.getInvokedMethod();
+ DexClassAndMethod reboundTarget = rebindSuperInvokeToMostSpecific(invokedMethod, context);
+ if (reboundTarget != null && reboundTarget.getReference() != invokedMethod) {
+ it.replaceCurrentInstruction(
+ new InvokeSuper(
+ reboundTarget.getReference(),
+ invoke.outValue(),
+ invoke.arguments(),
+ reboundTarget.getHolder().isInterface()));
+ }
+ continue;
}
if (current.isInvokeVirtual()) {
@@ -274,6 +291,33 @@
assert code.isConsistentSSA();
}
+ /** This rebinds invoke-super instructions to their most specific target. */
+ private DexClassAndMethod rebindSuperInvokeToMostSpecific(
+ DexMethod target, ProgramMethod context) {
+ DexEncodedMethod definition = appView.appInfo().lookupSuperTarget(target, context);
+ if (definition == null) {
+ return null;
+ }
+
+ DexClass holder = appView.definitionFor(definition.getHolderType());
+ if (holder == null) {
+ assert false;
+ return null;
+ }
+
+ if (holder.isInterface() && holder.getType() != context.getHolder().superType) {
+ // Not allowed.
+ return null;
+ }
+
+ DexClassAndMethod method = DexClassAndMethod.create(holder, definition);
+ if (AccessControl.isMemberAccessible(method, holder, context, appView).isPossiblyFalse()) {
+ return null;
+ }
+
+ return method;
+ }
+
/**
* This rebinds invoke-virtual instructions to their most specific target.
*
diff --git a/src/test/java/com/android/tools/r8/R8RunExamplesCommon.java b/src/test/java/com/android/tools/r8/R8RunExamplesCommon.java
index d288099..23ec0c9 100644
--- a/src/test/java/com/android/tools/r8/R8RunExamplesCommon.java
+++ b/src/test/java/com/android/tools/r8/R8RunExamplesCommon.java
@@ -24,7 +24,6 @@
import java.nio.file.Paths;
import java.util.Map;
import java.util.Set;
-import java.util.concurrent.ExecutionException;
import org.junit.Assume;
import org.junit.Before;
import org.junit.Rule;
@@ -187,7 +186,7 @@
}
@Test
- public void outputIsIdentical() throws IOException, InterruptedException, ExecutionException {
+ public void outputIsIdentical() throws IOException {
if (shouldCompileFail()) {
// We expected an exception, but got none.
// Return to ensure that this test fails due to the missing exception.
diff --git a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/CustomCollectionForwardingTest.java b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/CustomCollectionForwardingTest.java
index 0689a75..cf0d8ef 100644
--- a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/CustomCollectionForwardingTest.java
+++ b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/CustomCollectionForwardingTest.java
@@ -40,10 +40,6 @@
@Test
public void testCustomCollectionD8() throws Exception {
- expectThrowsWithHorizontalClassMergingIf(
- shrinkDesugaredLibrary
- && parameters.getApiLevel().isLessThan(AndroidApiLevel.N)
- && !parameters.getDexRuntimeVersion().isDefault());
KeepRuleConsumer keepRuleConsumer = createKeepRuleConsumer(parameters);
testForD8()
.addInnerClasses(CustomCollectionForwardingTest.class)
diff --git a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/CustomCollectionSuperCallsTest.java b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/CustomCollectionSuperCallsTest.java
index ce07cbf..ac48a3d 100644
--- a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/CustomCollectionSuperCallsTest.java
+++ b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/CustomCollectionSuperCallsTest.java
@@ -56,8 +56,6 @@
@Test
public void testCustomCollectionSuperCallsR8() throws Exception {
- expectThrowsWithHorizontalClassMergingIf(
- parameters.getApiLevel().isLessThan(AndroidApiLevel.N));
KeepRuleConsumer keepRuleConsumer = createKeepRuleConsumer(parameters);
R8TestRunResult r8TestRunResult =
testForR8(parameters.getBackend())
diff --git a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/jdktests/Jdk11StreamTests.java b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/jdktests/Jdk11StreamTests.java
index c75123e..9d9b5cb 100644
--- a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/jdktests/Jdk11StreamTests.java
+++ b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/jdktests/Jdk11StreamTests.java
@@ -181,10 +181,6 @@
@Test
public void testStream() throws Exception {
- expectThrowsWithHorizontalClassMergingIf(
- shrinkDesugaredLibrary
- && parameters.getApiLevel().isLessThan(AndroidApiLevel.N)
- && !parameters.getDexRuntimeVersion().isDefault());
Assume.assumeFalse(
"getAllFilesWithSuffixInDirectory() seems to find different files on Windows",
ToolHelper.isWindows());
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/reflection/GetClassTest.java b/src/test/java/com/android/tools/r8/ir/optimize/reflection/GetClassTest.java
index 406e16d..1452fd4 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/reflection/GetClassTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/reflection/GetClassTest.java
@@ -12,6 +12,7 @@
import com.android.tools.r8.CompilationMode;
import com.android.tools.r8.ForceInline;
import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.NoHorizontalClassMerging;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.ListUtils;
@@ -33,6 +34,7 @@
static class Sub extends Base {}
+ @NoHorizontalClassMerging
static class EffectivelyFinal {}
static class Reflection implements Callable<Class<?>> {
@@ -208,7 +210,6 @@
@Test
public void testR8() throws Exception {
boolean isRelease = mode == CompilationMode.RELEASE;
- expectThrowsWithHorizontalClassMergingIf(isRelease);
boolean expectCallPresent = !isRelease;
int expectedGetClassCount = isRelease ? 0 : 5;
int expectedConstClassCount = isRelease ? (parameters.isCfRuntime() ? 8 : 6) : 1;
@@ -216,6 +217,7 @@
.setMode(mode)
.addInnerClasses(GetClassTest.class)
.enableInliningAnnotations()
+ .enableNoHorizontalClassMergingAnnotations()
.addKeepMainRule(MAIN)
.noMinification()
.addOptionsModification(this::configure)
diff --git a/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsPropagationTest.java b/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsPropagationTest.java
index 816c073..4f65b48 100644
--- a/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsPropagationTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsPropagationTest.java
@@ -3,12 +3,14 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.shaking.assumenosideeffects;
+import static org.junit.Assume.assumeFalse;
import static org.junit.Assume.assumeTrue;
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.BooleanUtils;
import com.android.tools.r8.utils.StringUtils;
import java.util.Collection;
import org.junit.Test;
@@ -19,25 +21,6 @@
public class AssumenosideeffectsPropagationTest extends TestBase {
private static final Class<?> MAIN = SubsUser.class;
- private static final String JVM_OUTPUT = StringUtils.lines(
- "[Sub1, info]: message00",
- "[Base1, debug]: message00",
- "[Sub1, verbose]: message00",
- "[Sub1, info]: message1",
- "[Base1, debug]: message2",
- "[Sub1, verbose]: message3",
- "[Base2, info]: message08",
- "[AnotherSub2, debug]: message08",
- "[AnotherSub2, verbose]: message08",
- "[Base2, info]: message4",
- "[AnotherSub2, debug]: message5",
- "[AnotherSub2, verbose]: message6",
- "The end"
- );
- private static final String OUTPUT_WITHOUT_MESSAGES = StringUtils.lines(
- "The end"
- );
-
enum TestConfig {
SPECIFIC_RULES,
NON_SPECIFIC_RULES_PARTIAL,
@@ -84,33 +67,40 @@
}
}
- public String expectedOutput() {
+ public String expectedOutput(boolean enableHorizontalClassMerging) {
switch (this) {
case SPECIFIC_RULES:
- return StringUtils.lines(
- // Itf#info has side effects due to the missing Base2.
- "[Sub1, info]: message00",
- "[Base2, info]: message08",
- // Base2#info also has side effects.
- "[Base2, info]: message4",
- "The end"
- );
+ return enableHorizontalClassMerging
+ ? StringUtils.lines(
+ "[Base2, info]: message08",
+ // Base2#info also has side effects.
+ "[Base2, info]: message4",
+ "The end")
+ : StringUtils.lines(
+ // Itf#info has side effects due to the missing Base2.
+ "[Sub1, info]: message00",
+ "[Base2, info]: message08",
+ // Base2#info also has side effects.
+ "[Base2, info]: message4",
+ "The end");
case NON_SPECIFIC_RULES_PARTIAL:
- return StringUtils.lines(
- // TODO(b/133208961): Introduce comparison/meet of assume rules.
- // Itf has side effects for all methods, since we don't compute the meet yet.
- "[Sub1, info]: message00",
- "[Base1, debug]: message00",
- "[Sub1, verbose]: message00",
- "[Base2, info]: message08",
- "[AnotherSub2, debug]: message08",
- "[AnotherSub2, verbose]: message08",
- // Base2#debug also has side effects.
- "[AnotherSub2, debug]: message5",
- "The end"
- );
+ return enableHorizontalClassMerging
+ ? StringUtils.lines(
+ "[AnotherSub2, debug]: message08", "[AnotherSub2, debug]: message5", "The end")
+ : StringUtils.lines(
+ // TODO(b/133208961): Introduce comparison/meet of assume rules.
+ // Itf has side effects for all methods, since we don't compute the meet yet.
+ "[Sub1, info]: message00",
+ "[Base1, debug]: message00",
+ "[Sub1, verbose]: message00",
+ "[Base2, info]: message08",
+ "[AnotherSub2, debug]: message08",
+ "[AnotherSub2, verbose]: message08",
+ // Base2#debug also has side effects.
+ "[AnotherSub2, debug]: message5",
+ "The end");
case NON_SPECIFIC_RULES_ALL:
- return OUTPUT_WITHOUT_MESSAGES;
+ return StringUtils.lines("The end");
default:
throw new Unreachable();
}
@@ -119,38 +109,60 @@
private final TestParameters parameters;
private final TestConfig config;
+ private final boolean enableHorizontalClassMerging;
@Parameterized.Parameters(name = "{0} {1}")
public static Collection<Object[]> data() {
- return buildParameters(getTestParameters().withAllRuntimes().build(), TestConfig.values());
+ return buildParameters(
+ getTestParameters().withAllRuntimesAndApiLevels().build(),
+ TestConfig.values(),
+ BooleanUtils.values());
}
- public AssumenosideeffectsPropagationTest(TestParameters parameters, TestConfig config) {
+ public AssumenosideeffectsPropagationTest(
+ TestParameters parameters, TestConfig config, boolean enableHorizontalClassMerging) {
this.parameters = parameters;
this.config = config;
+ this.enableHorizontalClassMerging = enableHorizontalClassMerging;
}
@Test
public void testJVMOutput() throws Exception {
- assumeTrue(parameters.isCfRuntime() && config == TestConfig.SPECIFIC_RULES);
+ assumeTrue(parameters.isCfRuntime());
+ assumeTrue(config == TestConfig.SPECIFIC_RULES);
+ assumeFalse(enableHorizontalClassMerging);
testForJvm()
.addTestClasspath()
.run(parameters.getRuntime(), MAIN)
- .assertSuccessWithOutput(JVM_OUTPUT);
+ .assertSuccessWithOutputLines(
+ "[Sub1, info]: message00",
+ "[Base1, debug]: message00",
+ "[Sub1, verbose]: message00",
+ "[Sub1, info]: message1",
+ "[Base1, debug]: message2",
+ "[Sub1, verbose]: message3",
+ "[Base2, info]: message08",
+ "[AnotherSub2, debug]: message08",
+ "[AnotherSub2, verbose]: message08",
+ "[Base2, info]: message4",
+ "[AnotherSub2, debug]: message5",
+ "[AnotherSub2, verbose]: message6",
+ "The end");
}
@Test
public void testR8() throws Exception {
- expectThrowsWithHorizontalClassMergingIf(config != TestConfig.NON_SPECIFIC_RULES_ALL);
testForR8(parameters.getBackend())
.addInnerClasses(AssumenosideeffectsPropagationTest.class)
.addKeepMainRule(MAIN)
.addKeepRules(config.getKeepRules())
+ .addOptionsModification(
+ options -> options.enableHorizontalClassMerging = enableHorizontalClassMerging)
.enableInliningAnnotations()
.noMinification()
- .setMinApi(parameters.getRuntime())
+ .setMinApi(parameters.getApiLevel())
.run(parameters.getRuntime(), MAIN)
- .assertSuccessWithOutput(config.expectedOutput());
+ .assertSuccessWithOutput(config.expectedOutput(enableHorizontalClassMerging));
}
interface Itf {
