Don't probe local state in addDebugLocalStart.
Change-Id: I2c3e2c01ce373af0656020d81573ac01f0f38236
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/CfSourceCode.java b/src/main/java/com/android/tools/r8/ir/conversion/CfSourceCode.java
index 5f62c6c..8e91a62 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/CfSourceCode.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/CfSourceCode.java
@@ -192,8 +192,6 @@
private boolean inPrelude;
private Int2ObjectMap<DebugLocalInfo> incomingLocals;
private Int2ObjectMap<DebugLocalInfo> outgoingLocals;
- private Int2ReferenceMap<Int2ObjectMap<DebugLocalInfo>> definitelyLiveIncomingLocals =
- new Int2ReferenceOpenHashMap<>();
private Int2ReferenceMap<CfState.Snapshot> incomingState = new Int2ReferenceOpenHashMap<>();
private final CanonicalPositions canonicalPositions;
@@ -371,38 +369,21 @@
if (predecessorOffset == IRBuilder.INITIAL_BLOCK_OFFSET) {
return;
}
- if (currentInstructionIndex != predecessorOffset) {
- // If transfer is not still in the same block, then update the state to that of the successor.
- // The builder's lookup of local variables relies on this state for starting locals.
- currentInstructionIndex = successorOffset;
- state.reset(incomingState.get(currentInstructionIndex), false);
- setLocalVariableLists();
- // The transfer has not yet taken place, so the current position is that of the predecessor.
- int positionOffset = predecessorOffset;
- List<CfInstruction> instructions = code.getInstructions();
- CfInstruction instruction = instructions.get(positionOffset);
- while (0 < positionOffset && !(instruction instanceof CfPosition)) {
- instruction = instructions.get(--positionOffset);
- }
- if (instruction instanceof CfPosition) {
- CfPosition position = (CfPosition) instruction;
- state.setPosition(getCanonicalPosition(position.getPosition()));
- } else {
- state.setPosition(canonicalPositions.getPreamblePosition());
- }
- }
+ // The transfer has not yet taken place, so the current position is that of the predecessor.
+ state.setPosition(getCanonicalDebugPositionAtOffset(predecessorOffset));
+
// Manually compute the local variable change for the block transfer.
- LocalVariableList atSource = getLocalVariables(predecessorOffset);
- LocalVariableList atTarget = getLocalVariables(successorOffset);
+ Int2ObjectMap<DebugLocalInfo> atSource = getLocalVariables(predecessorOffset).locals;
+ Int2ObjectMap<DebugLocalInfo> atTarget = getLocalVariables(successorOffset).locals;
if (!isExceptional) {
- for (Entry<DebugLocalInfo> entry : atSource.locals.int2ObjectEntrySet()) {
- if (atTarget.locals.get(entry.getIntKey()) != entry.getValue()) {
+ for (Entry<DebugLocalInfo> entry : atSource.int2ObjectEntrySet()) {
+ if (atTarget.get(entry.getIntKey()) != entry.getValue()) {
builder.addDebugLocalEnd(entry.getIntKey(), entry.getValue());
}
}
}
- for (Entry<DebugLocalInfo> entry : atTarget.locals.int2ObjectEntrySet()) {
- if (atSource.locals.get(entry.getIntKey()) != entry.getValue()) {
+ for (Entry<DebugLocalInfo> entry : atTarget.int2ObjectEntrySet()) {
+ if (atSource.get(entry.getIntKey()) != entry.getValue()) {
builder.addDebugLocalStart(entry.getIntKey(), entry.getValue());
}
}
@@ -562,33 +543,15 @@
private void setLocalVariableLists() {
incomingLocals = getLocalVariables(currentInstructionIndex).locals;
- CfInstruction currentInstruction = code.getInstructions().get(currentInstructionIndex);
if (inPrelude) {
outgoingLocals = incomingLocals;
return;
}
- if (currentInstruction.isReturn() || currentInstruction instanceof CfThrow) {
- outgoingLocals = emptyMap();
- return;
- }
- if (isControlFlow(currentInstruction)) {
- // We need to read all locals that are not live on all successors to ensure liveness.
- // Determine outgoingLocals as the intersection of all successors' locals.
- outgoingLocals = null;
- int[] targets = getTargets(currentInstructionIndex);
- for (int target : targets) {
- Int2ObjectMap<DebugLocalInfo> locals = getLocalVariables(target).locals;
- outgoingLocals = intersectMaps(outgoingLocals, locals);
- }
- assert outgoingLocals != null;
- // Pass outgoingLocals to all successors.
- for (int target : targets) {
- Int2ObjectMap<DebugLocalInfo> existing = definitelyLiveIncomingLocals.get(target);
- definitelyLiveIncomingLocals.put(target, intersectMaps(existing, outgoingLocals));
- }
- } else {
- outgoingLocals = getLocalVariables(currentInstructionIndex + 1).locals;
- }
+ CfInstruction currentInstruction = code.getInstructions().get(currentInstructionIndex);
+ outgoingLocals =
+ !isControlFlow(currentInstruction)
+ ? getLocalVariables(currentInstructionIndex + 1).locals
+ : emptyMap();
}
private boolean localsChanged() {
@@ -613,27 +576,6 @@
}
}
- private Int2ObjectMap<DebugLocalInfo> intersectMaps(
- Int2ObjectMap<DebugLocalInfo> existing, Int2ObjectMap<DebugLocalInfo> update) {
- assert update != null;
- if (existing == null) {
- return update;
- }
- if (existing.size() > update.size()) {
- return intersectMaps(update, existing);
- }
- if (existing.equals(update)) {
- return existing;
- }
- Int2ObjectOpenHashMap<DebugLocalInfo> result = new Int2ObjectOpenHashMap<>();
- for (Entry<DebugLocalInfo> local : existing.int2ObjectEntrySet()) {
- if (local.getValue().equals(update.get(local.getIntKey()))) {
- result.put(local.getIntKey(), local.getValue());
- }
- }
- return result;
- }
-
private boolean isControlFlow(CfInstruction currentInstruction) {
return currentInstruction.isReturn()
|| currentInstruction.getTarget() != null
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 5c4d581..8fe972b 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
@@ -706,14 +706,6 @@
addInstruction(new Argument(value));
}
- private void addDebugLocalWrite(ValueType type, int dest, Value in) {
- assert options.debug;
- Value out = writeRegister(dest, type, ThrowingInfo.NO_THROW);
- DebugLocalWrite write = new DebugLocalWrite(out, in);
- assert !write.instructionTypeCanThrow();
- addInstruction(write);
- }
-
private Value getIncomingLocalValue(int register, DebugLocalInfo local) {
assert options.debug;
assert local != null;
@@ -729,26 +721,26 @@
}
public void addDebugLocalStart(int register, DebugLocalInfo local) {
+ assert local != null;
if (!options.debug) {
return;
}
- assert local != null;
- assert local == getOutgoingLocal(register) :
- "local-start mismatch: " + local + " != " + getOutgoingLocal(register)
- + " at " + currentInstructionOffset
- + " for source\n" + source.toString();
- // TODO(b/111251032): Here we lookup a value with type based on debug info. That's just wrong!
- ValueType valueType = ValueType.fromDexType(local.type);
- Value incomingValue = readRegisterIgnoreLocal(register, valueType, local);
// If the local was not introduced by the previous instruction, start it here.
+ Value incomingValue = getIncomingLocalValue(register, local);
if (incomingValue.getLocalInfo() != local
|| currentBlock.isEmpty()
|| currentBlock.getInstructions().getLast().outValue() != incomingValue) {
- addDebugLocalWrite(ValueType.fromDexType(local.type), register, incomingValue);
+ // Note that the write register must not lookup outgoing local information and the local is
+ // never considered clobbered by a start (if the in value has local info it must have been
+ // marked ended elsewhere).
+ Value out = writeRegister(register, incomingValue.outType(), ThrowingInfo.NO_THROW, local);
+ DebugLocalWrite write = new DebugLocalWrite(out, incomingValue);
+ addInstruction(write);
}
}
public void addDebugLocalEnd(int register, DebugLocalInfo local) {
+ assert local != null;
if (!options.debug) {
return;
}
@@ -951,7 +943,8 @@
// If the move is writing to a different local we must construct a new value.
DebugLocalInfo destLocal = getOutgoingLocal(dest);
if (destLocal != null && destLocal != in.getLocalInfo()) {
- addDebugLocalWrite(type, dest, in);
+ Value out = writeRegister(dest, type, ThrowingInfo.NO_THROW);
+ addInstruction(new DebugLocalWrite(out, in));
return;
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/JarSourceCode.java b/src/main/java/com/android/tools/r8/ir/conversion/JarSourceCode.java
index e528a90..1039817 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/JarSourceCode.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/JarSourceCode.java
@@ -476,6 +476,7 @@
|| successorOffset == EXCEPTIONAL_SYNC_EXIT_OFFSET) {
return;
}
+ // The transfer has not yet taken place, so the current position is that of the predecessor.
currentPosition = getCanonicalDebugPositionAtOffset(predecessorOffset);
LocalChangeAtOffset localChange = state.getLocalChange(predecessorOffset, successorOffset);
@@ -484,12 +485,8 @@
builder.addDebugLocalEnd(toClose.slot.register, toClose.info);
}
}
- List<Local> localsToOpen = localChange.getLocalsToOpen();
- if (!localsToOpen.isEmpty()) {
- state.restoreState(successorOffset);
- for (Local toOpen : localsToOpen) {
- builder.addDebugLocalStart(toOpen.slot.register, toOpen.info);
- }
+ for (Local toOpen : localChange.getLocalsToOpen()) {
+ builder.addDebugLocalStart(toOpen.slot.register, toOpen.info);
}
}
