Fix register allocation bug triggering in rare situations in debug mode.
When we use a debug move in place of an argument move that breaks
previous assumptions about the handling of blocked registers.
This change fixes the splitting or intervals from blocked registers
and make sure that argument splits are handled correctly in that
setting as well.
R=sgjesse@google.com
Bug: b/65355498
Change-Id: I8abdc980cf2d32cbb462924709599f55e5bdbdd5
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 52653a7..bc93fcf 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
@@ -1018,9 +1018,6 @@
}
private int getSpillRegister(LiveIntervals intervals) {
- if (intervals.isArgumentInterval()) {
- return intervals.getSplitParent().getRegister();
- }
int registerNumber = nextUnusedRegisterNumber++;
maxRegisterNumber = registerNumber;
if (intervals.getType() == MoveType.WIDE) {
@@ -1572,17 +1569,19 @@
LiveIntervals unhandledInterval,
boolean needsRegisterPair,
int candidate) {
+ List<LiveIntervals> newInactive = new ArrayList<>();
Iterator<LiveIntervals> inactiveIterator = inactive.iterator();
while (inactiveIterator.hasNext()) {
LiveIntervals intervals = inactiveIterator.next();
if ((intervals.usesRegister(candidate) ||
(needsRegisterPair && intervals.usesRegister(candidate + 1))) &&
intervals.overlaps(unhandledInterval)) {
- // If these assertions trigger we have changed the way blocked parts of intervals
- // are handled. If we ever get intervals with fixed registers in here, we need
- // to split them before the first use in the same way that we do when spilling
- // overlapping active intervals.
- assert !intervals.isLinked() || intervals.isArgumentInterval();
+ if (intervals.isLinked() && !intervals.isArgumentInterval()) {
+ int nextUsePosition = intervals.firstUseAfter(unhandledInterval.getStart());
+ LiveIntervals split = intervals.splitBefore(nextUsePosition);
+ split.setRegister(intervals.getRegister());
+ newInactive.add(split);
+ }
if (intervals.getStart() > unhandledInterval.getStart()) {
// The inactive live intervals hasn't started yet. Clear the temporary register
// assignment and move back to unhandled for register reassignment.
@@ -1597,6 +1596,7 @@
}
}
}
+ inactive.addAll(newInactive);
}
private void spillOverlappingActiveIntervals(
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 0d8c6b8..3c2210d 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
@@ -88,6 +88,9 @@
}
public boolean isRematerializable(LinearScanRegisterAllocator registerAllocator) {
+ if (value.isArgument()) {
+ return true;
+ }
// TODO(ager): rematerialize const string as well.
if (!value.isConstNumber()) {
return false;
diff --git a/src/main/java/com/android/tools/r8/ir/regalloc/RegisterMove.java b/src/main/java/com/android/tools/r8/ir/regalloc/RegisterMove.java
index 4a21e8a..313497c 100644
--- a/src/main/java/com/android/tools/r8/ir/regalloc/RegisterMove.java
+++ b/src/main/java/com/android/tools/r8/ir/regalloc/RegisterMove.java
@@ -26,7 +26,7 @@
this.dst = dst;
this.src = LinearScanRegisterAllocator.NO_REGISTER;
this.type = type;
- assert definition.isConstInstruction();
+ assert definition.isConstInstruction() || definition.isArgument();
this.definition = definition;
}
diff --git a/src/main/java/com/android/tools/r8/ir/regalloc/RegisterMoveScheduler.java b/src/main/java/com/android/tools/r8/ir/regalloc/RegisterMoveScheduler.java
index 007aa6f..a669737 100644
--- a/src/main/java/com/android/tools/r8/ir/regalloc/RegisterMoveScheduler.java
+++ b/src/main/java/com/android/tools/r8/ir/regalloc/RegisterMoveScheduler.java
@@ -121,8 +121,14 @@
Instruction instruction;
Value to = new FixedRegisterValue(move.type, move.dst);
if (move.definition != null) {
- ConstNumber number = move.definition.asConstNumber();
- instruction = new ConstNumber(number.type, to, number.getRawValue());
+ if (move.definition.isArgument()) {
+ int argumentRegister = move.definition.outValue().getLiveIntervals().getRegister();
+ Value from = new FixedRegisterValue(move.type, argumentRegister);
+ instruction = new Move(to, from);
+ } else {
+ ConstNumber number = move.definition.asConstNumber();
+ instruction = new ConstNumber(number.type, to, number.getRawValue());
+ }
} else {
Value from = new FixedRegisterValue(move.type, valueMap.get(move.src));
instruction = new Move(to, from);
diff --git a/src/main/java/com/android/tools/r8/ir/regalloc/SpillMoveSet.java b/src/main/java/com/android/tools/r8/ir/regalloc/SpillMoveSet.java
index 643a3c1..cd1d880 100644
--- a/src/main/java/com/android/tools/r8/ir/regalloc/SpillMoveSet.java
+++ b/src/main/java/com/android/tools/r8/ir/regalloc/SpillMoveSet.java
@@ -31,8 +31,6 @@
// The register allocator generating moves.
private LinearScanRegisterAllocator allocator;
// All registers below this number are arguments.
- // TODO(ager): Get rid of this field, we should deal with arguments and other values that can
- // be rematerialized differently.
private final int argumentRegisterLimit;
// Mapping from instruction numbers to the block that start with that instruction if any.
private final Map<Integer, BasicBlock> blockStartMap = new HashMap<>();
@@ -257,6 +255,10 @@
// disallowed at this point we know that argument registers do not change value and
// therefore we don't have to perform spill moves. Performing spill moves will also
// make art reject the code because it loses type information for the argument.
+ //
+ // TODO(ager): We are dealing with some of these moves as rematerialization. However,
+ // we are still generating actual moves back to the original argument register.
+ // We should get rid of this method and avoid generating the moves in the first place.
private void removeArgumentRestores(Set<SpillMove> moves) {
Iterator<SpillMove> moveIterator = moves.iterator();
while (moveIterator.hasNext()) {
