Make it less likely that Art lock verification gets confused.
When locking on argument values, use the argument register for all lock
operations whenever possible.
In all but the most obscure cases that should be possible. However,
it is not bullet-proof and when the argument registers become too high
we will have to use another register for it and lock verification
will get confused by the splitting and print a warning. That shouldn't
be treated as a fatal error, but we can attempt to avoid it as much
as possible.
R=acleung@google.com
Bug: 62475512
Change-Id: Ia4358e38aacfb90e79167d4b5144afa80d9cc30b
diff --git a/src/main/java/com/android/tools/r8/ir/code/Invoke.java b/src/main/java/com/android/tools/r8/ir/code/Invoke.java
index 8d0fa34..87edec8 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Invoke.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Invoke.java
@@ -91,8 +91,14 @@
}
protected int argumentRegisterValue(int i, DexBuilder builder) {
+ assert requiredArgumentRegisters() > 5;
if (i < arguments().size()) {
- return builder.allocatedRegisterForRangedArgument(arguments().get(i), getNumber());
+ // If argument values flow into ranged invokes, all the ranged invoke arguments
+ // are arguments to this method in order. Therefore, we use the incoming registers
+ // for the ranged invoke arguments. We know that arguments are always available there.
+ // If argument reuse is allowed there is no splitting and if argument reuse is disallowed
+ // the argument registers are never overwritten.
+ return builder.argumentOrAllocateRegister(arguments().get(i), getNumber());
}
return 0;
}
@@ -120,10 +126,10 @@
protected boolean argumentsConsecutive(DexBuilder builder) {
Value value = arguments().get(0);
- int next = builder.allocatedRegisterForRangedArgument(value, getNumber()) + value.requiredRegisters();
+ int next = builder.argumentOrAllocateRegister(value, getNumber()) + value.requiredRegisters();
for (int i = 1; i < arguments().size(); i++) {
value = arguments().get(i);
- assert next == builder.allocatedRegisterForRangedArgument(value, getNumber());
+ assert next == builder.argumentOrAllocateRegister(value, getNumber());
next += value.requiredRegisters();
}
return true;
diff --git a/src/main/java/com/android/tools/r8/ir/code/Monitor.java b/src/main/java/com/android/tools/r8/ir/code/Monitor.java
index 869d1d0..b3e155b 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Monitor.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Monitor.java
@@ -29,7 +29,15 @@
@Override
public void buildDex(DexBuilder builder) {
- int object = builder.allocatedRegister(object(), getNumber());
+ // If the monitor object is an argument, we use the argument register for all the monitor
+ // enters and exits in order to not confuse the Art verifier lock verification code.
+ // This is best effort. If the argument happens to be in a very high register we cannot
+ // do it and the lock verification can hit a case where it gets confused. Not much we
+ // can do about that, but this should avoid it in the most common cases.
+ int object = builder.argumentOrAllocateRegister(object(), getNumber());
+ if (object > maxInValueRegister()) {
+ object = builder.allocatedRegister(object(), getNumber());
+ }
if (type == Type.ENTER) {
builder.add(this, new MonitorEnter(object));
} else {
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/DexBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/DexBuilder.java
index 9cf14d8..50afe5f 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/DexBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/DexBuilder.java
@@ -318,8 +318,10 @@
return registerAllocator.getRegisterForValue(value, instructionNumber);
}
- public int allocatedRegisterForRangedArgument(Value value, int instructionNumber) {
- return registerAllocator.getRegisterForRangedArgument(value, instructionNumber);
+ // Get the argument register for a value if it is an argument, otherwise returns the
+ // allocated register at the instruction number.
+ public int argumentOrAllocateRegister(Value value, int instructionNumber) {
+ return registerAllocator.getArgumentOrAllocateRegisterForValue(value, instructionNumber);
}
public boolean argumentValueUsesHighRegister(Value value, int instructionNumber) {
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 4bf5dc6..4e7122d 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
@@ -421,12 +421,7 @@
}
@Override
- public int getRegisterForRangedArgument(Value value, int instructionNumber) {
- // If argument values flow into ranged invokes, all the ranged invoke arguments
- // are arguments to this method in order. Therefore, we use the incoming registers
- // for the ranged invoke arguments. We know that arguments are always available there.
- // If argument reuse is allowed there is no splitting and if argument reuse is disallowed
- // the argument registers are never overwritten.
+ public int getArgumentOrAllocateRegisterForValue(Value value, int instructionNumber) {
if (value.isArgument()) {
return getRegisterForIntervals(value.getLiveIntervals());
}
diff --git a/src/main/java/com/android/tools/r8/ir/regalloc/RegisterAllocator.java b/src/main/java/com/android/tools/r8/ir/regalloc/RegisterAllocator.java
index fc7c2d8..79ba8db 100644
--- a/src/main/java/com/android/tools/r8/ir/regalloc/RegisterAllocator.java
+++ b/src/main/java/com/android/tools/r8/ir/regalloc/RegisterAllocator.java
@@ -10,5 +10,5 @@
int registersUsed();
int getRegisterForValue(Value value, int instructionNumber);
boolean argumentValueUsesHighRegister(Value value, int instructionNumber);
- int getRegisterForRangedArgument(Value value, int instructionNumber);
+ int getArgumentOrAllocateRegisterForValue(Value value, int instructionNumber);
}
diff --git a/src/test/java/com/android/tools/r8/ir/regalloc/IdenticalAfterRegisterAllocationTest.java b/src/test/java/com/android/tools/r8/ir/regalloc/IdenticalAfterRegisterAllocationTest.java
index ba69bff..08e90c0 100644
--- a/src/test/java/com/android/tools/r8/ir/regalloc/IdenticalAfterRegisterAllocationTest.java
+++ b/src/test/java/com/android/tools/r8/ir/regalloc/IdenticalAfterRegisterAllocationTest.java
@@ -38,7 +38,7 @@
}
@Override
- public int getRegisterForRangedArgument(Value value, int instructionNumber) {
+ public int getArgumentOrAllocateRegisterForValue(Value value, int instructionNumber) {
return value.getNumber();
}
}
