Version 1.4.78
Cherry pick: Compare DEX instructions as last step in identicalAfterRegisterAllocation
CL: https://r8-review.googlesource.com/c/r8/+/36121
Bug: 127524985
Change-Id: Ib2043cb11f69eab28f3cd1c5f28f404ac68d2a4b
diff --git a/.gitignore b/.gitignore
index 22b29da..0a8d8b8 100644
--- a/.gitignore
+++ b/.gitignore
@@ -41,6 +41,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 308914a..8ff0521 100644
--- a/build.gradle
+++ b/build.gradle
@@ -335,6 +335,7 @@
"benchmarks/kotlin-benches",
"chrome",
"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/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 2fe03c0..11d013c 100644
--- a/src/main/java/com/android/tools/r8/Version.java
+++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@
// This field is accessed from release scripts using simple pattern matching.
// Therefore, changing this field could break our release scripts.
- public static final String LABEL = "1.4.77";
+ public static final String LABEL = "1.4.78";
private Version() {
}
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 0d05021..8cab73b 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -358,7 +358,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 6d21c5a..e30e9bb 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
@@ -473,6 +473,11 @@
return false;
}
}
+ // Finally check that the dex instructions for the generated code actually are the same.
+ if (allocator.getOptions().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 4b2fabb..d989d07 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
@@ -71,6 +71,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;
@@ -121,15 +122,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.getOptions());
+ 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() {
@@ -565,7 +591,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));
}
@@ -601,6 +628,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;
@@ -627,6 +659,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;
@@ -891,6 +928,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 {
@@ -927,6 +966,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 {
@@ -974,6 +1019,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 {
@@ -1005,6 +1056,11 @@
public int maxSize() {
return 0;
}
+
+ @Override
+ public boolean identicalInstructions(Info other, DexBuilder builder) {
+ return other instanceof FallThroughInfo;
+ }
}
private static class GotoInfo extends Info {
@@ -1131,6 +1187,11 @@
instructions.add(dex);
}
}
+
+ @Override
+ public boolean identicalInstructions(Info other, DexBuilder builder) {
+ return other instanceof GotoInfo;
+ }
}
public static class IfInfo extends Info {
@@ -1141,6 +1202,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();
}
@@ -1171,7 +1244,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;
@@ -1207,7 +1280,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);
@@ -1256,6 +1329,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 {
@@ -1279,6 +1362,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 190db23..ffe0bfb 100644
--- a/src/test/java/com/android/tools/r8/DXTestBuilder.java
+++ b/src/test/java/com/android/tools/r8/DXTestBuilder.java
@@ -3,12 +3,13 @@
// 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 java.io.IOException;
import java.nio.file.Path;
import java.util.ArrayList;
@@ -63,12 +64,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 2b66df4..af03f51 100644
--- a/src/test/java/com/android/tools/r8/TestBase.java
+++ b/src/test/java/com/android/tools/r8/TestBase.java
@@ -708,7 +708,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/TestShrinkerBuilder.java b/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
index 217463e..62b2b65 100644
--- a/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
+++ b/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
@@ -114,6 +114,10 @@
return self();
}
+ public T addKeepAllAttributes() {
+ return addKeepRules("-keepattributes *");
+ }
+
private static String getMethodLine(MethodReference method) {
// Should we encode modifiers in method references?
StringBuilder builder = new StringBuilder();
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..f91a08b
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/internal/Regression127524985.java
@@ -0,0 +1,57 @@
+// 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.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 Backend[] data() {
+ return Backend.values();
+ }
+
+ private final Backend backend;
+
+ public Regression127524985(Backend backend) {
+ this.backend = backend;
+ }
+
+ @Test
+ public void test() throws Throwable {
+ if (backend == Backend.CF) {
+ testForJvm()
+ .addClasspath(JAR)
+ .run(MAIN)
+ .assertSuccessWithOutput(EXPECTED);
+ }
+ (backend == Backend.DEX
+ ? testForD8()
+ : testForR8(backend)
+ .debug()
+ .noTreeShaking()
+ .noMinification()
+ .addKeepAllAttributes()
+ .addKeepRules("-dontwarn *"))
+ .addProgramFiles(JAR)
+ .compile()
+ .run(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
