Fix the workaround for overlapping long registers art bug.
We compute candidate registers in two places but only one of them
had the workaround code.
Creating a small test case is hard, so I added another full app
verification test to the suite to catch this.
R=sgjesse@google.com
Bug:
Change-Id: I00e7b77966edbdd6924f49dbe95e78f745fc8384
diff --git a/build.gradle b/build.gradle
index 0663b8a..45eade1 100644
--- a/build.gradle
+++ b/build.gradle
@@ -151,6 +151,7 @@
"gmscore/v8.tar.gz",
"gmscore/gmscore_v9.tar.gz",
"gmscore/gmscore_v10.tar.gz",
+ "photos/2017-06-06.tar.gz",
"youtube/youtube.android_11.47.tar.gz",
"youtube/youtube.android_12.10.tar.gz",
"youtube/youtube.android_12.17.tar.gz",
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 a981990..3a96ac2 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
@@ -870,7 +870,7 @@
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) {
+ if ((leftReg + 1) == register || (rightReg + 1) == register) {
return true;
}
return false;
@@ -977,14 +977,8 @@
// Get the register (pair) that is free the longest. That is the register with the largest
// free position.
- int candidate = getLargestCandidate(registerConstraint, freePositions, needsRegisterPair);
- if (needsOverlappingLongRegisterWorkaround(unhandledInterval)) {
- while (hasOverlappingLongRegisters(unhandledInterval, candidate)) {
- // Make the overlapping register unavailable for allocation and try again.
- freePositions.set(candidate, 0);
- candidate = getLargestCandidate(registerConstraint, freePositions, needsRegisterPair);
- }
- }
+ int candidate = getLargestValidCandidate(
+ unhandledInterval, registerConstraint, needsRegisterPair, freePositions);
int largestFreePosition = freePositions.get(candidate);
if (needsRegisterPair) {
largestFreePosition = Math.min(largestFreePosition, freePositions.get(candidate + 1));
@@ -1162,6 +1156,19 @@
return candidate;
}
+ private int getLargestValidCandidate(LiveIntervals unhandledInterval, int registerConstraint,
+ boolean needsRegisterPair, RegisterPositions freePositions) {
+ int candidate = getLargestCandidate(registerConstraint, freePositions, needsRegisterPair);
+ if (needsOverlappingLongRegisterWorkaround(unhandledInterval)) {
+ while (hasOverlappingLongRegisters(unhandledInterval, candidate)) {
+ // Make the overlapping register unavailable for allocation and try again.
+ freePositions.set(candidate, 0);
+ candidate = getLargestCandidate(registerConstraint, freePositions, needsRegisterPair);
+ }
+ }
+ return candidate;
+ }
+
private void allocateBlockedRegister(LiveIntervals unhandledInterval) {
int registerConstraint = unhandledInterval.getRegisterLimit();
if (registerConstraint < Constants.U16BIT_MAX) {
@@ -1225,9 +1232,9 @@
inactive, unhandledInterval, registerConstraint, usePositions, blockedPositions);
// Get the register (pair) that has the highest use position.
- assert unhandledInterval.requiredRegisters() <= 2;
boolean needsRegisterPair = unhandledInterval.requiredRegisters() == 2;
- int candidate = getLargestCandidate(registerConstraint, usePositions, needsRegisterPair);
+ int candidate = getLargestValidCandidate(
+ unhandledInterval, registerConstraint, needsRegisterPair, usePositions);
int largestUsePosition = usePositions.get(candidate);
int blockedPosition = blockedPositions.get(candidate);
if (needsRegisterPair) {
diff --git a/src/test/java/com/android/tools/r8/internal/D8PhotosVerificationTest.java b/src/test/java/com/android/tools/r8/internal/D8PhotosVerificationTest.java
new file mode 100644
index 0000000..de72fb2
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/internal/D8PhotosVerificationTest.java
@@ -0,0 +1,29 @@
+// Copyright (c) 2017, 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.CompilationException;
+import com.android.tools.r8.CompilationMode;
+import com.android.tools.r8.R8RunArtTestsTest.CompilerUnderTest;
+import com.android.tools.r8.shaking.ProguardRuleParserException;
+import java.io.IOException;
+import java.util.concurrent.ExecutionException;
+import org.junit.Test;
+
+public class D8PhotosVerificationTest extends CompilationTestBase {
+ public static final String PHOTOS =
+ "third_party/photos/2017-06-06/PhotosEnglishOnlyLegacy_proguard.jar";
+
+ public void runD8AndCheckVerification(CompilationMode mode, String version)
+ throws ProguardRuleParserException, ExecutionException, IOException, CompilationException {
+ runAndCheckVerification(
+ CompilerUnderTest.D8, mode, version, null, null, version);
+ }
+
+ @Test
+ public void verify()
+ throws ExecutionException, IOException, ProguardRuleParserException, CompilationException {
+ runD8AndCheckVerification(CompilationMode.RELEASE, PHOTOS);
+ }
+}
diff --git a/third_party/photos/2017-06-06.tar.gz.sha1 b/third_party/photos/2017-06-06.tar.gz.sha1
new file mode 100644
index 0000000..b21e5a6
--- /dev/null
+++ b/third_party/photos/2017-06-06.tar.gz.sha1
@@ -0,0 +1 @@
+80389d76881463daf28b47f402c5013f499966bf
\ No newline at end of file
