Rename IR Nop to DebugLocalReads and ensure it is never dead code.
R=ager
Bug:
Change-Id: Id9da9417dccf1220843e76f2e82620b6be791f11
diff --git a/src/main/java/com/android/tools/r8/graph/DexDebugEventBuilder.java b/src/main/java/com/android/tools/r8/graph/DexDebugEventBuilder.java
index 14e5288..a5a63e3 100644
--- a/src/main/java/com/android/tools/r8/graph/DexDebugEventBuilder.java
+++ b/src/main/java/com/android/tools/r8/graph/DexDebugEventBuilder.java
@@ -86,7 +86,7 @@
// If this is the end of the block clear out the pending state.
pendingLocals = null;
pendingLocalChanges = false;
- } else if (pc != emittedPc && !instruction.isNop()) {
+ } else if (pc != emittedPc && !instruction.isDebugLocalRead()) {
// For non-exit / pc-advancing instructions emit any pending changes once possible.
emitLocalChanges(pc);
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionIterator.java b/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionIterator.java
index 5089ca5..aebb9ed 100644
--- a/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionIterator.java
@@ -126,14 +126,14 @@
}
@Override
- public void removeOrReplaceByNop() {
+ public void removeOrReplaceByDebugLocalRead() {
if (current == null) {
throw new IllegalStateException();
}
if (current.getDebugValues().isEmpty()) {
remove();
} else {
- replaceCurrentInstruction(new Nop());
+ replaceCurrentInstruction(new DebugLocalRead());
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/Nop.java b/src/main/java/com/android/tools/r8/ir/code/DebugLocalRead.java
similarity index 76%
rename from src/main/java/com/android/tools/r8/ir/code/Nop.java
rename to src/main/java/com/android/tools/r8/ir/code/DebugLocalRead.java
index dc378de..f0eb4a5 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Nop.java
+++ b/src/main/java/com/android/tools/r8/ir/code/DebugLocalRead.java
@@ -10,25 +10,25 @@
import com.android.tools.r8.ir.optimize.Inliner.Constraint;
import com.android.tools.r8.utils.InternalOptions;
-public class Nop extends Instruction {
+public class DebugLocalRead extends Instruction {
- public Nop() {
+ public DebugLocalRead() {
super(null);
}
@Override
- public boolean isNop() {
+ public boolean isDebugLocalRead() {
return true;
}
@Override
- public Nop asNop() {
+ public DebugLocalRead asDebugLocalRead() {
return this;
}
@Override
public void buildDex(DexBuilder builder) {
- builder.addNop(this);
+ throw new Unreachable("Unexpected attempt to emit debug-local read.");
}
@Override
@@ -58,6 +58,8 @@
@Override
public boolean canBeDeadCode(IRCode code, InternalOptions options) {
- return getDebugValues().isEmpty();
+ // Reads are never dead code.
+ // They should also have a non-empty set of debug values (see RegAlloc::computeDebugInfo)
+ return false;
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionsIterator.java b/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionsIterator.java
index 1390620..1bedf26 100644
--- a/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionsIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionsIterator.java
@@ -53,7 +53,7 @@
}
@Override
- public void removeOrReplaceByNop() {
- instructionIterator.removeOrReplaceByNop();
+ public void removeOrReplaceByDebugLocalRead() {
+ instructionIterator.removeOrReplaceByDebugLocalRead();
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/Instruction.java b/src/main/java/com/android/tools/r8/ir/code/Instruction.java
index 801c684..15291a5 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Instruction.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Instruction.java
@@ -849,11 +849,11 @@
return null;
}
- public boolean isNop() {
+ public boolean isDebugLocalRead() {
return false;
}
- public Nop asNop() {
+ public DebugLocalRead asDebugLocalRead() {
return null;
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/InstructionIterator.java b/src/main/java/com/android/tools/r8/ir/code/InstructionIterator.java
index 56d81a2..2f2b3f1 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InstructionIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InstructionIterator.java
@@ -33,8 +33,8 @@
void add(Instruction instruction);
/**
- * Safe removal function that will insert a Nop to take over the debug values if any are
- * associated with the current instruction.
+ * Safe removal function that will insert a DebugLocalRead to take over the debug values if any
+ * are associated with the current instruction.
*/
- void removeOrReplaceByNop();
+ void removeOrReplaceByDebugLocalRead();
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/InstructionListIterator.java b/src/main/java/com/android/tools/r8/ir/code/InstructionListIterator.java
index d8ea70b..92491c3 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InstructionListIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InstructionListIterator.java
@@ -60,10 +60,10 @@
}
/**
- * Safe removal function that will insert a Nop to take over the debug values if any are
- * associated with the current instruction.
+ * Safe removal function that will insert a DebugLocalRead to take over the debug values if any
+ * are associated with the current instruction.
*/
- void removeOrReplaceByNop();
+ void removeOrReplaceByDebugLocalRead();
/**
* Remove the current instruction (aka the {@link Instruction} returned by the previous call to
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 3bdeefa..0d96df7 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
@@ -353,8 +353,7 @@
}
private static boolean isNopInstruction(com.android.tools.r8.ir.code.Instruction instruction) {
- return instruction.isNop()
- || instruction.isDebugLocalsChange()
+ return instruction.isDebugLocalsChange()
|| (instruction.isConstNumber() && !instruction.outValue().needsRegister());
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
index f2dfce4..fc516e4 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
@@ -35,6 +35,7 @@
import com.android.tools.r8.ir.code.ConstNumber;
import com.android.tools.r8.ir.code.ConstString;
import com.android.tools.r8.ir.code.ConstType;
+import com.android.tools.r8.ir.code.DebugLocalRead;
import com.android.tools.r8.ir.code.DebugLocalUninitialized;
import com.android.tools.r8.ir.code.DebugLocalWrite;
import com.android.tools.r8.ir.code.DebugPosition;
@@ -59,7 +60,6 @@
import com.android.tools.r8.ir.code.NewArrayEmpty;
import com.android.tools.r8.ir.code.NewArrayFilledData;
import com.android.tools.r8.ir.code.NewInstance;
-import com.android.tools.r8.ir.code.Nop;
import com.android.tools.r8.ir.code.Not;
import com.android.tools.r8.ir.code.NumberConversion;
import com.android.tools.r8.ir.code.NumericType;
@@ -608,7 +608,7 @@
// 1. The block is empty (eg, instructions from block entry until now materialized to nothing).
// 2. The block is non-empty (and the last instruction does not define the local to start).
if (currentBlock.getInstructions().isEmpty()) {
- addInstruction(new Nop());
+ addInstruction(new DebugLocalRead());
}
Instruction instruction = currentBlock.getInstructions().getLast();
assert instruction.outValue() != value;
@@ -630,7 +630,7 @@
// 2. The block has an instruction not defining the local being ended.
// 3. The block has an instruction defining the local being ended.
if (currentBlock.getInstructions().isEmpty()) {
- addInstruction(new Nop());
+ addInstruction(new DebugLocalRead());
}
Instruction instruction = currentBlock.getInstructions().getLast();
if (instruction.outValue() != value) {
@@ -645,7 +645,7 @@
if (instruction.isDebugLocalWrite()) {
DebugLocalWrite write = instruction.asDebugLocalWrite();
currentBlock.replaceCurrentDefinitions(value, write.src());
- currentBlock.listIterator(write).removeOrReplaceByNop();
+ currentBlock.listIterator(write).removeOrReplaceByDebugLocalRead();
} else {
instruction.outValue().clearLocalInfo();
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/JarState.java b/src/main/java/com/android/tools/r8/ir/conversion/JarState.java
index 828ff10..f11af49 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/JarState.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/JarState.java
@@ -176,7 +176,7 @@
// without conflating locals that are shared among different types. This issue arises because a
// debugging range can be larger than the definite-assignment scope of a local (eg, a local
// introduced in an unscoped switch case). To ensure that the SSA graph is valid we must introduce
- // the local before inserting any DebugLocalReads (we do so in the method prelude, but that can
+ // the local before inserting any DebugLocalRead (we do so in the method prelude, but that can
// potentially lead to phi functions merging locals of different move-types. Thus we allocate
// registers from the three distinct spaces.
private final int localsSize;
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 348fd02..fff55f0 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
@@ -1088,7 +1088,7 @@
user -> user.isCheckCast()
&& user.asCheckCast().getType().isSubtypeOf(checkCast.getType(), appInfo))) {
checkCast.outValue().replaceUsers(checkCast.inValues().get(0));
- it.removeOrReplaceByNop();
+ it.removeOrReplaceByDebugLocalRead();
}
}
}
@@ -1506,7 +1506,7 @@
write.outValue().replaceUsers(phi);
// Safely remove the write.
// TODO(zerny): Once phis become instructions, move debug values there instead of a nop.
- iterator.removeOrReplaceByNop();
+ iterator.removeOrReplaceByDebugLocalRead();
return;
}
}
@@ -1678,7 +1678,7 @@
shareCatchHandlers(instruction, candidate.definition)) {
instruction.outValue().replaceUsers(candidate);
eliminated = true;
- iterator.removeOrReplaceByNop();
+ iterator.removeOrReplaceByDebugLocalRead();
break; // Don't try any more candidates.
}
}
@@ -1927,13 +1927,13 @@
DexMethod invokedMethod = current.asInvokeMethod().getInvokedMethod();
if (matchesMethodOfThrowable(invokedMethod, throwableMethods.addSuppressed)) {
// Remove Throwable::addSuppressed(Throwable) call.
- iterator.removeOrReplaceByNop();
+ iterator.removeOrReplaceByDebugLocalRead();
} else if (matchesMethodOfThrowable(invokedMethod, throwableMethods.getSuppressed)) {
Value destValue = current.outValue();
if (destValue == null) {
// If the result of the call was not used we don't create
// an empty array and just remove the call.
- iterator.removeOrReplaceByNop();
+ iterator.removeOrReplaceByDebugLocalRead();
continue;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/DeadCodeRemover.java b/src/main/java/com/android/tools/r8/ir/optimize/DeadCodeRemover.java
index ebf69b5..2790c03 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/DeadCodeRemover.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/DeadCodeRemover.java
@@ -120,7 +120,7 @@
// All users will be removed for this instruction. Eagerly clear them so further inspection
// of this instruction during dead code elimination will terminate here.
outValue.clearUsers();
- iterator.removeOrReplaceByNop();
+ iterator.removeOrReplaceByDebugLocalRead();
}
}
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 a8b3972..7facea6 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
@@ -324,6 +324,12 @@
int spillCount = 0;
while (instructionIterator.hasNext()) {
Instruction instruction = instructionIterator.next();
+ if (instruction.isDebugLocalRead()) {
+ // Remove debug local reads now that local liveness is computed.
+ assert !instruction.getDebugValues().isEmpty();
+ instruction.clearDebugValues();
+ instructionIterator.remove();
+ }
int index = instruction.getNumber();
if (index == -1) {
spillCount++;
diff --git a/src/test/java/com/android/tools/r8/ir/regalloc/RegisterMoveSchedulerTest.java b/src/test/java/com/android/tools/r8/ir/regalloc/RegisterMoveSchedulerTest.java
index 8ab2de5..c8ea3bb 100644
--- a/src/test/java/com/android/tools/r8/ir/regalloc/RegisterMoveSchedulerTest.java
+++ b/src/test/java/com/android/tools/r8/ir/regalloc/RegisterMoveSchedulerTest.java
@@ -74,7 +74,7 @@
}
@Override
- public void removeOrReplaceByNop() {
+ public void removeOrReplaceByDebugLocalRead() {
remove();
}
