Strengthen inlining of side effect free methods
Bug: 174285670
Change-Id: Ibffc2f78be5a149803c86e5d7a772019ff4d58a0
diff --git a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
index 1dc0e89..2591960 100644
--- a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
+++ b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
@@ -680,14 +680,18 @@
// Library methods listed here are based on their original implementations. That is, we assume
// these cannot be overridden.
public final Set<DexMethod> libraryMethodsReturningNonNull =
- ImmutableSet.of(
- classMethods.getName,
- classMethods.getSimpleName,
- classMethods.forName,
- objectsMethods.requireNonNull,
- objectsMethods.requireNonNullWithMessage,
- objectsMethods.requireNonNullWithMessageSupplier,
- stringMembers.valueOf);
+ ImmutableSet.<DexMethod>builder()
+ .add(
+ classMethods.getName,
+ classMethods.getSimpleName,
+ classMethods.forName,
+ objectsMethods.requireNonNull,
+ objectsMethods.requireNonNullWithMessage,
+ objectsMethods.requireNonNullWithMessageSupplier,
+ stringMembers.valueOf)
+ .addAll(stringBufferMethods.appendMethods)
+ .addAll(stringBuilderMethods.appendMethods)
+ .build();
// TODO(b/119596718): More idempotent methods? Any singleton accessors? E.g.,
// java.util.Calendar#getInstance(...) // 4 variants
diff --git a/src/main/java/com/android/tools/r8/graph/DexMethod.java b/src/main/java/com/android/tools/r8/graph/DexMethod.java
index e00c1fe..d35b1a0 100644
--- a/src/main/java/com/android/tools/r8/graph/DexMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexMethod.java
@@ -261,6 +261,10 @@
return factory.isConstructor(this);
}
+ public boolean mustBeInlinedIntoInstanceInitializer(DexItemFactory dexItemFactory) {
+ return getName().startsWith(dexItemFactory.temporaryConstructorMethodPrefix);
+ }
+
public DexMethod withExtraArgumentPrepended(DexType type, DexItemFactory dexItemFactory) {
return dexItemFactory.createMethod(
holder, dexItemFactory.prependTypeToProto(type, proto), name);
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java
index 495be54..dd0486a 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java
@@ -155,7 +155,7 @@
new AllInstantiatedOrUninstantiated(appView),
new SameParentClass(),
new SameNestHost(),
- new PreserveMethodCharacteristics(),
+ new PreserveMethodCharacteristics(appView),
new SameFeatureSplit(appView),
new RespectPackageBoundaries(appView),
new DontMergeSynchronizedClasses(appView),
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/PreserveMethodCharacteristics.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/PreserveMethodCharacteristics.java
index 431fdb9..3adb35c 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/PreserveMethodCharacteristics.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/PreserveMethodCharacteristics.java
@@ -4,12 +4,14 @@
package com.android.tools.r8.horizontalclassmerging.policies;
+import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexMethodSignature;
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.MethodAccessFlags;
import com.android.tools.r8.horizontalclassmerging.MergeGroup;
import com.android.tools.r8.horizontalclassmerging.MultiClassPolicy;
+import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.OptionalBool;
import com.google.common.collect.Iterables;
import java.util.ArrayList;
@@ -28,9 +30,10 @@
static class MethodCharacteristics {
private final MethodAccessFlags accessFlags;
+ private final boolean isAssumeNoSideEffectsMethod;
private final OptionalBool isLibraryMethodOverride;
- private MethodCharacteristics(DexEncodedMethod method) {
+ private MethodCharacteristics(boolean isAssumeNoSideEffectsMethod, DexEncodedMethod method) {
this.accessFlags =
MethodAccessFlags.builder()
.setPrivate(method.getAccessFlags().isPrivate())
@@ -39,9 +42,16 @@
.setStrict(method.getAccessFlags().isStrict())
.setSynchronized(method.getAccessFlags().isSynchronized())
.build();
+ this.isAssumeNoSideEffectsMethod = isAssumeNoSideEffectsMethod;
this.isLibraryMethodOverride = method.isLibraryMethodOverride();
}
+ static MethodCharacteristics create(
+ AppView<AppInfoWithLiveness> appView, DexEncodedMethod method) {
+ return new MethodCharacteristics(
+ appView.appInfo().isAssumeNoSideEffectsMethod(method.getReference()), method);
+ }
+
@Override
public int hashCode() {
return (accessFlags.hashCode() << 2) | isLibraryMethodOverride.ordinal();
@@ -56,12 +66,17 @@
return false;
}
MethodCharacteristics characteristics = (MethodCharacteristics) obj;
- return isLibraryMethodOverride == characteristics.isLibraryMethodOverride
- && accessFlags.equals(characteristics.accessFlags);
+ return accessFlags.equals(characteristics.accessFlags)
+ && isAssumeNoSideEffectsMethod == characteristics.isAssumeNoSideEffectsMethod
+ && isLibraryMethodOverride == characteristics.isLibraryMethodOverride;
}
}
- public PreserveMethodCharacteristics() {}
+ private final AppView<AppInfoWithLiveness> appView;
+
+ public PreserveMethodCharacteristics(AppView<AppInfoWithLiveness> appView) {
+ this.appView = appView;
+ }
public static class TargetGroup {
@@ -72,12 +87,12 @@
return group;
}
- public boolean tryAdd(DexProgramClass clazz) {
+ public boolean tryAdd(AppView<AppInfoWithLiveness> appView, DexProgramClass clazz) {
Map<DexMethodSignature, MethodCharacteristics> newMethods = new HashMap<>();
for (DexEncodedMethod method : clazz.methods()) {
DexMethodSignature signature = method.getSignature();
MethodCharacteristics existingCharacteristics = methodMap.get(signature);
- MethodCharacteristics methodCharacteristics = new MethodCharacteristics(method);
+ MethodCharacteristics methodCharacteristics = MethodCharacteristics.create(appView, method);
if (existingCharacteristics == null) {
newMethods.put(signature, methodCharacteristics);
continue;
@@ -97,10 +112,10 @@
List<TargetGroup> groups = new ArrayList<>();
for (DexProgramClass clazz : group) {
- boolean added = Iterables.any(groups, targetGroup -> targetGroup.tryAdd(clazz));
+ boolean added = Iterables.any(groups, targetGroup -> targetGroup.tryAdd(appView, clazz));
if (!added) {
TargetGroup newGroup = new TargetGroup();
- added = newGroup.tryAdd(clazz);
+ added = newGroup.tryAdd(appView, clazz);
assert added;
groups.add(newGroup);
}
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 ee770e8..a1c4e7b 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
@@ -4,6 +4,7 @@
package com.android.tools.r8.ir.code;
+import static com.android.tools.r8.graph.DexProgramClass.asProgramClassOrNull;
import static com.android.tools.r8.ir.analysis.type.Nullability.definitelyNotNull;
import static com.android.tools.r8.ir.analysis.type.Nullability.maybeNull;
import static com.android.tools.r8.ir.code.DominatorTree.Assumption.MAY_HAVE_UNREACHABLE_BLOCKS;
@@ -12,6 +13,9 @@
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DebugLocalInfo;
import com.android.tools.r8.graph.DexField;
+import com.android.tools.r8.graph.DexItemFactory;
+import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.ProgramMethod;
@@ -19,6 +23,7 @@
import com.android.tools.r8.ir.analysis.type.TypeElement;
import com.android.tools.r8.ir.code.Phi.RegisterReadType;
import com.android.tools.r8.ir.optimize.NestUtils;
+import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.IteratorUtils;
import com.google.common.collect.ImmutableList;
@@ -269,6 +274,76 @@
}
@Override
+ public boolean replaceCurrentInstructionByNullCheckIfPossible(
+ AppView<?> appView, ProgramMethod context) {
+ Instruction toBeReplaced = current;
+ assert toBeReplaced != null;
+ assert toBeReplaced.isInstanceFieldInstruction() || toBeReplaced.isInvokeMethodWithReceiver();
+ if (toBeReplaced.hasUsedOutValue()) {
+ return false;
+ }
+ if (toBeReplaced.isInvokeDirect()) {
+ DexItemFactory dexItemFactory = appView.dexItemFactory();
+ DexMethod invokedMethod = toBeReplaced.asInvokeDirect().getInvokedMethod();
+ if (invokedMethod.isInstanceInitializer(dexItemFactory)
+ || invokedMethod.mustBeInlinedIntoInstanceInitializer(dexItemFactory)) {
+ return false;
+ }
+ }
+ if (toBeReplaced.instructionMayHaveSideEffects(
+ appView, context, Instruction.SideEffectAssumption.RECEIVER_NOT_NULL)) {
+ return false;
+ }
+ Value receiver =
+ toBeReplaced.isInstanceFieldInstruction()
+ ? toBeReplaced.asInstanceFieldInstruction().object()
+ : toBeReplaced.asInvokeMethodWithReceiver().getReceiver();
+ if (receiver.isNeverNull()) {
+ removeOrReplaceByDebugLocalRead();
+ return true;
+ }
+ InvokeMethod replacement;
+ if (appView.options().canUseRequireNonNull()) {
+ DexMethod requireNonNullMethod = appView.dexItemFactory().objectsMethods.requireNonNull;
+ replacement = new InvokeStatic(requireNonNullMethod, null, ImmutableList.of(receiver));
+ } else {
+ DexMethod getClassMethod = appView.dexItemFactory().objectMembers.getClass;
+ replacement = new InvokeVirtual(getClassMethod, null, ImmutableList.of(receiver));
+ }
+ replaceCurrentInstruction(replacement);
+ return true;
+ }
+
+ @Override
+ public boolean replaceCurrentInstructionByInitClassIfPossible(
+ AppView<AppInfoWithLiveness> appView, IRCode code, DexType type) {
+ Instruction toBeReplaced = current;
+ assert toBeReplaced != null;
+ assert toBeReplaced.isStaticFieldInstruction() || toBeReplaced.isInvokeStatic();
+ if (toBeReplaced.hasUsedOutValue()) {
+ return false;
+ }
+ ProgramMethod context = code.context();
+ if (toBeReplaced.instructionMayHaveSideEffects(
+ appView, context, Instruction.SideEffectAssumption.CLASS_ALREADY_INITIALIZED)) {
+ return false;
+ }
+ if (!type.classInitializationMayHaveSideEffectsInContext(appView, context)) {
+ removeOrReplaceByDebugLocalRead();
+ return true;
+ }
+ if (!appView.canUseInitClass()) {
+ return false;
+ }
+ DexProgramClass clazz = asProgramClassOrNull(appView.definitionFor(type));
+ if (clazz != null) {
+ Value dest = code.createValue(TypeElement.getInt());
+ replaceCurrentInstruction(new InitClass(dest, clazz.type));
+ }
+ return true;
+ }
+
+ @Override
public void replaceCurrentInstructionWithConstClass(
AppView<?> appView, IRCode code, DexType type, DebugLocalInfo localInfo) {
if (current == null) {
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 a96760f..757a581 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
@@ -11,7 +11,9 @@
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.ir.analysis.type.TypeElement;
+import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.InternalOptions;
import java.util.ListIterator;
import java.util.NoSuchElementException;
@@ -42,6 +44,18 @@
}
@Override
+ public boolean replaceCurrentInstructionByNullCheckIfPossible(
+ AppView<?> appView, ProgramMethod context) {
+ return instructionIterator.replaceCurrentInstructionByNullCheckIfPossible(appView, context);
+ }
+
+ @Override
+ public boolean replaceCurrentInstructionByInitClassIfPossible(
+ AppView<AppInfoWithLiveness> appView, IRCode code, DexType type) {
+ return instructionIterator.replaceCurrentInstructionByInitClassIfPossible(appView, code, type);
+ }
+
+ @Override
public void replaceCurrentInstructionWithConstClass(
AppView<?> appView, IRCode code, DexType type, DebugLocalInfo localInfo) {
instructionIterator.replaceCurrentInstructionWithConstClass(appView, code, type, localInfo);
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 e8ba6bd..689514f 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
@@ -130,7 +130,11 @@
}
public boolean hasUnusedOutValue() {
- return !hasOutValue() || !outValue().hasAnyUsers();
+ return !hasUsedOutValue();
+ }
+
+ public boolean hasUsedOutValue() {
+ return hasOutValue() && outValue().hasAnyUsers();
}
public Value outValue() {
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 c4cceea..6b17a04 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
@@ -11,7 +11,9 @@
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.ir.analysis.type.TypeElement;
+import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.InternalOptions;
import com.google.common.collect.Sets;
import java.util.ListIterator;
@@ -97,6 +99,11 @@
Value insertConstStringInstruction(AppView<?> appView, IRCode code, DexString value);
+ boolean replaceCurrentInstructionByNullCheckIfPossible(AppView<?> appView, ProgramMethod context);
+
+ boolean replaceCurrentInstructionByInitClassIfPossible(
+ AppView<AppInfoWithLiveness> appView, IRCode code, DexType type);
+
void replaceCurrentInstructionWithConstClass(
AppView<?> appView, IRCode code, DexType type, DebugLocalInfo localInfo);
diff --git a/src/main/java/com/android/tools/r8/ir/code/Invoke.java b/src/main/java/com/android/tools/r8/ir/code/Invoke.java
index 31fd8f4..5793ec1 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Invoke.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Invoke.java
@@ -144,6 +144,10 @@
abstract public DexType getReturnType();
+ public boolean hasArguments() {
+ return !arguments().isEmpty();
+ }
+
public boolean hasReturnTypeVoid(DexItemFactory factory) {
return getReturnType() == factory.voidType;
}
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 0fc2768..bbb6d27 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
@@ -10,7 +10,9 @@
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.ir.analysis.type.TypeElement;
+import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.InternalOptions;
import com.google.common.collect.Sets;
import java.util.ListIterator;
@@ -62,6 +64,18 @@
}
@Override
+ public boolean replaceCurrentInstructionByNullCheckIfPossible(
+ AppView<?> appView, ProgramMethod context) {
+ return currentBlockIterator.replaceCurrentInstructionByNullCheckIfPossible(appView, context);
+ }
+
+ @Override
+ public boolean replaceCurrentInstructionByInitClassIfPossible(
+ AppView<AppInfoWithLiveness> appView, IRCode code, DexType type) {
+ return currentBlockIterator.replaceCurrentInstructionByInitClassIfPossible(appView, code, type);
+ }
+
+ @Override
public void replaceCurrentInstructionWithConstClass(
AppView<?> appView, IRCode code, DexType type, DebugLocalInfo localInfo) {
currentBlockIterator.replaceCurrentInstructionWithConstClass(appView, code, type, localInfo);
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
index 7e75ee2..38c4e6f 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
@@ -982,6 +982,16 @@
continue;
}
+ if (invoke.isInvokeMethodWithReceiver()) {
+ if (iterator.replaceCurrentInstructionByNullCheckIfPossible(appView, context)) {
+ continue;
+ }
+ } else if (invoke.isInvokeStatic()
+ && iterator.replaceCurrentInstructionByInitClassIfPossible(
+ appView, code, resolutionResult.getResolvedHolder().getType())) {
+ continue;
+ }
+
// TODO(b/156853206): Should not duplicate resolution.
ProgramMethod singleTarget = oracle.lookupSingleTarget(invoke, context);
if (singleTarget == null) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java b/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
index 39ce729..2950d4c 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
@@ -3,7 +3,6 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.ir.optimize;
-import static com.android.tools.r8.graph.DexProgramClass.asProgramClassOrNull;
import static com.android.tools.r8.ir.analysis.type.Nullability.maybeNull;
import static com.google.common.base.Predicates.alwaysTrue;
@@ -14,7 +13,6 @@
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexMethod;
-import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexReference;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.ProgramMethod;
@@ -26,13 +24,10 @@
import com.android.tools.r8.ir.code.FieldInstruction;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.IRMetadata;
-import com.android.tools.r8.ir.code.InitClass;
import com.android.tools.r8.ir.code.InstancePut;
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.InvokeStatic;
-import com.android.tools.r8.ir.code.InvokeVirtual;
import com.android.tools.r8.ir.code.Position;
import com.android.tools.r8.ir.code.StaticGet;
import com.android.tools.r8.ir.code.StaticPut;
@@ -44,7 +39,6 @@
import com.android.tools.r8.shaking.ProguardMemberRuleReturnValue;
import com.android.tools.r8.utils.Reporter;
import com.android.tools.r8.utils.StringDiagnostic;
-import com.google.common.collect.ImmutableList;
import com.google.common.collect.Sets;
import java.util.ListIterator;
import java.util.Set;
@@ -216,8 +210,8 @@
}
if (current.isStaticGet()) {
StaticGet staticGet = current.asStaticGet();
- replaceInstructionByInitClassIfPossible(
- staticGet, staticGet.getField().holder, code, iterator, code.context());
+ iterator.replaceCurrentInstructionByInitClassIfPossible(
+ appView, code, staticGet.getField().holder);
}
replacement.setPosition(position);
if (block.hasCatchHandlers()) {
@@ -236,16 +230,12 @@
ListIterator<BasicBlock> blocks,
InstructionListIterator iterator,
InvokeMethod invoke) {
+ if (invoke.hasUnusedOutValue()) {
+ return;
+ }
+
DexMethod invokedMethod = invoke.getInvokedMethod();
- if (invokedMethod.proto.returnType.isVoidType()) {
- return;
- }
-
- if (!invoke.hasOutValue() || !invoke.outValue().hasNonDebugUsers()) {
- return;
- }
-
- DexType invokedHolder = invokedMethod.holder;
+ DexType invokedHolder = invokedMethod.getHolderType();
if (!invokedHolder.isClassType()) {
return;
}
@@ -302,10 +292,10 @@
invoke.setOutValue(null);
if (invoke.isInvokeMethodWithReceiver()) {
- replaceInstructionByNullCheckIfPossible(invoke, iterator, context);
+ iterator.replaceCurrentInstructionByNullCheckIfPossible(appView, context);
} else if (invoke.isInvokeStatic()) {
- replaceInstructionByInitClassIfPossible(
- invoke, singleTarget.getHolderType(), code, iterator, context);
+ iterator.replaceCurrentInstructionByInitClassIfPossible(
+ appView, code, singleTarget.getHolderType());
}
// Insert the definition of the replacement.
@@ -404,11 +394,11 @@
// To preserve side effects, original field-get is replaced by an explicit null-check, if
// the field-get instruction may only fail with an NPE, or the field-get remains as-is.
if (current.isInstanceGet()) {
- replaceInstructionByNullCheckIfPossible(current, iterator, context);
+ iterator.replaceCurrentInstructionByNullCheckIfPossible(appView, context);
} else {
assert current.isStaticGet();
- replaceInstructionByInitClassIfPossible(
- current, target.getHolderType(), code, iterator, context);
+ iterator.replaceCurrentInstructionByInitClassIfPossible(
+ appView, code, target.getHolderType());
}
// Insert the definition of the replacement.
@@ -426,58 +416,6 @@
}
}
- private void replaceInstructionByNullCheckIfPossible(
- Instruction instruction, InstructionListIterator iterator, ProgramMethod context) {
- assert instruction.isInstanceFieldInstruction() || instruction.isInvokeMethodWithReceiver();
- assert !instruction.hasOutValue() || !instruction.outValue().hasAnyUsers();
- if (instruction.instructionMayHaveSideEffects(
- appView, context, Instruction.SideEffectAssumption.RECEIVER_NOT_NULL)) {
- return;
- }
- Value receiver =
- instruction.isInstanceFieldInstruction()
- ? instruction.asInstanceFieldInstruction().object()
- : instruction.asInvokeMethodWithReceiver().getReceiver();
- if (receiver.isNeverNull()) {
- iterator.removeOrReplaceByDebugLocalRead();
- return;
- }
- InvokeMethod replacement;
- if (appView.options().canUseRequireNonNull()) {
- DexMethod requireNonNullMethod = appView.dexItemFactory().objectsMethods.requireNonNull;
- replacement = new InvokeStatic(requireNonNullMethod, null, ImmutableList.of(receiver));
- } else {
- DexMethod getClassMethod = appView.dexItemFactory().objectMembers.getClass;
- replacement = new InvokeVirtual(getClassMethod, null, ImmutableList.of(receiver));
- }
- iterator.replaceCurrentInstruction(replacement);
- }
-
- private void replaceInstructionByInitClassIfPossible(
- Instruction instruction,
- DexType holder,
- IRCode code,
- InstructionListIterator iterator,
- ProgramMethod context) {
- assert instruction.isStaticFieldInstruction() || instruction.isInvokeStatic();
- if (instruction.instructionMayHaveSideEffects(
- appView, context, Instruction.SideEffectAssumption.CLASS_ALREADY_INITIALIZED)) {
- return;
- }
- if (!holder.classInitializationMayHaveSideEffectsInContext(appView, context)) {
- iterator.removeOrReplaceByDebugLocalRead();
- return;
- }
- if (!appView.canUseInitClass()) {
- return;
- }
- DexProgramClass clazz = asProgramClassOrNull(appView.definitionFor(holder));
- if (clazz != null) {
- Value dest = code.createValue(TypeElement.getInt());
- iterator.replaceCurrentInstruction(new InitClass(dest, clazz.type));
- }
- }
-
private void replaceInstancePutByNullCheckIfNeverRead(
IRCode code, InstructionListIterator iterator, InstancePut current) {
DexEncodedField field = appView.appInfo().resolveField(current.getField()).getResolvedField();
@@ -492,7 +430,7 @@
return;
}
- replaceInstructionByNullCheckIfPossible(current, iterator, code.context());
+ iterator.replaceCurrentInstructionByNullCheckIfPossible(appView, code.context());
}
private void replaceStaticPutByInitClassIfNeverRead(
@@ -509,8 +447,7 @@
return;
}
- replaceInstructionByInitClassIfPossible(
- current, field.getHolderType(), code, iterator, code.context());
+ iterator.replaceCurrentInstructionByInitClassIfPossible(appView, code, field.getHolderType());
}
/**
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Outliner.java b/src/main/java/com/android/tools/r8/ir/optimize/Outliner.java
index 508e0f4..f673395 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/Outliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/Outliner.java
@@ -37,6 +37,7 @@
import com.android.tools.r8.ir.analysis.type.PrimitiveTypeElement;
import com.android.tools.r8.ir.analysis.type.TypeElement;
import com.android.tools.r8.ir.code.Add;
+import com.android.tools.r8.ir.code.Assume;
import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.BasicBlock.ThrowingInfo;
import com.android.tools.r8.ir.code.Binop;
@@ -782,7 +783,7 @@
// Process instruction. Returns true if an outline candidate was found.
private void processInstruction(Instruction instruction) {
// Figure out whether to include the instruction.
- boolean include = false;
+ boolean include;
int instructionIncrement = 1;
if (instruction.isConstInstruction()) {
if (index == start) {
@@ -811,13 +812,13 @@
includeInstruction(instruction);
// Check if this instruction ends the outline.
if (actualInstructions >= appView.options().outline.maxSize) {
- candidate(start, index + 1, actualInstructions);
+ candidate(start, index + 1);
} else {
index++;
}
} else if (index > start) {
// Do not add this instruction, candidate ends with previous instruction.
- candidate(start, index, actualInstructions);
+ candidate(start, index);
} else {
// Restart search from next instruction.
reset(index + 1);
@@ -830,7 +831,7 @@
int returnValueUsersLeftIfIncluded = returnValueUsersLeft;
if (returnValue != null) {
for (Value value : instruction.inValues()) {
- if (value == returnValue) {
+ if (value.getAliasedValue() == returnValue) {
returnValueUsersLeftIfIncluded--;
}
}
@@ -870,10 +871,9 @@
}
// Find the number of in-going arguments, if adding this instruction.
int newArgumentRegisters = argumentRegisters;
- if (instruction.inValues().size() > 0) {
- List<Value> inValues = orderedInValues(instruction, returnValue);
- for (int i = 0; i < inValues.size(); i++) {
- Value value = inValues.get(i);
+ if (invoke.hasArguments()) {
+ for (int i = 0; i < invoke.arguments().size(); i++) {
+ Value value = invoke.getArgument(i).getAliasedValue();
if (value == returnValue) {
continue;
}
@@ -884,7 +884,8 @@
newArgumentRegisters += value.requiredRegisters();
} else {
// For virtual calls only re-use the receiver argument.
- if (i > 0 || !arguments.contains(value)) {
+ Value receiver = invoke.asInvokeMethodWithReceiver().getReceiver().getAliasedValue();
+ if (value != receiver || !arguments.contains(value)) {
newArgumentRegisters += value.requiredRegisters();
}
}
@@ -907,7 +908,11 @@
offset++;
previous = instructions.get(index - offset);
} while (previous.isConstInstruction());
- if (!previous.isNewInstance() || previous.outValue() != returnValue) {
+ if (!previous.isNewInstance()) {
+ return false;
+ }
+ if (returnValue == null || returnValue != previous.outValue()) {
+ assert false;
return false;
}
// Clear pending new instance flag as the last thing, now the matching constructor is known
@@ -996,6 +1001,10 @@
// Add the current instruction to the outline.
private void includeInstruction(Instruction instruction) {
if (instruction.isAssume()) {
+ Assume assume = instruction.asAssume();
+ if (returnValue != null && assume.src().getAliasedValue() == returnValue) {
+ adjustReturnValueUsersLeft(assume.outValue().numberOfAllUsers() - 1);
+ }
return;
}
@@ -1005,12 +1014,7 @@
if (returnValue != null) {
for (Value value : inValues) {
if (value.getAliasedValue() == returnValue) {
- assert returnValueUsersLeft > 0;
- returnValueUsersLeft--;
- }
- if (returnValueUsersLeft == 0) {
- returnValue = null;
- returnType = appView.dexItemFactory().voidType;
+ adjustReturnValueUsersLeft(-1);
}
}
}
@@ -1080,9 +1084,18 @@
}
}
+ private void adjustReturnValueUsersLeft(int change) {
+ returnValueUsersLeft += change;
+ assert returnValueUsersLeft >= 0;
+ if (returnValueUsersLeft == 0) {
+ returnValue = null;
+ returnType = appView.dexItemFactory().voidType;
+ }
+ }
+
protected abstract void handle(int start, int end, Outline outline);
- private void candidate(int start, int index, int actualInstructions) {
+ private void candidate(int start, int index) {
List<Instruction> instructions = getInstructionArray();
assert !instructions.get(start).isConstInstruction();
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
index 143d776..0ce08a5 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
@@ -584,10 +584,13 @@
currentUsers = indirectOutValueUsers;
}
- assert !methodCallsOnInstance.isEmpty();
-
- inliner.performForcedInlining(
- method, code, methodCallsOnInstance, inliningIRProvider, Timing.empty());
+ if (!methodCallsOnInstance.isEmpty()) {
+ inliner.performForcedInlining(
+ method, code, methodCallsOnInstance, inliningIRProvider, Timing.empty());
+ } else {
+ assert indirectMethodCallsOnInstance.stream()
+ .noneMatch(method -> method.getDefinition().getOptimizationInfo().mayHaveSideEffects());
+ }
return true;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/library/LibraryMethodSideEffectModelCollection.java b/src/main/java/com/android/tools/r8/ir/optimize/library/LibraryMethodSideEffectModelCollection.java
index b1890af..ae91ac8 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/library/LibraryMethodSideEffectModelCollection.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/library/LibraryMethodSideEffectModelCollection.java
@@ -38,7 +38,10 @@
ImmutableMap.<DexMethod, BiPredicate<DexMethod, List<Value>>>builder()
.put(
dexItemFactory.stringMembers.constructor,
- (method, arguments) -> arguments.get(1).isNeverNull());
+ (method, arguments) -> arguments.get(1).isNeverNull())
+ .put(
+ dexItemFactory.stringMembers.valueOf,
+ (method, arguments) -> arguments.get(0).isNeverNull());
putAll(
builder,
dexItemFactory.stringBufferMethods.constructorMethods,
diff --git a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
index 97b2504..5619d97 100644
--- a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
+++ b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
@@ -575,6 +575,10 @@
return neverInlineDueToSingleCaller.contains(method.getReference());
}
+ public boolean isAssumeNoSideEffectsMethod(DexMethod method) {
+ return noSideEffects.containsKey(method);
+ }
+
public boolean isWhyAreYouNotInliningMethod(DexMethod method) {
return whyAreYouNotInlining.contains(method);
}
diff --git a/src/test/java/com/android/tools/r8/ir/conversion/StringSwitchConversionFromIfTest.java b/src/test/java/com/android/tools/r8/ir/conversion/StringSwitchConversionFromIfTest.java
index 294efee..bcdd93f 100644
--- a/src/test/java/com/android/tools/r8/ir/conversion/StringSwitchConversionFromIfTest.java
+++ b/src/test/java/com/android/tools/r8/ir/conversion/StringSwitchConversionFromIfTest.java
@@ -7,7 +7,7 @@
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertTrue;
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.TestBase;
@@ -87,7 +87,16 @@
// instruction may throw. This holds even if the string-switch instruction is
// compiled to a sequence of `if (x.equals("..."))` instructions that do not even
// use the hash code.
- assertNotEquals(0, hashCodeValues.size());
+ assertTrue(
+ code.collectArguments().get(0).uniqueUsers().stream()
+ .filter(Instruction::isInvokeMethod)
+ .map(Instruction::asInvokeMethod)
+ .map(invoke -> invoke.getInvokedMethod().getName().toSourceString())
+ .anyMatch(
+ name ->
+ name.equals("getClass")
+ || name.equals("hashCode")
+ || name.equals("requireNonNull")));
}
})
.run(parameters.getRuntime(), TestClass.class)
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/R8InliningTest.java b/src/test/java/com/android/tools/r8/ir/optimize/R8InliningTest.java
index af047b9..7049089 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/R8InliningTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/R8InliningTest.java
@@ -3,6 +3,8 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.ir.optimize;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isAbsent;
+import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
@@ -333,7 +335,7 @@
assertCounters(INLINABLE, NEVER_INLINABLE, countInvokes(inspector, m));
m = clazz.method("int", "notInlinableDueToMissingNpe", ImmutableList.of("inlining.A"));
- assertCounters(INLINABLE, NEVER_INLINABLE, countInvokes(inspector, m));
+ assertCounters(INLINABLE, ALWAYS_INLINABLE, countInvokes(inspector, m));
m = clazz.method("int", "notInlinableDueToSideEffect", ImmutableList.of("inlining.A"));
assertCounters(INLINABLE, NEVER_INLINABLE, countInvokes(inspector, m));
@@ -354,7 +356,12 @@
CodeInspector inspector =
new CodeInspector(getGeneratedFiles(), getGeneratedProguardMap(), null);
ClassSubject clazz = inspector.clazz(nullabilityClass);
- MethodSubject m = clazz.method("int", "conditionalOperator", ImmutableList.of("inlining.A"));
+ assertThat(clazz.uniqueMethodWithName("conditionalOperator"), isAbsent());
+
+ // The enum parameter may get unboxed.
+ MethodSubject m =
+ clazz.uniqueMethodWithName(
+ parameters.isCfRuntime() ? "moreControlFlows" : "moreControlFlows$enumunboxing$");
assertTrue(m.isPresent());
// Verify that a.b() is resolved to an inline instance-get.
@@ -365,27 +372,6 @@
InstructionSubject instruction = iterator.next();
if (instruction.isInstanceGet()) {
++instanceGetCount;
- } else if (instruction.isInvoke()) {
- ++invokeCount;
- }
- }
- assertEquals(1, instanceGetCount);
- assertEquals(0, invokeCount);
-
- // The enum parameter may get unboxed.
- m =
- clazz.uniqueMethodWithName(
- parameters.isCfRuntime() ? "moreControlFlows" : "moreControlFlows$enumunboxing$");
- assertTrue(m.isPresent());
-
- // Verify that a.b() is resolved to an inline instance-get.
- iterator = m.iterateInstructions();
- instanceGetCount = 0;
- invokeCount = 0;
- while (iterator.hasNext()) {
- InstructionSubject instruction = iterator.next();
- if (instruction.isInstanceGet()) {
- ++instanceGetCount;
} else if (instruction.isInvoke() && !isEnumInvoke(instruction)) {
++invokeCount;
}
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 c253c45..8d01a11 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
@@ -14,6 +14,7 @@
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.ir.analysis.type.Nullability;
import com.android.tools.r8.ir.analysis.type.TypeElement;
import com.android.tools.r8.ir.code.BasicBlock;
@@ -23,6 +24,7 @@
import com.android.tools.r8.ir.code.Move;
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.ir.code.ValueType;
+import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.InternalOptions;
import java.util.LinkedList;
import java.util.List;
@@ -62,6 +64,18 @@
}
@Override
+ public boolean replaceCurrentInstructionByNullCheckIfPossible(
+ AppView<?> appView, ProgramMethod context) {
+ throw new Unimplemented();
+ }
+
+ @Override
+ public boolean replaceCurrentInstructionByInitClassIfPossible(
+ AppView<AppInfoWithLiveness> appView, IRCode code, DexType type) {
+ throw new Unimplemented();
+ }
+
+ @Override
public void replaceCurrentInstructionWithConstClass(
AppView<?> appView, IRCode code, DexType type, DebugLocalInfo localInfo) {
throw new Unimplemented();
diff --git a/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumeNoSideEffectsForJavaLangClassTest.java b/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumeNoSideEffectsForJavaLangClassTest.java
index eb29f88..fed4e6d 100644
--- a/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumeNoSideEffectsForJavaLangClassTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumeNoSideEffectsForJavaLangClassTest.java
@@ -7,6 +7,7 @@
import static com.android.tools.r8.utils.codeinspector.CodeMatchers.invokesMethodWithName;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static com.android.tools.r8.utils.codeinspector.Matchers.onlyIf;
+import static org.hamcrest.CoreMatchers.anyOf;
import static org.hamcrest.MatcherAssert.assertThat;
import com.android.tools.r8.AssumeMayHaveSideEffects;
@@ -71,7 +72,11 @@
assertThat(
methodSubject,
onlyIf(maybeNullReceiver || maybeSubtype, invokesMethodWithName("hashCode")));
- assertThat(methodSubject, onlyIf(maybeNullReceiver, invokesMethodWithName("getClass")));
+ assertThat(
+ methodSubject,
+ onlyIf(
+ maybeNullReceiver,
+ anyOf(invokesMethodWithName("getClass"), invokesMethodWithName("requireNonNull"))));
assertThat(
methodSubject,
onlyIf(maybeNullReceiver || maybeSubtype, invokesMethodWithName("toString")));
diff --git a/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsPropagationTest.java b/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsPropagationTest.java
index 1c7f6ac..8130a6f 100644
--- a/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsPropagationTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsPropagationTest.java
@@ -111,7 +111,7 @@
private final TestConfig config;
private final boolean enableHorizontalClassMerging;
- @Parameterized.Parameters(name = "{0} {1}")
+ @Parameterized.Parameters(name = "{0}, config: {1}, horizontal: {2}")
public static Collection<Object[]> data() {
return buildParameters(
getTestParameters().withAllRuntimesAndApiLevels().build(),
diff --git a/src/test/java/com/android/tools/r8/smali/OutlineTest.java b/src/test/java/com/android/tools/r8/smali/OutlineTest.java
index e85f2d8..04ac085 100644
--- a/src/test/java/com/android/tools/r8/smali/OutlineTest.java
+++ b/src/test/java/com/android/tools/r8/smali/OutlineTest.java
@@ -252,7 +252,7 @@
" move-result-object v0",
" invoke-virtual { v0, v1 }, " + stringBuilderAppendSignature,
" move-result-object v0",
- " invoke-virtual { v0 }, " + stringBuilderToStringSignature,
+ " invoke-virtual { v0, v1 }, " + stringBuilderAppendSignature,
" move-result-object v0",
" const v0, 0",
" const v1, 1",
@@ -818,7 +818,7 @@
" move-result-object v0",
" invoke-virtual { v0, v1 }, " + stringBuilderAppendSignature,
" move-result-object v0",
- " invoke-virtual { v0 }, " + stringBuilderToStringSignature,
+ " invoke-virtual { v0, v1 }, " + stringBuilderAppendSignature,
" return-void");
String returnType2 = "java.lang.String";
@@ -866,14 +866,14 @@
ClassSubject clazz = inspector.clazz(OutlineOptions.CLASS_NAME);
assertTrue(clazz.isPresent());
assertEquals(3, clazz.getDexProgramClass().getMethodCollection().numberOfDirectMethods());
- // Collect the return types of the putlines for the body of method1 and method2.
+ // Collect the return types of the outlines for the body of method1 and method2.
List<DexType> r = new ArrayList<>();
for (DexEncodedMethod directMethod : clazz.getDexProgramClass().directMethods()) {
if (directMethod.getCode().asDexCode().instructions[0] instanceof InvokeVirtual) {
r.add(directMethod.method.proto.returnType);
}
}
- assert r.size() == 2;
+ assertEquals(2, r.size());
DexType r1 = r.get(0);
DexType r2 = r.get(1);
DexItemFactory factory = inspector.getFactory();
@@ -882,7 +882,7 @@
// Run the code.
String result = runArt(processedApplication);
- assertEquals("TestTestTestTest", result);
+ assertEquals("TestTestTestTestTest", result);
}
@Test
