Refactor util to add null throwing code.
Bug: 72443802, 124246610
Change-Id: I4dbc555f8399f60dd45cc4b9a631f8a0d72f0ded
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 5f10bd1..9aa0cba 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
@@ -7,6 +7,7 @@
import static com.android.tools.r8.ir.code.DominatorTree.Assumption.MAY_HAVE_UNREACHABLE_BLOCKS;
import com.android.tools.r8.graph.AppInfo;
+import com.android.tools.r8.graph.AppInfoWithSubtyping;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.ir.analysis.type.TypeAnalysis;
@@ -182,6 +183,68 @@
}
@Override
+ public void replaceCurrentInstructionWithThrowNull(
+ AppView<? extends AppInfoWithSubtyping> appView,
+ IRCode code,
+ ListIterator<BasicBlock> blockIterator,
+ Set<BasicBlock> blocksToRemove) {
+ if (current == null) {
+ throw new IllegalStateException();
+ }
+ BasicBlock block = current.getBlock();
+ assert !blocksToRemove.contains(block);
+
+ BasicBlock normalSuccessorBlock = split(code, blockIterator);
+ previous();
+
+ // Unlink all blocks that are dominated by successor.
+ {
+ DominatorTree dominatorTree = new DominatorTree(code, MAY_HAVE_UNREACHABLE_BLOCKS);
+ blocksToRemove.addAll(block.unlink(normalSuccessorBlock, dominatorTree));
+ }
+
+ // Insert constant null before the instruction.
+ previous();
+ ConstNumber constNumberInstruction = code.createConstNull();
+ // Note that we only keep position info for throwing instructions in release mode.
+ constNumberInstruction.setPosition(
+ appView.options().debug ? current.getPosition() : Position.none());
+ add(constNumberInstruction);
+ next();
+
+ // Replace the instruction by throw.
+ Throw throwInstruction = new Throw(constNumberInstruction.outValue());
+ for (Value inValue : current.inValues()) {
+ if (inValue.hasLocalInfo()) {
+ // Add this value as a debug value to avoid changing its live range.
+ throwInstruction.addDebugValue(inValue);
+ }
+ }
+ replaceCurrentInstruction(throwInstruction);
+ next();
+ remove();
+
+ // Remove all catch handlers where the guard does not include NullPointerException.
+ if (block.hasCatchHandlers()) {
+ CatchHandlers<BasicBlock> catchHandlers = block.getCatchHandlers();
+ catchHandlers.forEach(
+ (guard, target) -> {
+ if (blocksToRemove.contains(target)) {
+ // Already removed previously. This may happen if two catch handlers have the same
+ // target.
+ return;
+ }
+ if (!appView.appInfo().isSubtype(appView.dexItemFactory().npeType, guard)) {
+ // TODO(christofferqa): Consider updating previous dominator tree instead of
+ // rebuilding it from scratch.
+ DominatorTree dominatorTree = new DominatorTree(code, MAY_HAVE_UNREACHABLE_BLOCKS);
+ blocksToRemove.addAll(block.unlink(target, dominatorTree));
+ }
+ });
+ }
+ }
+
+ @Override
public BasicBlock split(IRCode code, ListIterator<BasicBlock> blocksIterator) {
List<BasicBlock> blocks = code.blocks;
assert blocksIterator == null || IteratorUtils.peekPrevious(blocksIterator) == block;
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 0a1bf67..8186ca2 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
@@ -5,11 +5,13 @@
package com.android.tools.r8.ir.code;
import com.android.tools.r8.graph.AppInfo;
+import com.android.tools.r8.graph.AppInfoWithSubtyping;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexType;
import java.util.ArrayList;
import java.util.List;
import java.util.ListIterator;
+import java.util.Set;
public interface InstructionListIterator
extends InstructionIterator, PreviousUntilIterator<Instruction> {
@@ -49,6 +51,25 @@
}
/**
+ * Replace the current instruction with null throwing instructions.
+ *
+ * @param appView with subtype info through which we can test if the guard is subtype of NPE.
+ * @param code the IR code for the block this iterator originates from.
+ * @param blockIterator basic block iterator used to iterate the blocks.
+ * @param blocksToRemove set passed where blocks that were detached from the graph, but not
+ * removed yet are added. When inserting `throw null`, catch handlers whose guard does not
+ * catch NPE will be removed, but not yet removed using the passed block
+ * <code>blockIterator</code>. When iterating using <code>blockIterator</code> after then
+ * method returns the blocks in this set must be skipped when iterating with the active
+ * <code>blockIterator</code> and ultimately removed.
+ */
+ void replaceCurrentInstructionWithThrowNull(
+ AppView<? extends AppInfoWithSubtyping> appView,
+ IRCode code,
+ ListIterator<BasicBlock> blockIterator,
+ Set<BasicBlock> blocksToRemove);
+
+ /**
* Split the block into two blocks at the point of the {@link ListIterator} cursor. The existing
* block will have all the instructions before the cursor, and the new block all the
* instructions after the cursor.
@@ -65,7 +86,6 @@
*/
BasicBlock split(IRCode code, ListIterator<BasicBlock> blockIterator);
-
default BasicBlock split(IRCode code) {
return split(code, null);
}
@@ -117,9 +137,9 @@
* @param blockIterator basic block iterator used to iterate the blocks. This must be positioned
* just after the block for which this is the instruction iterator. After this method returns
* it will be positioned just after the basic block returned.
- * @param blocksToRemove list passed where blocks that where detached from the graph, but not
+ * @param blocksToRemove list passed where blocks that were detached from the graph, but not
* removed are added. When inlining an inlinee that always throws blocks in the <code>code
- * </code> can be detached, and not simply removed unsing the passed <code>blockIterator
+ * </code> can be detached, and not simply removed using the passed <code>blockIterator
* </code>. When iterating using <code>blockIterator</code> after then method returns the
* blocks in this list must be skipped when iterating with the active <code>blockIterator
* </code> and ultimately removed.
diff --git a/src/main/java/com/android/tools/r8/ir/code/LinearFlowInstructionIterator.java b/src/main/java/com/android/tools/r8/ir/code/LinearFlowInstructionIterator.java
index 01c66a8..12de1ce 100644
--- a/src/main/java/com/android/tools/r8/ir/code/LinearFlowInstructionIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/LinearFlowInstructionIterator.java
@@ -5,10 +5,12 @@
package com.android.tools.r8.ir.code;
import com.android.tools.r8.graph.AppInfo;
+import com.android.tools.r8.graph.AppInfoWithSubtyping;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexType;
import java.util.List;
import java.util.ListIterator;
+import java.util.Set;
public class LinearFlowInstructionIterator implements InstructionIterator, InstructionListIterator {
@@ -36,6 +38,16 @@
}
@Override
+ public void replaceCurrentInstructionWithThrowNull(
+ AppView<? extends AppInfoWithSubtyping> appView,
+ IRCode code,
+ ListIterator<BasicBlock> blockIterator,
+ Set<BasicBlock> blocksToRemove) {
+ currentBlockIterator.replaceCurrentInstructionWithThrowNull(
+ appView, code, blockIterator, blocksToRemove);
+ }
+
+ @Override
public BasicBlock split(IRCode code, ListIterator<BasicBlock> blockIterator) {
return currentBlockIterator.split(code, blockIterator);
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java b/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java
index 7ca48ed..31ae148 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java
@@ -4,7 +4,6 @@
package com.android.tools.r8.ir.optimize;
-import static com.android.tools.r8.ir.code.DominatorTree.Assumption.MAY_HAVE_UNREACHABLE_BLOCKS;
import static com.android.tools.r8.ir.optimize.UninstantiatedTypeOptimization.Strategy.ALLOW_ARGUMENT_REMOVAL;
import static com.android.tools.r8.ir.optimize.UninstantiatedTypeOptimization.Strategy.DISALLOW_ARGUMENT_REMOVAL;
@@ -27,16 +26,11 @@
import com.android.tools.r8.ir.analysis.type.Nullability;
import com.android.tools.r8.ir.analysis.type.TypeLatticeElement;
import com.android.tools.r8.ir.code.BasicBlock;
-import com.android.tools.r8.ir.code.CatchHandlers;
-import com.android.tools.r8.ir.code.ConstNumber;
-import com.android.tools.r8.ir.code.DominatorTree;
import com.android.tools.r8.ir.code.FieldInstruction;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.InstructionListIterator;
import com.android.tools.r8.ir.code.InvokeMethod;
-import com.android.tools.r8.ir.code.Position;
-import com.android.tools.r8.ir.code.Throw;
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.ir.optimize.MemberPoolCollection.MemberPool;
import com.android.tools.r8.logging.Log;
@@ -425,8 +419,8 @@
// would lead to an IncompatibleClassChangeError (see MemberResolutionTest#lookupStaticField-
// WithFieldGetFromNullReferenceDirectly).
if (!receiver.getTypeLattice().isDefinitelyNull()) {
- replaceCurrentInstructionWithThrowNull(
- instruction, blockIterator, instructionIterator, code, blocksToBeRemoved);
+ instructionIterator.replaceCurrentInstructionWithThrowNull(
+ appView, code, blockIterator, blocksToBeRemoved);
++numberOfInstanceGetOrInstancePutWithNullReceiver;
replacedByThrowNull = true;
}
@@ -512,8 +506,8 @@
if (invoke.isInvokeMethodWithReceiver()) {
Value receiver = invoke.asInvokeMethodWithReceiver().getReceiver();
if (receiver.isAlwaysNull(appView)) {
- replaceCurrentInstructionWithThrowNull(
- invoke, blockIterator, instructionIterator, code, blocksToBeRemoved);
+ instructionIterator.replaceCurrentInstructionWithThrowNull(
+ appView, code, blockIterator, blocksToBeRemoved);
++numberOfInvokesWithNullReceiver;
return;
}
@@ -530,8 +524,8 @@
for (int i = 0; i < invoke.arguments().size(); i++) {
Value argument = invoke.arguments().get(i);
if (argument.isAlwaysNull(appView) && facts.get(i)) {
- replaceCurrentInstructionWithThrowNull(
- invoke, blockIterator, instructionIterator, code, blocksToBeRemoved);
+ instructionIterator.replaceCurrentInstructionWithThrowNull(
+ appView, code, blockIterator, blocksToBeRemoved);
++numberOfInvokesWithNullArgument;
return;
}
@@ -539,63 +533,4 @@
}
}
- private void replaceCurrentInstructionWithThrowNull(
- Instruction instruction,
- ListIterator<BasicBlock> blockIterator,
- InstructionListIterator instructionIterator,
- IRCode code,
- Set<BasicBlock> blocksToBeRemoved) {
- BasicBlock block = instruction.getBlock();
- assert !blocksToBeRemoved.contains(block);
-
- BasicBlock normalSuccessorBlock = instructionIterator.split(code, blockIterator);
- instructionIterator.previous();
-
- // Unlink all blocks that are dominated by successor.
- {
- DominatorTree dominatorTree = new DominatorTree(code, MAY_HAVE_UNREACHABLE_BLOCKS);
- blocksToBeRemoved.addAll(block.unlink(normalSuccessorBlock, dominatorTree));
- }
-
- // Insert constant null before the instruction.
- instructionIterator.previous();
- ConstNumber constNumberInstruction = code.createConstNull();
- // Note that we only keep position info for throwing instructions in release mode.
- constNumberInstruction.setPosition(
- appView.options().debug ? instruction.getPosition() : Position.none());
- instructionIterator.add(constNumberInstruction);
- instructionIterator.next();
-
- // Replace the instruction by throw.
- Throw throwInstruction = new Throw(constNumberInstruction.outValue());
- for (Value inValue : instruction.inValues()) {
- if (inValue.hasLocalInfo()) {
- // Add this value as a debug value to avoid changing its live range.
- throwInstruction.addDebugValue(inValue);
- }
- }
- instructionIterator.replaceCurrentInstruction(throwInstruction);
- instructionIterator.next();
- instructionIterator.remove();
-
- // Remove all catch handlers where the guard does not include NullPointerException.
- if (block.hasCatchHandlers()) {
- CatchHandlers<BasicBlock> catchHandlers = block.getCatchHandlers();
- catchHandlers.forEach(
- (guard, target) -> {
- if (blocksToBeRemoved.contains(target)) {
- // Already removed previously. This may happen if two catch handlers have the same
- // target.
- return;
- }
- if (!appView.appInfo().isSubtype(appView.dexItemFactory().npeType, guard)) {
- // TODO(christofferqa): Consider updating previous dominator tree instead of
- // rebuilding it from scratch.
- DominatorTree dominatorTree = new DominatorTree(code, MAY_HAVE_UNREACHABLE_BLOCKS);
- blocksToBeRemoved.addAll(block.unlink(target, dominatorTree));
- }
- });
- }
- }
-
}
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 b30a9cf..0f32501 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
@@ -7,6 +7,7 @@
import com.android.tools.r8.errors.Unimplemented;
import com.android.tools.r8.graph.AppInfo;
+import com.android.tools.r8.graph.AppInfoWithSubtyping;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexApplication;
import com.android.tools.r8.graph.DexType;
@@ -22,6 +23,8 @@
import java.util.LinkedList;
import java.util.List;
import java.util.ListIterator;
+import java.util.Set;
+
import org.junit.Test;
public class RegisterMoveSchedulerTest {
@@ -45,6 +48,15 @@
}
@Override
+ public void replaceCurrentInstructionWithThrowNull(
+ AppView<? extends AppInfoWithSubtyping> appView,
+ IRCode code,
+ ListIterator<BasicBlock> blockIterator,
+ Set<BasicBlock> blocksToRemove) {
+ throw new Unimplemented();
+ }
+
+ @Override
public boolean hasNext() {
return it.hasNext();
}
