Revert "Make addPossiblyThrowingInstructionsToPossiblyThrowingBlock() work in more cases"
This reverts commit d1f378da757bf94aa1288d6969969dce7da64ad4.
Revert "Fix AssertionError in Goto.toString() when Goto is no the last instruction in a block"
This reverts commit 147b0770d9a01ce6477cb25b6d3ca906e32830d8.
Revert "Ensure "current" is used only for remove/replace in BasicBlockInstructionListIterator."
This reverts commit 138bf7df5ecc311ea872eab47986cf4231ae6242.
Reason for revert: AssertionError builfing R8 lib.
Bug: b/244238384
Change-Id: Id748e579c0a0c1efa5ed84dad9c16ebac1f3ca75
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 cc9dd41..640d3bd 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,7 +30,6 @@
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;
@@ -136,8 +135,8 @@
}
/**
- * Adds an instruction to the block. The instruction will be added just before the instruction
- * that would be returned by a call to next().
+ * Adds an instruction to the block. The instruction will be added just before the current cursor
+ * position.
*
* <p>The instruction will be assigned to the block it is added to.
*
@@ -154,71 +153,58 @@
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,
- Collection<Instruction> instructionsToAdd,
+ Instruction[] instructions,
InternalOptions options) {
- // 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;
+ InstructionListIterator iterator = this;
+ if (!block.hasCatchHandlers()) {
+ iterator.addAll(instructions);
+ return iterator;
}
-
- 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);
- }
- do {
- boolean addedThrowing = dstIterator.addUntilThrowing(srcIterator);
- if (!addedThrowing || (!srcIterator.hasNext() && splitBeforeAdding)) {
- break;
+ 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;
+ }
}
- BasicBlock nextBlock =
- dstIterator.splitCopyCatchHandlers(
- code, blockIterator, options, UnaryOperator.identity());
- dstIterator = nextBlock.listIterator(code);
- } while (srcIterator.hasNext());
-
- return dstIterator;
+ // Add the first throwing instruction without splitting the block.
+ if (i < instructions.length) {
+ assert instructions[i].instructionTypeCanThrow();
+ iterator.add(instructions[i]);
+ i++;
+ }
+ }
+ 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;
+ }
+ }
+ // Add the current throwing instruction to the split block.
+ if (i < instructions.length) {
+ assert instructions[i].instructionTypeCanThrow();
+ iterator.add(instructions[i]);
+ i++;
+ }
+ }
+ return iterator;
}
@Override
@@ -351,33 +337,31 @@
current = newInstruction;
}
- private Position getPreviousPosition() {
- // Cannot use "current" because it is invalidated by peekNext().
- Instruction prev = peekPrevious();
- return prev != null ? prev.getPosition() : block.getPosition();
- }
-
- private void addNewNonThrowing(Instruction instruction, InternalOptions options) {
- assert !instruction.instructionTypeCanThrow();
- if (!hasInsertionPosition()) {
- // We keep position info only for throwing instructions in release mode.
- instruction.setPosition(options.debug ? getPreviousPosition() : Position.none());
- }
- add(instruction);
- }
-
@Override
public Value insertConstNumberInstruction(
IRCode code, InternalOptions options, long value, TypeElement type) {
ConstNumber constNumberInstruction = code.createNumberConstant(value, type);
- addNewNonThrowing(constNumberInstruction, options);
+ // Note that we only keep position info for throwing instructions in release mode.
+ if (!hasInsertionPosition()) {
+ Position position;
+ if (options.debug) {
+ position = current != null ? current.getPosition() : block.getPosition();
+ } else {
+ position = Position.none();
+ }
+ constNumberInstruction.setPosition(position);
+ }
+ add(constNumberInstruction);
return constNumberInstruction.outValue();
}
@Override
public Value insertConstStringInstruction(AppView<?> appView, IRCode code, DexString value) {
ConstString constStringInstruction = code.createStringConstant(appView, value);
- addNewNonThrowing(constStringInstruction, appView.options());
+ // Note that we only keep position info for throwing instructions in release mode.
+ constStringInstruction.setPosition(
+ appView.options().debug ? current.getPosition() : Position.none());
+ add(constStringInstruction);
return constStringInstruction.outValue();
}
@@ -716,7 +700,7 @@
assert hasNext();
// Get the position at which the block is being split.
- Position position = getPreviousPosition();
+ Position position = current != null ? current.getPosition() : block.getPosition();
// Prepare the new block, placing the exception handlers on the block with the throwing
// instruction.
diff --git a/src/main/java/com/android/tools/r8/ir/code/Goto.java b/src/main/java/com/android/tools/r8/ir/code/Goto.java
index 9beaff7..41ad960 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Goto.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Goto.java
@@ -8,7 +8,6 @@
import com.android.tools.r8.ir.conversion.CfBuilder;
import com.android.tools.r8.ir.conversion.DexBuilder;
import com.android.tools.r8.lightir.LirBuilder;
-import com.android.tools.r8.utils.ListUtils;
import java.util.List;
import java.util.ListIterator;
@@ -73,11 +72,7 @@
@Override
public String toString() {
- BasicBlock myBlock = getBlock();
- // Avoids BasicBlock.exit(), since it will assert when block is invalid.
- if (myBlock != null
- && !myBlock.getSuccessors().isEmpty()
- && ListUtils.last(myBlock.getInstructions()) == this) {
+ if (getBlock() != null && !getBlock().getSuccessors().isEmpty()) {
return super.toString() + "block " + getTarget().getNumberAsString();
}
return super.toString() + "block <unknown>";
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 9db4576..325cea8 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,
- Collection<Instruction> instructionsToAdd,
+ Instruction[] instructions,
InternalOptions options) {
return instructionIterator.addPossiblyThrowingInstructionsToPossiblyThrowingBlock(
- code, blockIterator, instructionsToAdd, options);
+ code, blockIterator, instructions, 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 c9907ee..247079e 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,9 +17,7 @@
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;
@@ -35,39 +33,12 @@
}
}
- 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,
- Collection<Instruction> instructionsToAdd,
+ Instruction[] instructions,
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 62f3374..21ab96c 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,
- Collection<Instruction> instructionsToAdd,
+ Instruction[] instructions,
InternalOptions options) {
return currentBlockIterator.addPossiblyThrowingInstructionsToPossiblyThrowingBlock(
- code, blockIterator, instructionsToAdd, options);
+ code, blockIterator, instructions, 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 f6f2204..c579123 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,
- Collection<Instruction> instructionsToAdd,
+ Instruction[] instructions,
InternalOptions options) {
throw new Unimplemented();
}
