Version 1.3.46
Merge: Minor cleanup of class inliner
CL: https://r8-review.googlesource.com/c/r8/+/32221
Merge: Check for inline-ability before force-inlining in class inliner
CL: https://r8-review.googlesource.com/c/r8/+/32138
Merge: Add class inliner test for invoke-super inlining
CL: https://r8-review.googlesource.com/c/r8/+/32133
Merge: Abort gracefully from class inlining in case of ineligible users
CL: https://r8-review.googlesource.com/c/r8/+/32104
Merge: Add failing test for class inliner
CL: https://r8-review.googlesource.com/c/r8/+/32103
Merge: Output abort reason in class inliner
CL: https://r8-review.googlesource.com/c/r8/+/31960
Merge: Attempt to make test independent of Java version
CL: https://r8-review.googlesource.com/c/r8/+/31961
Merge: Extend class inliner's escape analysis
CL: https://r8-review.googlesource.com/c/r8/+/31920
Merge: Partial fix for class inliner's escape analysis
CL: https://r8-review.googlesource.com/c/r8/+/31901
Merge: Add failing test for class inliner bug
CL: https://r8-review.googlesource.com/c/r8/+/31808
Merge: Extend error messages in class inliner
CL: https://r8-review.googlesource.com/c/r8/+/31508
Merge: Introduce -neverclassinline config option
CL: https://r8-review.googlesource.com/c/r8/+/30891
Bug: 120182628, 121119666
Change-Id: I1cab5c66c3f066960f82163d5287600e4959cbf6
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 8b14556..3d5acb9 100644
--- a/src/main/java/com/android/tools/r8/Version.java
+++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@
// This field is accessed from release scripts using simple pattern matching.
// Therefore, changing this field could break our release scripts.
- public static final String LABEL = "1.3.45";
+ public static final String LABEL = "1.3.46";
private Version() {
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
index c3db796..481b6c1 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -390,12 +390,6 @@
}
}
- public boolean hasDebugPositions() {
- checkIfObsolete();
- assert code != null && code.isDexCode();
- return code.asDexCode().hasDebugPositions();
- }
-
public int getClassFileVersion() {
checkIfObsolete();
assert classFileVersion >= 0;
diff --git a/src/main/java/com/android/tools/r8/graph/ParameterUsagesInfo.java b/src/main/java/com/android/tools/r8/graph/ParameterUsagesInfo.java
index 84cf331..aac25ba 100644
--- a/src/main/java/com/android/tools/r8/graph/ParameterUsagesInfo.java
+++ b/src/main/java/com/android/tools/r8/graph/ParameterUsagesInfo.java
@@ -4,9 +4,14 @@
package com.android.tools.r8.graph;
+import com.android.tools.r8.ir.code.If;
import com.android.tools.r8.ir.code.If.Type;
+import com.android.tools.r8.ir.code.InstanceGet;
+import com.android.tools.r8.ir.code.InstancePut;
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.Invoke;
+import com.android.tools.r8.ir.code.InvokeMethodWithReceiver;
+import com.android.tools.r8.ir.code.Return;
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.utils.Pair;
import com.google.common.collect.ImmutableList;
@@ -38,32 +43,63 @@
}
public final static class ParameterUsage {
+
public final int index;
public final Set<Type> ifZeroTest;
public final List<Pair<Invoke.Type, DexMethod>> callsReceiver;
- public final boolean returnValue;
- ParameterUsage(int index, Set<Type> ifZeroTest,
- List<Pair<Invoke.Type, DexMethod>> callsReceiver, boolean returnValue) {
+ // If a field of this argument is assigned: arg.f = x.
+ public final boolean hasFieldAssignment;
+
+ // If a field of this argument is read: x = arg.f.
+ public final boolean hasFieldRead;
+
+ // If this argument is assigned to a field: x.f = arg.
+ public final boolean isAssignedToField;
+
+ // If this argument is returned: return arg.
+ public final boolean isReturned;
+
+ ParameterUsage(
+ int index,
+ Set<Type> ifZeroTest,
+ List<Pair<Invoke.Type, DexMethod>> callsReceiver,
+ boolean hasFieldAssignment,
+ boolean hasFieldRead,
+ boolean isAssignedToField,
+ boolean isReturned) {
this.index = index;
- this.ifZeroTest = ifZeroTest.isEmpty()
- ? Collections.emptySet() : ImmutableSet.copyOf(ifZeroTest);
- this.callsReceiver = callsReceiver.isEmpty()
- ? Collections.emptyList() : ImmutableList.copyOf(callsReceiver);
- this.returnValue = returnValue;
+ this.ifZeroTest =
+ ifZeroTest.isEmpty() ? Collections.emptySet() : ImmutableSet.copyOf(ifZeroTest);
+ this.callsReceiver =
+ callsReceiver.isEmpty() ? Collections.emptyList() : ImmutableList.copyOf(callsReceiver);
+ this.hasFieldAssignment = hasFieldAssignment;
+ this.hasFieldRead = hasFieldRead;
+ this.isAssignedToField = isAssignedToField;
+ this.isReturned = isReturned;
}
public boolean notUsed() {
- return ifZeroTest == null && callsReceiver == null && !returnValue;
+ return ifZeroTest == null
+ && callsReceiver == null
+ && !hasFieldAssignment
+ && !hasFieldRead
+ && !isAssignedToField
+ && !isReturned;
}
}
public static class ParameterUsageBuilder {
+
private final int index;
private final Value arg;
private final Set<Type> ifZeroTestTypes = new HashSet<>();
private final List<Pair<Invoke.Type, DexMethod>> callsOnReceiver = new ArrayList<>();
- private boolean returnValue = false;
+
+ private boolean hasFieldAssignment = false;
+ private boolean hasFieldRead = false;
+ private boolean isAssignedToField = false;
+ private boolean isReturned = false;
public ParameterUsageBuilder(Value arg, int index) {
this.arg = arg;
@@ -72,31 +108,82 @@
// Returns false if the instruction is not supported.
public boolean note(Instruction instruction) {
- if (instruction.isInvokeMethodWithReceiver() &&
- instruction.inValues().lastIndexOf(arg) == 0) {
- callsOnReceiver.add(new Pair<>(
- instruction.asInvokeMethodWithReceiver().getType(),
- instruction.asInvokeMethodWithReceiver().getInvokedMethod()));
- return true;
+ if (instruction.isIf()) {
+ return note(instruction.asIf());
}
-
- if (instruction.isIf() && instruction.asIf().isZeroTest()) {
- assert instruction.inValues().size() == 1 && instruction.inValues().get(0) == arg;
- ifZeroTestTypes.add(instruction.asIf().getType());
- return true;
+ if (instruction.isInstanceGet()) {
+ return note(instruction.asInstanceGet());
}
-
+ if (instruction.isInstancePut()) {
+ return note(instruction.asInstancePut());
+ }
+ if (instruction.isInvokeMethodWithReceiver()) {
+ return note(instruction.asInvokeMethodWithReceiver());
+ }
if (instruction.isReturn()) {
- assert instruction.inValues().size() == 1 && instruction.inValues().get(0) == arg;
- returnValue = true;
- return true;
+ return note(instruction.asReturn());
}
-
return false;
}
public ParameterUsage build() {
- return new ParameterUsage(index, ifZeroTestTypes, callsOnReceiver, returnValue);
+ return new ParameterUsage(
+ index,
+ ifZeroTestTypes,
+ callsOnReceiver,
+ hasFieldAssignment,
+ hasFieldRead,
+ isAssignedToField,
+ isReturned);
+ }
+
+ private boolean note(If ifInstruction) {
+ if (ifInstruction.asIf().isZeroTest()) {
+ assert ifInstruction.inValues().size() == 1 && ifInstruction.inValues().get(0) == arg;
+ ifZeroTestTypes.add(ifInstruction.asIf().getType());
+ return true;
+ }
+ return false;
+ }
+
+ private boolean note(InstanceGet instanceGetInstruction) {
+ assert arg != instanceGetInstruction.outValue();
+ if (instanceGetInstruction.object() == arg) {
+ hasFieldRead = true;
+ return true;
+ }
+ return false;
+ }
+
+ private boolean note(InstancePut instancePutInstruction) {
+ assert arg != instancePutInstruction.outValue();
+ if (instancePutInstruction.object() == arg) {
+ hasFieldAssignment = true;
+ isAssignedToField |= instancePutInstruction.value() == arg;
+ return true;
+ }
+ if (instancePutInstruction.value() == arg) {
+ isAssignedToField = true;
+ return true;
+ }
+ return false;
+ }
+
+ private boolean note(InvokeMethodWithReceiver invokeInstruction) {
+ if (invokeInstruction.inValues().lastIndexOf(arg) == 0) {
+ callsOnReceiver.add(
+ new Pair<>(
+ invokeInstruction.asInvokeMethodWithReceiver().getType(),
+ invokeInstruction.asInvokeMethodWithReceiver().getInvokedMethod()));
+ return true;
+ }
+ return false;
+ }
+
+ private boolean note(Return returnInstruction) {
+ assert returnInstruction.inValues().size() == 1 && returnInstruction.inValues().get(0) == arg;
+ isReturned = true;
+ return true;
}
}
}
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 6f62d21..8c5f51b 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
@@ -31,7 +31,7 @@
import java.util.Set;
import java.util.function.Function;
-public abstract class Instruction {
+public abstract class Instruction implements InstructionOrPhi {
protected Value outValue = null;
protected final List<Value> inValues = new ArrayList<>();
@@ -499,6 +499,11 @@
return debugValues != null ? debugValues : ImmutableSet.of();
}
+ @Override
+ public boolean isInstruction() {
+ return true;
+ }
+
public boolean isArrayGet() {
return false;
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/InstructionOrPhi.java b/src/main/java/com/android/tools/r8/ir/code/InstructionOrPhi.java
new file mode 100644
index 0000000..10110a0
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/ir/code/InstructionOrPhi.java
@@ -0,0 +1,16 @@
+// Copyright (c) 2018, 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.code;
+
+public interface InstructionOrPhi {
+
+ default boolean isInstruction() {
+ return false;
+ }
+
+ default boolean isPhi() {
+ return false;
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/ir/code/Phi.java b/src/main/java/com/android/tools/r8/ir/code/Phi.java
index f3301e7..61d3883 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Phi.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Phi.java
@@ -20,7 +20,7 @@
import java.util.Map.Entry;
import java.util.Set;
-public class Phi extends Value {
+public class Phi extends Value implements InstructionOrPhi {
public enum RegisterReadType {
NORMAL,
diff --git a/src/main/java/com/android/tools/r8/ir/code/Value.java b/src/main/java/com/android/tools/r8/ir/code/Value.java
index 96f765f..1d8e88b 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Value.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Value.java
@@ -277,6 +277,11 @@
return users.getFirst();
}
+ public Phi firstPhiUser() {
+ assert !phiUsers.isEmpty();
+ return phiUsers.getFirst();
+ }
+
public Set<Phi> uniquePhiUsers() {
if (uniquePhiUsers != null) {
return uniquePhiUsers;
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 1aad1b9..ac5dd28 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
@@ -487,10 +487,9 @@
BiConsumer<IRCode, DexEncodedMethod> outlineHandler =
outliner == null ? Outliner::noProcessing : outliner.identifyCandidateMethods();
callGraph.forEachMethod(
- (method, isProcessedConcurrently) -> {
- processMethod(
- method, directFeedback, isProcessedConcurrently, callGraph, outlineHandler);
- },
+ (method, isProcessedConcurrently) ->
+ processMethod(
+ method, directFeedback, isProcessedConcurrently, callGraph, outlineHandler),
executorService);
timing.end();
}
@@ -841,14 +840,21 @@
// lambda, it is not get collected by merger.
assert options.enableInlining && inliner != null;
classInliner.processMethodCode(
- appInfo.withLiveness(), codeRewriter, method, code, isProcessedConcurrently,
- methodsToInline -> inliner.performForcedInlining(method, code, methodsToInline),
- Suppliers.memoize(() -> inliner.createDefaultOracle(
- method, code,
- isProcessedConcurrently, callSiteInformation,
- Integer.MAX_VALUE / 2, Integer.MAX_VALUE / 2)
- )
- );
+ appInfo.withLiveness(),
+ codeRewriter,
+ method,
+ code,
+ isProcessedConcurrently,
+ inliner,
+ Suppliers.memoize(
+ () ->
+ inliner.createDefaultOracle(
+ method,
+ code,
+ isProcessedConcurrently,
+ callSiteInformation,
+ Integer.MAX_VALUE / 2,
+ Integer.MAX_VALUE / 2)));
assert code.isConsistentSSA();
}
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 d11946c..809ef70 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
@@ -818,8 +818,18 @@
continue;
}
- if (insn.isInstanceGet() ||
- (insn.isInstancePut() && insn.asInstancePut().object() == receiver)) {
+ if (insn.isInstanceGet() || insn.isInstancePut()) {
+ if (insn.isInstancePut()) {
+ InstancePut instancePutInstruction = insn.asInstancePut();
+ // Only allow field writes to the receiver.
+ if (instancePutInstruction.object() != receiver) {
+ return;
+ }
+ // Do not allow the receiver to escape via a field write.
+ if (instancePutInstruction.value() == receiver) {
+ return;
+ }
+ }
DexField field = insn.asFieldInstruction().getField();
if (field.clazz == clazz.type && clazz.lookupInstanceField(field) != null) {
// Require only accessing instance fields of the *current* class.
@@ -880,7 +890,7 @@
public void identifyParameterUsages(
DexEncodedMethod method, IRCode code, OptimizationFeedback feedback) {
List<ParameterUsage> usages = new ArrayList<>();
- List<Value> values = code.collectArguments(true);
+ List<Value> values = code.collectArguments();
for (int i = 0; i < values.size(); i++) {
Value value = values.get(i);
if (value.numberOfPhiUsers() > 0) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
index 0ef4597..ec6a2fe 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
@@ -4,22 +4,20 @@
package com.android.tools.r8.ir.optimize.classinliner;
-import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.Instruction;
-import com.android.tools.r8.ir.code.InvokeMethod;
+import com.android.tools.r8.ir.code.InstructionOrPhi;
import com.android.tools.r8.ir.desugar.LambdaRewriter;
import com.android.tools.r8.ir.optimize.CodeRewriter;
-import com.android.tools.r8.ir.optimize.Inliner.InliningInfo;
+import com.android.tools.r8.ir.optimize.Inliner;
import com.android.tools.r8.ir.optimize.InliningOracle;
import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
import com.google.common.collect.Streams;
import java.util.Iterator;
import java.util.List;
-import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Predicate;
import java.util.function.Supplier;
@@ -31,10 +29,6 @@
private final int totalMethodInstructionLimit;
private final ConcurrentHashMap<DexClass, Boolean> knownClasses = new ConcurrentHashMap<>();
- public interface InlinerAction {
- void inline(Map<InvokeMethod, InliningInfo> methods);
- }
-
public ClassInliner(DexItemFactory factory,
LambdaRewriter lambdaRewriter, int totalMethodInstructionLimit) {
this.factory = factory;
@@ -125,13 +119,14 @@
DexEncodedMethod method,
IRCode code,
Predicate<DexEncodedMethod> isProcessedConcurrently,
- InlinerAction inliner,
+ Inliner inliner,
Supplier<InliningOracle> defaultOracle) {
// Collect all the new-instance and static-get instructions in the code before inlining.
- List<Instruction> roots = Streams.stream(code.instructionIterator())
- .filter(insn -> insn.isNewInstance() || insn.isStaticGet())
- .collect(Collectors.toList());
+ List<Instruction> roots =
+ Streams.stream(code.instructionIterator())
+ .filter(insn -> insn.isNewInstance() || insn.isStaticGet())
+ .collect(Collectors.toList());
// We loop inlining iterations until there was no inlining, but still use same set
// of roots to avoid infinite inlining. Looping makes possible for some roots to
@@ -145,20 +140,26 @@
while (rootsIterator.hasNext()) {
Instruction root = rootsIterator.next();
InlineCandidateProcessor processor =
- new InlineCandidateProcessor(factory, appInfo, lambdaRewriter,
+ new InlineCandidateProcessor(
+ factory,
+ appInfo,
+ lambdaRewriter,
+ inliner,
clazz -> isClassEligible(appInfo, clazz),
- isProcessedConcurrently, method, root);
+ isProcessedConcurrently,
+ method,
+ root);
// Assess eligibility of instance and class.
if (!processor.isInstanceEligible() ||
!processor.isClassAndUsageEligible()) {
// This root will never be inlined.
- rootsIterator.remove();
continue;
}
// Assess users eligibility and compute inlining of direct calls and extra methods needed.
- if (!processor.areInstanceUsersEligible(method.method.getHolder(), defaultOracle)) {
+ InstructionOrPhi ineligibleUser = processor.areInstanceUsersEligible(defaultOracle);
+ if (ineligibleUser != null) {
// This root may succeed if users change in future.
continue;
}
@@ -169,7 +170,7 @@
}
// Inline the class instance.
- boolean anyInlinedMethods = processor.processInlining(code, inliner);
+ boolean anyInlinedMethods = processor.processInlining(code, defaultOracle);
// Restore normality.
code.removeAllTrivialPhis();
@@ -183,7 +184,7 @@
} while (repeat);
}
- private boolean isClassEligible(AppInfo appInfo, DexClass clazz) {
+ private boolean isClassEligible(AppInfoWithLiveness appInfo, DexClass clazz) {
Boolean eligible = knownClasses.get(clazz);
if (eligible == null) {
Boolean computed = computeClassEligible(appInfo, clazz);
@@ -198,9 +199,12 @@
// - is not an abstract class or interface
// - does not declare finalizer
// - does not trigger any static initializers except for its own
- private boolean computeClassEligible(AppInfo appInfo, DexClass clazz) {
- if (clazz == null || clazz.isLibraryClass() ||
- clazz.accessFlags.isAbstract() || clazz.accessFlags.isInterface()) {
+ private boolean computeClassEligible(AppInfoWithLiveness appInfo, DexClass clazz) {
+ if (clazz == null
+ || clazz.isLibraryClass()
+ || clazz.accessFlags.isAbstract()
+ || clazz.accessFlags.isInterface()
+ || appInfo.neverClassInline.contains(clazz.type)) {
return false;
}
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 9baa647..c669507 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
@@ -24,17 +24,18 @@
import com.android.tools.r8.ir.code.If;
import com.android.tools.r8.ir.code.InstanceGet;
import com.android.tools.r8.ir.code.Instruction;
+import com.android.tools.r8.ir.code.InstructionOrPhi;
import com.android.tools.r8.ir.code.Invoke.Type;
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.Value;
import com.android.tools.r8.ir.desugar.LambdaRewriter;
+import com.android.tools.r8.ir.optimize.Inliner;
import com.android.tools.r8.ir.optimize.Inliner.InlineAction;
import com.android.tools.r8.ir.optimize.Inliner.InliningInfo;
import com.android.tools.r8.ir.optimize.Inliner.Reason;
import com.android.tools.r8.ir.optimize.InliningOracle;
-import com.android.tools.r8.ir.optimize.classinliner.ClassInliner.InlinerAction;
import com.android.tools.r8.kotlin.KotlinInfo;
import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
import com.android.tools.r8.utils.Pair;
@@ -56,6 +57,7 @@
private final DexItemFactory factory;
private final AppInfoWithLiveness appInfo;
private final LambdaRewriter lambdaRewriter;
+ private final Inliner inliner;
private final Predicate<DexClass> isClassEligible;
private final Predicate<DexEncodedMethod> isProcessedConcurrently;
private final DexEncodedMethod method;
@@ -76,12 +78,17 @@
private int estimatedCombinedSizeForInlining = 0;
InlineCandidateProcessor(
- DexItemFactory factory, AppInfoWithLiveness appInfo,
- LambdaRewriter lambdaRewriter, Predicate<DexClass> isClassEligible,
+ DexItemFactory factory,
+ AppInfoWithLiveness appInfo,
+ LambdaRewriter lambdaRewriter,
+ Inliner inliner,
+ Predicate<DexClass> isClassEligible,
Predicate<DexEncodedMethod> isProcessedConcurrently,
- DexEncodedMethod method, Instruction root) {
+ DexEncodedMethod method,
+ Instruction root) {
this.factory = factory;
this.lambdaRewriter = lambdaRewriter;
+ this.inliner = inliner;
this.isClassEligible = isClassEligible;
this.method = method;
this.root = root;
@@ -101,8 +108,8 @@
return false;
}
- eligibleClass = isNewInstance() ?
- root.asNewInstance().clazz : root.asStaticGet().getField().type;
+ eligibleClass =
+ root.isNewInstance() ? root.asNewInstance().clazz : root.asStaticGet().getField().type;
eligibleClassDefinition = appInfo.definitionFor(eligibleClass);
if (eligibleClassDefinition == null && lambdaRewriter != null) {
// Check if the class is synthesized for a desugared lambda
@@ -128,7 +135,7 @@
return false;
}
- if (isNewInstance()) {
+ if (root.isNewInstance()) {
// NOTE: if the eligible class does not directly extend java.lang.Object,
// we also have to guarantee that it is initialized with initializer classified as
// TrivialInstanceInitializer. This will be checked in areInstanceUsersEligible(...).
@@ -215,53 +222,78 @@
!appInfo.isPinned(eligibleClassDefinition.lookupStaticField(instanceField).field);
}
- // Checks if the inlining candidate instance users are eligible,
- // see comment on processMethodCode(...).
- boolean areInstanceUsersEligible(
- DexType invocationContext, Supplier<InliningOracle> defaultOracle) {
+ /**
+ * Checks if the inlining candidate instance users are eligible, see comment on {@link
+ * ClassInliner#processMethodCode}.
+ *
+ * @return null if all users are eligible, or the first ineligible user.
+ */
+ protected InstructionOrPhi areInstanceUsersEligible(Supplier<InliningOracle> defaultOracle) {
// No Phi users.
if (eligibleInstance.numberOfPhiUsers() > 0) {
- return false; // Not eligible.
+ return eligibleInstance.firstPhiUser(); // Not eligible.
}
for (Instruction user : eligibleInstance.uniqueUsers()) {
// Field read/write.
- if (user.isInstanceGet() ||
- (user.isInstancePut() && user.asInstancePut().value() != eligibleInstance)) {
+ if (user.isInstanceGet()
+ || (user.isInstancePut() && user.asInstancePut().value() != eligibleInstance)) {
DexField field = user.asFieldInstruction().getField();
- if (field.clazz == eligibleClass &&
- eligibleClassDefinition.lookupInstanceField(field) != null) {
+ if (field.clazz == eligibleClass
+ && eligibleClassDefinition.lookupInstanceField(field) != null) {
// Since class inliner currently only supports classes directly extending
// java.lang.Object, we don't need to worry about fields defined in superclasses.
continue;
}
- return false; // Not eligible.
+ return user; // Not eligible.
}
- // Eligible constructor call (for new instance roots only).
- if (user.isInvokeDirect() && root.isNewInstance()) {
- InliningInfo inliningInfo = isEligibleConstructorCall(user.asInvokeDirect());
- if (inliningInfo != null) {
- methodCallsOnInstance.put(user.asInvokeDirect(), inliningInfo);
- continue;
- }
- }
-
- // Eligible virtual method call on the instance as a receiver.
- if (user.isInvokeVirtual() || user.isInvokeInterface()) {
- InliningInfo inliningInfo =
- isEligibleDirectVirtualMethodCall(user.asInvokeMethodWithReceiver());
- if (inliningInfo != null) {
- methodCallsOnInstance.put(user.asInvokeMethodWithReceiver(), inliningInfo);
- continue;
- }
- }
-
- // Eligible usage as an invocation argument.
if (user.isInvokeMethod()) {
- if (isExtraMethodCallEligible(defaultOracle, user.asInvokeMethod(), invocationContext)) {
- continue;
+ InvokeMethod invokeMethod = user.asInvokeMethod();
+ DexEncodedMethod singleTarget = findSingleTarget(invokeMethod);
+ if (!isEligibleSingleTarget(singleTarget)) {
+ return user; // Not eligible.
}
+
+ // Eligible constructor call (for new instance roots only).
+ if (user.isInvokeDirect()) {
+ InvokeDirect invoke = user.asInvokeDirect();
+ if (factory.isConstructor(invoke.getInvokedMethod())) {
+ boolean isCorrespondingConstructorCall =
+ root.isNewInstance()
+ && !invoke.inValues().isEmpty()
+ && root.outValue() == invoke.inValues().get(0);
+ if (isCorrespondingConstructorCall) {
+ InliningInfo inliningInfo =
+ isEligibleConstructorCall(user.asInvokeDirect(), singleTarget, defaultOracle);
+ if (inliningInfo != null) {
+ methodCallsOnInstance.put(user.asInvokeDirect(), inliningInfo);
+ continue;
+ }
+ }
+ }
+ }
+
+ // Eligible virtual method call on the instance as a receiver.
+ if (user.isInvokeVirtual() || user.isInvokeInterface()) {
+ InvokeMethodWithReceiver invoke = user.asInvokeMethodWithReceiver();
+ InliningInfo inliningInfo = isEligibleDirectVirtualMethodCall(invoke, singleTarget);
+ if (inliningInfo != null) {
+ methodCallsOnInstance.put(invoke, inliningInfo);
+ continue;
+ }
+ }
+
+ // Eligible usage as an invocation argument.
+ if (isExtraMethodCall(invokeMethod)) {
+ assert !invokeMethod.isInvokeSuper();
+ assert !invokeMethod.isInvokePolymorphic();
+ if (isExtraMethodCallEligible(invokeMethod, singleTarget, defaultOracle)) {
+ continue;
+ }
+ }
+
+ return user; // Not eligible.
}
// Allow some IF instructions.
@@ -274,9 +306,9 @@
}
}
- return false; // Not eligible.
+ return user; // Not eligible.
}
- return true;
+ return null; // Eligible.
}
// Process inlining, includes the following steps:
@@ -289,10 +321,34 @@
// * remove root instruction
//
// Returns `true` if at least one method was inlined.
- boolean processInlining(IRCode code, InlinerAction inliner) {
+ boolean processInlining(IRCode code, Supplier<InliningOracle> defaultOracle) {
replaceUsagesAsUnusedArgument(code);
- boolean anyInlinedMethods = forceInlineExtraMethodInvocations(inliner);
- anyInlinedMethods |= forceInlineDirectMethodInvocations(inliner);
+
+ boolean anyInlinedMethods = forceInlineExtraMethodInvocations(code);
+ if (anyInlinedMethods) {
+ // Reset the collections.
+ methodCallsOnInstance.clear();
+ extraMethodCalls.clear();
+ unusedArguments.clear();
+ estimatedCombinedSizeForInlining = 0;
+
+ // Repeat user analysis
+ InstructionOrPhi ineligibleUser = areInstanceUsersEligible(defaultOracle);
+ if (ineligibleUser != null) {
+ // We introduced a user that we cannot handle in the class inliner as a result of force
+ // inlining. Abort gracefully from class inlining without removing the instance.
+ //
+ // Alternatively we would need to collect additional information about the behavior of
+ // methods (which is bad for memory), or we would need to analyze the called methods before
+ // inlining them. The latter could be good solution, since we are going to build IR for the
+ // methods that need to be inlined anyway.
+ return true;
+ }
+ assert extraMethodCalls.isEmpty();
+ assert unusedArguments.isEmpty();
+ }
+
+ anyInlinedMethods |= forceInlineDirectMethodInvocations(code);
removeMiscUsages(code);
removeFieldReads(code);
removeFieldWrites();
@@ -317,36 +373,20 @@
unusedArguments.clear();
}
- private boolean forceInlineExtraMethodInvocations(InlinerAction inliner) {
+ private boolean forceInlineExtraMethodInvocations(IRCode code) {
if (extraMethodCalls.isEmpty()) {
return false;
}
-
// Inline extra methods.
- inliner.inline(extraMethodCalls);
-
- // Reset the collections.
- methodCallsOnInstance.clear();
- extraMethodCalls.clear();
- unusedArguments.clear();
- estimatedCombinedSizeForInlining = 0;
-
- // Repeat user analysis
- if (!areInstanceUsersEligible(null, () -> {
- throw new Unreachable("Inlining oracle is expected to be needed");
- })) {
- throw new Unreachable("Analysis must succeed after inlining of extra methods");
- }
- assert extraMethodCalls.isEmpty();
- assert unusedArguments.isEmpty();
+ inliner.performForcedInlining(method, code, extraMethodCalls);
return true;
}
- private boolean forceInlineDirectMethodInvocations(InlinerAction inliner) {
+ private boolean forceInlineDirectMethodInvocations(IRCode code) {
if (methodCallsOnInstance.isEmpty()) {
return false;
}
- inliner.inline(methodCallsOnInstance);
+ inliner.performForcedInlining(method, code, methodCallsOnInstance);
return true;
}
@@ -390,7 +430,11 @@
continue;
}
- throw new Unreachable("Unexpected usage left after method inlining: " + user);
+ throw new Unreachable(
+ "Unexpected usage left in method `"
+ + method.method.toSourceString()
+ + "` after inlining: "
+ + user);
}
if (needToRemoveUnreachableBlocks) {
@@ -414,7 +458,11 @@
continue;
}
- throw new Unreachable("Unexpected usage left after method inlining: " + user);
+ throw new Unreachable(
+ "Unexpected usage left in method `"
+ + method.method.toSourceString()
+ + "` after inlining: "
+ + user);
}
}
@@ -437,45 +485,57 @@
private void removeFieldWrites() {
for (Instruction user : eligibleInstance.uniqueUsers()) {
if (!user.isInstancePut()) {
- throw new Unreachable("Unexpected usage left after field reads removed: " + user);
+ throw new Unreachable(
+ "Unexpected usage left in method `"
+ + method.method.toSourceString()
+ + "` after field reads removed: "
+ + user);
}
if (user.asInstancePut().getField().clazz != eligibleClass) {
- throw new Unreachable("Unexpected field write left after field reads removed: " + user);
+ throw new Unreachable(
+ "Unexpected field write left in method `"
+ + method.method.toSourceString()
+ + "` after field reads removed: "
+ + user);
}
removeInstruction(user);
}
}
- private InliningInfo isEligibleConstructorCall(InvokeDirect initInvoke) {
- // Must be a constructor of the exact same class.
- DexMethod init = initInvoke.getInvokedMethod();
- if (!factory.isConstructor(init)) {
- return null;
- }
+ private InliningInfo isEligibleConstructorCall(
+ InvokeDirect invoke, DexEncodedMethod singleTarget, Supplier<InliningOracle> defaultOracle) {
+ assert factory.isConstructor(invoke.getInvokedMethod());
+ assert isEligibleSingleTarget(singleTarget);
+
// Must be a constructor called on the receiver.
- if (initInvoke.inValues().lastIndexOf(eligibleInstance) != 0) {
+ if (invoke.inValues().lastIndexOf(eligibleInstance) != 0) {
return null;
}
- assert init.holder == eligibleClass
- : "Inlined constructor? [invoke: " + initInvoke +
- ", expected class: " + eligibleClass + "]";
+ DexMethod init = invoke.getInvokedMethod();
+ // Must be a constructor of the exact same class.
+ if (init.holder != eligibleClass) {
+ // Calling a constructor on a class that is different from the type of the instance.
+ // Gracefully abort class inlining (see the test B116282409).
+ return null;
+ }
- DexEncodedMethod definition = findSingleTarget(init, true);
- if (definition == null || isProcessedConcurrently.test(definition)) {
+ // Check that the `eligibleInstance` does not escape via the constructor.
+ ParameterUsage parameterUsage = singleTarget.getOptimizationInfo().getParameterUsages(0);
+ if (!isEligibleParameterUsage(parameterUsage, invoke, defaultOracle)) {
return null;
}
// Don't inline code w/o normal returns into block with catch handlers (b/64432527).
- if (initInvoke.getBlock().hasCatchHandlers() &&
- definition.getOptimizationInfo().neverReturnsNormally()) {
+ if (invoke.getBlock().hasCatchHandlers()
+ && singleTarget.getOptimizationInfo().neverReturnsNormally()) {
return null;
}
if (isDesugaredLambda) {
// Lambda desugaring synthesizes eligible constructors.
- markSizeForInlining(definition);
- return new InliningInfo(definition, eligibleClass);
+ markSizeForInlining(singleTarget);
+ return new InliningInfo(singleTarget, eligibleClass);
}
// If the superclass of the initializer is NOT java.lang.Object, the super class
@@ -485,34 +545,27 @@
// any class initializers in superclass chain or in superinterfaces, see
// details in ClassInliner::computeClassEligible(...).
if (eligibleClassDefinition.superType != factory.objectType) {
- TrivialInitializer info = definition.getOptimizationInfo().getTrivialInitializerInfo();
+ TrivialInitializer info = singleTarget.getOptimizationInfo().getTrivialInitializerInfo();
if (!(info instanceof TrivialInstanceInitializer)) {
return null;
}
}
- if (!definition.isInliningCandidate(method, Reason.SIMPLE, appInfo)) {
- // We won't be able to inline it here.
-
- // Note that there may be some false negatives here since the method may
- // reference private fields of its class which are supposed to be replaced
- // with arguments after inlining. We should try and improve it later.
-
- // Using -allowaccessmodification mitigates this.
- return null;
- }
-
- return definition.getOptimizationInfo().getClassInlinerEligibility() != null
- ? new InliningInfo(definition, eligibleClass) : null;
+ return singleTarget.getOptimizationInfo().getClassInlinerEligibility() != null
+ ? new InliningInfo(singleTarget, eligibleClass)
+ : null;
}
- private InliningInfo isEligibleDirectVirtualMethodCall(InvokeMethodWithReceiver invoke) {
+ private InliningInfo isEligibleDirectVirtualMethodCall(
+ InvokeMethodWithReceiver invoke, DexEncodedMethod singleTarget) {
+ assert isEligibleSingleTarget(singleTarget);
if (invoke.inValues().lastIndexOf(eligibleInstance) > 0) {
return null; // Instance passed as an argument.
}
return isEligibleVirtualMethodCall(
!invoke.getBlock().hasCatchHandlers(),
invoke.getInvokedMethod(),
+ singleTarget,
eligibility ->
!eligibility.returnsReceiver
|| invoke.outValue() == null
@@ -520,13 +573,21 @@
}
private InliningInfo isEligibleIndirectVirtualMethodCall(DexMethod callee) {
- return isEligibleVirtualMethodCall(false, callee, eligibility -> !eligibility.returnsReceiver);
+ DexEncodedMethod singleTarget = eligibleClassDefinition.lookupVirtualMethod(callee);
+ if (isEligibleSingleTarget(singleTarget)) {
+ return isEligibleVirtualMethodCall(
+ false, callee, singleTarget, eligibility -> !eligibility.returnsReceiver);
+ }
+ return null;
}
private InliningInfo isEligibleVirtualMethodCall(
boolean allowMethodsWithoutNormalReturns,
DexMethod callee,
+ DexEncodedMethod singleTarget,
Predicate<ClassInlinerEligibility> eligibilityAcceptanceCheck) {
+ assert isEligibleSingleTarget(singleTarget);
+
// We should not inline a method if the invocation has type interface or virtual and the
// signature of the invocation resolves to a private or static method.
ResolutionResult resolutionResult = appInfo.resolveMethod(callee.holder, callee);
@@ -534,23 +595,13 @@
&& !resolutionResult.asSingleTarget().isVirtualMethod()) {
return null;
}
-
- DexEncodedMethod singleTarget = findSingleTarget(callee, false);
- if (singleTarget == null
- || !singleTarget.isVirtualMethod()
- || isProcessedConcurrently.test(singleTarget)) {
+ if (!singleTarget.isVirtualMethod()) {
return null;
}
if (method == singleTarget) {
return null; // Don't inline itself.
}
-
if (isDesugaredLambda) {
- // If this is the call to method of the desugared lambda, we consider only calls
- // to main lambda method eligible (for both direct and indirect calls).
- if (singleTarget.accessFlags.isBridge()) {
- return null;
- }
markSizeForInlining(singleTarget);
return new InliningInfo(singleTarget, eligibleClass);
}
@@ -573,21 +624,27 @@
return null;
}
- if (!singleTarget.isInliningCandidate(method, Reason.SIMPLE, appInfo)) {
- // We won't be able to inline it here.
-
- // Note that there may be some false negatives here since the method may
- // reference private fields of its class which are supposed to be replaced
- // with arguments after inlining. We should try and improve it later.
-
- // Using -allowaccessmodification mitigates this.
- return null;
- }
-
markSizeForInlining(singleTarget);
return new InliningInfo(singleTarget, eligibleClass);
}
+ private boolean isExtraMethodCall(InvokeMethod invoke) {
+ if (invoke.isInvokeDirect() && factory.isConstructor(invoke.getInvokedMethod())) {
+ return false;
+ }
+ if (invoke.isInvokeMethodWithReceiver()
+ && invoke.asInvokeMethodWithReceiver().getReceiver() == eligibleInstance) {
+ return false;
+ }
+ if (invoke.isInvokeSuper()) {
+ return false;
+ }
+ if (invoke.isInvokePolymorphic()) {
+ return false;
+ }
+ return true;
+ }
+
// Analyzes if a method invoke the eligible instance is passed to is eligible. In short,
// it can be eligible if:
//
@@ -607,42 +664,52 @@
// -- method itself can be inlined
//
private boolean isExtraMethodCallEligible(
- Supplier<InliningOracle> defaultOracle,
- InvokeMethod invokeMethod,
- DexType invocationContext) {
+ InvokeMethod invoke, DexEncodedMethod singleTarget, Supplier<InliningOracle> defaultOracle) {
+ // Don't consider constructor invocations and super calls, since we don't want to forcibly
+ // inline them.
+ assert isExtraMethodCall(invoke);
+ assert isEligibleSingleTarget(singleTarget);
- List<Value> arguments = Lists.newArrayList(invokeMethod.inValues());
+ List<Value> arguments = Lists.newArrayList(invoke.inValues());
- // Don't consider constructor invocations and super calls, since
- // we don't want to forcibly inline them.
- if (invokeMethod.isInvokeSuper() ||
- (invokeMethod.isInvokeDirect() && factory.isConstructor(invokeMethod.getInvokedMethod()))) {
+ // If we got here with invocation on receiver the user is ineligible.
+ if (invoke.isInvokeMethodWithReceiver() && arguments.get(0) == eligibleInstance) {
return false;
}
- // Remove receiver from arguments.
- if (invokeMethod.isInvokeMethodWithReceiver()) {
- if (arguments.get(0) == eligibleInstance) {
- // If we got here with invocation on receiver the user is ineligible.
- return false;
- }
- arguments.remove(0);
- }
-
- // Need single target.
- DexEncodedMethod singleTarget = invokeMethod.lookupSingleTarget(appInfo, invocationContext);
- if (singleTarget == null || isProcessedConcurrently.test(singleTarget)) {
- return false; // Not eligible.
- }
-
OptimizationInfo optimizationInfo = singleTarget.getOptimizationInfo();
// Don't inline code w/o normal returns into block with catch handlers (b/64432527).
- if (invokeMethod.getBlock().hasCatchHandlers() && optimizationInfo.neverReturnsNormally()) {
+ if (invoke.getBlock().hasCatchHandlers() && optimizationInfo.neverReturnsNormally()) {
return false;
}
// Go through all arguments, see if all usages of eligibleInstance are good.
+ if (!isEligibleParameterUsages(invoke, arguments, singleTarget, defaultOracle)) {
+ return false;
+ }
+
+ for (int argIndex = 0; argIndex < arguments.size(); argIndex++) {
+ Value argument = arguments.get(argIndex);
+ if (argument == eligibleInstance && optimizationInfo.getParameterUsages(argIndex).notUsed()) {
+ // Reference can be removed since it's not used.
+ unusedArguments.add(new Pair<>(invoke, argIndex));
+ }
+ }
+
+ extraMethodCalls.put(invoke, new InliningInfo(singleTarget, null));
+
+ // Looks good.
+ markSizeForInlining(singleTarget);
+ return true;
+ }
+
+ private boolean isEligibleParameterUsages(
+ InvokeMethod invoke,
+ List<Value> arguments,
+ DexEncodedMethod singleTarget,
+ Supplier<InliningOracle> defaultOracle) {
+ // Go through all arguments, see if all usages of eligibleInstance are good.
for (int argIndex = 0; argIndex < arguments.size(); argIndex++) {
Value argument = arguments.get(argIndex);
if (argument != eligibleInstance) {
@@ -650,56 +717,79 @@
}
// Have parameter usage info?
+ OptimizationInfo optimizationInfo = singleTarget.getOptimizationInfo();
ParameterUsage parameterUsage = optimizationInfo.getParameterUsages(argIndex);
- if (parameterUsage == null) {
- return false; // Don't know anything.
+ if (!isEligibleParameterUsage(parameterUsage, invoke, defaultOracle)) {
+ return false;
}
+ }
+ return true;
+ }
- if (parameterUsage.notUsed()) {
- // Reference can be removed since it's not used.
- unusedArguments.add(new Pair<>(invokeMethod, argIndex));
- continue;
- }
+ private boolean isEligibleParameterUsage(
+ ParameterUsage parameterUsage, InvokeMethod invoke, Supplier<InliningOracle> defaultOracle) {
+ if (parameterUsage == null) {
+ return false; // Don't know anything.
+ }
- if (parameterUsage.returnValue &&
- !(invokeMethod.outValue() == null || invokeMethod.outValue().numberOfAllUsers() == 0)) {
+ if (parameterUsage.notUsed()) {
+ return true;
+ }
+
+ if (parameterUsage.isAssignedToField) {
+ return false;
+ }
+
+ if (parameterUsage.isReturned) {
+ if (!(invoke.outValue() == null || invoke.outValue().numberOfAllUsers() == 0)) {
// Used as return value which is not ignored.
return false;
}
+ }
- if (!Sets.difference(parameterUsage.ifZeroTest, ALLOWED_ZERO_TEST_TYPES).isEmpty()) {
- // Used in unsupported zero-check-if kinds.
- return false;
- }
+ if (!Sets.difference(parameterUsage.ifZeroTest, ALLOWED_ZERO_TEST_TYPES).isEmpty()) {
+ // Used in unsupported zero-check-if kinds.
+ return false;
+ }
- for (Pair<Type, DexMethod> call : parameterUsage.callsReceiver) {
- if (call.getFirst() != Type.VIRTUAL && call.getFirst() != Type.INTERFACE) {
- // Don't support direct and super calls yet.
- return false;
- }
+ for (Pair<Type, DexMethod> call : parameterUsage.callsReceiver) {
+ Type type = call.getFirst();
+ DexMethod target = call.getSecond();
+ if (type == Type.VIRTUAL || type == Type.INTERFACE) {
// Is the method called indirectly still eligible?
- InliningInfo potentialInliningInfo = isEligibleIndirectVirtualMethodCall(call.getSecond());
+ InliningInfo potentialInliningInfo = isEligibleIndirectVirtualMethodCall(target);
if (potentialInliningInfo == null) {
return false;
}
-
- // Check if the method is inline-able by standard inliner.
- InlineAction inlineAction =
- invokeMethod.computeInlining(defaultOracle.get(), method.method.holder);
- if (inlineAction == null) {
+ } else if (type == Type.DIRECT) {
+ if (!isTrivialInitializer(target)) {
+ // Only calls to trivial initializers are supported at this point.
return false;
}
+ } else {
+ // Static and super calls are not yet supported.
+ return false;
}
- extraMethodCalls.put(invokeMethod, new InliningInfo(singleTarget, null));
+ // Check if the method is inline-able by standard inliner.
+ InlineAction inlineAction = invoke.computeInlining(defaultOracle.get(), method.method.holder);
+ if (inlineAction == null) {
+ return false;
+ }
}
-
- // Looks good.
- markSizeForInlining(singleTarget);
return true;
}
+ private boolean isTrivialInitializer(DexMethod method) {
+ if (method == appInfo.dexItemFactory.objectMethods.constructor) {
+ return true;
+ }
+ DexEncodedMethod encodedMethod = appInfo.definitionFor(method);
+ return encodedMethod != null
+ && encodedMethod.getOptimizationInfo().getTrivialInitializerInfo() != null;
+ }
+
private boolean exemptFromInstructionLimit(DexEncodedMethod inlinee) {
DexType inlineeHolder = inlinee.method.holder;
if (isDesugaredLambda && inlineeHolder == eligibleClass) {
@@ -723,20 +813,49 @@
}
}
- private boolean isNewInstance() {
- return root.isNewInstance();
- }
-
- private DexEncodedMethod findSingleTarget(DexMethod callee, boolean isDirect) {
+ private DexEncodedMethod findSingleTarget(InvokeMethod invoke) {
+ if (isExtraMethodCall(invoke)) {
+ DexType invocationContext = method.method.holder;
+ return invoke.lookupSingleTarget(appInfo, invocationContext);
+ }
// We don't use computeSingleTarget(...) on invoke since it sometimes fails to
// find the single target, while this code may be more successful since we exactly
// know what is the actual type of the receiver.
// Note that we also intentionally limit ourselves to methods directly defined in
// the instance's class. This may be improved later.
- return isDirect
- ? eligibleClassDefinition.lookupDirectMethod(callee)
- : eligibleClassDefinition.lookupVirtualMethod(callee);
+ return invoke.isInvokeDirect()
+ ? eligibleClassDefinition.lookupDirectMethod(invoke.getInvokedMethod())
+ : eligibleClassDefinition.lookupVirtualMethod(invoke.getInvokedMethod());
+ }
+
+ private boolean isEligibleSingleTarget(DexEncodedMethod singleTarget) {
+ if (singleTarget == null) {
+ return false;
+ }
+ if (isProcessedConcurrently.test(singleTarget)) {
+ return false;
+ }
+ if (isDesugaredLambda && !singleTarget.accessFlags.isBridge()) {
+ // OK if this is the call to the main method of a desugared lambda (for both direct and
+ // indirect calls).
+ //
+ // Note: This is needed because lambda methods are generally processed in the same batch as
+ // they are class inlined, which means that the call to isInliningCandidate() below will
+ // return false.
+ return true;
+ }
+ if (!singleTarget.isInliningCandidate(method, Reason.SIMPLE, appInfo)) {
+ // If `singleTarget` is not an inlining candidate, we won't be able to inline it here.
+ //
+ // Note that there may be some false negatives here since the method may
+ // reference private fields of its class which are supposed to be replaced
+ // with arguments after inlining. We should try and improve it later.
+ //
+ // Using -allowaccessmodification mitigates this.
+ return false;
+ }
+ return true;
}
private void removeInstruction(Instruction instruction) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/staticizer/StaticizingProcessor.java b/src/main/java/com/android/tools/r8/ir/optimize/staticizer/StaticizingProcessor.java
index 79f18ed..ee7edd0 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/staticizer/StaticizingProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/staticizer/StaticizingProcessor.java
@@ -81,14 +81,12 @@
processMethodsConcurrently(methodsToBeStaticized, this::removeReferencesToThis, feedback);
// Convert instance methods into static methods with an extra parameter.
- Set<DexEncodedMethod> staticizedMethods = staticizeMethodSymbols();
+ Set<DexEncodedMethod> methods = staticizeMethodSymbols();
// Process all other methods that may reference singleton fields and call methods on them.
// (Note that we exclude the former instance methods, but include new static methods created as
// a result of staticizing.)
- Set<DexEncodedMethod> methods = Sets.newIdentityHashSet();
methods.addAll(referencingExtraMethods);
- methods.addAll(staticizedMethods);
methods.addAll(hostClassInits.keySet());
processMethodsConcurrently(methods, this::rewriteReferences, feedback);
}
diff --git a/src/main/java/com/android/tools/r8/shaking/ClassInlineRule.java b/src/main/java/com/android/tools/r8/shaking/ClassInlineRule.java
new file mode 100644
index 0000000..e7dc76c
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/shaking/ClassInlineRule.java
@@ -0,0 +1,105 @@
+// Copyright (c) 2018, 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.shaking;
+
+import com.android.tools.r8.errors.Unreachable;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.position.Position;
+import java.util.List;
+
+public class ClassInlineRule extends ProguardConfigurationRule {
+
+ public enum Type {
+ NEVER
+ }
+
+ public static class Builder extends ProguardConfigurationRule.Builder<ClassInlineRule, Builder> {
+
+ private Builder() {
+ super();
+ }
+
+ Type type;
+
+ @Override
+ public Builder self() {
+ return this;
+ }
+
+ public Builder setType(Type type) {
+ this.type = type;
+ return this;
+ }
+
+ @Override
+ public ClassInlineRule build() {
+ return new ClassInlineRule(
+ origin,
+ getPosition(),
+ source,
+ classAnnotation,
+ classAccessFlags,
+ negatedClassAccessFlags,
+ classTypeNegated,
+ classType,
+ classNames,
+ inheritanceAnnotation,
+ inheritanceClassName,
+ inheritanceIsExtends,
+ memberRules,
+ type);
+ }
+ }
+
+ private final Type type;
+
+ protected ClassInlineRule(
+ Origin origin,
+ Position position,
+ String source,
+ ProguardTypeMatcher classAnnotation,
+ ProguardAccessFlags classAccessFlags,
+ ProguardAccessFlags negatedClassAccessFlags,
+ boolean classTypeNegated,
+ ProguardClassType classType,
+ ProguardClassNameList classNames,
+ ProguardTypeMatcher inheritanceAnnotation,
+ ProguardTypeMatcher inheritanceClassName,
+ boolean inheritanceIsExtends,
+ List<ProguardMemberRule> memberRules,
+ Type type) {
+ super(
+ origin,
+ position,
+ source,
+ classAnnotation,
+ classAccessFlags,
+ negatedClassAccessFlags,
+ classTypeNegated,
+ classType,
+ classNames,
+ inheritanceAnnotation,
+ inheritanceClassName,
+ inheritanceIsExtends,
+ memberRules);
+ this.type = type;
+ }
+
+ public static Builder builder() {
+ return new Builder();
+ }
+
+ public Type getType() {
+ return type;
+ }
+
+ @Override
+ String typeString() {
+ switch (type) {
+ case NEVER:
+ return "neverclassinline";
+ }
+ throw new Unreachable("Unknown class inline type " + type);
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
index fc95770..31d60ea 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -1807,6 +1807,10 @@
*/
public final Set<DexMethod> neverInline;
/**
+ * All types that *must* never be inlined due to a configuration directive (testing only).
+ */
+ public final Set<DexType> neverClassInline;
+ /**
* All types that *must* never be merged due to a configuration directive (testing only).
*/
public final Set<DexType> neverMerge;
@@ -1869,6 +1873,7 @@
this.alwaysInline = enqueuer.rootSet.alwaysInline;
this.forceInline = enqueuer.rootSet.forceInline;
this.neverInline = enqueuer.rootSet.neverInline;
+ this.neverClassInline = enqueuer.rootSet.neverClassInline;
this.neverMerge = enqueuer.rootSet.neverMerge;
this.identifierNameStrings = joinIdentifierNameStrings(
enqueuer.rootSet.identifierNameStrings, enqueuer.identifierNameStrings);
@@ -1912,6 +1917,7 @@
this.alwaysInline = previous.alwaysInline;
this.forceInline = previous.forceInline;
this.neverInline = previous.neverInline;
+ this.neverClassInline = previous.neverClassInline;
this.neverMerge = previous.neverMerge;
this.identifierNameStrings = previous.identifierNameStrings;
this.prunedTypes = mergeSets(previous.prunedTypes, removedClasses);
@@ -1968,6 +1974,7 @@
assert lense.assertDefinitionNotModified(
previous.neverMerge.stream().map(this::definitionFor).filter(Objects::nonNull)
.collect(Collectors.toList()));
+ this.neverClassInline = rewriteItems(previous.neverClassInline, lense::lookupType);
this.neverMerge = previous.neverMerge;
this.identifierNameStrings =
lense.rewriteReferencesConservatively(previous.identifierNameStrings);
@@ -2012,6 +2019,7 @@
this.alwaysInline = previous.alwaysInline;
this.forceInline = previous.forceInline;
this.neverInline = previous.neverInline;
+ this.neverClassInline = previous.neverClassInline;
this.neverMerge = previous.neverMerge;
this.identifierNameStrings = previous.identifierNameStrings;
this.prunedTypes = previous.prunedTypes;
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
index 7d5f9a5..7694299 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
@@ -16,7 +16,6 @@
import com.android.tools.r8.position.Position;
import com.android.tools.r8.position.TextPosition;
import com.android.tools.r8.position.TextRange;
-import com.android.tools.r8.shaking.InlineRule.Type;
import com.android.tools.r8.shaking.ProguardConfiguration.Builder;
import com.android.tools.r8.shaking.ProguardTypeMatcher.ClassOrType;
import com.android.tools.r8.shaking.ProguardTypeMatcher.MatchSpecificType;
@@ -353,16 +352,19 @@
} else if (acceptString("packageobfuscationdictionary")) {
configurationBuilder.setPackageObfuscationDictionary(parseFileName(false));
} else if (acceptString("alwaysinline")) {
- InlineRule rule = parseInlineRule(Type.ALWAYS, optionStart);
+ InlineRule rule = parseInlineRule(InlineRule.Type.ALWAYS, optionStart);
configurationBuilder.addRule(rule);
} else if (allowTestOptions && acceptString("forceinline")) {
- InlineRule rule = parseInlineRule(Type.FORCE, optionStart);
+ InlineRule rule = parseInlineRule(InlineRule.Type.FORCE, optionStart);
configurationBuilder.addRule(rule);
// Insert a matching -checkdiscard rule to ensure force inlining happens.
ProguardCheckDiscardRule ruled = rule.asProguardCheckDiscardRule();
configurationBuilder.addRule(ruled);
} else if (allowTestOptions && acceptString("neverinline")) {
- InlineRule rule = parseInlineRule(Type.NEVER, optionStart);
+ InlineRule rule = parseInlineRule(InlineRule.Type.NEVER, optionStart);
+ configurationBuilder.addRule(rule);
+ } else if (allowTestOptions && acceptString("neverclassinline")) {
+ ClassInlineRule rule = parseClassInlineRule(ClassInlineRule.Type.NEVER, optionStart);
configurationBuilder.addRule(rule);
} else if (allowTestOptions && acceptString("nevermerge")) {
ClassMergingRule rule = parseClassMergingRule(ClassMergingRule.Type.NEVER, optionStart);
@@ -597,6 +599,17 @@
return keepRuleBuilder.build();
}
+ private ClassInlineRule parseClassInlineRule(ClassInlineRule.Type type, Position start)
+ throws ProguardRuleParserException {
+ ClassInlineRule.Builder keepRuleBuilder =
+ ClassInlineRule.builder().setOrigin(origin).setStart(start).setType(type);
+ parseClassSpec(keepRuleBuilder, false);
+ Position end = getPosition();
+ keepRuleBuilder.setSource(getSourceSnippet(contents, start, end));
+ keepRuleBuilder.setEnd(end);
+ return keepRuleBuilder.build();
+ }
+
private ClassMergingRule parseClassMergingRule(ClassMergingRule.Type type, Position start)
throws ProguardRuleParserException {
ClassMergingRule.Builder keepRuleBuilder =
diff --git a/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java b/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
index d90812c..ff87d3d 100644
--- a/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
+++ b/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
@@ -65,6 +65,7 @@
private final Set<DexMethod> alwaysInline = Sets.newIdentityHashSet();
private final Set<DexMethod> forceInline = Sets.newIdentityHashSet();
private final Set<DexMethod> neverInline = Sets.newIdentityHashSet();
+ private final Set<DexType> neverClassInline = Sets.newIdentityHashSet();
private final Set<DexType> neverMerge = Sets.newIdentityHashSet();
private final Map<DexDefinition, Map<DexDefinition, ProguardKeepRule>> dependentNoShrinking =
new IdentityHashMap<>();
@@ -263,6 +264,10 @@
}
} else if (rule instanceof InlineRule) {
markMatchingMethods(clazz, memberKeepRules, rule, null);
+ } else if (rule instanceof ClassInlineRule) {
+ if (allRulesSatisfied(memberKeepRules, clazz)) {
+ markClass(clazz, rule);
+ }
} else if (rule instanceof ProguardAssumeValuesRule) {
markMatchingVisibleMethods(clazz, memberKeepRules, rule, null);
markMatchingVisibleFields(clazz, memberKeepRules, rule, null);
@@ -333,6 +338,7 @@
alwaysInline,
forceInline,
neverInline,
+ neverClassInline,
neverMerge,
noSideEffects,
assumedValues,
@@ -812,20 +818,26 @@
} else if (context instanceof ProguardCheckDiscardRule) {
checkDiscarded.add(item);
} else if (context instanceof InlineRule) {
- switch (((InlineRule) context).getType()) {
- case ALWAYS:
- if (item.isDexEncodedMethod()) {
+ if (item.isDexEncodedMethod()) {
+ switch (((InlineRule) context).getType()) {
+ case ALWAYS:
alwaysInline.add(item.asDexEncodedMethod().method);
- }
- break;
- case FORCE:
- if (item.isDexEncodedMethod()) {
+ break;
+ case FORCE:
forceInline.add(item.asDexEncodedMethod().method);
- }
- break;
- case NEVER:
- if (item.isDexEncodedMethod()) {
+ break;
+ case NEVER:
neverInline.add(item.asDexEncodedMethod().method);
+ break;
+ default:
+ throw new Unreachable();
+ }
+ }
+ } else if (context instanceof ClassInlineRule) {
+ switch (((ClassInlineRule) context).getType()) {
+ case NEVER:
+ if (item.isDexClass()) {
+ neverClassInline.add(item.asDexClass().type);
}
break;
default:
@@ -861,6 +873,7 @@
public final Set<DexMethod> alwaysInline;
public final Set<DexMethod> forceInline;
public final Set<DexMethod> neverInline;
+ public final Set<DexType> neverClassInline;
public final Set<DexType> neverMerge;
public final Map<DexDefinition, ProguardMemberRule> noSideEffects;
public final Map<DexDefinition, ProguardMemberRule> assumedValues;
@@ -878,6 +891,7 @@
Set<DexMethod> alwaysInline,
Set<DexMethod> forceInline,
Set<DexMethod> neverInline,
+ Set<DexType> neverClassInline,
Set<DexType> neverMerge,
Map<DexDefinition, ProguardMemberRule> noSideEffects,
Map<DexDefinition, ProguardMemberRule> assumedValues,
@@ -893,6 +907,7 @@
this.alwaysInline = Collections.unmodifiableSet(alwaysInline);
this.forceInline = Collections.unmodifiableSet(forceInline);
this.neverInline = Collections.unmodifiableSet(neverInline);
+ this.neverClassInline = Collections.unmodifiableSet(neverClassInline);
this.neverMerge = Collections.unmodifiableSet(neverMerge);
this.noSideEffects = Collections.unmodifiableMap(noSideEffects);
this.assumedValues = Collections.unmodifiableMap(assumedValues);
diff --git a/src/test/examples/inlining/AlwaysInline.java b/src/test/examples/inlining/AlwaysInline.java
index 815d50a..22cb31e 100644
--- a/src/test/examples/inlining/AlwaysInline.java
+++ b/src/test/examples/inlining/AlwaysInline.java
@@ -3,6 +3,8 @@
// BSD-style license that can be found in the LICENSE file.
package inlining;
-public @interface AlwaysInline {
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Target;
-}
+@Target({ElementType.METHOD})
+public @interface AlwaysInline {}
diff --git a/src/test/examples/inlining/NeverInline.java b/src/test/examples/inlining/NeverInline.java
new file mode 100644
index 0000000..0c5b581
--- /dev/null
+++ b/src/test/examples/inlining/NeverInline.java
@@ -0,0 +1,10 @@
+// Copyright (c) 2018, 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 inlining;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Target;
+
+@Target({ElementType.METHOD})
+public @interface NeverInline {}
diff --git a/src/test/java/com/android/tools/r8/NeverClassInline.java b/src/test/java/com/android/tools/r8/NeverClassInline.java
new file mode 100644
index 0000000..aa7f9b5
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/NeverClassInline.java
@@ -0,0 +1,10 @@
+// Copyright (c) 2018, 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;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Target;
+
+@Target({ElementType.TYPE})
+public @interface NeverClassInline {}
diff --git a/src/test/java/com/android/tools/r8/NeverMerge.java b/src/test/java/com/android/tools/r8/NeverMerge.java
index 7fc97b9..7c6922a 100644
--- a/src/test/java/com/android/tools/r8/NeverMerge.java
+++ b/src/test/java/com/android/tools/r8/NeverMerge.java
@@ -3,4 +3,8 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Target;
+
+@Target({ElementType.TYPE})
public @interface NeverMerge {}
diff --git a/src/test/java/com/android/tools/r8/R8RunExamplesAndroidOTest.java b/src/test/java/com/android/tools/r8/R8RunExamplesAndroidOTest.java
index 101fbb8..5f6f362 100644
--- a/src/test/java/com/android/tools/r8/R8RunExamplesAndroidOTest.java
+++ b/src/test/java/com/android/tools/r8/R8RunExamplesAndroidOTest.java
@@ -108,7 +108,9 @@
.withOptionConsumer(opts -> opts.enableClassInlining = true)
.withBuilderTransformation(
b -> b.addProguardConfiguration(PROGUARD_OPTIONS, Origin.unknown()))
- .withDexCheck(inspector -> checkLambdaCount(inspector, 24, "lambdadesugaring"))
+ // TODO(b/120814598): Should be 24. Some lambdas are not class inlined because parameter
+ // usages for lambda methods are not present for the class inliner.
+ .withDexCheck(inspector -> checkLambdaCount(inspector, 46, "lambdadesugaring"))
.run();
}
@@ -128,7 +130,9 @@
.withOptionConsumer(opts -> opts.enableClassInlining = true)
.withBuilderTransformation(
b -> b.addProguardConfiguration(PROGUARD_OPTIONS, Origin.unknown()))
- .withDexCheck(inspector -> checkLambdaCount(inspector, 24, "lambdadesugaring"))
+ // TODO(b/120814598): Should be 24. Some lambdas are not class inlined because parameter
+ // usages for lambda methods are not present for the class inliner.
+ .withDexCheck(inspector -> checkLambdaCount(inspector, 46, "lambdadesugaring"))
.run();
}
@@ -150,7 +154,9 @@
.withOptionConsumer(opts -> opts.enableClassInlining = true)
.withBuilderTransformation(
b -> b.addProguardConfiguration(PROGUARD_OPTIONS_N_PLUS, Origin.unknown()))
- .withDexCheck(inspector -> checkLambdaCount(inspector, 5, "lambdadesugaringnplus"))
+ // TODO(b/120814598): Should be 5. Some lambdas are not class inlined because parameter
+ // usages for lambda methods are not present for the class inliner.
+ .withDexCheck(inspector -> checkLambdaCount(inspector, 22, "lambdadesugaringnplus"))
.run();
}
@@ -172,7 +178,9 @@
.withOptionConsumer(opts -> opts.enableClassInlining = true)
.withBuilderTransformation(
b -> b.addProguardConfiguration(PROGUARD_OPTIONS_N_PLUS, Origin.unknown()))
- .withDexCheck(inspector -> checkLambdaCount(inspector, 5, "lambdadesugaringnplus"))
+ // TODO(b/120814598): Should be 5. Some lambdas are not class inlined because parameter
+ // usages for lambda methods are not present for the class inliner.
+ .withDexCheck(inspector -> checkLambdaCount(inspector, 22, "lambdadesugaringnplus"))
.run();
}
diff --git a/src/test/java/com/android/tools/r8/R8TestBuilder.java b/src/test/java/com/android/tools/r8/R8TestBuilder.java
index 3a249e6..5550415 100644
--- a/src/test/java/com/android/tools/r8/R8TestBuilder.java
+++ b/src/test/java/com/android/tools/r8/R8TestBuilder.java
@@ -30,6 +30,7 @@
}
private boolean enableInliningAnnotations = false;
+ private boolean enableClassInliningAnnotations = false;
private boolean enableMergeAnnotations = false;
@Override
@@ -41,7 +42,7 @@
R8TestCompileResult internalCompile(
Builder builder, Consumer<InternalOptions> optionsConsumer, Supplier<AndroidApp> app)
throws CompilationFailedException {
- if (enableInliningAnnotations || enableMergeAnnotations) {
+ if (enableInliningAnnotations || enableClassInliningAnnotations || enableMergeAnnotations) {
ToolHelper.allowTestProguardOptions(builder);
}
StringBuilder proguardMapBuilder = new StringBuilder();
@@ -71,6 +72,14 @@
return self();
}
+ public R8TestBuilder enableClassInliningAnnotations() {
+ if (!enableClassInliningAnnotations) {
+ enableClassInliningAnnotations = true;
+ addKeepRules("-neverclassinline @com.android.tools.r8.NeverClassInline class *");
+ }
+ return self();
+ }
+
public R8TestBuilder enableMergeAnnotations() {
if (!enableMergeAnnotations) {
enableMergeAnnotations = true;
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/BuilderWithInheritanceTest.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/BuilderWithInheritanceTest.java
new file mode 100644
index 0000000..c3deab3
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/BuilderWithInheritanceTest.java
@@ -0,0 +1,97 @@
+// Copyright (c) 2018, 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.classinliner;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.junit.Assert.assertThat;
+
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.NeverMerge;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+/** Regression test for b/120182628. */
+@RunWith(Parameterized.class)
+public class BuilderWithInheritanceTest extends TestBase {
+
+ private final Backend backend;
+
+ @Parameters(name = "Backend: {0}")
+ public static Backend[] data() {
+ return Backend.values();
+ }
+
+ public BuilderWithInheritanceTest(Backend backend) {
+ this.backend = backend;
+ }
+
+ @Test
+ public void test() throws Exception {
+ CodeInspector inspector =
+ testForR8(backend)
+ .addInnerClasses(BuilderWithInheritanceTest.class)
+ .addKeepMainRule(TestClass.class)
+ .enableInliningAnnotations()
+ .enableMergeAnnotations()
+ .run(TestClass.class)
+ .assertSuccessWithOutput("42")
+ .inspector();
+ assertThat(inspector.clazz(Builder.class), isPresent());
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ A a = new Builder(42).build();
+ System.out.print(a.get());
+ }
+ }
+
+ static class A {
+
+ private final int f;
+
+ public A(int f) {
+ this.f = f;
+ }
+
+ public int get() {
+ return f;
+ }
+ }
+
+ @NeverMerge
+ static class BuilderBase {
+
+ protected int f;
+
+ public BuilderBase(int f) {
+ this.f = f;
+ }
+
+ @NeverInline
+ public static int get(BuilderBase obj) {
+ return obj.f;
+ }
+ }
+
+ static class Builder extends BuilderBase {
+
+ public Builder(int f) {
+ super(f);
+ }
+
+ public A build() {
+ // After force inlining of get() there will be a field-read "this.f", which is not allowed by
+ // the class inliner, because the class inliner only handles reads from fields that are
+ // declared on Builder.
+ return new A(get(this));
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java
index caa697b..585a696 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.ir.optimize.classinliner;
+import static com.android.tools.r8.ir.desugar.LambdaRewriter.LAMBDA_CLASS_NAME_PREFIX;
import static org.hamcrest.CoreMatchers.containsString;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
@@ -43,6 +44,7 @@
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import com.android.tools.r8.utils.codeinspector.FieldAccessInstructionSubject;
+import com.android.tools.r8.utils.codeinspector.FoundClassSubject;
import com.android.tools.r8.utils.codeinspector.InstructionSubject;
import com.android.tools.r8.utils.codeinspector.NewInstanceInstructionSubject;
import com.google.common.collect.Sets;
@@ -352,14 +354,23 @@
"java.lang.StringBuilder"),
collectTypes(clazz, "testStatelessLambda", "void"));
+ // TODO(b/120814598): Should only be "java.lang.StringBuilder". Lambdas are not class inlined
+ // because parameter usage is not available for each lambda constructor.
+ Set<String> expectedTypes = Sets.newHashSet("java.lang.StringBuilder");
+ expectedTypes.addAll(
+ inspector.allClasses().stream()
+ .map(FoundClassSubject::getFinalName)
+ .filter(name -> name.contains(LAMBDA_CLASS_NAME_PREFIX))
+ .collect(Collectors.toList()));
assertEquals(
- Sets.newHashSet(
- "java.lang.StringBuilder"),
+ expectedTypes,
collectTypes(clazz, "testStatefulLambda", "void", "java.lang.String", "java.lang.String"));
- assertEquals(0,
- inspector.allClasses().stream()
- .filter(ClassSubject::isSynthesizedJavaLambdaClass).count());
+ // TODO(b/120814598): Should be 0. Lambdas are not class inlined because parameter usage is not
+ // available for each lambda constructor.
+ assertEquals(
+ 3,
+ inspector.allClasses().stream().filter(ClassSubject::isSynthesizedJavaLambdaClass).count());
}
private Set<String> collectTypes(
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliningOracleTest.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliningOracleTest.java
new file mode 100644
index 0000000..6982656
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliningOracleTest.java
@@ -0,0 +1,74 @@
+// Copyright (c) 2018, 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.classinliner;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.junit.Assert.assertThat;
+
+import com.android.tools.r8.NeverClassInline;
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.NeverMerge;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import org.junit.Test;
+
+/** Regression test for b/121119666. */
+public class ClassInliningOracleTest extends TestBase {
+
+ @Test
+ public void test() throws Exception {
+ CodeInspector inspector =
+ testForR8(Backend.DEX)
+ .addInnerClasses(ClassInliningOracleTest.class)
+ .addKeepMainRule(TestClass.class)
+ .enableInliningAnnotations()
+ .enableClassInliningAnnotations()
+ .enableMergeAnnotations()
+ .compile()
+ .inspector();
+ assertThat(inspector.clazz(Builder.class), isPresent());
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ // In order to class inline the Builder, we would need to force-inline the help() method.
+ // We can't do this alone, though, since force-inlining of help() would lead to an illegal
+ // invoke-super instruction in main().
+ Builder builder = new Builder();
+ new Helper().help(builder);
+ System.out.print(builder.build());
+ }
+ }
+
+ @NeverMerge
+ static class HelperBase {
+
+ @NeverInline
+ public void hello() {
+ System.out.println("Hello");
+ }
+ }
+
+ @NeverClassInline
+ static class Helper extends HelperBase {
+
+ @NeverInline
+ public void help(Builder builder) {
+ // TODO(b/120959040): To avoid unused argument removal; should be replaced by a testing rule).
+ if (builder != null) {
+ super.hello();
+ }
+ }
+ }
+
+ static class Builder {
+
+ @NeverInline
+ public Object build() {
+ return new Object();
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/EscapingBuilderTest.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/EscapingBuilderTest.java
new file mode 100644
index 0000000..85857b3
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/EscapingBuilderTest.java
@@ -0,0 +1,166 @@
+// Copyright (c) 2018, 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.classinliner;
+
+import static com.android.tools.r8.ir.optimize.classinliner.EscapingBuilderTest.TestClass.escape;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.CoreMatchers.not;
+import static org.junit.Assert.assertThat;
+
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import org.junit.Test;
+
+/** Regression test for b/120182628. */
+public class EscapingBuilderTest extends TestBase {
+
+ @Test
+ public void test() throws Exception {
+ testForR8(Backend.DEX)
+ .addInnerClasses(EscapingBuilderTest.class)
+ .addKeepMainRule(TestClass.class)
+ .enableInliningAnnotations()
+ .compile()
+ .inspect(this::inspect);
+ }
+
+ private void inspect(CodeInspector inspector) {
+ assertThat(inspector.clazz(Builder0.class), not(isPresent()));
+ assertThat(inspector.clazz(Builder1.class), isPresent());
+ assertThat(inspector.clazz(Builder2.class), isPresent());
+ assertThat(inspector.clazz(Builder3.class), isPresent());
+ assertThat(inspector.clazz(Builder4.class), isPresent());
+ assertThat(inspector.clazz(Builder5.class), isPresent());
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ testBuilder0();
+ testBuilder1();
+ testBuilder2();
+ testBuilder3();
+ testBuilder4();
+ testBuilder5();
+ }
+
+ private static void testBuilder0() {
+ new Builder0().build();
+ }
+
+ private static void testBuilder1() {
+ new Builder1().build();
+ }
+
+ private static void testBuilder2() {
+ new Builder2().init().build();
+ }
+
+ private static void testBuilder3() {
+ Builder3 builder3 = new Builder3();
+ builder3.init(builder3).build();
+ }
+
+ private static void testBuilder4() {
+ Builder4 builder4 = new Builder4();
+ builder4.init(builder4).build();
+ }
+
+ private static void testBuilder5() {
+ Builder5.init(new Builder5()).build();
+ }
+
+ @NeverInline
+ public static void escape(Object o) {}
+ }
+
+ // Simple builder that should be class inlined.
+ static class Builder0 {
+
+ public Object build() {
+ return new Object();
+ }
+ }
+
+ // Builder that escapes via field `f` that is assigned in the constructor.
+ static class Builder1 {
+
+ public Builder1 f = this;
+
+ public Object build() {
+ escape(f);
+ return new Object();
+ }
+ }
+
+ // Builder that escapes via field `f` that is assigned in a virtual method.
+ static class Builder2 {
+
+ public Builder2 f;
+
+ @NeverInline
+ public Builder2 init() {
+ f = this;
+ return this;
+ }
+
+ public Object build() {
+ escape(f);
+ return new Object();
+ }
+ }
+
+ // Builder that escapes via field `f` that is assigned in a virtual method.
+ static class Builder3 {
+
+ public Builder3 f;
+
+ @NeverInline
+ public Builder3 init(Builder3 builder) {
+ f = builder;
+ return this;
+ }
+
+ public Object build() {
+ escape(f);
+ return new Object();
+ }
+ }
+
+ // Builder that escapes via field `f` that is assigned in a virtual method.
+ static class Builder4 {
+
+ public Builder4 f;
+
+ @NeverInline
+ public Builder4 init(Builder4 builder) {
+ builder.f = builder;
+ return this;
+ }
+
+ public Object build() {
+ escape(f);
+ return new Object();
+ }
+ }
+
+ // Builder that escapes via field `f` that is assigned in a static method.
+ static class Builder5 {
+
+ public Builder5 f;
+
+ @NeverInline
+ public static Builder5 init(Builder5 builder) {
+ builder.f = builder;
+ return builder;
+ }
+
+ public Object build() {
+ escape(f);
+ return new Object();
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/naming/AdaptResourceFileContentsTest.java b/src/test/java/com/android/tools/r8/naming/AdaptResourceFileContentsTest.java
index b03c257..cc5b7b7 100644
--- a/src/test/java/com/android/tools/r8/naming/AdaptResourceFileContentsTest.java
+++ b/src/test/java/com/android/tools/r8/naming/AdaptResourceFileContentsTest.java
@@ -169,7 +169,8 @@
getProguardConfig(enableAdaptResourceFileContents, adaptResourceFileContentsPathFilter),
"-neverinline class com.android.tools.r8.naming.AdaptResourceFileContentsTestClass$B {",
" public void method();",
- "}");
+ "}",
+ "-neverclassinline class com.android.tools.r8.naming.AdaptResourceFileContentsTestClass$B");
}
@Test
@@ -325,13 +326,7 @@
.addDataResources(getDataResources())
.enableProguardTestOptions()
.addKeepRules(proguardConfig)
- .addOptionsModification(
- o -> {
- // TODO(christofferqa): Class inliner should respect -neverinline.
- o.enableClassInlining = false;
- o.enableVerticalClassMerging = true;
- o.dataResourceConsumer = dataResourceConsumer;
- })
+ .addOptionsModification(o -> o.dataResourceConsumer = dataResourceConsumer)
.compile();
}
diff --git a/src/test/java/com/android/tools/r8/naming/AdaptResourceFileNamesTest.java b/src/test/java/com/android/tools/r8/naming/AdaptResourceFileNamesTest.java
index 8876860..7d4c47a 100644
--- a/src/test/java/com/android/tools/r8/naming/AdaptResourceFileNamesTest.java
+++ b/src/test/java/com/android/tools/r8/naming/AdaptResourceFileNamesTest.java
@@ -27,6 +27,7 @@
import com.android.tools.r8.utils.ArchiveResourceProvider;
import com.android.tools.r8.utils.FileUtils;
import com.android.tools.r8.utils.KeepingDiagnosticHandler;
+import com.android.tools.r8.utils.StringUtils;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import java.io.File;
@@ -95,15 +96,15 @@
private static String getProguardConfigWithNeverInline(
boolean enableAdaptResourceFileNames, String adaptResourceFileNamesPathFilter) {
- return String.join(
- System.lineSeparator(),
+ return StringUtils.lines(
getProguardConfig(enableAdaptResourceFileNames, adaptResourceFileNamesPathFilter),
"-neverinline class " + adaptresourcefilenames.A.class.getName() + " {",
" public void method();",
"}",
"-neverinline class " + adaptresourcefilenames.B.Inner.class.getName() + " {",
" public void method();",
- "}");
+ "}",
+ "-neverclassinline class *");
}
@Test
@@ -264,9 +265,6 @@
return ToolHelper.runR8(
command,
options -> {
- // TODO(christofferqa): Class inliner should respect -neverinline.
- options.enableClassInlining = false;
- options.enableVerticalClassMerging = true;
options.dataResourceConsumer = dataResourceConsumer;
options.proguardMapConsumer = proguardMapConsumer;
options.testing.suppressExperimentalCfBackendWarning = true;