Extend iget canonicalization to non-this reads
Fixes: b/237372251
Change-Id: I4ceb4871f12b16999bf3e47bc5d0691ed79996a5
diff --git a/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java b/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
index 432311b..550ea8b 100644
--- a/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
+++ b/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
@@ -22,6 +22,7 @@
import com.android.tools.r8.ir.conversion.IRBuilder;
import com.android.tools.r8.utils.CfgPrinter;
import com.android.tools.r8.utils.InternalOptions;
+import com.android.tools.r8.utils.IterableUtils;
import com.android.tools.r8.utils.ListUtils;
import com.android.tools.r8.utils.StringUtils;
import com.android.tools.r8.utils.StringUtils.BraceType;
@@ -53,6 +54,7 @@
import java.util.function.BiFunction;
import java.util.function.Consumer;
import java.util.function.Function;
+import java.util.function.Predicate;
/**
* Basic block abstraction.
@@ -718,6 +720,10 @@
return instructions;
}
+ public <T extends Instruction> Iterable<T> getInstructions(Predicate<Instruction> predicate) {
+ return IterableUtils.filter(getInstructions(), predicate);
+ }
+
public Iterable<Instruction> instructionsAfter(Instruction instruction) {
return () -> iterator(instruction);
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/InstructionIterator.java b/src/main/java/com/android/tools/r8/ir/code/InstructionIterator.java
index 539f0d7..04ee188 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InstructionIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InstructionIterator.java
@@ -19,6 +19,13 @@
Instruction previous();
+ default void previous(int times) {
+ while (times > 0) {
+ previous();
+ times--;
+ }
+ }
+
/**
* Peek the next instruction.
*
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java b/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java
index 7a0a731..8d8410a 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java
@@ -9,6 +9,7 @@
import static com.android.tools.r8.ir.code.Opcodes.DEX_ITEM_BASED_CONST_STRING;
import static com.android.tools.r8.ir.code.Opcodes.INSTANCE_GET;
import static com.android.tools.r8.ir.code.Opcodes.STATIC_GET;
+import static com.android.tools.r8.utils.MapUtils.ignoreKey;
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AppInfoWithClassHierarchy;
@@ -22,6 +23,7 @@
import com.android.tools.r8.ir.analysis.value.AbstractValue;
import com.android.tools.r8.ir.analysis.value.SingleFieldValue;
import com.android.tools.r8.ir.code.BasicBlock;
+import com.android.tools.r8.ir.code.BasicBlockIterator;
import com.android.tools.r8.ir.code.ConstClass;
import com.android.tools.r8.ir.code.ConstNumber;
import com.android.tools.r8.ir.code.ConstString;
@@ -32,15 +34,27 @@
import com.android.tools.r8.ir.code.InstanceGet;
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.InstructionListIterator;
+import com.android.tools.r8.ir.code.InstructionOrPhi;
+import com.android.tools.r8.ir.code.InvokeDirect;
+import com.android.tools.r8.ir.code.NewInstance;
+import com.android.tools.r8.ir.code.Phi;
+import com.android.tools.r8.ir.code.Position;
import com.android.tools.r8.ir.code.StaticGet;
import com.android.tools.r8.ir.code.Value;
+import com.android.tools.r8.utils.OptionalBool;
+import com.android.tools.r8.utils.WorkList;
+import com.google.common.collect.Sets;
import it.unimi.dsi.fastutil.Hash.Strategy;
import it.unimi.dsi.fastutil.objects.Object2ObjectLinkedOpenCustomHashMap;
import it.unimi.dsi.fastutil.objects.Object2ObjectMap;
import it.unimi.dsi.fastutil.objects.Object2ObjectSortedMap.FastSortedEntrySet;
import java.util.ArrayList;
+import java.util.Collections;
+import java.util.IdentityHashMap;
import java.util.Iterator;
import java.util.List;
+import java.util.Map;
+import java.util.Set;
/**
* Canonicalize constants.
@@ -54,7 +68,9 @@
private final CodeRewriter codeRewriter;
private final ProgramMethod context;
private final IRCode code;
- private final boolean isAccessingVolatileField;
+
+ private OptionalBool isAccessingVolatileField = OptionalBool.unknown();
+ private Set<InstanceGet> ineligibleInstanceGetInstructions;
public ConstantCanonicalizer(
AppView<?> appView, CodeRewriter codeRewriter, ProgramMethod context, IRCode code) {
@@ -62,10 +78,22 @@
this.codeRewriter = codeRewriter;
this.context = context;
this.code = code;
- this.isAccessingVolatileField = computeIsAccessingVolatileField(appView, code);
}
- private static boolean computeIsAccessingVolatileField(AppView<?> appView, IRCode code) {
+ private ConstantCanonicalizer clear() {
+ isAccessingVolatileField = OptionalBool.unknown();
+ ineligibleInstanceGetInstructions = null;
+ return this;
+ }
+
+ private boolean getOrComputeIsAccessingVolatileField() {
+ if (isAccessingVolatileField.isUnknown()) {
+ isAccessingVolatileField = OptionalBool.of(computeIsAccessingVolatileField());
+ }
+ return isAccessingVolatileField.isTrue();
+ }
+
+ private boolean computeIsAccessingVolatileField() {
if (!appView.hasClassHierarchy()) {
// Conservatively return true.
return true;
@@ -83,8 +111,48 @@
return false;
}
- public void canonicalize() {
- Object2ObjectLinkedOpenCustomHashMap<Instruction, List<Value>> valuesDefinedByConstant =
+ private Set<InstanceGet> getOrComputeIneligibleInstanceGetInstructions() {
+ if (ineligibleInstanceGetInstructions == null) {
+ ineligibleInstanceGetInstructions = computeIneligibleInstanceGetInstructions();
+ }
+ return ineligibleInstanceGetInstructions;
+ }
+
+ private Set<InstanceGet> computeIneligibleInstanceGetInstructions() {
+ Set<InstanceGet> ineligibleInstanceGetInstructions = Sets.newIdentityHashSet();
+ for (BasicBlock catchHandlerBlock : computeDirectAndIndirectCatchHandlerBlocks()) {
+ for (InstanceGet instanceGet :
+ catchHandlerBlock.<InstanceGet>getInstructions(Instruction::isInstanceGet)) {
+ // If the receiver is defined in a block with catch handlers and the definition of the
+ // receiver is not throwing (typically defined by an assume instruction or a phi), then we
+ // cant split the block and copy the catch handlers, since the canonicalized constant would
+ // then not be defined on the exceptional edge.
+ Value object = instanceGet.object();
+ if (!object.isDefinedByInstructionSatisfying(Instruction::instructionTypeCanThrow)
+ && object.getBlock().hasCatchHandlers()) {
+ ineligibleInstanceGetInstructions.add(instanceGet);
+ }
+ }
+ }
+ return ineligibleInstanceGetInstructions;
+ }
+
+ private Set<BasicBlock> computeDirectAndIndirectCatchHandlerBlocks() {
+ WorkList<BasicBlock> catchHandlerBlocks = WorkList.newIdentityWorkList();
+ for (BasicBlock block : code.getBlocks()) {
+ if (block.entry().isMoveException()) {
+ catchHandlerBlocks.addIfNotSeen(block);
+ }
+ }
+ while (catchHandlerBlocks.hasNext()) {
+ BasicBlock block = catchHandlerBlocks.next();
+ catchHandlerBlocks.addIfNotSeen(block.getSuccessors());
+ }
+ return catchHandlerBlocks.getSeenSet();
+ }
+
+ public ConstantCanonicalizer canonicalize() {
+ Object2ObjectLinkedOpenCustomHashMap<Instruction, List<Instruction>> valuesDefinedByConstant =
new Object2ObjectLinkedOpenCustomHashMap<>(
new Strategy<Instruction>() {
@@ -125,77 +193,100 @@
});
// Collect usages of constants that can be canonicalized.
- for (Instruction current : code.instructions()) {
- if (isConstantCanonicalizationCandidate(current)) {
- List<Value> oldValuesDefinedByConstant =
- valuesDefinedByConstant.computeIfAbsent(current, k -> new ArrayList<>());
- oldValuesDefinedByConstant.add(current.outValue());
+ for (Instruction instruction : code.instructions()) {
+ if (isConstantCanonicalizationCandidate(instruction)) {
+ valuesDefinedByConstant
+ .computeIfAbsent(instruction, ignoreKey(ArrayList::new))
+ .add(instruction);
}
}
if (valuesDefinedByConstant.isEmpty()) {
- return;
+ return clear();
}
// Double-check the entry block does not have catch handlers.
// Otherwise, we need to split it before moving canonicalized const-string, which may throw.
assert !code.entryBlock().hasCatchHandlers();
- FastSortedEntrySet<Instruction, List<Value>> entries =
+ FastSortedEntrySet<Instruction, List<Instruction>> entries =
valuesDefinedByConstant.object2ObjectEntrySet();
// Sort the most frequently used constant first and exclude constant use only one time, such
// as the {@code MAX_CANONICALIZED_CONSTANT} will be canonicalized into the entry block.
- Iterator<Object2ObjectMap.Entry<Instruction, List<Value>>> iterator =
+ Iterator<Object2ObjectMap.Entry<Instruction, List<Instruction>>> iterator =
entries.stream()
.filter(a -> a.getValue().size() > 1)
.sorted((a, b) -> Integer.compare(b.getValue().size(), a.getValue().size()))
.limit(MAX_CANONICALIZED_CONSTANT)
.iterator();
-
if (!iterator.hasNext()) {
- return;
+ return clear();
}
boolean shouldSimplifyIfs = false;
+
+ // Insert instructions in the entry block.
+ Map<InstructionOrPhi, List<Instruction>> pendingInsertions = new IdentityHashMap<>();
do {
- Object2ObjectMap.Entry<Instruction, List<Value>> entry = iterator.next();
+ Object2ObjectMap.Entry<Instruction, List<Instruction>> entry = iterator.next();
Instruction canonicalizedConstant = entry.getKey();
assert canonicalizedConstant.instructionTypeCanBeCanonicalized();
- Instruction newConst;
+ Instruction newInstruction;
if (canonicalizedConstant.getBlock().isEntry()) {
- newConst = canonicalizedConstant;
+ newInstruction = canonicalizedConstant;
} else {
- switch (canonicalizedConstant.opcode()) {
- case CONST_CLASS:
- newConst = ConstClass.copyOf(code, canonicalizedConstant.asConstClass());
- break;
- case CONST_NUMBER:
- newConst = ConstNumber.copyOf(code, canonicalizedConstant.asConstNumber());
- break;
- case CONST_STRING:
- newConst = ConstString.copyOf(code, canonicalizedConstant.asConstString());
- break;
- case DEX_ITEM_BASED_CONST_STRING:
- newConst =
- DexItemBasedConstString.copyOf(
- code, canonicalizedConstant.asDexItemBasedConstString());
- break;
- case INSTANCE_GET:
- newConst = InstanceGet.copyOf(code, canonicalizedConstant.asInstanceGet());
- break;
- case STATIC_GET:
- newConst = StaticGet.copyOf(code, canonicalizedConstant.asStaticGet());
- break;
- default:
- throw new Unreachable();
+ newInstruction = createMaterializingInstruction(canonicalizedConstant);
+ InstructionOrPhi insertionPoint = getInsertionPointForCanonicalizedConstant(newInstruction);
+ if (insertionPoint == null) {
+ insertCanonicalizedConstantInEntryBlock(newInstruction);
+ } else {
+ // Record that this instruction needs to be inserted at the insertion position. Note that
+ // the insertion position may later be moved if it is itself subject to canonicalization.
+ addPendingInsertion(insertionPoint, newInstruction, pendingInsertions);
}
- insertCanonicalizedConstant(newConst);
}
- for (Value outValue : entry.getValue()) {
- outValue.replaceUsers(newConst.outValue());
+ // Remove the canonicalized instructions.
+ for (Instruction oldInstruction : entry.getValue()) {
+ if (oldInstruction != newInstruction) {
+ oldInstruction.outValue().replaceUsers(newInstruction.outValue());
+ oldInstruction
+ .getBlock()
+ .listIterator(code, oldInstruction)
+ .removeOrReplaceByDebugLocalRead();
+
+ // If the removed instruction is an insertion point for another constant, then record that
+ // the constant should instead be inserted at the point where the removed instruction has
+ // been moved to.
+ for (Instruction pendingInsertion :
+ removePendingInsertions(oldInstruction, pendingInsertions)) {
+ addPendingInsertion(newInstruction, pendingInsertion, pendingInsertions);
+ }
+ }
}
- shouldSimplifyIfs |= newConst.outValue().hasUserThatMatches(Instruction::isIf);
+ shouldSimplifyIfs |= newInstruction.outValue().hasUserThatMatches(Instruction::isIf);
} while (iterator.hasNext());
+ // Insert instructions that cannot be inserted in the entry block.
+ if (!pendingInsertions.isEmpty()) {
+ BasicBlockIterator blockIterator = code.listIterator();
+ while (blockIterator.hasNext()) {
+ BasicBlock block = blockIterator.next();
+ InstructionListIterator instructionIterator = block.listIterator(code);
+ for (Phi insertionPoint : block.getPhis()) {
+ instructionIterator =
+ insertPendingInsertions(
+ blockIterator, instructionIterator, insertionPoint, pendingInsertions);
+ }
+ while (instructionIterator.hasNext()) {
+ Instruction insertionPoint = instructionIterator.next();
+ instructionIterator =
+ insertPendingInsertions(
+ blockIterator, instructionIterator, insertionPoint, pendingInsertions);
+ }
+ }
+ }
+
+ assert pendingInsertions.isEmpty();
+
shouldSimplifyIfs |= code.removeAllDeadAndTrivialPhis();
if (shouldSimplifyIfs) {
@@ -203,6 +294,112 @@
}
assert code.isConsistentSSA(appView);
+ return clear();
+ }
+
+ private void addPendingInsertion(
+ InstructionOrPhi insertionPoint,
+ Instruction newInstruction,
+ Map<InstructionOrPhi, List<Instruction>> pendingInsertions) {
+ pendingInsertions
+ .computeIfAbsent(insertionPoint, ignoreKey(ArrayList::new))
+ .add(newInstruction);
+ }
+
+ private InstructionListIterator insertPendingInsertions(
+ BasicBlockIterator blockIterator,
+ InstructionListIterator instructionIterator,
+ InstructionOrPhi insertionPoint,
+ Map<InstructionOrPhi, List<Instruction>> pendingInsertions) {
+ List<Instruction> pendingInsertionsAtInsertionPoint =
+ removePendingInsertions(insertionPoint, pendingInsertions);
+ if (pendingInsertionsAtInsertionPoint.isEmpty()) {
+ return instructionIterator;
+ }
+ WorkList<Instruction> worklist =
+ WorkList.newIdentityWorkList(pendingInsertionsAtInsertionPoint);
+ while (worklist.hasNext()) {
+ Instruction newInstruction = worklist.next();
+ List<Instruction> pendingInsertionsAfterNewInstruction =
+ removePendingInsertions(newInstruction, pendingInsertions);
+ if (pendingInsertionsAfterNewInstruction.isEmpty()) {
+ instructionIterator =
+ insertCanonicalizedConstantAtInsertionPoint(
+ blockIterator, instructionIterator, insertionPoint, newInstruction);
+ } else {
+ // Process pending insertions before the current instruction to ensure the current
+ // instruction ends up first in the instruction stream.
+ worklist.addIfNotSeen(pendingInsertionsAfterNewInstruction);
+ worklist.addIgnoringSeenSet(newInstruction);
+ }
+ }
+ return instructionIterator;
+ }
+
+ private List<Instruction> removePendingInsertions(
+ InstructionOrPhi insertionPoint, Map<InstructionOrPhi, List<Instruction>> pendingInsertions) {
+ List<Instruction> pendingInstructionsAtInsertionPosition =
+ pendingInsertions.remove(insertionPoint);
+ return pendingInstructionsAtInsertionPosition != null
+ ? pendingInstructionsAtInsertionPosition
+ : Collections.emptyList();
+ }
+
+ private InstructionOrPhi getInsertionPointForCanonicalizedConstant(Instruction newInstruction) {
+ switch (newInstruction.opcode()) {
+ case CONST_CLASS:
+ case CONST_NUMBER:
+ case CONST_STRING:
+ case DEX_ITEM_BASED_CONST_STRING:
+ case STATIC_GET:
+ // Insert in entry block.
+ return null;
+ case INSTANCE_GET:
+ {
+ InstanceGet instanceGet = newInstruction.asInstanceGet();
+ Value object = instanceGet.object();
+ if (object.isThis()) {
+ return null;
+ }
+ if (object.isPhi()) {
+ return object.asPhi();
+ }
+ Instruction definition = object.getDefinition();
+ if (definition.isArgument()) {
+ return code.getLastArgument();
+ }
+ if (definition.isNewInstance()) {
+ InvokeDirect uniqueConstructorInvoke =
+ definition.asNewInstance().getUniqueConstructorInvoke(appView.dexItemFactory());
+ // This is guaranteed to be non-null by isConstantCanonicalizationCandidate.
+ assert uniqueConstructorInvoke != null;
+ return uniqueConstructorInvoke;
+ }
+ return definition;
+ }
+ default:
+ throw new Unreachable();
+ }
+ }
+
+ private Instruction createMaterializingInstruction(Instruction canonicalizedConstant) {
+ switch (canonicalizedConstant.opcode()) {
+ case CONST_CLASS:
+ return ConstClass.copyOf(code, canonicalizedConstant.asConstClass());
+ case CONST_NUMBER:
+ return ConstNumber.copyOf(code, canonicalizedConstant.asConstNumber());
+ case CONST_STRING:
+ return ConstString.copyOf(code, canonicalizedConstant.asConstString());
+ case DEX_ITEM_BASED_CONST_STRING:
+ return DexItemBasedConstString.copyOf(
+ code, canonicalizedConstant.asDexItemBasedConstString());
+ case INSTANCE_GET:
+ return InstanceGet.copyOf(code, canonicalizedConstant.asInstanceGet());
+ case STATIC_GET:
+ return StaticGet.copyOf(code, canonicalizedConstant.asStaticGet());
+ default:
+ throw new Unreachable();
+ }
}
public boolean isConstantCanonicalizationCandidate(Instruction instruction) {
@@ -230,16 +427,21 @@
case INSTANCE_GET:
{
InstanceGet instanceGet = instruction.asInstanceGet();
- if (!instanceGet.object().isThis()) {
- // TODO(b/236661949): Generalize this to more receivers. For canonicalization we need
- // the receiver to be non-null (or the instruction can throw) and we also currently
- // require the receiver to be defined on-entry, since we always hoist constants to the
- // entry block.
+ if (instanceGet.instructionMayHaveSideEffects(appView, context)) {
return false;
}
+ if (instanceGet.object().isDefinedByInstructionSatisfying(Instruction::isNewInstance)) {
+ NewInstance newInstance = instanceGet.object().getDefinition().asNewInstance();
+ if (newInstance.getUniqueConstructorInvoke(appView.dexItemFactory()) == null) {
+ return false;
+ }
+ }
if (!isReadOfFinalFieldOutsideInitializer(instanceGet)) {
return false;
}
+ if (getOrComputeIneligibleInstanceGetInstructions().contains(instanceGet)) {
+ return false;
+ }
break;
}
case STATIC_GET:
@@ -276,7 +478,7 @@
}
private boolean isReadOfFinalFieldOutsideInitializer(FieldGet fieldGet) {
- if (isAccessingVolatileField) {
+ if (getOrComputeIsAccessingVolatileField()) {
// A final field may be initialized concurrently. A requirement for this is that the field is
// volatile. However, the reading or writing of another volatile field also allows for
// concurrently initializing a non-volatile field. See also redundant field load elimination.
@@ -350,7 +552,7 @@
return singleFieldValue.getField().lookupOnClass(fieldHolder) != null;
}
- private void insertCanonicalizedConstant(Instruction canonicalizedConstant) {
+ private void insertCanonicalizedConstantInEntryBlock(Instruction canonicalizedConstant) {
BasicBlock entryBlock = code.entryBlock();
// Insert the constant instruction at the start of the block right after the argument
// instructions. It is important that the const instruction is put before any instruction
@@ -367,6 +569,76 @@
it.add(canonicalizedConstant);
}
+ private InstructionListIterator insertCanonicalizedConstantAtInsertionPoint(
+ BasicBlockIterator blockIterator,
+ InstructionListIterator instructionIterator,
+ InstructionOrPhi insertionPoint,
+ Instruction newInstruction) {
+ // If the insertion point is a phi, then we're inserting the new instruction before all other
+ // instructions in the block.
+ assert !insertionPoint.isPhi() || !instructionIterator.hasPrevious();
+ // If the insertion point is not a phi, the iterator is positioned immediately after the
+ // insertion point.
+ assert insertionPoint.isPhi() || instructionIterator.peekPrevious() == insertionPoint;
+ newInstruction.setPosition(
+ getPositionForCanonicalizationConstantAtInsertionPoint(insertionPoint, newInstruction));
+ if (newInstruction.instructionTypeCanThrow()
+ && insertionPoint.getBlock().hasCatchHandlers()
+ && insertionPoint.getBlock().canThrow()) {
+ // Split the block and rewind the block iterator to the insertion block.
+ BasicBlock splitBlock =
+ instructionIterator.splitCopyCatchHandlers(code, blockIterator, appView.options());
+ BasicBlock previousBlock =
+ blockIterator.previousUntil(block -> block == insertionPoint.getBlock());
+ assert previousBlock == insertionPoint.getBlock();
+ blockIterator.next();
+ if (insertionPoint.isPhi()) {
+ // Add new instruction before the goto and position the instruction iterator before the
+ // first instruction (i.e., at the phi position).
+ assert insertionPoint.getBlock().getInstructions().size() == 1;
+ instructionIterator.previous();
+ instructionIterator.add(newInstruction);
+ instructionIterator.previous();
+ assert !instructionIterator.hasPrevious();
+ } else {
+ // Add the new instruction after the insertion point. If the block containing the insertion
+ // point can throw, we insert the new instruction in the beginning of the split block.
+ // Otherwise, we insert it in the end of the insertion block.
+ if (insertionPoint.getBlock().canThrow()) {
+ assert !splitBlock.canThrow();
+ splitBlock.listIterator(code).add(newInstruction);
+ instructionIterator.previous(2);
+ Instruction next = instructionIterator.next();
+ assert next == insertionPoint;
+ } else {
+ assert splitBlock.canThrow();
+ instructionIterator.previous();
+ instructionIterator.add(newInstruction);
+ instructionIterator.previous(2);
+ Instruction next = instructionIterator.next();
+ assert next == insertionPoint;
+ }
+ }
+ } else {
+ instructionIterator.add(newInstruction);
+ Instruction previous = instructionIterator.previous();
+ assert previous == newInstruction;
+ }
+ return instructionIterator;
+ }
+
+ private Position getPositionForCanonicalizationConstantAtInsertionPoint(
+ InstructionOrPhi insertionPoint, Instruction newInstruction) {
+ Position insertionPosition =
+ insertionPoint.isPhi()
+ ? insertionPoint.getBlock().getPosition()
+ : insertionPoint.asInstruction().getPosition();
+ if (newInstruction.instructionTypeCanThrow() && insertionPosition.isNone()) {
+ return Position.syntheticNone();
+ }
+ return insertionPosition;
+ }
+
private static boolean constantUsedByInvokeRange(Instruction constant) {
for (Instruction user : constant.outValue().uniqueUsers()) {
if (user.isInvoke() && user.asInvoke().requiredArgumentRegisters() > 5) {
diff --git a/src/main/java/com/android/tools/r8/utils/IterableUtils.java b/src/main/java/com/android/tools/r8/utils/IterableUtils.java
index 1c4aef5..cd6dcb6 100644
--- a/src/main/java/com/android/tools/r8/utils/IterableUtils.java
+++ b/src/main/java/com/android/tools/r8/utils/IterableUtils.java
@@ -88,7 +88,8 @@
return -1;
}
- public static <T> Iterable<T> filter(Iterable<T> iterable, Predicate<? super T> predicate) {
+ public static <S, T extends S> Iterable<T> filter(
+ Iterable<S> iterable, Predicate<? super S> predicate) {
return () -> IteratorUtils.filter(iterable.iterator(), predicate);
}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/canonicalization/InstanceGetOnNewInstanceCanonicalizationTest.java b/src/test/java/com/android/tools/r8/ir/optimize/canonicalization/InstanceGetOnNewInstanceCanonicalizationTest.java
new file mode 100644
index 0000000..d8950eb
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/canonicalization/InstanceGetOnNewInstanceCanonicalizationTest.java
@@ -0,0 +1,70 @@
+// Copyright (c) 2022, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.ir.optimize.canonicalization;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.utils.codeinspector.InstructionSubject;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class InstanceGetOnNewInstanceCanonicalizationTest extends TestBase {
+
+ @Parameter(0)
+ public TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepMainRule(Main.class)
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .inspect(
+ inspector -> {
+ MethodSubject mainMethodSubject = inspector.clazz(Main.class).mainMethod();
+ assertThat(mainMethodSubject, isPresent());
+ assertEquals(
+ parameters.isCfRuntime() ? 2 : 1,
+ mainMethodSubject
+ .streamInstructions()
+ .filter(InstructionSubject::isInstanceGet)
+ .count());
+ })
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines("Hello, world!");
+ }
+
+ static class Main {
+
+ final String field = System.currentTimeMillis() > 0 ? ", " : null;
+
+ public static void main(String[] args) {
+ Main main = new Main();
+ if (System.currentTimeMillis() > 0) {
+ System.out.print("Hello");
+ System.out.print(main.field);
+ System.out.println("world!");
+ } else {
+ System.out.println(main.field);
+ }
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/canonicalization/InstanceGetOnPhiCanonicalizationTest.java b/src/test/java/com/android/tools/r8/ir/optimize/canonicalization/InstanceGetOnPhiCanonicalizationTest.java
new file mode 100644
index 0000000..8fafc8f
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/canonicalization/InstanceGetOnPhiCanonicalizationTest.java
@@ -0,0 +1,70 @@
+// Copyright (c) 2022, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.ir.optimize.canonicalization;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.utils.codeinspector.InstructionSubject;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class InstanceGetOnPhiCanonicalizationTest extends TestBase {
+
+ @Parameter(0)
+ public TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepMainRule(Main.class)
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .inspect(
+ inspector -> {
+ MethodSubject mainMethodSubject = inspector.clazz(Main.class).mainMethod();
+ assertThat(mainMethodSubject, isPresent());
+ assertEquals(
+ parameters.isCfRuntime() ? 2 : 1,
+ mainMethodSubject
+ .streamInstructions()
+ .filter(InstructionSubject::isInstanceGet)
+ .count());
+ })
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines("Hello, world!");
+ }
+
+ static class Main {
+
+ final String field = System.currentTimeMillis() > 0 ? ", " : null;
+
+ public static void main(String[] args) {
+ Main main = System.currentTimeMillis() > 0 ? new Main() : new Main();
+ if (System.currentTimeMillis() > 0) {
+ System.out.print("Hello");
+ System.out.print(main.field);
+ System.out.println("world!");
+ } else {
+ System.out.println(main.field);
+ }
+ }
+ }
+}