Revert "Revised workaround for Android 6.0 dex2oat divergence issue."
This reverts commit cf110979123d03ab2405d04b7c0ce9d0ea1539bf.
Reason for revert: CL leads to NPE during register allocation in internal tests.
Change-Id: I0a65d7516ea1bd08232f1f9418e0b50948a78682
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 6288eaa..05fd603 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
@@ -36,7 +36,6 @@
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.InstructionListIterator;
import com.android.tools.r8.ir.code.InvokeMethod;
-import com.android.tools.r8.ir.code.InvokeStatic;
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.ir.desugar.CovariantReturnTypeAnnotationTransformer;
import com.android.tools.r8.ir.desugar.InterfaceMethodRewriter;
@@ -94,7 +93,6 @@
import java.util.function.BiConsumer;
import java.util.function.Function;
import java.util.function.Predicate;
-import java.util.function.Supplier;
import java.util.stream.Collectors;
public class IRConverter {
@@ -1243,116 +1241,20 @@
}
}
- /**
- * For each block, we look to see if the header matches:
- *
- * <pre>
- * pseudo-instructions*
- * v2 <- long-{mul,div} v0 v1
- * pseudo-instructions*
- * v5 <- long-{add,sub} v3 v4
- * </pre>
- *
- * where v2 ~=~ v3 or v2 ~=~ v4 (with ~=~ being equal or an alias of) and the block is not a
- * fallthrough target.
- */
private void materializeInstructionBeforeLongOperationsWorkaround(IRCode code) {
if (!options.canHaveDex2OatLinkedListBug()) {
return;
}
- DexItemFactory factory = options.itemFactory;
- final Supplier<DexMethod> javaLangLangSignum =
- Suppliers.memoize(
- () ->
- factory.createMethod(
- factory.createString("Ljava/lang/Long;"),
- factory.createString("signum"),
- factory.intDescriptor,
- new DexString[] {factory.longDescriptor}));
for (BasicBlock block : code.blocks) {
InstructionListIterator it = block.listIterator();
- Instruction firstMaterializing = it.nextUntil(IRConverter::isNotPseudoInstruction);
- if (!isLongMulOrDiv(firstMaterializing)) {
- continue;
- }
- Instruction secondMaterializing = it.nextUntil(IRConverter::isNotPseudoInstruction);
- if (!isLongAddOrSub(secondMaterializing)) {
- continue;
- }
- if (isFallthoughTarget(block)) {
- continue;
- }
- Value outOfMulOrDiv = firstMaterializing.outValue();
- for (Value inOfAddOrSub : secondMaterializing.inValues()) {
- if (isAliasOf(inOfAddOrSub, outOfMulOrDiv)) {
- it = block.listIterator();
- it.nextUntil(i -> i == firstMaterializing);
- Value longValue = firstMaterializing.inValues().get(0);
- InvokeStatic invokeLongSignum =
- new InvokeStatic(
- javaLangLangSignum.get(), null, Collections.singletonList(longValue));
- ensureThrowingInstructionBefore(code, firstMaterializing, it, invokeLongSignum);
- return;
- }
+ Instruction firstMaterializing =
+ it.nextUntil(IRConverter::isMaterializingInstructionOnArtArmVersionM);
+ if (needsInstructionBeforeLongOperation(firstMaterializing)) {
+ ensureInstructionBefore(code, firstMaterializing, it);
}
}
}
- private static boolean isAliasOf(Value usedValue, Value definingValue) {
- while (true) {
- if (usedValue == definingValue) {
- return true;
- }
- Instruction definition = usedValue.definition;
- if (definition == null || !definition.isMove()) {
- return false;
- }
- usedValue = definition.asMove().src();
- }
- }
-
- private static boolean isNotPseudoInstruction(Instruction instruction) {
- return !(instruction.isDebugInstruction() || instruction.isMove());
- }
-
- private static boolean isLongMulOrDiv(Instruction instruction) {
- return instruction != null
- && instruction.outValue() != null
- && instruction.outValue().getTypeLattice().isLong()
- && (instruction.isMul() || instruction.isDiv());
- }
-
- private static boolean isLongAddOrSub(Instruction instruction) {
- return instruction != null
- && instruction.outValue() != null
- && instruction.outValue().getTypeLattice().isLong()
- && (instruction.isAdd() || instruction.isSub());
- }
-
- private static boolean isFallthoughTarget(BasicBlock block) {
- for (BasicBlock pred : block.getPredecessors()) {
- if (pred.exit().fallthroughBlock() == block) {
- return true;
- }
- }
- return false;
- }
-
- private void ensureThrowingInstructionBefore(
- IRCode code, Instruction addBefore, InstructionListIterator it, Instruction instruction) {
- Instruction check = it.previous();
- assert addBefore == check;
- BasicBlock block = check.getBlock();
- if (block.hasCatchHandlers()) {
- // Split the block and copy back catch handlers to the header block.
- BasicBlock split = it.split(code);
- block.copyCatchHandlers(code, code.listIterator(code.blocks.size()), split);
- it = block.listIterator(block.getInstructions().size() - 1);
- }
- instruction.setPosition(addBefore.getPosition());
- it.add(instruction);
- }
-
private static void ensureInstructionBefore(
IRCode code, Instruction addBefore, InstructionListIterator it) {
// Force materialize a constant-zero before the long operation.
@@ -1371,6 +1273,33 @@
it.add(fixitUser);
}
+ private static boolean needsInstructionBeforeLongOperation(Instruction instruction) {
+ // The cortex fixup will only trigger on long sub and long add instructions.
+ if (!((instruction.isAdd() || instruction.isSub()) && instruction.outType().isWide())) {
+ return false;
+ }
+ // If the block with the instruction is a fallthrough block, then it can't end up being
+ // preceded by the incorrectly linked prologue/epilogue..
+ BasicBlock block = instruction.getBlock();
+ for (BasicBlock pred : block.getPredecessors()) {
+ if (pred.exit().fallthroughBlock() == block) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ private static boolean isMaterializingInstructionOnArtArmVersionM(Instruction instruction) {
+ return !instruction.isDebugInstruction()
+ && !instruction.isMove()
+ && !isPossiblyNonMaterializingLongOperationOnArtArmVersionM(instruction);
+ }
+
+ private static boolean isPossiblyNonMaterializingLongOperationOnArtArmVersionM(
+ Instruction instruction) {
+ return (instruction.isMul() || instruction.isDiv()) && instruction.outType().isWide();
+ }
+
private void printC1VisualizerHeader(DexEncodedMethod method) {
if (printer != null) {
printer.begin("compilation");
diff --git a/src/test/java/com/android/tools/r8/DXTestBuilder.java b/src/test/java/com/android/tools/r8/DXTestBuilder.java
index 190db23..8312729 100644
--- a/src/test/java/com/android/tools/r8/DXTestBuilder.java
+++ b/src/test/java/com/android/tools/r8/DXTestBuilder.java
@@ -67,11 +67,6 @@
}
@Override
- public DXTestBuilder addProgramClassFileData(Collection<byte[]> classes) {
- throw new Unimplemented("No support for adding classfile data directly");
- }
-
- @Override
public DXTestBuilder addProgramFiles(Collection<Path> files) {
injars.addAll(files);
return self();
diff --git a/src/test/java/com/android/tools/r8/JvmTestBuilder.java b/src/test/java/com/android/tools/r8/JvmTestBuilder.java
index 6feb460..4e0bedf 100644
--- a/src/test/java/com/android/tools/r8/JvmTestBuilder.java
+++ b/src/test/java/com/android/tools/r8/JvmTestBuilder.java
@@ -147,12 +147,6 @@
"No support for adding paths directly (we need to compute the descriptor)");
}
- @Override
- public JvmTestBuilder addProgramClassFileData(Collection<byte[]> files) {
- throw new Unimplemented(
- "No support for adding classfile data directly (we need to compute the descriptor)");
- }
-
public JvmTestBuilder addClasspath(Path... paths) {
return addClasspath(Arrays.asList(paths));
}
diff --git a/src/test/java/com/android/tools/r8/ProguardTestBuilder.java b/src/test/java/com/android/tools/r8/ProguardTestBuilder.java
index fa4c41c..28ce1e6 100644
--- a/src/test/java/com/android/tools/r8/ProguardTestBuilder.java
+++ b/src/test/java/com/android/tools/r8/ProguardTestBuilder.java
@@ -184,12 +184,6 @@
}
@Override
- public ProguardTestBuilder addProgramClassFileData(Collection<byte[]> classes) {
- throw new Unimplemented(
- "No support for adding classfile data directly (we need to compute the descriptor)");
- }
-
- @Override
public ProguardTestBuilder addKeepRules(Collection<String> rules) {
config.addAll(rules);
return self();
diff --git a/src/test/java/com/android/tools/r8/TestBuilder.java b/src/test/java/com/android/tools/r8/TestBuilder.java
index 7f200c1..4977d94 100644
--- a/src/test/java/com/android/tools/r8/TestBuilder.java
+++ b/src/test/java/com/android/tools/r8/TestBuilder.java
@@ -39,12 +39,6 @@
public abstract T addProgramFiles(Collection<Path> files);
- public abstract T addProgramClassFileData(Collection<byte[]> classes);
-
- public T addProgramClassFileData(byte[]... classes) {
- return addProgramClassFileData(Arrays.asList(classes));
- }
-
public T addProgramClasses(Class<?>... classes) {
return addProgramClasses(Arrays.asList(classes));
}
diff --git a/src/test/java/com/android/tools/r8/TestCompileResult.java b/src/test/java/com/android/tools/r8/TestCompileResult.java
index bf704ea..12d1990 100644
--- a/src/test/java/com/android/tools/r8/TestCompileResult.java
+++ b/src/test/java/com/android/tools/r8/TestCompileResult.java
@@ -6,7 +6,6 @@
import static com.android.tools.r8.TestBase.Backend.DEX;
import com.android.tools.r8.TestBase.Backend;
-import com.android.tools.r8.ToolHelper.DexVm;
import com.android.tools.r8.ToolHelper.ProcessResult;
import com.android.tools.r8.debug.CfDebugTestConfig;
import com.android.tools.r8.debug.DebugTestConfig;
@@ -99,17 +98,4 @@
ProcessResult result = ToolHelper.runArtRaw(out.toString(), mainClass);
return createRunResult(app, result);
}
-
- public TestRunResult runDex2Oat() throws IOException {
- return runDex2Oat(ToolHelper.getDexVm());
- }
-
- public TestRunResult runDex2Oat(DexVm vm) throws IOException {
- assert getBackend() == DEX;
- Path tmp = state.getNewTempFolder();
- Path jarFile = tmp.resolve("out.jar");
- Path oatFile = tmp.resolve("out.oat");
- app.writeToZip(jarFile, OutputMode.DexIndexed);
- return new TestRunResult(app, ToolHelper.runDex2OatRaw(jarFile, oatFile, vm));
- }
}
diff --git a/src/test/java/com/android/tools/r8/TestCompilerBuilder.java b/src/test/java/com/android/tools/r8/TestCompilerBuilder.java
index 9452a63..810d292 100644
--- a/src/test/java/com/android/tools/r8/TestCompilerBuilder.java
+++ b/src/test/java/com/android/tools/r8/TestCompilerBuilder.java
@@ -5,7 +5,6 @@
import com.android.tools.r8.TestBase.Backend;
import com.android.tools.r8.debug.DebugTestConfig;
-import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.utils.AndroidApiLevel;
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.AndroidAppConsumers;
@@ -114,14 +113,6 @@
}
@Override
- public T addProgramClassFileData(Collection<byte[]> classes) {
- for (byte[] clazz : classes) {
- builder.addClassProgramData(clazz, Origin.unknown());
- }
- return self();
- }
-
- @Override
public T addProgramFiles(Collection<Path> files) {
builder.addProgramFiles(files);
return self();
@@ -133,9 +124,4 @@
builder.addLibraryFiles(files);
return self();
}
-
- public T noDesugaring() {
- builder.setDisableDesugaring(true);
- return self();
- }
}
diff --git a/src/test/java/com/android/tools/r8/TestRunResult.java b/src/test/java/com/android/tools/r8/TestRunResult.java
index 8b6461f..46dc4cc 100644
--- a/src/test/java/com/android/tools/r8/TestRunResult.java
+++ b/src/test/java/com/android/tools/r8/TestRunResult.java
@@ -26,14 +26,6 @@
this.result = result;
}
- public String getStdOut() {
- return result.stdout;
- }
-
- public String getStdErr() {
- return result.stderr;
- }
-
public TestRunResult assertSuccess() {
assertEquals(errorMessage("Expected run to succeed."), 0, result.exitCode);
return this;
diff --git a/src/test/java/com/android/tools/r8/ToolHelper.java b/src/test/java/com/android/tools/r8/ToolHelper.java
index 70be34e..d4639a1 100644
--- a/src/test/java/com/android/tools/r8/ToolHelper.java
+++ b/src/test/java/com/android/tools/r8/ToolHelper.java
@@ -1438,42 +1438,39 @@
}
public static void runDex2Oat(Path file, Path outFile) throws IOException {
- runDex2Oat(file, outFile, getDexVm());
- }
-
- public static void runDex2Oat(Path file, Path outFile, DexVm vm) throws IOException {
- ProcessResult result = runDex2OatRaw(file, outFile, vm);
- if (result.exitCode != 0) {
- fail("dex2oat failed, exit code " + result.exitCode + ", stderr:\n" + result.stderr);
- }
- if (result.stderr != null && result.stderr.contains("Verification error")) {
- fail("Verification error: \n" + result.stderr);
- }
- }
-
- public static ProcessResult runDex2OatRaw(Path file, Path outFile, DexVm vm) throws IOException {
- Assume.assumeTrue(ToolHelper.isDex2OatSupported());
+ DexVm vm = getDexVm();
if (vm.isOlderThanOrEqual(DexVm.ART_5_1_1_HOST)) {
// TODO(b/79191363): Support running dex2oat for past android versions.
// Run default dex2oat for tests on old runtimes.
vm = DexVm.ART_DEFAULT;
}
+ runDex2Oat(file, outFile, vm);
+ }
+
+ public static void runDex2Oat(Path file, Path outFile, DexVm vm) throws IOException {
+ Assume.assumeTrue(ToolHelper.isDex2OatSupported());
// TODO(jmhenaff): find a way to run this on windows (push dex and run on device/emulator?)
Assume.assumeTrue(!ToolHelper.isWindows());
assert Files.exists(file);
assert ByteStreams.toByteArray(Files.newInputStream(file)).length > 0;
List<String> command = new ArrayList<>();
command.add(getDex2OatPath(vm).toString());
- command.add("--android-root=" + getProductPath(vm) + "/system");
+ command.add("--android-root=" + getProductPath(vm));
command.add("--runtime-arg");
command.add("-Xnorelocate");
+ command.add("--boot-image=" + getProductBootImagePath(vm));
command.add("--dex-file=" + file.toAbsolutePath());
command.add("--oat-file=" + outFile.toAbsolutePath());
- // TODO(zerny): Create a proper interface for invoking dex2oat. Hardcoding arm64 here is a hack!
command.add("--instruction-set=arm64");
ProcessBuilder builder = new ProcessBuilder(command);
builder.environment().put("LD_LIBRARY_PATH", getDexVmLibPath(vm).toString());
- return runProcess(builder);
+ ProcessResult result = runProcess(builder);
+ if (result.exitCode != 0) {
+ fail("dex2oat failed, exit code " + result.exitCode + ", stderr:\n" + result.stderr);
+ }
+ if (result.stderr.contains("Verification error")) {
+ fail("Verification error: \n" + result.stderr);
+ }
}
public static ProcessResult runProguardRaw(
diff --git a/src/test/java/com/android/tools/r8/regress/b118075510/Regress118075510Runner.java b/src/test/java/com/android/tools/r8/regress/b118075510/Regress118075510Runner.java
deleted file mode 100644
index 12bd452..0000000
--- a/src/test/java/com/android/tools/r8/regress/b118075510/Regress118075510Runner.java
+++ /dev/null
@@ -1,54 +0,0 @@
-// Copyright (c) 2018, 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.regress.b118075510;
-
-import com.android.tools.r8.AsmTestBase;
-import com.android.tools.r8.CompilationFailedException;
-import com.android.tools.r8.D8TestCompileResult;
-import com.android.tools.r8.utils.AndroidApiLevel;
-import com.android.tools.r8.utils.StringUtils;
-import com.android.tools.r8.utils.codeinspector.CodeInspector;
-import com.android.tools.r8.utils.codeinspector.MethodSubject;
-import java.io.IOException;
-import java.util.concurrent.ExecutionException;
-import org.junit.Assert;
-import org.junit.Test;
-
-public class Regress118075510Runner extends AsmTestBase {
-
- public static final Class<?> CLASS = Regress118075510Test.class;
- public static final String EXPECTED = StringUtils.lines("0", "0");
-
- @Test
- public void test()
- throws CompilationFailedException, IOException, ExecutionException, NoSuchMethodException {
- testForJvm().addTestClasspath().run(CLASS).assertSuccessWithOutput(EXPECTED);
-
- D8TestCompileResult d8Result =
- testForD8().addProgramClasses(CLASS).setMinApi(AndroidApiLevel.M).release().compile();
-
- CodeInspector inspector = d8Result.inspector();
- checkMethodContainsLongSignum(inspector, "fooNoTryCatch");
- checkMethodContainsLongSignum(inspector, "fooWithTryCatch");
- // Check the program runs on ART/Dalvik
- d8Result.run(CLASS).assertSuccessWithOutput(EXPECTED);
- // Check the program can be dex2oat compiled to arm64. This will diverge without the fixup.
- d8Result.runDex2Oat().assertSuccess();
- }
-
- private void checkMethodContainsLongSignum(CodeInspector inspector, String methodName)
- throws NoSuchMethodException {
- MethodSubject method = inspector.method(CLASS.getMethod(methodName, long.class, long.class));
- Assert.assertTrue(
- "Did not contain Long.signum workaround in "
- + methodName
- + ":\n"
- + method.getMethod().codeToString(),
- method
- .streamInstructions()
- .anyMatch(
- i ->
- i.isInvoke() && i.getMethod().qualifiedName().equals("java.lang.Long.signum")));
- }
-}
diff --git a/src/test/java/com/android/tools/r8/regress/b118075510/Regress118075510Test.java b/src/test/java/com/android/tools/r8/regress/b118075510/Regress118075510Test.java
deleted file mode 100644
index cc4dece..0000000
--- a/src/test/java/com/android/tools/r8/regress/b118075510/Regress118075510Test.java
+++ /dev/null
@@ -1,38 +0,0 @@
-// Copyright (c) 2018, 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.regress.b118075510;
-
-public class Regress118075510Test {
-
- public static void fooNoTryCatch(long a, long b) {
- // Call a method on the runner class that will not be on the classpath at runtime.
- // This causes the optimizing 6.0.1 compiler to delegate to the old quick compiler.
- if (a == b) Regress118075510Runner.class.getMethods();
- // The else branch of the conditional here ends up with an invalid previous pointer at its
- // block header (ie, previous of mul-long (2addr) points to epilogue-end which is self-linked.
- System.out.println(a < b ? 0 : a * b + b);
- }
-
- public static void fooWithTryCatch(long a, long b) {
- // Call a method on the runner class that will not be on the classpath at runtime.
- // This causes the optimizing 6.0.1 compiler to delegate to the old quick compiler.
- if (a == b) Regress118075510Runner.class.getMethods();
- // The else branch of the conditional here ends up with an invalid previous pointer at its
- // block header (ie, previous of mul-long (2addr) points to epilogue-end which is self-linked.
- try {
- if (a < b) {
- System.out.println((long) 0);
- } else {
- System.out.println(a * b + b);
- }
- } catch (RuntimeException e) {
- e.printStackTrace();
- }
- }
-
- public static void main(String[] args) {
- fooNoTryCatch(args.length, 456);
- fooWithTryCatch(456, args.length);
- }
-}
diff --git a/src/test/java/com/android/tools/r8/regress/b77842465/Regress77842465.java b/src/test/java/com/android/tools/r8/regress/b77842465/Regress77842465.java
index 3ab3467..cbc6e1b 100644
--- a/src/test/java/com/android/tools/r8/regress/b77842465/Regress77842465.java
+++ b/src/test/java/com/android/tools/r8/regress/b77842465/Regress77842465.java
@@ -5,20 +5,29 @@
import com.android.tools.r8.AsmTestBase;
import com.android.tools.r8.CompilationFailedException;
-import com.android.tools.r8.utils.AndroidApiLevel;
+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.origin.Origin;
import java.io.IOException;
+import java.nio.file.Path;
import org.junit.Test;
public class Regress77842465 extends AsmTestBase {
@Test
public void test() throws CompilationFailedException, IOException {
- testForD8()
- .addProgramClassFileData(Regress77842465Dump.dump())
- .noDesugaring()
- .setMinApi(AndroidApiLevel.M)
- .compile()
- .runDex2Oat()
- .assertSuccess();
+
+ Path dexOut = temp.getRoot().toPath().resolve("out.jar");
+ Path oatOut = temp.getRoot().toPath().resolve("out.odex");
+
+ D8.run(D8Command.builder()
+ .addClassProgramData(Regress77842465Dump.dump(), Origin.unknown())
+ .setOutput(dexOut, OutputMode.DexIndexed)
+ .setDisableDesugaring(true)
+ .build());
+
+ ToolHelper.runDex2Oat(dexOut, oatOut);
}
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java
index 98bb8d0..1e21552 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java
@@ -5,10 +5,8 @@
package com.android.tools.r8.utils.codeinspector;
import com.android.tools.r8.graph.DexEncodedMethod;
-import com.google.common.collect.Streams;
import java.util.Iterator;
import java.util.function.Predicate;
-import java.util.stream.Stream;
public abstract class MethodSubject extends MemberSubject {
@@ -42,8 +40,4 @@
public abstract LineNumberTable getLineNumberTable();
public abstract boolean hasLocalVariableTable();
-
- public Stream<InstructionSubject> streamInstructions() {
- return Streams.stream(iterateInstructions());
- }
}
