Merge "Detach ProguardConfiguration from InternalOptions as much as possible."
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
index 6f94b1e..23b6b70 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -459,7 +459,7 @@
codeRewriter.commonSubexpressionElimination(code);
codeRewriter.simplifyArrayConstruction(code);
codeRewriter.rewriteMoveResult(code);
- codeRewriter.splitConstants(code);
+ codeRewriter.splitRangeInvokeConstants(code);
codeRewriter.foldConstants(code);
codeRewriter.rewriteSwitch(code);
codeRewriter.simplifyIf(code);
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
index e1c0ccb..48a64b8 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
@@ -63,7 +63,6 @@
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
-import java.util.IdentityHashMap;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
@@ -652,68 +651,35 @@
assert code.isConsistentSSA();
}
- // Constants are canonicalized in the entry block. We split some of them when it is likely
- // that having them canonicalized in the entry block will lead to poor code quality.
- public void splitConstants(IRCode code) {
+ // Split constants that flow into ranged invokes. This gives the register allocator more
+ // freedom in assigning register to ranged invokes which can greatly reduce the number
+ // of register needed (and thereby code size as well).
+ public void splitRangeInvokeConstants(IRCode code) {
for (BasicBlock block : code.blocks) {
- // Split constants that flow into phis. It is likely that these constants will have moves
- // generated for them anyway and we might as well insert a const instruction in the right
- // predecessor block.
- splitPhiConstants(code, block);
- // Split constants that flow into ranged invokes. This gives the register allocator more
- // freedom in assigning register to ranged invokes which can greatly reduce the number
- // of register needed (and thereby code size as well).
- splitRangedInvokeConstants(code, block);
- }
- }
-
- private void splitRangedInvokeConstants(IRCode code, BasicBlock block) {
- InstructionListIterator it = block.listIterator();
- while (it.hasNext()) {
- Instruction current = it.next();
- if (current.isInvoke() && current.asInvoke().requiredArgumentRegisters() > 5) {
- Invoke invoke = current.asInvoke();
- it.previous();
- Map<ConstNumber, ConstNumber> oldToNew = new HashMap<>();
- for (int i = 0; i < invoke.inValues().size(); i++) {
- Value value = invoke.inValues().get(i);
- if (value.isConstNumber() && value.numberOfUsers() > 1) {
- ConstNumber definition = value.getConstInstruction().asConstNumber();
- Value originalValue = definition.outValue();
- ConstNumber newNumber = oldToNew.get(definition);
- if (newNumber == null) {
- newNumber = ConstNumber.copyOf(code, definition);
- it.add(newNumber);
- oldToNew.put(definition, newNumber);
+ InstructionListIterator it = block.listIterator();
+ while (it.hasNext()) {
+ Instruction current = it.next();
+ if (current.isInvoke() && current.asInvoke().requiredArgumentRegisters() > 5) {
+ Invoke invoke = current.asInvoke();
+ it.previous();
+ Map<ConstNumber, ConstNumber> oldToNew = new HashMap<>();
+ for (int i = 0; i < invoke.inValues().size(); i++) {
+ Value value = invoke.inValues().get(i);
+ if (value.isConstNumber() && value.numberOfUsers() > 1) {
+ ConstNumber definition = value.getConstInstruction().asConstNumber();
+ Value originalValue = definition.outValue();
+ ConstNumber newNumber = oldToNew.get(definition);
+ if (newNumber == null) {
+ newNumber = ConstNumber.copyOf(code, definition);
+ it.add(newNumber);
+ oldToNew.put(definition, newNumber);
+ }
+ invoke.inValues().set(i, newNumber.outValue());
+ originalValue.removeUser(invoke);
+ newNumber.outValue().addUser(invoke);
}
- invoke.inValues().set(i, newNumber.outValue());
- originalValue.removeUser(invoke);
- newNumber.outValue().addUser(invoke);
}
- }
- it.next();
- }
- }
- }
-
- private void splitPhiConstants(IRCode code, BasicBlock block) {
- for (int i = 0; i < block.getPredecessors().size(); i++) {
- Map<ConstNumber, ConstNumber> oldToNew = new IdentityHashMap<>();
- BasicBlock predecessor = block.getPredecessors().get(i);
- for (Phi phi : block.getPhis()) {
- Value operand = phi.getOperand(i);
- if (!operand.isPhi() && operand.isConstNumber()) {
- ConstNumber definition = operand.getConstInstruction().asConstNumber();
- ConstNumber newNumber = oldToNew.get(definition);
- Value originalValue = definition.outValue();
- if (newNumber == null) {
- newNumber = ConstNumber.copyOf(code, definition);
- oldToNew.put(definition, newNumber);
- insertConstantInBlock(newNumber, predecessor);
- }
- phi.getOperands().set(i, newNumber.outValue());
- originalValue.removePhiUser(phi);
- newNumber.outValue().addPhiUser(phi);
+ it.next();
}
}
}
@@ -730,7 +696,7 @@
List<Instruction> toInsertInThisBlock = new ArrayList<>();
while (it.hasNext()) {
Instruction instruction = it.next();
- if (instruction.isConstNumber()) {
+ if (instruction.isConstNumber() && instruction.outValue().numberOfAllUsers() != 0) {
// Collect the blocks for all users of the constant.
List<BasicBlock> userBlocks = new LinkedList<>();
for (Instruction user : instruction.outValue().uniqueUsers()) {
diff --git a/src/main/java/com/android/tools/r8/ir/regalloc/LiveIntervals.java b/src/main/java/com/android/tools/r8/ir/regalloc/LiveIntervals.java
index 12db3d9..9a810d9 100644
--- a/src/main/java/com/android/tools/r8/ir/regalloc/LiveIntervals.java
+++ b/src/main/java/com/android/tools/r8/ir/regalloc/LiveIntervals.java
@@ -96,10 +96,22 @@
// these computations. We use the unadjusted real register number to make sure that
// isRematerializable for the same intervals does not change from one phase of
// compilation to the next.
+ if (getMaxNonSpilledRegister() == NO_REGISTER) {
+ assert allSplitsAreSpilled();
+ return true;
+ }
int max = registerAllocator.unadjustedRealRegisterFromAllocated(getMaxNonSpilledRegister());
return max < Constants.U8BIT_MAX;
}
+ private boolean allSplitsAreSpilled() {
+ assert isSpilled();
+ for (LiveIntervals splitChild : splitChildren) {
+ assert splitChild.isSpilled();
+ }
+ return true;
+ }
+
public boolean isSpilledAndRematerializable(LinearScanRegisterAllocator allocator) {
return isSpilled() && isRematerializable(allocator);
}
diff --git a/src/test/debugTestResources/Locals.java b/src/test/debugTestResources/Locals.java
index 3ab5d7b..d42dd61 100644
--- a/src/test/debugTestResources/Locals.java
+++ b/src/test/debugTestResources/Locals.java
@@ -211,6 +211,39 @@
public static void nop() {}
+ public static int tempInCase(int x) {
+ int res = 0;
+ for (int i = 0; i < x; ++i) {
+ int rem = x - i;
+ switch (rem) {
+ case 1:
+ return res;
+ case 5:
+ int tmp = res + x + i;
+ res += tmp;
+ break;
+ case 10:
+ i++;
+ break;
+ default:
+ res += rem;
+ }
+ res += rem % 2;
+ }
+ res *= x;
+ return res;
+ }
+
+ public static int localSwap(int x, int y) {
+ int sum = x + y;
+ {
+ int t = x;
+ x = y;
+ y = t;
+ }
+ return sum + x + y;
+ }
+
public static void main(String[] args) {
noLocals();
unusedLocals();
@@ -224,6 +257,7 @@
stepEmptyForLoopBody1(3);
stepEmptyForLoopBody2(3);
stepNonEmptyForLoopBody(3);
+ tempInCase(42);
+ localSwap(1, 2);
}
-
}
diff --git a/src/test/java/com/android/tools/r8/debug/LocalsTest.java b/src/test/java/com/android/tools/r8/debug/LocalsTest.java
index 16d1a60..02fb694 100644
--- a/src/test/java/com/android/tools/r8/debug/LocalsTest.java
+++ b/src/test/java/com/android/tools/r8/debug/LocalsTest.java
@@ -423,4 +423,51 @@
checkLocal("i", I3),
run());
}
+
+ @Test
+ public void tempInCase() throws Throwable {
+ runDebugTest(
+ "Locals",
+ breakpoint("Locals", "tempInCase"),
+ run(),
+ checkLine(SOURCE_FILE, 215),
+ checkLocal("x", Value.createInt(42)),
+ stepOver(),
+ checkLine(SOURCE_FILE, 216),
+ checkLocal("res", Value.createInt(0)),
+ checkNoLocal("i"),
+ stepOver(),
+ checkLine(SOURCE_FILE, 217),
+ stepOver(),
+ checkLine(SOURCE_FILE, 218),
+ checkLocal("rem", Value.createInt(42)),
+ setLocal("rem", Value.createInt(1)),
+ stepOver(),
+ checkLine(SOURCE_FILE, 220),
+ checkLocal("res", Value.createInt(0)),
+ run());
+ }
+
+ @Test
+ public void localSwap() throws Throwable {
+ runDebugTest(
+ "Locals",
+ breakpoint("Locals", "localSwap"),
+ run(),
+ checkLine(SOURCE_FILE, 238),
+ stepOver(),
+ checkLine(SOURCE_FILE, 240),
+ stepOver(),
+ checkLine(SOURCE_FILE, 241),
+ checkLocal("x", Value.createInt(1)),
+ checkLocal("y", Value.createInt(2)),
+ checkLocal("t", Value.createInt(1)),
+ stepOver(),
+ stepOver(),
+ checkLine(SOURCE_FILE, 244),
+ checkLocal("x", Value.createInt(2)),
+ checkLocal("y", Value.createInt(1)),
+ checkNoLocal("t"),
+ run());
+ }
}
diff --git a/src/test/java/com/android/tools/r8/debuginfo/BackBranchToSelfTestRunner.java b/src/test/java/com/android/tools/r8/debuginfo/BackBranchToSelfTestRunner.java
index 24eae68..0ad8ecd 100644
--- a/src/test/java/com/android/tools/r8/debuginfo/BackBranchToSelfTestRunner.java
+++ b/src/test/java/com/android/tools/r8/debuginfo/BackBranchToSelfTestRunner.java
@@ -22,11 +22,11 @@
assertEquals(expected, runOnArt(d8App, clazz.getCanonicalName()));
assertEquals(expected, runOnArt(dxApp, clazz.getCanonicalName()));
- checkBackBranchToSelf(inspectMethod(d8App, clazz, "int", "backBranchToSelf", "boolean"), false);
- checkBackBranchToSelf(inspectMethod(dxApp, clazz, "int", "backBranchToSelf", "boolean"), true);
+ checkBackBranchToSelf(inspectMethod(d8App, clazz, "int", "backBranchToSelf", "boolean"));
+ checkBackBranchToSelf(inspectMethod(dxApp, clazz, "int", "backBranchToSelf", "boolean"));
}
- private void checkBackBranchToSelf(DebugInfoInspector info, boolean dx) {
+ private void checkBackBranchToSelf(DebugInfoInspector info) {
info.checkStartLine(10);
info.checkLineHasExactLocals(10, "loop", "boolean");
info.checkNoLine(11);
diff --git a/src/test/java/com/android/tools/r8/smali/IfSimplificationTest.java b/src/test/java/com/android/tools/r8/smali/IfSimplificationTest.java
index 6b4a431..919f569 100644
--- a/src/test/java/com/android/tools/r8/smali/IfSimplificationTest.java
+++ b/src/test/java/com/android/tools/r8/smali/IfSimplificationTest.java
@@ -180,8 +180,8 @@
":return",
" return v0");
DexCode code = method.getCode().asDexCode();
- assertEquals(12, code.instructions.length);
- assertTrue(code.instructions[11] instanceof Return);
+ assertEquals(10, code.instructions.length);
+ assertTrue(code.instructions[9] instanceof Return);
}
@Test
@@ -443,6 +443,6 @@
DexCode code = method.getCode().asDexCode();
// TODO(sgjesse): Maybe this test is too fragile, as it leaves quite a lot of code, so the
// expectation might need changing with other optimizations.
- assertEquals(29, code.instructions.length);
+ assertEquals(27, code.instructions.length);
}
}
diff --git a/src/test/java/com/android/tools/r8/smali/SwitchRewritingTest.java b/src/test/java/com/android/tools/r8/smali/SwitchRewritingTest.java
index f10add0..7f89da7 100644
--- a/src/test/java/com/android/tools/r8/smali/SwitchRewritingTest.java
+++ b/src/test/java/com/android/tools/r8/smali/SwitchRewritingTest.java
@@ -7,7 +7,6 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
-import com.android.tools.r8.R8;
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.code.Const;
import com.android.tools.r8.code.Const4;
@@ -292,12 +291,11 @@
DexCode code = method.getCode().asDexCode();
if (key == 0) {
assertEquals(5, code.instructions.length);
- assertTrue(code.instructions[0] instanceof IfEqz);
+ assertTrue(code.instructions[2] instanceof IfEqz);
} else {
assertEquals(6, code.instructions.length);
- assertTrue(some16BitConst(code.instructions[0]));
- assertTrue(code.instructions[1] instanceof IfEq);
- assertTrue(code.instructions[2] instanceof Const4);
+ assertTrue(some16BitConst(code.instructions[2]));
+ assertTrue(code.instructions[3] instanceof IfEq);
}
}
@@ -351,9 +349,9 @@
DexEncodedMethod method = getMethod(app, signature);
DexCode code = method.getCode().asDexCode();
if (twoCaseWillUsePackedSwitch(key1, key2)) {
- assertTrue(code.instructions[0] instanceof PackedSwitch);
+ assertTrue(code.instructions[3] instanceof PackedSwitch);
} else {
- assertTrue(code.instructions[0] instanceof SparseSwitch);
+ assertTrue(code.instructions[3] instanceof SparseSwitch);
}
}
@@ -423,10 +421,22 @@
MethodSignature signature = new MethodSignature("Test", "test", "int", ImmutableList.of("int"));
DexEncodedMethod method = getMethod(app, signature);
DexCode code = method.getCode().asDexCode();
+ int packedSwitchCount = 0;
+ int sparseSwitchCount = 0;
+ for (Instruction instruction : code.instructions) {
+ if (instruction instanceof PackedSwitch) {
+ packedSwitchCount++;
+ }
+ if (instruction instanceof SparseSwitch) {
+ sparseSwitchCount++;
+ }
+ }
if (keyStep <= 2) {
- assertTrue(code.instructions[0] instanceof PackedSwitch);
+ assertEquals(1, packedSwitchCount);
+ assertEquals(0, sparseSwitchCount);
} else {
- assertTrue(code.instructions[0] instanceof SparseSwitch);
+ assertEquals(0, packedSwitchCount);
+ assertEquals(1, sparseSwitchCount);
}
}
diff --git a/third_party/framework.tar.gz.sha1 b/third_party/framework.tar.gz.sha1
index 7c11eec..675bb98 100644
--- a/third_party/framework.tar.gz.sha1
+++ b/third_party/framework.tar.gz.sha1
@@ -1 +1 @@
-d36913d75cde7c1293959698c45cf62342c709e0
\ No newline at end of file
+58224fa0baf2e9bd995918db690c35b2200c83a2
\ No newline at end of file
diff --git a/tools/test.py b/tools/test.py
index ba459e7..8e016c7 100755
--- a/tools/test.py
+++ b/tools/test.py
@@ -68,6 +68,7 @@
utils.upload_html_to_cloud_storage(upload_dir, destination)
url = 'http://storage.googleapis.com/%s/%s/test/index.html' % (BUCKET, u_dir)
print 'Test results available at: %s' % url
+ print '@@@STEP_LINK@Test failures@%s@@@' % url
def Main():
(options, args) = ParseOptions()
@@ -112,7 +113,7 @@
return_code = gradle.RunGradle(gradle_args + ['-Pdex_vm=%s' % art_vm],
throw_on_failure=False)
if return_code != 0:
- if options.archive_failures:
+ if options.archive_failures and os.name != 'nt':
archive_failures()
return return_code
diff --git a/tools/utils.py b/tools/utils.py
index b7abb45..dab1dd1 100644
--- a/tools/utils.py
+++ b/tools/utils.py
@@ -56,7 +56,7 @@
def upload_html_to_cloud_storage(directory, destination):
# Upload and make the content encoding right for viewing directly
- cmd = [sys.executable, 'gsutil.py', 'cp', '-z', 'html', '-a',
+ cmd = ['gsutil.py', 'cp', '-z', 'html', '-a',
'public-read', '-R', directory, destination]
PrintCmd(cmd)
subprocess.check_call(cmd)
