Merge "Remove old constant splitting heuristics that doesn't help."
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/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);
}
}
