Leverage processInvokesNeverReturningNormally() for optimizing failing null checks
Bug: 150269949
Change-Id: I27473b258413d5a56e7454e5e622217453cf1155
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 363c6a1..50cec6e 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
@@ -87,10 +87,20 @@
}
@Override
+ public boolean hasInsertionPosition() {
+ return position != null;
+ }
+
+ @Override
public void setInsertionPosition(Position position) {
this.position = position;
}
+ @Override
+ public void unsetInsertionPosition() {
+ this.position = null;
+ }
+
/**
* Adds an instruction to the block. The instruction will be added just before the current cursor
* position.
@@ -212,13 +222,15 @@
IRCode code, InternalOptions options, long value, TypeElement type) {
ConstNumber constNumberInstruction = code.createNumberConstant(value, type);
// Note that we only keep position info for throwing instructions in release mode.
- Position position;
- if (options.debug) {
- position = current != null ? current.getPosition() : block.getPosition();
- } else {
- position = Position.none();
+ if (!hasInsertionPosition()) {
+ Position position;
+ if (options.debug) {
+ position = current != null ? current.getPosition() : block.getPosition();
+ } else {
+ position = Position.none();
+ }
+ constNumberInstruction.setPosition(position);
}
- constNumberInstruction.setPosition(position);
add(constNumberInstruction);
return constNumberInstruction.outValue();
}
@@ -308,8 +320,13 @@
new DominatorTree(code, MAY_HAVE_UNREACHABLE_BLOCKS),
affectedValues));
- InstructionListIterator throwBlockInstructionIterator =
- throwBlock == block ? this : throwBlock.listIterator(code);
+ InstructionListIterator throwBlockInstructionIterator;
+ if (throwBlock == block) {
+ throwBlockInstructionIterator = this;
+ } else {
+ throwBlockInstructionIterator = throwBlock.listIterator(code);
+ throwBlockInstructionIterator.setInsertionPosition(position);
+ }
// Insert constant null before the goto instruction.
Value nullValue =
@@ -321,7 +338,9 @@
// Replace the instruction by throw.
Throw throwInstruction = new Throw(nullValue);
- if (toBeReplaced.getPosition().isSome()) {
+ if (hasInsertionPosition()) {
+ throwInstruction.setPosition(position);
+ } else if (toBeReplaced.getPosition().isSome()) {
throwInstruction.setPosition(toBeReplaced.getPosition());
} else {
// The instruction that is being removed cannot throw, and thus it must be unreachable as we
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 47ce330..bec911e 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
@@ -152,11 +152,6 @@
}
@Override
- public void setInsertionPosition(Position position) {
- instructionIterator.setInsertionPosition(position);
- }
-
- @Override
public void replaceCurrentInstruction(Instruction newInstruction, Set<Value> affectedValues) {
instructionIterator.replaceCurrentInstruction(newInstruction, affectedValues);
}
@@ -165,4 +160,19 @@
public void removeOrReplaceByDebugLocalRead() {
instructionIterator.removeOrReplaceByDebugLocalRead();
}
+
+ @Override
+ public boolean hasInsertionPosition() {
+ return instructionIterator.hasInsertionPosition();
+ }
+
+ @Override
+ public void setInsertionPosition(Position position) {
+ instructionIterator.setInsertionPosition(position);
+ }
+
+ @Override
+ public void unsetInsertionPosition() {
+ instructionIterator.unsetInsertionPosition();
+ }
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/InstanceFieldInstruction.java b/src/main/java/com/android/tools/r8/ir/code/InstanceFieldInstruction.java
index 9077c97..faebe44 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InstanceFieldInstruction.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InstanceFieldInstruction.java
@@ -16,6 +16,9 @@
Value object();
+ boolean instructionInstanceCanThrow(
+ AppView<?> appView, ProgramMethod context, SideEffectAssumption assumption);
+
boolean instructionMayHaveSideEffects(
AppView<?> appView, ProgramMethod context, SideEffectAssumption assumption);
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 e4e96af..ee7d57e 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
@@ -1461,17 +1461,70 @@
return false;
}
- public enum SideEffectAssumption {
- NONE,
- CLASS_ALREADY_INITIALIZED,
- RECEIVER_NOT_NULL;
+ public static class SideEffectAssumption {
- boolean canAssumeClassIsAlreadyInitialized() {
- return this == CLASS_ALREADY_INITIALIZED;
+ public static final SideEffectAssumption NONE = new SideEffectAssumption();
+
+ public static final SideEffectAssumption CLASS_ALREADY_INITIALIZED =
+ new SideEffectAssumption() {
+
+ @Override
+ public boolean canAssumeClassIsAlreadyInitialized() {
+ return true;
+ }
+ };
+
+ public static final SideEffectAssumption INVOKED_METHOD_DOES_NOT_HAVE_SIDE_EFFECTS =
+ new SideEffectAssumption() {
+
+ @Override
+ public boolean canAssumeInvokedMethodDoesNotHaveSideEffects() {
+ return true;
+ }
+ };
+
+ public static final SideEffectAssumption RECEIVER_NOT_NULL =
+ new SideEffectAssumption() {
+
+ @Override
+ public boolean canAssumeReceiverIsNotNull() {
+ return true;
+ }
+ };
+
+ public boolean canAssumeClassIsAlreadyInitialized() {
+ return false;
}
- boolean canAssumeReceiverIsNotNull() {
- return this == RECEIVER_NOT_NULL;
+ public boolean canAssumeInvokedMethodDoesNotHaveSideEffects() {
+ return false;
+ }
+
+ public boolean canAssumeReceiverIsNotNull() {
+ return false;
+ }
+
+ public SideEffectAssumption join(SideEffectAssumption other) {
+ return new SideEffectAssumption() {
+
+ @Override
+ public boolean canAssumeClassIsAlreadyInitialized() {
+ return SideEffectAssumption.this.canAssumeClassIsAlreadyInitialized()
+ || other.canAssumeClassIsAlreadyInitialized();
+ }
+
+ @Override
+ public boolean canAssumeInvokedMethodDoesNotHaveSideEffects() {
+ return SideEffectAssumption.this.canAssumeInvokedMethodDoesNotHaveSideEffects()
+ || other.canAssumeInvokedMethodDoesNotHaveSideEffects();
+ }
+
+ @Override
+ public boolean canAssumeReceiverIsNotNull() {
+ return SideEffectAssumption.this.canAssumeInvokedMethodDoesNotHaveSideEffects()
+ || other.canAssumeInvokedMethodDoesNotHaveSideEffects();
+ }
+ };
}
}
}
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 d83aa64..2790e66 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
@@ -58,10 +58,18 @@
*/
void removeOrReplaceByDebugLocalRead();
+ default boolean hasInsertionPosition() {
+ return false;
+ }
+
default void setInsertionPosition(Position position) {
// Intentionally empty.
}
+ default void unsetInsertionPosition() {
+ // Intentionally empty.
+ }
+
default Value insertConstNullInstruction(IRCode code, InternalOptions options) {
return insertConstNumberInstruction(code, options, 0, TypeElement.getNull());
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/InvokeMethodWithReceiver.java b/src/main/java/com/android/tools/r8/ir/code/InvokeMethodWithReceiver.java
index 877d359..79f7152 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InvokeMethodWithReceiver.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InvokeMethodWithReceiver.java
@@ -225,6 +225,10 @@
return true;
}
+ if (assumption.canAssumeInvokedMethodDoesNotHaveSideEffects()) {
+ return false;
+ }
+
DexEncodedMethod resolvedMethod = resolutionResult.getResolvedMethod();
if (appViewWithLiveness.appInfo().noSideEffects.containsKey(getInvokedMethod())
|| appViewWithLiveness.appInfo().noSideEffects.containsKey(resolvedMethod.getReference())) {
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
index 6634df6..4c127fa 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -69,7 +69,6 @@
import com.android.tools.r8.ir.optimize.RedundantFieldLoadElimination;
import com.android.tools.r8.ir.optimize.ReflectionOptimizer;
import com.android.tools.r8.ir.optimize.ServiceLoaderRewriter;
-import com.android.tools.r8.ir.optimize.UninstantiatedTypeOptimization;
import com.android.tools.r8.ir.optimize.classinliner.ClassInliner;
import com.android.tools.r8.ir.optimize.enums.EnumUnboxer;
import com.android.tools.r8.ir.optimize.enums.EnumValueOptimizer;
@@ -159,7 +158,6 @@
private final Devirtualizer devirtualizer;
private final CovariantReturnTypeAnnotationTransformer covariantReturnTypeAnnotationTransformer;
private final StringSwitchRemover stringSwitchRemover;
- private final UninstantiatedTypeOptimization uninstantiatedTypeOptimization;
private final TypeChecker typeChecker;
private final DesugaredLibraryAPIConverter desugaredLibraryAPIConverter;
private final ServiceLoaderRewriter serviceLoaderRewriter;
@@ -254,7 +252,6 @@
this.lensCodeRewriter = null;
this.identifierNameStringMarker = null;
this.devirtualizer = null;
- this.uninstantiatedTypeOptimization = null;
this.typeChecker = null;
this.d8NestBasedAccessDesugaring = null;
this.stringSwitchRemover = null;
@@ -320,10 +317,6 @@
}
this.devirtualizer =
options.enableDevirtualization ? new Devirtualizer(appViewWithLiveness) : null;
- this.uninstantiatedTypeOptimization =
- options.enableUninstantiatedTypeOptimization
- ? new UninstantiatedTypeOptimization(appViewWithLiveness)
- : null;
this.typeChecker = new TypeChecker(appViewWithLiveness);
this.d8NestBasedAccessDesugaring = null;
this.serviceLoaderRewriter =
@@ -351,7 +344,6 @@
this.lensCodeRewriter = null;
this.identifierNameStringMarker = null;
this.devirtualizer = null;
- this.uninstantiatedTypeOptimization = null;
this.typeChecker = null;
this.d8NestBasedAccessDesugaring =
options.shouldDesugarNests() ? new D8NestBasedAccessDesugaring(appView) : null;
@@ -1318,14 +1310,6 @@
assert code.verifyTypes(appView);
- if (uninstantiatedTypeOptimization != null) {
- timing.begin("Rewrite uninstantiated types");
- uninstantiatedTypeOptimization.rewrite(code);
- timing.end();
- }
-
- assert code.verifyTypes(appView);
-
timing.begin("Remove trivial type checks/casts");
codeRewriter.removeTrivialCheckCastAndInstanceOfInstructions(code);
timing.end();
@@ -1367,8 +1351,8 @@
timing.begin("Propogate sparse conditionals");
new SparseConditionalConstantPropagation(appView, code).run();
timing.end();
- timing.begin("Rewrite always throwing invokes");
- codeRewriter.processMethodsNeverReturningNormally(code);
+ timing.begin("Rewrite always throwing instructions");
+ codeRewriter.optimizeAlwaysThrowingInstructions(code);
timing.end();
timing.begin("Simplify control flow");
if (codeRewriter.simplifyControlFlow(code)) {
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 def219f..c6447c1 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
@@ -51,8 +51,10 @@
import com.android.tools.r8.ir.code.IRMetadata;
import com.android.tools.r8.ir.code.If;
import com.android.tools.r8.ir.code.If.Type;
+import com.android.tools.r8.ir.code.InstanceFieldInstruction;
import com.android.tools.r8.ir.code.InstanceOf;
import com.android.tools.r8.ir.code.Instruction;
+import com.android.tools.r8.ir.code.Instruction.SideEffectAssumption;
import com.android.tools.r8.ir.code.InstructionIterator;
import com.android.tools.r8.ir.code.InstructionListIterator;
import com.android.tools.r8.ir.code.InstructionOrPhi;
@@ -60,6 +62,7 @@
import com.android.tools.r8.ir.code.Invoke;
import com.android.tools.r8.ir.code.InvokeDirect;
import com.android.tools.r8.ir.code.InvokeMethod;
+import com.android.tools.r8.ir.code.InvokeMethodWithReceiver;
import com.android.tools.r8.ir.code.InvokeNewArray;
import com.android.tools.r8.ir.code.InvokeStatic;
import com.android.tools.r8.ir.code.InvokeVirtual;
@@ -78,7 +81,9 @@
import com.android.tools.r8.ir.code.Xor;
import com.android.tools.r8.ir.conversion.IRConverter;
import com.android.tools.r8.ir.optimize.controlflow.SwitchCaseAnalyzer;
+import com.android.tools.r8.ir.optimize.info.MethodOptimizationInfo;
import com.android.tools.r8.ir.regalloc.LinearScanRegisterAllocator;
+import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.BooleanUtils;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.InternalOutputMode;
@@ -115,6 +120,7 @@
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.BitSet;
import java.util.Collection;
import java.util.Deque;
import java.util.HashMap;
@@ -2809,69 +2815,94 @@
return changed;
}
- // Find all method invocations that never returns normally, split the block
- // after each such invoke instruction and follow it with a block throwing a
- // null value (which should result in NPE). Note that this throw is not
+ // Find all instructions that always throw, split the block after each such instruction and follow
+ // it with a block throwing a null value (which should result in NPE). Note that this throw is not
// expected to be ever reached, but is intended to satisfy verifier.
- public void processMethodsNeverReturningNormally(IRCode code) {
+ public void optimizeAlwaysThrowingInstructions(IRCode code) {
if (!appView.appInfo().hasLiveness()) {
return;
}
+ AppView<AppInfoWithLiveness> appViewWithLiveness = appView.withLiveness();
+ Set<Value> affectedValues = Sets.newIdentityHashSet();
+ Set<BasicBlock> blocksToRemove = Sets.newIdentityHashSet();
ListIterator<BasicBlock> blockIterator = code.listIterator();
+ ProgramMethod context = code.context();
while (blockIterator.hasNext()) {
BasicBlock block = blockIterator.next();
if (block.getNumber() != 0 && block.getPredecessors().isEmpty()) {
continue;
}
- InstructionListIterator insnIterator = block.listIterator(code);
- while (insnIterator.hasNext()) {
- Instruction insn = insnIterator.next();
- if (!insn.isInvokeMethod()) {
+ if (blocksToRemove.contains(block)) {
+ continue;
+ }
+ InstructionListIterator instructionIterator = block.listIterator(code);
+ while (instructionIterator.hasNext()) {
+ Instruction instruction = instructionIterator.next();
+ if (instruction.throwsOnNullInput()) {
+ Value inValue = instruction.getNonNullInput();
+ if (inValue.isAlwaysNull(appView)) {
+ // Insert `throw null` after the instruction if it is not guaranteed to throw an NPE.
+ if (instruction.isInstanceFieldInstruction()) {
+ InstanceFieldInstruction instanceFieldInstruction =
+ instruction.asInstanceFieldInstruction();
+ if (instanceFieldInstruction.instructionInstanceCanThrow(
+ appView, context, SideEffectAssumption.RECEIVER_NOT_NULL)) {
+ instructionIterator.next();
+ }
+ } else if (instruction.isInvokeMethodWithReceiver()) {
+ InvokeMethodWithReceiver invoke = instruction.asInvokeMethodWithReceiver();
+ SideEffectAssumption assumption =
+ SideEffectAssumption.RECEIVER_NOT_NULL.join(
+ SideEffectAssumption.INVOKED_METHOD_DOES_NOT_HAVE_SIDE_EFFECTS);
+ if (invoke.instructionMayHaveSideEffects(appView, context, assumption)) {
+ instructionIterator.next();
+ }
+ }
+ instructionIterator.replaceCurrentInstructionWithThrowNull(
+ appViewWithLiveness, code, blockIterator, blocksToRemove, affectedValues);
+ continue;
+ }
+ }
+
+ if (!instruction.isInvokeMethod()) {
continue;
}
- InvokeMethod invoke = insn.asInvokeMethod();
+ InvokeMethod invoke = instruction.asInvokeMethod();
DexEncodedMethod singleTarget =
invoke.lookupSingleTarget(appView.withLiveness(), code.context());
- if (singleTarget == null || !singleTarget.getOptimizationInfo().neverReturnsNormally()) {
+ if (singleTarget == null) {
continue;
}
- // Split the block.
- {
- BasicBlock newBlock = insnIterator.split(code, blockIterator);
- assert !insnIterator.hasNext(); // must be pointing *after* inserted GoTo.
- // Move block iterator back so current block is 'newBlock'.
- blockIterator.previous();
+ MethodOptimizationInfo optimizationInfo = singleTarget.getOptimizationInfo();
- newBlock.unlinkSinglePredecessorSiblingsAllowed();
+ // If the invoke instruction is a null check, we can remove it.
+ boolean isNullCheck = false;
+ if (optimizationInfo.hasNonNullParamOrThrow()) {
+ BitSet nonNullParamOrThrow = optimizationInfo.getNonNullParamOrThrow();
+ for (int i = 0; i < invoke.arguments().size(); i++) {
+ Value argument = invoke.arguments().get(i);
+ if (argument.isAlwaysNull(appView) && nonNullParamOrThrow.get(i)) {
+ isNullCheck = true;
+ break;
+ }
+ }
}
-
- // We want to follow the invoke instruction with 'throw null', which should
- // be unreachable but is needed to satisfy the verifier. Note that we have
- // to put 'throw null' into a separate block to make sure we don't get two
- // throwing instructions in the block having catch handler. This new block
- // does not need catch handlers.
- Instruction gotoInsn = insnIterator.previous();
- assert gotoInsn.isGoto();
- assert insnIterator.hasNext();
- BasicBlock throwNullBlock = insnIterator.split(code, blockIterator);
- InstructionListIterator throwNullInsnIterator = throwNullBlock.listIterator(code);
-
- // Insert 'null' constant.
- ConstNumber nullConstant = code.createConstNull(gotoInsn.getLocalInfo());
- nullConstant.setPosition(invoke.getPosition());
- throwNullInsnIterator.add(nullConstant);
-
- // Replace Goto with Throw.
- Throw notReachableThrow = new Throw(nullConstant.outValue());
- Instruction insnGoto = throwNullInsnIterator.next();
- assert insnGoto.isGoto();
- throwNullInsnIterator.replaceCurrentInstruction(notReachableThrow);
+ // If the invoke instruction never returns normally, we can insert a throw null instruction
+ // after the invoke.
+ if (isNullCheck || optimizationInfo.neverReturnsNormally()) {
+ instructionIterator.setInsertionPosition(invoke.getPosition());
+ instructionIterator.next();
+ instructionIterator.replaceCurrentInstructionWithThrowNull(
+ appViewWithLiveness, code, blockIterator, blocksToRemove, affectedValues);
+ instructionIterator.unsetInsertionPosition();
+ }
}
}
- Set<Value> affectedValues = code.removeUnreachableBlocks();
+ code.removeBlocks(blocksToRemove);
+ assert code.getUnreachableBlocks().isEmpty();
if (!affectedValues.isEmpty()) {
new TypeAnalysis(appView).narrowing(affectedValues);
}
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 cb1eec7..6a8b6a5 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
@@ -7,7 +7,6 @@
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;
-import com.android.tools.r8.graph.AppInfoWithClassHierarchy;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexItemFactory;
@@ -18,21 +17,11 @@
import com.android.tools.r8.graph.FieldAccessInfo;
import com.android.tools.r8.graph.FieldAccessInfoCollection;
import com.android.tools.r8.graph.GraphLense.NestedGraphLense;
-import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.graph.RewrittenPrototypeDescription;
import com.android.tools.r8.graph.RewrittenPrototypeDescription.ArgumentInfoCollection;
import com.android.tools.r8.graph.RewrittenPrototypeDescription.RemovedArgumentInfo;
import com.android.tools.r8.graph.TopDownClassHierarchyTraversal;
-import com.android.tools.r8.ir.analysis.type.TypeAnalysis;
import com.android.tools.r8.ir.analysis.value.AbstractValue;
-import com.android.tools.r8.ir.code.BasicBlock;
-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.Instruction.SideEffectAssumption;
-import com.android.tools.r8.ir.code.InstructionListIterator;
-import com.android.tools.r8.ir.code.InvokeMethod;
-import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.ir.optimize.MemberPoolCollection.MemberPool;
import com.android.tools.r8.ir.optimize.info.OptimizationFeedback;
import com.android.tools.r8.ir.optimize.info.OptimizationFeedbackSimple;
@@ -45,11 +34,9 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
-import java.util.BitSet;
import java.util.HashMap;
import java.util.HashSet;
import java.util.IdentityHashMap;
-import java.util.ListIterator;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ExecutorService;
@@ -356,95 +343,4 @@
return dexItemFactory.createMethod(method.holder, newProto, method.name);
}
-
- public void rewrite(IRCode code) {
- Set<BasicBlock> blocksToBeRemoved = Sets.newIdentityHashSet();
- ListIterator<BasicBlock> blockIterator = code.listIterator();
- Set<Value> affectedValues = Sets.newIdentityHashSet();
- while (blockIterator.hasNext()) {
- BasicBlock block = blockIterator.next();
- if (blocksToBeRemoved.contains(block)) {
- continue;
- }
- InstructionListIterator instructionIterator = block.listIterator(code);
- while (instructionIterator.hasNext()) {
- Instruction instruction = instructionIterator.next();
- if (instruction.throwsOnNullInput()) {
- Value couldBeNullValue = instruction.getNonNullInput();
- if (isThrowNullCandidate(couldBeNullValue, instruction, appView, code.context())) {
- instructionIterator.replaceCurrentInstructionWithThrowNull(
- appView, code, blockIterator, blocksToBeRemoved, affectedValues);
- continue;
- }
- }
- if (instruction.isInvokeMethod()) {
- rewriteInvoke(
- instruction.asInvokeMethod(),
- blockIterator,
- instructionIterator,
- code,
- affectedValues,
- blocksToBeRemoved);
- }
- }
- }
- code.removeBlocks(blocksToBeRemoved);
- code.removeAllDeadAndTrivialPhis(affectedValues);
- code.removeUnreachableBlocks();
- if (!affectedValues.isEmpty()) {
- new TypeAnalysis(appView).narrowing(affectedValues);
- }
- assert code.isConsistentSSA();
- }
-
- private static boolean isThrowNullCandidate(
- Value couldBeNullValue,
- Instruction current,
- AppView<? extends AppInfoWithClassHierarchy> appView,
- ProgramMethod context) {
- if (!couldBeNullValue.isAlwaysNull(appView)) {
- return false;
- }
- if (current.isFieldInstruction()) {
- // Other resolution-related errors come first.
- FieldInstruction fieldInstruction = current.asFieldInstruction();
- // We can't replace the current instruction with `throw null` if it may throw another
- // exception than NullPointerException.
- if (fieldInstruction.instructionInstanceCanThrow(
- appView, context, SideEffectAssumption.RECEIVER_NOT_NULL)) {
- return false;
- }
- }
- return true;
- }
-
- // invoke instructions with a null receiver has already been rewritten to `throw null`.
- // At this point, we attempt to explore non-null-param-or-throw optimization info and replace
- // the invocation with `throw null` if an argument is known to be null and the method is going to
- // throw for that null argument.
- private void rewriteInvoke(
- InvokeMethod invoke,
- ListIterator<BasicBlock> blockIterator,
- InstructionListIterator instructionIterator,
- IRCode code,
- Set<Value> affectedValues,
- Set<BasicBlock> blocksToBeRemoved) {
- DexEncodedMethod target = invoke.lookupSingleTarget(appView, code.context());
- if (target == null) {
- return;
- }
-
- BitSet facts = target.getOptimizationInfo().getNonNullParamOrThrow();
- if (facts != null) {
- for (int i = 0; i < invoke.arguments().size(); i++) {
- Value argument = invoke.arguments().get(i);
- if (argument.isAlwaysNull(appView) && facts.get(i)) {
- instructionIterator.next();
- instructionIterator.replaceCurrentInstructionWithThrowNull(
- appView, code, blockIterator, blocksToBeRemoved, affectedValues);
- return;
- }
- }
- }
- }
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfo.java b/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfo.java
index 1b79eb8..f919a1e 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfo.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfo.java
@@ -44,6 +44,10 @@
public abstract ParameterUsage getParameterUsages(int parameter);
+ public final boolean hasNonNullParamOrThrow() {
+ return getNonNullParamOrThrow() != null;
+ }
+
public abstract BitSet getNonNullParamOrThrow();
public abstract BitSet getNonNullParamOnNormalExits();
diff --git a/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java b/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java
index 6326718..34fc2fc 100644
--- a/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java
+++ b/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java
@@ -513,9 +513,6 @@
// Addition of checks for super-class-initialization cause this to abort on non-ToT art.
"008-exceptions",
- // Fails due to non-matching Exception messages.
- "201-built-in-except-detail-messages",
-
// Generally fails on non-R8/D8 running.
"156-register-dex-file-multi-loader",
"412-new-array",
@@ -525,9 +522,6 @@
// Addition of checks for super-class-initialization cause this to abort on non-ToT art.
"008-exceptions",
- // Fails due to non-matching Exception messages.
- "201-built-in-except-detail-messages",
-
// Generally fails on non-R8/D8 running.
"004-checker-UnsafeTest18",
"005-annotations",
@@ -544,9 +538,6 @@
// Addition of checks for super-class-initialization cause this to abort on non-ToT art.
"008-exceptions",
- // Fails due to non-matching Exception messages.
- "201-built-in-except-detail-messages",
-
// Generally fails on non R8/D8 running.
"004-checker-UnsafeTest18",
"004-NativeAllocations",
@@ -567,9 +558,6 @@
// Addition of checks for super-class-initialization cause this to abort on non-ToT art.
"008-exceptions",
- // Fails due to non-matching Exception messages.
- "201-built-in-except-detail-messages",
-
// Generally fails on non R8/D8 running.
"004-checker-UnsafeTest18",
"004-NativeAllocations",
@@ -590,9 +578,6 @@
// Addition of checks for super-class-initialization cause this to abort on non-ToT art.
"008-exceptions",
- // Fails due to non-matching Exception messages.
- "201-built-in-except-detail-messages",
-
// Generally fails on non R8/D8 running.
"004-checker-UnsafeTest18",
"004-NativeAllocations",
@@ -821,6 +806,11 @@
.put("138-duplicate-classes-check", TestCondition.any())
// Array index out of bounds exception.
.put("150-loadlibrary", TestCondition.any())
+ // Fails due to non-matching Exception messages.
+ .put(
+ "201-built-in-except-detail-messages",
+ TestCondition.match(
+ TestCondition.compilers(CompilerUnderTest.R8, CompilerUnderTest.R8_AFTER_D8)))
// Uses dex file version 37 and therefore only runs on Android N and above.
.put(
"370-dex-v37",
@@ -1170,13 +1160,6 @@
"435-new-instance"
);
- private static List<String> requireUninstantiatedTypeOptimizationToBeDisabled = ImmutableList.of(
- // This test inspects the message of the exception that is thrown when calling a virtual
- // method with a null-receiver. This message changes when the invocation is rewritten to
- // "throw null".
- "201-built-in-except-detail-messages"
- );
-
private static List<String> hasMissingClasses = ImmutableList.of(
"091-override-package-private-method",
"003-omnibus-opcodes",
@@ -1279,8 +1262,6 @@
private final boolean disableInlining;
// Whether to disable class inlining
private final boolean disableClassInlining;
- // Whether to disable the uninitialized type optimization.
- private final boolean disableUninstantiatedTypeOptimization;
// Has missing classes.
private final boolean hasMissingClasses;
// Explicitly disable desugaring.
@@ -1304,7 +1285,6 @@
boolean outputMayDiffer,
boolean disableInlining,
boolean disableClassInlining,
- boolean disableUninstantiatedTypeOptimization,
boolean hasMissingClasses,
boolean disableDesugaring,
List<String> keepRules,
@@ -1323,7 +1303,6 @@
this.outputMayDiffer = outputMayDiffer;
this.disableInlining = disableInlining;
this.disableClassInlining = disableClassInlining;
- this.disableUninstantiatedTypeOptimization = disableUninstantiatedTypeOptimization;
this.hasMissingClasses = hasMissingClasses;
this.disableDesugaring = disableDesugaring;
this.keepRules = keepRules;
@@ -1354,7 +1333,6 @@
disableInlining,
true, // Disable class inlining for JCTF tests.
false,
- false,
true, // Disable desugaring for JCTF tests.
ImmutableList.of(),
null);
@@ -1383,7 +1361,6 @@
disableInlining,
true, // Disable class inlining for JCTF tests.
false,
- false,
true, // Disable desugaring for JCTF tests.
ImmutableList.of(),
null);
@@ -1548,7 +1525,6 @@
outputMayDiffer.contains(name),
requireInliningToBeDisabled.contains(name),
requireClassInliningToBeDisabled.contains(name),
- requireUninstantiatedTypeOptimizationToBeDisabled.contains(name),
hasMissingClasses.contains(name),
false,
keepRules.getOrDefault(name, ImmutableList.of()),
@@ -1619,7 +1595,6 @@
private final boolean disableInlining;
private final boolean disableClassInlining;
- private final boolean disableUninstantiatedTypeOptimization;
private final boolean hasMissingClasses;
private final boolean disableDesugaring;
private final List<String> keepRules;
@@ -1628,7 +1603,6 @@
private CompilationOptions(TestSpecification spec) {
this.disableInlining = spec.disableInlining;
this.disableClassInlining = spec.disableClassInlining;
- this.disableUninstantiatedTypeOptimization = spec.disableUninstantiatedTypeOptimization;
this.hasMissingClasses = spec.hasMissingClasses;
this.disableDesugaring = spec.disableDesugaring;
this.keepRules = spec.keepRules;
@@ -1643,9 +1617,6 @@
if (disableClassInlining) {
options.enableClassInlining = false;
}
- if (disableUninstantiatedTypeOptimization) {
- options.enableUninstantiatedTypeOptimization = false;
- }
// Some tests actually rely on missing classes for what they test.
options.ignoreMissingClasses = hasMissingClasses;
if (configuration != null) {
@@ -1664,7 +1635,6 @@
CompilationOptions options = (CompilationOptions) o;
return disableInlining == options.disableInlining
&& disableClassInlining == options.disableClassInlining
- && disableUninstantiatedTypeOptimization == options.disableUninstantiatedTypeOptimization
&& hasMissingClasses == options.hasMissingClasses;
}
@@ -1673,7 +1643,6 @@
return Objects.hash(
disableInlining,
disableClassInlining,
- disableUninstantiatedTypeOptimization,
hasMissingClasses);
}
}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/AlwaysThrowNullTest.java b/src/test/java/com/android/tools/r8/ir/optimize/AlwaysThrowNullTest.java
index a46524a..3ce1ee1 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/AlwaysThrowNullTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/AlwaysThrowNullTest.java
@@ -166,7 +166,7 @@
@Parameterized.Parameters(name = "{0}")
public static TestParametersCollection data() {
- return getTestParameters().withAllRuntimes().build();
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
}
private final TestParameters parameters;
@@ -228,7 +228,7 @@
testForD8()
.release()
.addProgramClassesAndInnerClasses(MAIN)
- .setMinApi(parameters.getRuntime())
+ .setMinApi(parameters.getApiLevel())
.run(parameters.getRuntime(), MAIN)
.assertSuccessWithOutput(JAVA_OUTPUT);
test(result, false);
@@ -244,10 +244,9 @@
.enableMemberValuePropagationAnnotations()
.addKeepMainRule(MAIN)
.noMinification()
- .setMinApi(parameters.getRuntime())
+ .setMinApi(parameters.getApiLevel())
.run(parameters.getRuntime(), MAIN)
.assertSuccessWithOutput(JAVA_OUTPUT);
test(result, true);
}
-
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java
index c5ca6c9..bfcda02 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java
@@ -50,6 +50,10 @@
boolean isConstNull();
+ default boolean isConstString() {
+ return isConstString(JumboStringMode.ALLOW);
+ }
+
boolean isConstString(JumboStringMode jumboStringMode);
boolean isConstString(String value, JumboStringMode jumboStringMode);
