Make addPossiblyThrowingInstructionsToPossiblyThrowingBlock() work in more cases
It did not work when the cursor was before a throwing instruction. It
would split the block, which would move the throwing instruction to the
subsequent block, and then add another throwing instruction to the
subsequent block.
This also adds an override that takes a Collection<> to avoid needing an
array.
Bug: b/244238384
Change-Id: I608e72cb5e1100355820141404d874ddee961cc6
diff --git a/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionListIterator.java b/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionListIterator.java
index 640d3bd..af49f66 100644
--- a/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionListIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionListIterator.java
@@ -30,6 +30,7 @@
import com.google.common.collect.Sets;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.Iterator;
import java.util.List;
import java.util.ListIterator;
import java.util.Set;
@@ -135,8 +136,8 @@
}
/**
- * Adds an instruction to the block. The instruction will be added just before the current cursor
- * position.
+ * Adds an instruction to the block. The instruction will be added just before the instruction
+ * that would be returned by a call to next().
*
* <p>The instruction will be assigned to the block it is added to.
*
@@ -153,58 +154,71 @@
metadata.record(instruction);
}
+ private boolean hasPriorThrowingInstruction() {
+ Instruction next = peekNext();
+ for (Instruction ins : block.getInstructions()) {
+ if (ins == next) {
+ break;
+ }
+ if (ins.instructionTypeCanThrow()) {
+ return true;
+ }
+ }
+ return false;
+ }
+
@Override
public InstructionListIterator addPossiblyThrowingInstructionsToPossiblyThrowingBlock(
IRCode code,
BasicBlockIterator blockIterator,
- Instruction[] instructions,
+ Collection<Instruction> instructionsToAdd,
InternalOptions options) {
- InstructionListIterator iterator = this;
- if (!block.hasCatchHandlers()) {
- iterator.addAll(instructions);
- return iterator;
+ // Assert that we are not inserting after the final jump, and also store peekNext() for later.
+ Instruction origNext = null;
+ assert (origNext = peekNext()) != null;
+ InstructionListIterator ret =
+ addPossiblyThrowingInstructionsToPossiblyThrowingBlockImpl(
+ this, code, blockIterator, instructionsToAdd, options);
+ assert ret.peekNext() == origNext;
+ return ret;
+ }
+
+ // Use a static method to ensure dstIterator is used instead of "this".
+ private static InstructionListIterator addPossiblyThrowingInstructionsToPossiblyThrowingBlockImpl(
+ BasicBlockInstructionListIterator dstIterator,
+ IRCode code,
+ BasicBlockIterator blockIterator,
+ Collection<Instruction> instructionsToAdd,
+ InternalOptions options) {
+ if (!dstIterator.block.hasCatchHandlers() || instructionsToAdd.isEmpty()) {
+ dstIterator.addAll(instructionsToAdd);
+ return dstIterator;
}
- int i = 0;
- if (!block.canThrow()) {
- // Add all non-throwing instructions up until the first throwing instruction.
- for (; i < instructions.length; i++) {
- Instruction materializingInstruction = instructions[i];
- if (!materializingInstruction.instructionTypeCanThrow()) {
- iterator.add(materializingInstruction);
- } else {
- break;
- }
- }
- // Add the first throwing instruction without splitting the block.
- if (i < instructions.length) {
- assert instructions[i].instructionTypeCanThrow();
- iterator.add(instructions[i]);
- i++;
- }
+
+ Iterator<Instruction> srcIterator = instructionsToAdd.iterator();
+
+ // If the throwing instruction is before the cursor, then we must split the block first.
+ // If there is one afterwards, we can add instructions and when we split, the throwing one
+ // will be moved to the split block.
+ boolean splitBeforeAdding = dstIterator.hasPriorThrowingInstruction();
+ if (splitBeforeAdding) {
+ BasicBlock nextBlock =
+ dstIterator.splitCopyCatchHandlers(
+ code, blockIterator, options, UnaryOperator.identity());
+ dstIterator = nextBlock.listIterator(code);
}
- for (; i < instructions.length; i++) {
- BasicBlock splitBlock = iterator.splitCopyCatchHandlers(code, blockIterator, options);
- BasicBlock previousBlock = blockIterator.positionAfterPreviousBlock(splitBlock);
- assert previousBlock == splitBlock;
- iterator = splitBlock.listIterator(code);
- // Add all non-throwing instructions up until the next throwing instruction to the split
- // block.
- for (; i < instructions.length; i++) {
- Instruction materializingInstruction = instructions[i];
- if (!materializingInstruction.instructionTypeCanThrow()) {
- iterator.add(materializingInstruction);
- } else {
- break;
- }
+ do {
+ boolean addedThrowing = dstIterator.addUntilThrowing(srcIterator);
+ if (!addedThrowing || (!srcIterator.hasNext() && splitBeforeAdding)) {
+ break;
}
- // Add the current throwing instruction to the split block.
- if (i < instructions.length) {
- assert instructions[i].instructionTypeCanThrow();
- iterator.add(instructions[i]);
- i++;
- }
- }
- return iterator;
+ BasicBlock nextBlock =
+ dstIterator.splitCopyCatchHandlers(
+ code, blockIterator, options, UnaryOperator.identity());
+ dstIterator = nextBlock.listIterator(code);
+ } while (srcIterator.hasNext());
+
+ return dstIterator;
}
@Override
diff --git a/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionListIterator.java b/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionListIterator.java
index 325cea8..9db4576 100644
--- a/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionListIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionListIterator.java
@@ -231,10 +231,10 @@
public InstructionListIterator addPossiblyThrowingInstructionsToPossiblyThrowingBlock(
IRCode code,
BasicBlockIterator blockIterator,
- Instruction[] instructions,
+ Collection<Instruction> instructionsToAdd,
InternalOptions options) {
return instructionIterator.addPossiblyThrowingInstructionsToPossiblyThrowingBlock(
- code, blockIterator, instructions, options);
+ code, blockIterator, instructionsToAdd, options);
}
@Override
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 247079e..c9907ee 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
@@ -17,7 +17,9 @@
import com.android.tools.r8.utils.ConsumerUtils;
import com.android.tools.r8.utils.InternalOptions;
import com.google.common.collect.Sets;
+import java.util.Arrays;
import java.util.Collection;
+import java.util.Iterator;
import java.util.ListIterator;
import java.util.Set;
import java.util.function.Consumer;
@@ -33,12 +35,39 @@
}
}
+ default void addAll(Collection<Instruction> instructions) {
+ for (Instruction instruction : instructions) {
+ add(instruction);
+ }
+ }
+
+ default boolean addUntilThrowing(Iterator<Instruction> srcIterator) {
+ while (srcIterator.hasNext()) {
+ // Add all non-throwing instructions up until the first throwing instruction.
+ Instruction instruction = srcIterator.next();
+ add(instruction);
+ if (instruction.instructionTypeCanThrow()) {
+ return true;
+ }
+ }
+ return false;
+ }
+
InstructionListIterator addPossiblyThrowingInstructionsToPossiblyThrowingBlock(
IRCode code,
BasicBlockIterator blockIterator,
- Instruction[] instructions,
+ Collection<Instruction> instructionsToAdd,
InternalOptions options);
+ default InstructionListIterator addPossiblyThrowingInstructionsToPossiblyThrowingBlock(
+ IRCode code,
+ BasicBlockIterator blockIterator,
+ Instruction[] instructionsToAdd,
+ InternalOptions options) {
+ return addPossiblyThrowingInstructionsToPossiblyThrowingBlock(
+ code, blockIterator, Arrays.asList(instructionsToAdd), options);
+ }
+
BasicBlock addThrowingInstructionToPossiblyThrowingBlock(
IRCode code,
ListIterator<BasicBlock> blockIterator,
diff --git a/src/main/java/com/android/tools/r8/ir/code/LinearFlowInstructionListIterator.java b/src/main/java/com/android/tools/r8/ir/code/LinearFlowInstructionListIterator.java
index 21ab96c..62f3374 100644
--- a/src/main/java/com/android/tools/r8/ir/code/LinearFlowInstructionListIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/LinearFlowInstructionListIterator.java
@@ -192,10 +192,10 @@
public InstructionListIterator addPossiblyThrowingInstructionsToPossiblyThrowingBlock(
IRCode code,
BasicBlockIterator blockIterator,
- Instruction[] instructions,
+ Collection<Instruction> instructionsToAdd,
InternalOptions options) {
return currentBlockIterator.addPossiblyThrowingInstructionsToPossiblyThrowingBlock(
- code, blockIterator, instructions, options);
+ code, blockIterator, instructionsToAdd, options);
}
@Override
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 c579123..f6f2204 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
@@ -200,7 +200,7 @@
public InstructionListIterator addPossiblyThrowingInstructionsToPossiblyThrowingBlock(
IRCode code,
BasicBlockIterator blockIterator,
- Instruction[] instructions,
+ Collection<Instruction> instructionsToAdd,
InternalOptions options) {
throw new Unimplemented();
}
