Version 1.0.15
Merge: Extend the check for instructions taking long operands overlapping
their result (resolved conflicts in R8RunArtTestsTest)
CL: https://r8-review.googlesource.com/c/r8/+/17600
(merge conflicts in R8RunArtTestsTest)
Change-Id: I49a4903cc1d707f0ab92ff2a3e018778084b7c18
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index f1fb55c..8442b2d 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 = "v1.0.14";
+ public static final String LABEL = "v1.0.15";
private Version() {
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/Add.java b/src/main/java/com/android/tools/r8/ir/code/Add.java
index 783d672..2abc743 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Add.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Add.java
@@ -35,12 +35,6 @@
@Override
public com.android.tools.r8.code.Instruction CreateLong(int dest, int left, int right) {
- // The dalvik jit had a bug where the long operations add, sub, or, xor and and would write
- // the first part of the result long before reading the second part of the input longs.
- // Therefore, there can be no overlap of the second part of an input long and the first
- // part of the output long.
- assert dest != left + 1;
- assert dest != right + 1;
return new AddLong(dest, left, right);
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/And.java b/src/main/java/com/android/tools/r8/ir/code/And.java
index ad46715..c0bdb2a 100644
--- a/src/main/java/com/android/tools/r8/ir/code/And.java
+++ b/src/main/java/com/android/tools/r8/ir/code/And.java
@@ -41,12 +41,6 @@
@Override
public com.android.tools.r8.code.Instruction CreateLong(int dest, int left, int right) {
- // The dalvik jit had a bug where the long operations add, sub, or, xor and and would write
- // the first part of the result long before reading the second part of the input longs.
- // Therefore, there can be no overlap of the second part of an input long and the first
- // part of the output long.
- assert dest != left + 1;
- assert dest != right + 1;
return new AndLong(dest, left, right);
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/Or.java b/src/main/java/com/android/tools/r8/ir/code/Or.java
index 3df3a83..2cfe9b9 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Or.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Or.java
@@ -40,12 +40,6 @@
@Override
public com.android.tools.r8.code.Instruction CreateLong(int dest, int left, int right) {
- // The dalvik jit had a bug where the long operations add, sub, or, xor and and would write
- // the first part of the result long before reading the second part of the input longs.
- // Therefore, there can be no overlap of the second part of an input long and the first
- // part of the output long.
- assert dest != left + 1;
- assert dest != right + 1;
return new OrLong(dest, left, right);
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/Sub.java b/src/main/java/com/android/tools/r8/ir/code/Sub.java
index f9f5920..f2a173b 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Sub.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Sub.java
@@ -39,12 +39,6 @@
@Override
public com.android.tools.r8.code.Instruction CreateLong(int dest, int left, int right) {
- // The dalvik jit had a bug where the long operations add, sub, or, xor and and would write
- // the first part of the result long before reading the second part of the input longs.
- // Therefore, there can be no overlap of the second part of an input long and the first
- // part of the output long.
- assert dest != left + 1;
- assert dest != right + 1;
return new SubLong(dest, left, right);
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/Xor.java b/src/main/java/com/android/tools/r8/ir/code/Xor.java
index d2897ed..0f37a65 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Xor.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Xor.java
@@ -39,12 +39,6 @@
@Override
public com.android.tools.r8.code.Instruction CreateLong(int dest, int left, int right) {
- // The dalvik jit had a bug where the long operations add, sub, or, xor and and would write
- // the first part of the result long before reading the second part of the input longs.
- // Therefore, there can be no overlap of the second part of an input long and the first
- // part of the output long.
- assert dest != left + 1;
- assert dest != right + 1;
return new XorLong(dest, left, right);
}
diff --git a/src/main/java/com/android/tools/r8/ir/regalloc/LinearScanRegisterAllocator.java b/src/main/java/com/android/tools/r8/ir/regalloc/LinearScanRegisterAllocator.java
index 15c186f..f67aade 100644
--- a/src/main/java/com/android/tools/r8/ir/regalloc/LinearScanRegisterAllocator.java
+++ b/src/main/java/com/android/tools/r8/ir/regalloc/LinearScanRegisterAllocator.java
@@ -1072,6 +1072,46 @@
return arrayReg == register;
}
+ private boolean needsSingleResultOverlappingLongOperandsWorkaround(LiveIntervals intervals) {
+ if (!options.canHaveCmpLongBug()) {
+ return false;
+ }
+ if (intervals.requiredRegisters() == 2) {
+ // Not the live range for a single value and therefore not the output of cmp-long.
+ return false;
+ }
+ if (intervals.getValue().isPhi()) {
+ // If this writes a new register pair it will be via a move and not an cmp-long operation.
+ return false;
+ }
+ if (intervals.getSplitParent() != intervals) {
+ // This is a split of a parent interval and therefore if this leads to a write of a
+ // register it will be via a move and not an cmp-long operation.
+ return false;
+ }
+ Instruction definition = intervals.getValue().definition;
+ return definition.isCmp() && definition.asCmp().inValues().get(0).outType().isWide();
+ }
+
+ private boolean singleOverlappingLong(int register1, int register2) {
+ return register1 == register2 || register1 == (register2 + 1);
+ }
+
+ // Is one of the cmp-long argument registers the same as the register we are
+ // allocating for the result?
+ private boolean isSingleResultOverlappingLongOperands(LiveIntervals intervals, int register) {
+ assert needsSingleResultOverlappingLongOperandsWorkaround(intervals);
+ Value left = intervals.getValue().definition.asCmp().leftValue();
+ Value right = intervals.getValue().definition.asCmp().rightValue();
+ int leftReg =
+ left.getLiveIntervals().getSplitCovering(intervals.getStart()).getRegister();
+ int rightReg =
+ right.getLiveIntervals().getSplitCovering(intervals.getStart()).getRegister();
+ assert leftReg != NO_REGISTER;
+ assert rightReg != NO_REGISTER;
+ return singleOverlappingLong(register, leftReg) || singleOverlappingLong(register, rightReg);
+ }
+
// The dalvik jit had a bug where the long operations add, sub, or, xor and and would write
// the first part of the result long before reading the second part of the input longs.
//
@@ -1081,7 +1121,10 @@
//
// Dalvik would add v0 and v2 and write that to v3. It would then read v1 and v3 and produce
// the wrong result.
- private boolean needsOverlappingLongRegisterWorkaround(LiveIntervals intervals) {
+ private boolean needsLongResultOverlappingLongOperandsWorkaround(LiveIntervals intervals) {
+ if (!options.canHaveOverlappingLongRegisterBug()) {
+ return false;
+ }
if (intervals.requiredRegisters() == 1) {
// Not the live range for a wide value.
return false;
@@ -1107,8 +1150,14 @@
return false;
}
- private boolean hasOverlappingLongRegisters(LiveIntervals unhandledInterval, int register) {
- assert needsOverlappingLongRegisterWorkaround(unhandledInterval);
+ private boolean longOverlappingLong(int register1, int register2) {
+ return register1 == register2 || register1 == (register2 + 1)
+ || (register1 + 1) == register2 || (register1 + 1) == (register2 + 1);
+ }
+
+ private boolean isLongResultOverlappingLongOperands(
+ LiveIntervals unhandledInterval, int register) {
+ assert needsLongResultOverlappingLongOperandsWorkaround(unhandledInterval);
Value left = unhandledInterval.getValue().definition.asBinop().leftValue();
Value right = unhandledInterval.getValue().definition.asBinop().rightValue();
int leftReg =
@@ -1117,11 +1166,9 @@
right.getLiveIntervals().getSplitCovering(unhandledInterval.getStart()).getRegister();
assert leftReg != NO_REGISTER && rightReg != NO_REGISTER;
// The dalvik bug is actually only for overlap with the second operand, For now we
- // make sure that there is no overlap with either operand.
- if ((leftReg + 1) == register || (rightReg + 1) == register) {
- return true;
- }
- return false;
+ // make sure that there is no overlap with either register of either operand. Some vendor
+ // optimization have bees seen to need this more conservative check.
+ return longOverlappingLong(register, leftReg) || longOverlappingLong(register, rightReg);
}
private boolean allocateSingleInterval(LiveIntervals unhandledInterval, ArgumentReuseMode mode) {
@@ -1350,8 +1397,8 @@
}
if (freePosition >= unhandledInterval.getEnd()) {
// Check for overlapping long registers issue.
- if (needsOverlappingLongRegisterWorkaround(unhandledInterval) &&
- hasOverlappingLongRegisters(unhandledInterval, register)) {
+ if (needsLongResultOverlappingLongOperandsWorkaround(unhandledInterval) &&
+ isLongResultOverlappingLongOperands(unhandledInterval, register)) {
return false;
}
// Check for aget-wide bug in recent Art VMs.
@@ -1447,9 +1494,24 @@
if (candidate == REGISTER_CANDIDATE_NOT_FOUND) {
return candidate;
}
- if (needsOverlappingLongRegisterWorkaround(unhandledInterval)) {
+ if (needsLongResultOverlappingLongOperandsWorkaround(unhandledInterval)) {
int lastCandidate = candidate;
- while (hasOverlappingLongRegisters(unhandledInterval, candidate)) {
+ while (isLongResultOverlappingLongOperands(unhandledInterval, candidate)) {
+ // Make the overlapping register unavailable for allocation and try again.
+ freePositions.set(candidate, 0);
+ candidate = getLargestCandidate(registerConstraint, freePositions, needsRegisterPair, type);
+ // If there are only invalid candidates of the give type we will end up with the same
+ // candidate returned again once we have tried them all. In that case we didn't find a
+ // valid register candidate and we need to broaden the search to other types.
+ if (lastCandidate == candidate) {
+ return REGISTER_CANDIDATE_NOT_FOUND;
+ }
+ lastCandidate = candidate;
+ }
+ }
+ if (needsSingleResultOverlappingLongOperandsWorkaround(unhandledInterval)) {
+ int lastCandidate = candidate;
+ while (isSingleResultOverlappingLongOperands(unhandledInterval, candidate)) {
// Make the overlapping register unavailable for allocation and try again.
freePositions.set(candidate, 0);
candidate = getLargestCandidate(registerConstraint, freePositions, needsRegisterPair, type);
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 610d547..45834d7 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -482,4 +482,17 @@
public boolean canHaveThisTypeVerifierBug() {
return minApiLevel < AndroidApiLevel.M.getLevel();
}
+
+ // The dalvik jit had a bug where the long operations add, sub, or, xor and and would write
+ // the first part of the result long before reading the second part of the input longs.
+ public boolean canHaveOverlappingLongRegisterBug() {
+ return minApiLevel < AndroidApiLevel.L.getLevel();
+ }
+
+ // Some dalvik versions found in the wild perform invalid JIT compilation of cmp-long
+ // instructions where the result register overlaps with the input registers.
+ // See b/74084493.
+ public boolean canHaveCmpLongBug() {
+ return minApiLevel < AndroidApiLevel.L.getLevel();
+ }
}
diff --git a/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java b/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java
index f3fb918..0ef4ef4 100644
--- a/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java
+++ b/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java
@@ -145,6 +145,10 @@
.put("974-verify-interface-super", AndroidApiLevel.N.getLevel())
// Desugaring of interface private methods is not yet supported.
.put("975-iface-private", AndroidApiLevel.N.getLevel())
+ // The extended check for overlapping long registers cause this to run out of registers.
+ .put("421-large-frame", AndroidApiLevel.L.getLevel())
+ // The extended check for overlapping long registers cause this to run out of registers.
+ .put("551-checker-shifter-operand", AndroidApiLevel.L.getLevel())
.build();
// Tests that timeout when run with Art.
