Merge "Compare DEX instructions as last step in identicalAfterRegisterAllocation"
diff --git a/.gitignore b/.gitignore
index b8248ed..482b1bf 100644
--- a/.gitignore
+++ b/.gitignore
@@ -45,6 +45,8 @@
third_party/dart-sdk.tar.gz
third_party/desugar/desugar_*/
third_party/desugar/desugar_*.tar.gz
+third_party/internal/*
+!third_party/internal/*.sha1
third_party/gmail/*
!third_party/gmail/*.sha1
third_party/gmscore/*
diff --git a/build.gradle b/build.gradle
index 6523153..80b4c80 100644
--- a/build.gradle
+++ b/build.gradle
@@ -346,6 +346,7 @@
"classlib",
"cf_segments",
"desugar/desugar_20180308",
+ "internal/issue-127524985",
"framework",
"gmail/gmail_android_170604.16",
"gmail/gmail_android_180826.15",
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
index 4955807..0ba6cb1 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -343,7 +343,7 @@
public void setCode(IRCode ir, RegisterAllocator registerAllocator, InternalOptions options) {
checkIfObsolete();
- final DexBuilder builder = new DexBuilder(ir, registerAllocator, options);
+ final DexBuilder builder = new DexBuilder(ir, registerAllocator);
setCode(builder.build());
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/Instruction.java b/src/main/java/com/android/tools/r8/ir/code/Instruction.java
index 4714251..f845dc5 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Instruction.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Instruction.java
@@ -469,6 +469,11 @@
return false;
}
}
+ // Finally check that the dex instructions for the generated code actually are the same.
+ if (allocator.options().isGeneratingDex()
+ && !DexBuilder.identicalInstructionsAfterBuildingDexCode(this, other, allocator)) {
+ return false;
+ }
return true;
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/DexBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/DexBuilder.java
index f295ddd..2eaa046 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/DexBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/DexBuilder.java
@@ -70,6 +70,7 @@
import it.unimi.dsi.fastutil.ints.Int2ReferenceMap;
import it.unimi.dsi.fastutil.ints.Int2ReferenceOpenHashMap;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.ListIterator;
@@ -120,15 +121,40 @@
BasicBlock nextBlock;
- public DexBuilder(
- IRCode ir,
- RegisterAllocator registerAllocator,
- InternalOptions options) {
+ public DexBuilder(IRCode ir, RegisterAllocator registerAllocator) {
+ this(ir, registerAllocator, ir.options);
assert ir != null;
assert registerAllocator != null;
+ }
+
+ private DexBuilder(IRCode ir, RegisterAllocator registerAllocator, InternalOptions options) {
this.ir = ir;
this.registerAllocator = registerAllocator;
this.options = options;
+ if (isBuildingForComparison()) {
+ instructionToInfo = new Info[1];
+ }
+ }
+
+ public static boolean identicalInstructionsAfterBuildingDexCode(
+ com.android.tools.r8.ir.code.Instruction a,
+ com.android.tools.r8.ir.code.Instruction b,
+ RegisterAllocator allocator) {
+ DexBuilder builder = new DexBuilder(null, allocator, allocator.options());
+ Info infoA = buildInfoForComparison(a, builder);
+ Info infoB = buildInfoForComparison(b, builder);
+ return infoA.identicalInstructions(infoB, builder);
+ }
+
+ private static Info buildInfoForComparison(
+ com.android.tools.r8.ir.code.Instruction instruction, DexBuilder builder) {
+ instruction.buildDex(builder);
+ assert builder.instructionToInfo.length == 1;
+ return builder.instructionToInfo[0];
+ }
+
+ private boolean isBuildingForComparison() {
+ return ir == null;
}
private void reset() {
@@ -561,7 +587,8 @@
public void add(com.android.tools.r8.ir.code.Instruction instr, Instruction dex) {
assert !instr.isGoto();
- assert !instr.isDexItemBasedConstString()
+ assert isBuildingForComparison()
+ || !instr.isDexItemBasedConstString()
|| ir.method.getOptimizationInfo().useIdentifierNameString();
add(instr, new FixedSizeInfo(instr, dex));
}
@@ -597,6 +624,11 @@
}
private void add(com.android.tools.r8.ir.code.Instruction ir, Info info) {
+ if (isBuildingForComparison()) {
+ // We are building for instruction comparison, so just set the info.
+ setSingleInfo(info);
+ return;
+ }
assert ir != null;
assert info != null;
assert getInfo(ir) == null;
@@ -623,6 +655,11 @@
instructionToInfo[instructionNumberToIndex(instruction.getNumber())] = info;
}
+ private void setSingleInfo(Info info) {
+ assert instructionToInfo.length == 1;
+ instructionToInfo[0] = info;
+ }
+
private Info getTargetInfo(BasicBlock block) {
InstructionIterator iterator = block.iterator();
com.android.tools.r8.ir.code.Instruction instruction = null;
@@ -887,6 +924,8 @@
public com.android.tools.r8.ir.code.Instruction getIR() {
return ir;
}
+
+ public abstract boolean identicalInstructions(Info other, DexBuilder builder);
}
private static class FixedSizeInfo extends Info {
@@ -923,6 +962,12 @@
public void addInstructions(DexBuilder builder, List<Instruction> instructions) {
instructions.add(instruction);
}
+
+ @Override
+ public boolean identicalInstructions(Info other, DexBuilder builder) {
+ return other instanceof FixedSizeInfo
+ && instruction.equals(((FixedSizeInfo) other).instruction);
+ }
}
private static class MultiFixedSizeInfo extends Info {
@@ -970,6 +1015,12 @@
public int getSize() {
return size;
}
+
+ @Override
+ public boolean identicalInstructions(Info other, DexBuilder builder) {
+ return other instanceof MultiFixedSizeInfo
+ && Arrays.equals(instructions, ((MultiFixedSizeInfo) other).instructions);
+ }
}
private static class FallThroughInfo extends Info {
@@ -1001,6 +1052,11 @@
public int maxSize() {
return 0;
}
+
+ @Override
+ public boolean identicalInstructions(Info other, DexBuilder builder) {
+ return other instanceof FallThroughInfo;
+ }
}
private static class GotoInfo extends Info {
@@ -1127,6 +1183,11 @@
instructions.add(dex);
}
}
+
+ @Override
+ public boolean identicalInstructions(Info other, DexBuilder builder) {
+ return other instanceof GotoInfo;
+ }
}
public static class IfInfo extends Info {
@@ -1137,6 +1198,18 @@
super(branch);
}
+ private int getRegister(int operandIndex, DexBuilder builder) {
+ If branch = getBranch();
+ return builder.allocatedRegister(branch.inValues().get(operandIndex), branch.getNumber());
+ }
+
+ private int[] getRegisters(DexBuilder builder) {
+ if (getBranch().isZeroTest()) {
+ return new int[] {getRegister(0, builder)};
+ }
+ return new int[] {getRegister(0, builder), getRegister(1, builder)};
+ }
+
private If getBranch() {
return (If) getIR();
}
@@ -1167,7 +1240,7 @@
int source = builder.getInfo(branch).getOffset();
int target = builder.getInfo(branch.getTrueTarget().entry()).getOffset();
int relativeOffset = target - source;
- int register1 = builder.allocatedRegister(branch.inValues().get(0), branch.getNumber());
+ int register1 = getRegister(0, builder);
if (relativeOffset < 0) {
builder.hasBackwardsBranch = true;
@@ -1203,7 +1276,7 @@
break;
}
} else {
- int register2 = builder.allocatedRegister(branch.inValues().get(1), branch.getNumber());
+ int register2 = getRegister(1, builder);
switch (getBranch().getType()) {
case EQ:
instruction = new IfEq(register1, register2, relativeOffset);
@@ -1252,6 +1325,16 @@
public int getSize() {
return size;
}
+
+ @Override
+ public boolean identicalInstructions(Info other, DexBuilder builder) {
+ if (!(other instanceof IfInfo)) {
+ return false;
+ }
+ IfInfo otherInfo = (IfInfo) other;
+ return getBranch().getType() == otherInfo.getBranch().getType()
+ && Arrays.equals(getRegisters(builder), otherInfo.getRegisters(builder));
+ }
}
public static class MoveInfo extends Info {
@@ -1275,6 +1358,16 @@
}
@Override
+ public boolean identicalInstructions(Info other, DexBuilder builder) {
+ if (!(other instanceof MoveInfo)) {
+ return false;
+ }
+ MoveInfo moveInfo = (MoveInfo) other;
+ return srcRegister(builder) == moveInfo.srcRegister(builder)
+ && destRegister(builder) == moveInfo.destRegister(builder);
+ }
+
+ @Override
public int computeSize(DexBuilder builder) {
int srcRegister = srcRegister(builder);
int destRegister = destRegister(builder);
diff --git a/src/test/java/com/android/tools/r8/DXTestBuilder.java b/src/test/java/com/android/tools/r8/DXTestBuilder.java
index e9e7f9c..8462f7b 100644
--- a/src/test/java/com/android/tools/r8/DXTestBuilder.java
+++ b/src/test/java/com/android/tools/r8/DXTestBuilder.java
@@ -3,12 +3,14 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8;
+import com.android.tools.r8.ClassFileConsumer.ArchiveConsumer;
import com.android.tools.r8.D8Command.Builder;
import com.android.tools.r8.TestBase.Backend;
import com.android.tools.r8.ToolHelper.ProcessResult;
import com.android.tools.r8.errors.Unimplemented;
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.InternalOptions;
+import com.android.tools.r8.utils.ListUtils;
import com.android.tools.r8.utils.StringUtils;
import java.io.IOException;
import java.nio.file.Path;
@@ -64,12 +66,33 @@
@Override
public DXTestBuilder addProgramClasses(Collection<Class<?>> classes) {
- throw new Unimplemented("No support for adding classes directly");
+ return addProgramClassFileData(
+ ListUtils.map(
+ classes,
+ c -> {
+ try {
+ return ToolHelper.getClassAsBytes(c);
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+ }));
}
@Override
public DXTestBuilder addProgramClassFileData(Collection<byte[]> classes) {
- throw new Unimplemented("No support for adding classfile data directly");
+ try {
+ Path out = getState().getNewTempFolder().resolve("out.jar");
+ ArchiveConsumer consumer = new ArchiveConsumer(out);
+ for (byte[] clazz : classes) {
+ String descriptor = TestBase.extractClassDescriptor(clazz);
+ consumer.accept(ByteDataView.of(clazz), descriptor, null);
+ }
+ consumer.finished(null);
+ addProgramFiles(out);
+ return self();
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
}
@Override
diff --git a/src/test/java/com/android/tools/r8/TestBase.java b/src/test/java/com/android/tools/r8/TestBase.java
index 8fad46b..063e871 100644
--- a/src/test/java/com/android/tools/r8/TestBase.java
+++ b/src/test/java/com/android/tools/r8/TestBase.java
@@ -721,7 +721,7 @@
* Run application on Art with the specified main class.
*/
protected ProcessResult runOnArtRaw(AndroidApp app, Class mainClass) throws IOException {
- return runOnArtRaw(app, mainClass.getCanonicalName());
+ return runOnArtRaw(app, mainClass.getTypeName());
}
/**
diff --git a/src/test/java/com/android/tools/r8/TestParametersBuilder.java b/src/test/java/com/android/tools/r8/TestParametersBuilder.java
index 12735cf..1c3f77c 100644
--- a/src/test/java/com/android/tools/r8/TestParametersBuilder.java
+++ b/src/test/java/com/android/tools/r8/TestParametersBuilder.java
@@ -106,6 +106,11 @@
vm -> startInclusive.isOlderThanOrEqual(vm) && vm.isOlderThanOrEqual(endInclusive));
}
+ /** Add all available DEX runtimes that support native multidex. */
+ public TestParametersBuilder withNativeMultidexDexRuntimes() {
+ return withDexRuntimesStartingFromIncluding(DexVm.Version.V5_1_1);
+ }
+
/** Add all available DEX runtimes starting from and including {@param startInclusive}. */
public TestParametersBuilder withDexRuntimesStartingFromIncluding(DexVm.Version startInclusive) {
return withDexRuntimeFilter(vm -> startInclusive.isOlderThanOrEqual(vm));
diff --git a/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java b/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
index 5e3e100..7d2d474 100644
--- a/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
+++ b/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
@@ -121,6 +121,10 @@
return self();
}
+ public T addKeepAllAttributes() {
+ return addKeepRules("-keepattributes *");
+ }
+
public abstract T addApplyMapping(String proguardMap);
private static String getMethodLine(MethodReference method) {
diff --git a/src/test/java/com/android/tools/r8/internal/Regression127524985.java b/src/test/java/com/android/tools/r8/internal/Regression127524985.java
new file mode 100644
index 0000000..7de4078
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/internal/Regression127524985.java
@@ -0,0 +1,60 @@
+// 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.internal;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.utils.StringUtils;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class Regression127524985 extends TestBase {
+
+ private static final String MAIN = "com.android.tools.r8.internal.Regression127524985$Main";
+
+ private static final Path JAR =
+ Paths.get("third_party/internal/issue-127524985/issue-127524985.jar");
+
+ private static final String EXPECTED = StringUtils.lines("true");
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimes().build();
+ }
+
+ private final TestParameters parameters;
+
+ public Regression127524985(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Throwable {
+ if (parameters.isCfRuntime()) {
+ testForJvm()
+ .addClasspath(JAR)
+ .run(parameters.getRuntime(), MAIN)
+ .assertSuccessWithOutput(EXPECTED);
+ }
+ (parameters.isDexRuntime()
+ ? testForD8()
+ : testForR8(parameters.getBackend())
+ .debug()
+ .noTreeShaking()
+ .noMinification()
+ .addKeepAllAttributes()
+ .addKeepRules("-dontwarn *"))
+ .addProgramFiles(JAR)
+ .apply(parameters::setMinApiForRuntime)
+ .compile()
+ .run(parameters.getRuntime(), MAIN)
+ .assertSuccessWithOutput(EXPECTED);
+ }
+}
diff --git a/third_party/internal/issue-127524985.tar.gz.sha1 b/third_party/internal/issue-127524985.tar.gz.sha1
new file mode 100644
index 0000000..a1b57e3
--- /dev/null
+++ b/third_party/internal/issue-127524985.tar.gz.sha1
@@ -0,0 +1 @@
+c1d03d36435cc08d1ee218c94e8dbfc74df975f1
\ No newline at end of file
