Move Objects.requireNonNull and Lomg.compare handling into BackportedMethodRewriter
This adds support for arbitrary IR rewriting while handling a method that needs backported. For these two specific methods, use the same IR manipulation as before, but it is now centralized with the other, template-based backports for Objects and Long.
Test: tools/test.py --no-internal -v *Backport*Test*
Change-Id: I7e011bd2c9acbd280db39005e1fbac5849151ce1
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 db352a2..1d00f15 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
@@ -1163,7 +1163,6 @@
}
codeRewriter.rewriteKnownArrayLengthCalls(code);
- codeRewriter.rewriteLongCompareAndRequireNonNull(code, options);
codeRewriter.rewriteAssertionErrorTwoArgumentConstructor(code, options);
codeRewriter.commonSubexpressionElimination(code);
codeRewriter.simplifyArrayConstruction(code);
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/BackportedMethodRewriter.java b/src/main/java/com/android/tools/r8/ir/desugar/BackportedMethodRewriter.java
index 4298d1b..e19e587 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/BackportedMethodRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/BackportedMethodRewriter.java
@@ -108,11 +108,10 @@
continue;
}
- DexMethod newMethod = provider.provideMethod(appView);
- iterator.replaceCurrentInstruction(
- new InvokeStatic(newMethod, invoke.outValue(), invoke.inValues()));
+ provider.rewriteInvoke(invoke, iterator, appView);
if (provider.requiresGenerationOfCode()) {
+ DexMethod newMethod = provider.provideMethod(appView);
methodProviders.putIfAbsent(newMethod, provider);
holders.add(code.method.method.holder);
}
@@ -269,9 +268,6 @@
}
private void initializeAndroidKMethodProviders(DexItemFactory factory) {
- // Note: Long.compare rewriting is handled by CodeRewriter since there is a dedicated
- // bytecode which supports the operation.
-
// Byte
DexType type = factory.boxedByteType;
// int Byte.compare(byte a, byte b)
@@ -296,6 +292,14 @@
method = factory.createMethod(type, proto, name);
addProvider(new MethodGenerator(method, IntegerMethods::new));
+ // Long
+ type = factory.boxedLongType;
+ // int Long.compare(long a, long b)
+ name = factory.createString("compare");
+ proto = factory.createProto(factory.intType, factory.longType, factory.longType);
+ method = factory.createMethod(type, proto, name);
+ addProvider(new InvokeRewriter(method, LongMethods::rewriteCompare));
+
// Boolean
type = factory.boxedBooleanType;
// int Boolean.compare(boolean a, boolean b)
@@ -346,7 +350,11 @@
method = factory.createMethod(type, proto, name);
addProvider(new MethodGenerator(method, ObjectsMethods::new));
- // Note: Objects.requireNonNull(T) rewriting is handled by CodeRewriter for now.
+ // T Objects.requireNonNull(T obj)
+ name = factory.createString("requireNonNull");
+ proto = factory.createProto(factory.objectType, factory.objectType);
+ method = factory.createMethod(type, proto, name);
+ addProvider(new InvokeRewriter(method, ObjectsMethods::rewriteRequireNonNull));
// T Objects.requireNonNull(T obj, String message)
name = factory.createString("requireNonNull");
@@ -1052,6 +1060,9 @@
this.method = method;
}
+ public abstract void rewriteInvoke(InvokeMethod invoke, InstructionListIterator iterator,
+ AppView<?> appView);
+
public abstract DexMethod provideMethod(AppView<?> appView);
public abstract TemplateMethodCode generateTemplateMethod(
@@ -1074,6 +1085,13 @@
}
@Override
+ public void rewriteInvoke(InvokeMethod invoke, InstructionListIterator iterator,
+ AppView<?> appView) {
+ iterator.replaceCurrentInstruction(
+ new InvokeStatic(provideMethod(appView), invoke.outValue(), invoke.inValues()));
+ }
+
+ @Override
public DexMethod provideMethod(AppView<?> appView) {
if (targetMethod != null) {
return targetMethod;
@@ -1096,6 +1114,33 @@
}
}
+ private static final class InvokeRewriter extends MethodProvider {
+ private final MethodInvokeRewriter rewriter;
+
+ InvokeRewriter(DexMethod method, MethodInvokeRewriter rewriter) {
+ super(method);
+ this.rewriter = rewriter;
+ }
+
+ @Override public void rewriteInvoke(InvokeMethod invoke, InstructionListIterator iterator,
+ AppView<?> appView) {
+ rewriter.rewrite(invoke, iterator, appView.dexItemFactory());
+ }
+
+ @Override public boolean requiresGenerationOfCode() {
+ return false;
+ }
+
+ @Override public DexMethod provideMethod(AppView<?> appView) {
+ throw new Unreachable();
+ }
+
+ @Override
+ public TemplateMethodCode generateTemplateMethod(InternalOptions options, DexMethod method) {
+ throw new Unreachable();
+ }
+ }
+
private static class MethodGenerator extends MethodProvider {
private final TemplateMethodFactory factory;
@@ -1113,6 +1158,13 @@
}
@Override
+ public void rewriteInvoke(InvokeMethod invoke, InstructionListIterator iterator,
+ AppView<?> appView) {
+ iterator.replaceCurrentInstruction(
+ new InvokeStatic(provideMethod(appView), invoke.outValue(), invoke.inValues()));
+ }
+
+ @Override
public DexMethod provideMethod(AppView<?> appView) {
if (generatedMethod != null) {
return generatedMethod;
@@ -1179,4 +1231,8 @@
TemplateMethodCode create(InternalOptions options, DexMethod method, String name);
}
+
+ private interface MethodInvokeRewriter {
+ void rewrite(InvokeMethod invoke, InstructionListIterator iterator, DexItemFactory factory);
+ }
}
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/backports/LongMethods.java b/src/main/java/com/android/tools/r8/ir/desugar/backports/LongMethods.java
index e3bf3b5..639f8d4 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/backports/LongMethods.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/backports/LongMethods.java
@@ -4,9 +4,17 @@
package com.android.tools.r8.ir.desugar.backports;
+import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.ir.code.Cmp;
+import com.android.tools.r8.ir.code.Cmp.Bias;
+import com.android.tools.r8.ir.code.InstructionListIterator;
+import com.android.tools.r8.ir.code.InvokeMethod;
+import com.android.tools.r8.ir.code.NumericType;
+import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.ir.synthetic.TemplateMethodCode;
import com.android.tools.r8.utils.InternalOptions;
+import java.util.List;
public final class LongMethods extends TemplateMethodCode {
public LongMethods(InternalOptions options, DexMethod method, String methodName) {
@@ -198,4 +206,12 @@
return new String(buf, i, buf.length - i);
}
}
+
+ public static void rewriteCompare(InvokeMethod invoke, InstructionListIterator iterator,
+ DexItemFactory factory) {
+ List<Value> inValues = invoke.inValues();
+ assert inValues.size() == 2;
+ iterator.replaceCurrentInstruction(
+ new Cmp(NumericType.LONG, Bias.NONE, invoke.outValue(), inValues.get(0), inValues.get(1)));
+ }
}
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/backports/ObjectsMethods.java b/src/main/java/com/android/tools/r8/ir/desugar/backports/ObjectsMethods.java
index 4fc3cb1..6ac64e9 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/backports/ObjectsMethods.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/backports/ObjectsMethods.java
@@ -4,7 +4,11 @@
package com.android.tools.r8.ir.desugar.backports;
+import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.ir.code.InstructionListIterator;
+import com.android.tools.r8.ir.code.InvokeMethod;
+import com.android.tools.r8.ir.code.InvokeVirtual;
import com.android.tools.r8.ir.synthetic.TemplateMethodCode;
import com.android.tools.r8.utils.InternalOptions;
import java.util.Arrays;
@@ -128,4 +132,15 @@
}
return fromIndex;
}
+
+ public static void rewriteRequireNonNull(InvokeMethod invoke, InstructionListIterator iterator,
+ DexItemFactory factory) {
+ InvokeVirtual getClass =
+ new InvokeVirtual(factory.objectMethods.getClass, null, invoke.inValues());
+ if (invoke.outValue() != null) {
+ invoke.outValue().replaceUsers(invoke.inValues().get(0));
+ invoke.setOutValue(null);
+ }
+ iterator.replaceCurrentInstruction(getClass);
+ }
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
index 2511133..7c1053c 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
@@ -42,8 +42,6 @@
import com.android.tools.r8.ir.code.Binop;
import com.android.tools.r8.ir.code.CatchHandlers;
import com.android.tools.r8.ir.code.CheckCast;
-import com.android.tools.r8.ir.code.Cmp;
-import com.android.tools.r8.ir.code.Cmp.Bias;
import com.android.tools.r8.ir.code.ConstInstruction;
import com.android.tools.r8.ir.code.ConstNumber;
import com.android.tools.r8.ir.code.ConstString;
@@ -3742,39 +3740,6 @@
assert code.isConsistentSSA();
}
- public void rewriteLongCompareAndRequireNonNull(IRCode code, InternalOptions options) {
- if (options.canUseJava7CompareAndObjectsOperations()) {
- return;
- }
-
- InstructionListIterator iterator = code.instructionListIterator();
- while (iterator.hasNext()) {
- Instruction current = iterator.next();
- if (current.isInvokeMethod()) {
- DexMethod invokedMethod = current.asInvokeMethod().getInvokedMethod();
- if (invokedMethod == dexItemFactory.longMethods.compare) {
- // Rewrite calls to Long.compare for sdk versions that do not have that method.
- List<Value> inValues = current.inValues();
- assert inValues.size() == 2;
- iterator.replaceCurrentInstruction(
- new Cmp(NumericType.LONG, Bias.NONE, current.outValue(), inValues.get(0),
- inValues.get(1)));
- } else if (invokedMethod == dexItemFactory.objectsMethods.requireNonNull) {
- // Rewrite calls to Objects.requireNonNull(Object) because Javac 9 start to use it for
- // synthesized null checks.
- InvokeVirtual callToGetClass = new InvokeVirtual(dexItemFactory.objectMethods.getClass,
- null, current.inValues());
- if (current.outValue() != null) {
- current.outValue().replaceUsers(current.inValues().get(0));
- current.setOutValue(null);
- }
- iterator.replaceCurrentInstruction(callToGetClass);
- }
- }
- }
- assert code.isConsistentSSA();
- }
-
public void rewriteAssertionErrorTwoArgumentConstructor(IRCode code, InternalOptions options) {
if (options.canUseAssertionErrorTwoArgumentConstructor()) {
return;
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index c9407a8..e35245d 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -1081,10 +1081,6 @@
return intermediate || hasMinApi(AndroidApiLevel.L);
}
- public boolean canUseJava7CompareAndObjectsOperations() {
- return isGeneratingClassFiles() || hasMinApi(AndroidApiLevel.K);
- }
-
public boolean canUseSuppressedExceptions() {
return isGeneratingClassFiles() || hasMinApi(AndroidApiLevel.K);
}
diff --git a/src/test/examples/rewrite/LongCompare.java b/src/test/examples/rewrite/LongCompare.java
deleted file mode 100644
index 7c96c10..0000000
--- a/src/test/examples/rewrite/LongCompare.java
+++ /dev/null
@@ -1,40 +0,0 @@
-// Copyright (c) 2017, 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 rewrite;
-
-public class LongCompare {
-
- public static int simpleCompare(long l1, long l2) {
- try {
- return Long.compare(l1, l2);
- } catch (Throwable t) {
- System.out.println(t);
- }
- return 2;
- }
-
- public static long getValue1() {
- return 123456789L;
- }
-
- public static long getValue2() {
- return 0;
- }
-
- public static boolean complexCompare(long l1, long l2) {
- return Long.compare(getValue1(), l1) == 0 && Long.compare(l2, getValue2()) > 0;
- }
-
- public static void main(String[] args) {
- System.out.println(simpleCompare(123456789L, 987654321L));
- System.out.println(simpleCompare(Long.MAX_VALUE, 0L));
- System.out.println(simpleCompare(Long.MIN_VALUE, 0L));
- System.out.println(simpleCompare(Long.MAX_VALUE, Long.MAX_VALUE));
-
- System.out.println(complexCompare(123456789L, 1));
- System.out.println(complexCompare(123456789L, -1));
- System.out.println(complexCompare(1234567890L, 1));
- System.out.println(complexCompare(1234567890L, -1));
- }
-}
diff --git a/src/test/examples/rewrite/RequireNonNull.java b/src/test/examples/rewrite/RequireNonNull.java
deleted file mode 100644
index d8868bf..0000000
--- a/src/test/examples/rewrite/RequireNonNull.java
+++ /dev/null
@@ -1,39 +0,0 @@
-// Copyright (c) 2017, 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 rewrite;
-
-import java.util.Objects;
-
-public class RequireNonNull {
-
- public static void main(String[] args) {
- RequireNonNull o = new RequireNonNull();
- System.out.println(o.nonnullRemove().toString());
- System.out.println(o.nonnullRemove(o).toString());
- o.nonnullWithPhiInstruction(true, o);
- o.nonnullWithPhiInstruction(false, o);
- }
-
- private Object nonnullRemove() {
- return Objects.requireNonNull(this);
- }
-
- private Object nonnullRemove(Object o) {
- Objects.requireNonNull(o);
- return o;
- }
-
- private void nonnullWithPhiInstruction(boolean b, Object input) {
- Object o = null;
- if (b) {
- o = Objects.requireNonNull(input);
- }
- System.out.println(o);
- }
-
- @Override
- public String toString() {
- return "toString";
- }
-}
diff --git a/src/test/java/com/android/tools/r8/rewrite/longcompare/LongCompare.java b/src/test/java/com/android/tools/r8/rewrite/longcompare/LongCompare.java
deleted file mode 100644
index 16b7922..0000000
--- a/src/test/java/com/android/tools/r8/rewrite/longcompare/LongCompare.java
+++ /dev/null
@@ -1,80 +0,0 @@
-// Copyright (c) 2017, 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.rewrite.longcompare;
-
-import com.android.tools.r8.CompilationFailedException;
-import com.android.tools.r8.D8;
-import com.android.tools.r8.D8Command;
-import com.android.tools.r8.OutputMode;
-import com.android.tools.r8.ToolHelper;
-import com.android.tools.r8.ToolHelper.ArtCommandBuilder;
-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.InstructionSubject;
-import com.android.tools.r8.utils.codeinspector.InvokeInstructionSubject;
-import com.android.tools.r8.utils.codeinspector.MethodSubject;
-import java.io.IOException;
-import java.nio.file.Path;
-import java.nio.file.Paths;
-import java.util.Arrays;
-import java.util.Iterator;
-import org.junit.Assert;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
-
-/**
- * Tests checking that Long.compare() is always rewritten into long compare instruction.
- */
-public class LongCompare {
-
- @Rule
- public TemporaryFolder tmpOutputDir = ToolHelper.getTemporaryFolderForTest();
-
- void compileWithD8(Path intputPath, Path outputPath)
- throws IOException, CompilationFailedException {
- D8.run(
- D8Command.builder()
- .addProgramFiles(intputPath)
- .setOutput(outputPath, OutputMode.DexIndexed)
- .build());
- }
-
- void runTest(Path dexFile) {
- ArtCommandBuilder builder = new ArtCommandBuilder(ToolHelper.getDexVm());
- builder.appendClasspath(dexFile.toString());
- builder.setMainClass("rewrite.LongCompare");
- try {
- String output = ToolHelper.runArt(builder);
- Assert
- .assertEquals(StringUtils.lines("-1", "1", "-1", "0", "true", "false", "false", "false"),
- output);
- } catch (IOException e) {
- Assert.fail();
- }
- }
-
- @Test
- public void testLongCompareIsRewritten() throws Exception {
- final Path inputPath = Paths.get(ToolHelper.EXAMPLES_BUILD_DIR + "/rewrite.jar");
- Path outputPath = tmpOutputDir.newFolder().toPath();
-
- compileWithD8(inputPath, outputPath);
-
- Path dexPath = outputPath.resolve("classes.dex");
-
- CodeInspector codeInspector = new CodeInspector(dexPath);
- ClassSubject classSubject = codeInspector.clazz("rewrite.LongCompare");
- MethodSubject methodSubject = classSubject
- .method("int", "simpleCompare", Arrays.asList("long", "long"));
- // Check that exception handler is removed since it is no longer needed.
- Assert.assertFalse(methodSubject.getMethod().getCode().asDexCode().usesExceptionHandling());
- Iterator<InvokeInstructionSubject> iterator =
- methodSubject.iterateInstructions(InstructionSubject::isInvoke);
- Assert.assertFalse(iterator.hasNext());
-
- runTest(dexPath);
- }
-}
diff --git a/src/test/java/com/android/tools/r8/rewrite/longcompare/RequireNonNullRewriteTest.java b/src/test/java/com/android/tools/r8/rewrite/longcompare/RequireNonNullRewriteTest.java
deleted file mode 100644
index 278bc8b..0000000
--- a/src/test/java/com/android/tools/r8/rewrite/longcompare/RequireNonNullRewriteTest.java
+++ /dev/null
@@ -1,91 +0,0 @@
-// Copyright (c) 2017, 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.rewrite.longcompare;
-
-import com.android.tools.r8.CompilationFailedException;
-import com.android.tools.r8.CompilationMode;
-import com.android.tools.r8.D8;
-import com.android.tools.r8.D8Command;
-import com.android.tools.r8.OutputMode;
-import com.android.tools.r8.ToolHelper;
-import com.android.tools.r8.ToolHelper.ArtCommandBuilder;
-import com.android.tools.r8.ToolHelper.ProcessResult;
-import com.android.tools.r8.utils.codeinspector.ClassSubject;
-import com.android.tools.r8.utils.codeinspector.CodeInspector;
-import com.android.tools.r8.utils.codeinspector.InstructionSubject;
-import com.android.tools.r8.utils.codeinspector.InvokeInstructionSubject;
-import com.android.tools.r8.utils.codeinspector.MethodSubject;
-import java.io.IOException;
-import java.nio.file.Path;
-import java.nio.file.Paths;
-import java.util.Collections;
-import java.util.Iterator;
-import java.util.concurrent.ExecutionException;
-import org.junit.Assert;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
-
-public class RequireNonNullRewriteTest {
-
- @Rule
- public TemporaryFolder tmpOutputDir = ToolHelper.getTemporaryFolderForTest();
-
- void compileWithD8(Path intputPath, Path outputPath, CompilationMode mode)
- throws IOException, CompilationFailedException {
- D8.run(
- D8Command.builder()
- .setMode(mode)
- .addProgramFiles(intputPath)
- .setOutput(outputPath, OutputMode.DexIndexed)
- .build());
- }
-
- void runTest(Path jarFile, Path dexFile) {
- String mainClass = "rewrite.RequireNonNull";
- ArtCommandBuilder builder = new ArtCommandBuilder(ToolHelper.getDexVm());
- builder.appendClasspath(dexFile.toString());
- builder.setMainClass(mainClass);
- try {
- String output = ToolHelper.runArt(builder);
- ProcessResult javaResult = ToolHelper.runJava(jarFile, mainClass);
- Assert.assertEquals(javaResult.stdout, output);
- } catch (IOException e) {
- Assert.fail();
- }
- }
-
- @Test
- public void testDebugRequireNonNullIsRewritten() throws Exception {
- runTest(CompilationMode.DEBUG);
- }
-
- @Test
- public void testReleaseRequireNonNullIsRewritten() throws Exception {
- runTest(CompilationMode.RELEASE);
- }
-
- private void runTest(CompilationMode mode)
- throws IOException, ExecutionException, CompilationFailedException {
- final Path inputPath = Paths.get(ToolHelper.EXAMPLES_BUILD_DIR + "/rewrite.jar");
- Path outputPath = tmpOutputDir.newFolder().toPath();
-
- compileWithD8(inputPath, outputPath, mode);
-
- Path dexPath = outputPath.resolve("classes.dex");
-
- CodeInspector codeInspector = new CodeInspector(dexPath);
- ClassSubject classSubject = codeInspector.clazz("rewrite.RequireNonNull");
- MethodSubject methodSubject = classSubject
- .method("java.lang.Object", "nonnullRemove", Collections.emptyList());
-
- Iterator<InvokeInstructionSubject> iterator =
- methodSubject.iterateInstructions(InstructionSubject::isInvoke);
- Assert.assertTrue(iterator.hasNext());
- Assert.assertEquals("getClass", iterator.next().invokedMethod().name.toString());
- Assert.assertFalse(iterator.hasNext());
-
- runTest(inputPath, dexPath);
- }
-}
