Merge "Extend the check for instructions taking long operands overlapping their result"
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 fefe8c5..acf4395 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
@@ -1090,6 +1090,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.
//
@@ -1099,7 +1139,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;
@@ -1125,8 +1168,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 =
@@ -1135,11 +1184,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);
}
// Intervals overlap a move exception interval if one of the splits of the intervals does.
@@ -1416,8 +1463,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.
@@ -1513,9 +1560,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 3414d43..1073cf9 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -501,4 +501,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 32c0175..094097f 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)
// Desugaring of interface private methods is not yet supported.
.put("975-iface-private", AndroidApiLevel.N)
+ // The extended check for overlapping long registers cause this to run out of registers.
+ .put("421-large-frame", AndroidApiLevel.L)
+ // The extended check for overlapping long registers cause this to run out of registers.
+ .put("551-checker-shifter-operand", AndroidApiLevel.L)
.build();
// Tests that timeout when run with Art.
