Merge equivalent instance initializers
Bug: 189296638
Change-Id: Ibca97395154568252cf04d78c9ec54c3987a5de5
diff --git a/src/main/java/com/android/tools/r8/cf/code/CfFieldInstruction.java b/src/main/java/com/android/tools/r8/cf/code/CfFieldInstruction.java
index 592ee31..cd8d6b7 100644
--- a/src/main/java/com/android/tools/r8/cf/code/CfFieldInstruction.java
+++ b/src/main/java/com/android/tools/r8/cf/code/CfFieldInstruction.java
@@ -42,6 +42,10 @@
spec.withInt(f -> f.opcode).withItem(f -> f.field).withItem(f -> f.declaringField);
}
+ public CfFieldInstruction(int opcode, DexField field) {
+ this(opcode, field, field);
+ }
+
public CfFieldInstruction(int opcode, DexField field, DexField declaringField) {
this.opcode = opcode;
this.field = field;
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 646c9a0..a42388a 100644
--- a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
+++ b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
@@ -2238,6 +2238,10 @@
return createMethod(holder, createProto(voidType), classConstructorMethodName);
}
+ public DexMethod createInstanceInitializer(DexType holder, DexType... parameters) {
+ return createMethod(holder, createProto(voidType, parameters), constructorMethodName);
+ }
+
public DexMethod createInstanceInitializerWithFreshProto(
DexMethod method, List<DexType> extraTypes, Predicate<DexMethod> isFresh) {
assert method.isInstanceInitializer(this);
@@ -2258,8 +2262,6 @@
private DexMethod createInstanceInitializerWithFreshProto(
DexProto proto, List<DexType> extraTypes, Function<DexProto, Optional<DexMethod>> isFresh) {
- assert !extraTypes.isEmpty();
-
Queue<Iterable<DexProto>> tryProtos = new LinkedList<>();
Iterator<DexProto> current = IterableUtils.singleton(proto).iterator();
@@ -2276,6 +2278,7 @@
if (object.isPresent()) {
return object.get();
}
+ assert !extraTypes.isEmpty();
tryProtos.add(
Iterables.transform(extraTypes, extraType -> appendTypeToProto(tryProto, extraType)));
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexType.java b/src/main/java/com/android/tools/r8/graph/DexType.java
index a38770a..96c3084 100644
--- a/src/main/java/com/android/tools/r8/graph/DexType.java
+++ b/src/main/java/com/android/tools/r8/graph/DexType.java
@@ -93,6 +93,11 @@
return descriptor;
}
+ public int getRequiredRegisters() {
+ assert !isVoidType();
+ return isWideType() ? 2 : 1;
+ }
+
@Override
public int computeHashCode() {
return descriptor.hashCode();
diff --git a/src/main/java/com/android/tools/r8/graph/DexTypeUtils.java b/src/main/java/com/android/tools/r8/graph/DexTypeUtils.java
new file mode 100644
index 0000000..b6d9757
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/graph/DexTypeUtils.java
@@ -0,0 +1,89 @@
+// Copyright (c) 2021, 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.graph;
+
+import com.android.tools.r8.ir.analysis.type.ClassTypeElement;
+import com.android.tools.r8.ir.analysis.type.InterfaceCollection;
+import com.android.tools.r8.utils.ObjectUtils;
+import java.util.Iterator;
+
+public class DexTypeUtils {
+
+ public static DexType computeLeastUpperBound(
+ AppView<? extends AppInfoWithClassHierarchy> appView, Iterable<DexType> types) {
+ DexItemFactory dexItemFactory = appView.dexItemFactory();
+
+ Iterator<DexType> iterator = types.iterator();
+ assert iterator.hasNext();
+
+ DexType result = iterator.next();
+ while (iterator.hasNext()) {
+ result = computeLeastUpperBound(appView, result, iterator.next());
+ }
+ return result;
+ }
+
+ public static DexType computeLeastUpperBound(
+ AppView<? extends AppInfoWithClassHierarchy> appView, DexType type, DexType other) {
+ if (type == other) {
+ return type;
+ }
+
+ DexItemFactory dexItemFactory = appView.dexItemFactory();
+ if (type == dexItemFactory.objectType
+ || other == dexItemFactory.objectType
+ || type.isArrayType() != other.isArrayType()) {
+ return dexItemFactory.objectType;
+ }
+
+ if (type.isArrayType()) {
+ assert other.isArrayType();
+ int arrayDimension = type.getNumberOfLeadingSquareBrackets();
+ if (other.getNumberOfLeadingSquareBrackets() != arrayDimension) {
+ return dexItemFactory.objectType;
+ }
+
+ DexType baseType = type.toBaseType(dexItemFactory);
+ DexType otherBaseType = other.toBaseType(dexItemFactory);
+ if (baseType.isPrimitiveType() || otherBaseType.isPrimitiveType()) {
+ assert baseType != otherBaseType;
+ return dexItemFactory.objectType;
+ }
+
+ return dexItemFactory.createArrayType(
+ arrayDimension, computeLeastUpperBound(appView, baseType, otherBaseType));
+ }
+
+ assert !type.isArrayType();
+ assert !other.isArrayType();
+
+ boolean isInterface =
+ type.isClassType()
+ && ObjectUtils.getBooleanOrElse(
+ appView.definitionFor(type), DexClass::isInterface, false);
+ boolean otherIsInterface =
+ other.isClassType()
+ && ObjectUtils.getBooleanOrElse(
+ appView.definitionFor(other), DexClass::isInterface, false);
+ if (isInterface != otherIsInterface) {
+ return dexItemFactory.objectType;
+ }
+
+ if (isInterface) {
+ assert otherIsInterface;
+ InterfaceCollection interfaceCollection =
+ ClassTypeElement.computeLeastUpperBoundOfInterfaces(
+ appView, InterfaceCollection.singleton(type), InterfaceCollection.singleton(other));
+ return interfaceCollection.hasSingleKnownInterface()
+ ? interfaceCollection.getSingleKnownInterface()
+ : dexItemFactory.objectType;
+ }
+
+ assert !isInterface;
+ assert !otherIsInterface;
+
+ return ClassTypeElement.computeLeastUpperBoundOfClasses(appView.appInfo(), type, other);
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java
index 999869c..3927d11 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java
@@ -62,13 +62,18 @@
private final Mode mode;
private final MergeGroup group;
private final DexItemFactory dexItemFactory;
- private final ClassInitializerMerger classInitializerMerger;
private final HorizontalClassMergerGraphLens.Builder lensBuilder;
private final ClassMethodsBuilder classMethodsBuilder = new ClassMethodsBuilder();
private final Reference2IntMap<DexType> classIdentifiers = new Reference2IntOpenHashMap<>();
- private final ClassStaticFieldsMerger classStaticFieldsMerger;
+
+ // Field mergers.
private final ClassInstanceFieldsMerger classInstanceFieldsMerger;
+ private final ClassStaticFieldsMerger classStaticFieldsMerger;
+
+ // Method mergers.
+ private final ClassInitializerMerger classInitializerMerger;
+ private final InstanceInitializerMergerCollection instanceInitializerMergers;
private final Collection<VirtualMethodMerger> virtualMethodMergers;
private ClassMerger(
@@ -82,10 +87,18 @@
this.group = group;
this.lensBuilder = lensBuilder;
this.mode = mode;
- this.virtualMethodMergers = virtualMethodMergers;
- this.classInitializerMerger = ClassInitializerMerger.create(group);
+
+ // Field mergers.
this.classStaticFieldsMerger = new ClassStaticFieldsMerger(appView, lensBuilder, group);
this.classInstanceFieldsMerger = new ClassInstanceFieldsMerger(appView, lensBuilder, group);
+
+ // Method mergers.
+ this.classInitializerMerger = ClassInitializerMerger.create(group);
+ this.instanceInitializerMergers =
+ InstanceInitializerMergerCollection.create(
+ appView, classIdentifiers, group, classInstanceFieldsMerger, lensBuilder, mode);
+ this.virtualMethodMergers = virtualMethodMergers;
+
buildClassIdentifierMap();
}
@@ -175,12 +188,8 @@
}
void mergeInstanceInitializers(SyntheticArgumentClass syntheticArgumentClass) {
- InstanceInitializerMergerCollection instanceInitializerMergers =
- InstanceInitializerMergerCollection.create(appView, group, lensBuilder, mode);
instanceInitializerMergers.forEach(
- merger ->
- merger.merge(
- classMethodsBuilder, lensBuilder, classIdentifiers, syntheticArgumentClass));
+ merger -> merger.merge(classMethodsBuilder, syntheticArgumentClass));
}
void mergeMethods(
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 c05af00..4ad6f55 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java
@@ -212,6 +212,9 @@
for (ClassMerger merger : classMergers) {
merger.mergeGroup(syntheticArgumentClass, syntheticClassInitializerConverterBuilder);
}
+
+ // Clear type elements cache after IR building.
+ appView.dexItemFactory().clearTypeElementsCache();
}
/**
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMergerCfCode.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMergerCfCode.java
new file mode 100644
index 0000000..839b6d5
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMergerCfCode.java
@@ -0,0 +1,33 @@
+// Copyright (c) 2021, 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.horizontalclassmerging;
+
+import com.android.tools.r8.cf.code.CfInstruction;
+import com.android.tools.r8.cf.code.CfTryCatch;
+import com.android.tools.r8.graph.CfCode;
+import com.android.tools.r8.graph.DexType;
+import java.util.List;
+
+/**
+ * Similar to CfCode, but with a marker that makes it possible to recognize this is synthesized by
+ * the horizontal class merger.
+ */
+public class HorizontalClassMergerCfCode extends CfCode {
+
+ HorizontalClassMergerCfCode(
+ DexType originalHolder,
+ int maxStack,
+ int maxLocals,
+ List<CfInstruction> instructions,
+ List<CfTryCatch> tryCatchRanges,
+ List<LocalVariableInfo> localVariables) {
+ super(originalHolder, maxStack, maxLocals, instructions, tryCatchRanges, localVariables);
+ }
+
+ @Override
+ public boolean isHorizontalClassMergingCode() {
+ return true;
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMergerGraphLens.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMergerGraphLens.java
index abadbc4..20eb5a3 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMergerGraphLens.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMergerGraphLens.java
@@ -120,6 +120,11 @@
newMethodSignatures);
}
+ DexMethod getRenamedMethodSignature(DexMethod method) {
+ assert newMethodSignatures.containsKey(method);
+ return newMethodSignatures.get(method);
+ }
+
void recordNewFieldSignature(DexField oldFieldSignature, DexField newFieldSignature) {
fieldMap.put(oldFieldSignature, newFieldSignature);
}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerAnalysis.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerAnalysis.java
index febdbe6..9a6390a 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerAnalysis.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerAnalysis.java
@@ -4,14 +4,204 @@
package com.android.tools.r8.horizontalclassmerging;
+import static com.android.tools.r8.ir.code.Opcodes.ARGUMENT;
+import static com.android.tools.r8.ir.code.Opcodes.ASSUME;
+import static com.android.tools.r8.ir.code.Opcodes.CONST_CLASS;
+import static com.android.tools.r8.ir.code.Opcodes.CONST_NUMBER;
+import static com.android.tools.r8.ir.code.Opcodes.CONST_STRING;
+import static com.android.tools.r8.ir.code.Opcodes.DEX_ITEM_BASED_CONST_STRING;
+import static com.android.tools.r8.ir.code.Opcodes.GOTO;
+import static com.android.tools.r8.ir.code.Opcodes.INSTANCE_PUT;
+import static com.android.tools.r8.ir.code.Opcodes.INVOKE_DIRECT;
+import static com.android.tools.r8.ir.code.Opcodes.RETURN;
+
+import com.android.tools.r8.graph.AppInfoWithClassHierarchy;
+import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.DexField;
+import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.graph.ProgramField;
import com.android.tools.r8.graph.ProgramMethod;
+import com.android.tools.r8.horizontalclassmerging.HorizontalClassMerger.Mode;
+import com.android.tools.r8.ir.analysis.value.AbstractValue;
+import com.android.tools.r8.ir.code.BasicBlock;
+import com.android.tools.r8.ir.code.IRCode;
+import com.android.tools.r8.ir.code.InstancePut;
+import com.android.tools.r8.ir.code.Instruction;
+import com.android.tools.r8.ir.code.InvokeDirect;
+import com.android.tools.r8.ir.code.Value;
+import com.android.tools.r8.ir.optimize.info.MethodOptimizationInfo;
+import com.android.tools.r8.ir.optimize.info.field.InstanceFieldInitializationInfo;
+import com.android.tools.r8.ir.optimize.info.field.InstanceFieldInitializationInfoFactory;
+import com.android.tools.r8.ir.optimize.info.initializer.InstanceInitializerInfo;
+import com.android.tools.r8.utils.WorkList;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
+import java.util.ArrayList;
+import java.util.List;
public class InstanceInitializerAnalysis {
public static InstanceInitializerDescription analyze(
- ProgramMethod instanceInitializer, HorizontalClassMergerGraphLens.Builder lensBuilder) {
- // TODO(b/189296638): Return an InstanceInitializerDescription if the given instance initializer
- // is parent constructor call followed by a sequence of instance field puts.
+ AppView<? extends AppInfoWithClassHierarchy> appView,
+ MergeGroup group,
+ ProgramMethod instanceInitializer,
+ ClassInstanceFieldsMerger instanceFieldsMerger,
+ Mode mode) {
+ InstanceInitializerDescription.Builder builder =
+ InstanceInitializerDescription.builder(appView, instanceInitializer);
+
+ if (mode.isFinal()) {
+ // TODO(b/189296638): We can't build IR in the final round of class merging without simulating
+ // that we're in D8.
+ MethodOptimizationInfo optimizationInfo =
+ instanceInitializer.getDefinition().getOptimizationInfo();
+ if (optimizationInfo.mayHaveSideEffects()) {
+ return null;
+ }
+ InstanceInitializerInfo instanceInitializerInfo =
+ optimizationInfo.getContextInsensitiveInstanceInitializerInfo();
+ if (!instanceInitializerInfo.hasParent()) {
+ // We don't know the parent constructor of the first constructor.
+ return null;
+ }
+ DexMethod parent = instanceInitializerInfo.getParent();
+ if (parent.getArity() > 0) {
+ return null;
+ }
+ builder.addInvokeConstructor(parent, ImmutableList.of());
+ return builder.build();
+ }
+
+ IRCode code = instanceInitializer.buildIR(appView);
+ WorkList<BasicBlock> workList = WorkList.newIdentityWorkList(code.entryBlock());
+ while (workList.hasNext()) {
+ BasicBlock block = workList.next();
+ for (Instruction instruction : block.getInstructions()) {
+ switch (instruction.opcode()) {
+ case ARGUMENT:
+ case ASSUME:
+ case CONST_CLASS:
+ case CONST_NUMBER:
+ case CONST_STRING:
+ case DEX_ITEM_BASED_CONST_STRING:
+ case RETURN:
+ break;
+
+ case GOTO:
+ if (!workList.addIfNotSeen(instruction.asGoto().getTarget())) {
+ return invalid();
+ }
+ break;
+
+ case INSTANCE_PUT:
+ {
+ InstancePut instancePut = instruction.asInstancePut();
+
+ // This must initialize a field on the receiver.
+ if (!instancePut.object().getAliasedValue().isThis()) {
+ return invalid();
+ }
+
+ // Check that this writes a field on the enclosing class.
+ DexField fieldReference = instancePut.getField();
+ DexField lensRewrittenFieldReference =
+ appView.graphLens().lookupField(fieldReference);
+ if (lensRewrittenFieldReference.getHolderType()
+ != instanceInitializer.getHolderType()) {
+ return invalid();
+ }
+
+ ProgramField sourceField =
+ instanceInitializer.getHolder().lookupProgramField(lensRewrittenFieldReference);
+ if (sourceField == null) {
+ return invalid();
+ }
+
+ InstanceFieldInitializationInfo initializationInfo =
+ getInitializationInfo(instancePut.value(), appView, instanceInitializer);
+ if (initializationInfo == null) {
+ return invalid();
+ }
+
+ ProgramField targetField = instanceFieldsMerger.getTargetField(sourceField);
+ assert targetField != null;
+
+ builder.addInstancePut(targetField.getReference(), initializationInfo);
+ break;
+ }
+
+ case INVOKE_DIRECT:
+ {
+ InvokeDirect invoke = instruction.asInvokeDirect();
+
+ // This must initialize the receiver.
+ if (!invoke.getReceiver().getAliasedValue().isThis()) {
+ return invalid();
+ }
+
+ DexMethod invokedMethod = invoke.getInvokedMethod();
+ DexMethod lensRewrittenInvokedMethod =
+ appView
+ .graphLens()
+ .lookupInvokeDirect(invokedMethod, instanceInitializer)
+ .getReference();
+
+ // TODO(b/189296638): Consider allowing constructor forwarding.
+ if (!lensRewrittenInvokedMethod.isInstanceInitializer(appView.dexItemFactory())
+ || lensRewrittenInvokedMethod.getHolderType() != group.getSuperType()) {
+ return invalid();
+ }
+
+ // Extract the non-receiver arguments.
+ List<InstanceFieldInitializationInfo> arguments =
+ new ArrayList<>(invoke.arguments().size() - 1);
+ for (Value argument : Iterables.skip(invoke.arguments(), 1)) {
+ InstanceFieldInitializationInfo initializationInfo =
+ getInitializationInfo(argument, appView, instanceInitializer);
+ if (initializationInfo == null) {
+ return invalid();
+ }
+ arguments.add(initializationInfo);
+ }
+
+ if (!builder.addInvokeConstructor(invokedMethod, arguments)) {
+ return invalid();
+ }
+ }
+ break;
+
+ default:
+ // Not allowed.
+ return invalid();
+ }
+ }
+ }
+
+ return builder.isValid() ? builder.build() : null;
+ }
+
+ private static InstanceFieldInitializationInfo getInitializationInfo(
+ Value value,
+ AppView<? extends AppInfoWithClassHierarchy> appView,
+ ProgramMethod instanceInitializer) {
+ InstanceFieldInitializationInfoFactory factory =
+ appView.instanceFieldInitializationInfoFactory();
+
+ Value root = value.getAliasedValue();
+ if (root.isDefinedByInstructionSatisfying(Instruction::isArgument)) {
+ return factory.createArgumentInitializationInfo(
+ value.getDefinition().asArgument().getIndex());
+ }
+
+ AbstractValue abstractValue = value.getAbstractValue(appView, instanceInitializer);
+ if (abstractValue.isSingleConstValue()) {
+ return abstractValue.asSingleConstValue();
+ }
+
+ return null;
+ }
+
+ private static InstanceInitializerDescription invalid() {
return null;
}
}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerDescription.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerDescription.java
index 41f2c4e..b67c652 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerDescription.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerDescription.java
@@ -4,14 +4,44 @@
package com.android.tools.r8.horizontalclassmerging;
+import com.android.tools.r8.cf.code.CfConstClass;
+import com.android.tools.r8.cf.code.CfConstNull;
+import com.android.tools.r8.cf.code.CfConstNumber;
+import com.android.tools.r8.cf.code.CfConstString;
+import com.android.tools.r8.cf.code.CfDexItemBasedConstString;
+import com.android.tools.r8.cf.code.CfFieldInstruction;
+import com.android.tools.r8.cf.code.CfInstruction;
+import com.android.tools.r8.cf.code.CfInvoke;
+import com.android.tools.r8.cf.code.CfLabel;
+import com.android.tools.r8.cf.code.CfLoad;
+import com.android.tools.r8.cf.code.CfPosition;
+import com.android.tools.r8.cf.code.CfReturnVoid;
+import com.android.tools.r8.code.Instruction;
+import com.android.tools.r8.code.InvokeDirectRange;
+import com.android.tools.r8.code.ReturnVoid;
+import com.android.tools.r8.graph.AppInfoWithClassHierarchy;
+import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.CfCode;
+import com.android.tools.r8.graph.DexCode;
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.DexType;
+import com.android.tools.r8.graph.DexTypeList;
import com.android.tools.r8.graph.ProgramMethod;
+import com.android.tools.r8.ir.analysis.value.SingleConstValue;
+import com.android.tools.r8.ir.analysis.value.SingleDexItemBasedStringValue;
+import com.android.tools.r8.ir.code.Position;
+import com.android.tools.r8.ir.code.ValueType;
import com.android.tools.r8.ir.optimize.info.field.InstanceFieldInitializationInfo;
+import com.android.tools.r8.utils.IntBox;
+import com.android.tools.r8.utils.IterableUtils;
+import com.google.common.collect.ImmutableList;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
+import org.objectweb.asm.Opcodes;
/**
* A simple abstraction of an instance initializer's code, which allows a parent constructor call
@@ -19,24 +49,219 @@
*/
public class InstanceInitializerDescription {
- private final int arity;
- private final Map<DexField, InstanceFieldInitializationInfo> instanceFieldAssignments;
+ // Field assignments that happen prior to the parent constructor call.
+ //
+ // Most fields are generally assigned after the parent constructor call, but both javac and
+ // kotlinc may assign instance fields prior to the parent constructor call. For example, the
+ // synthetic this$0 field for non-static inner classes is typically assigned prior to the parent
+ // constructor call.
+ private final Map<DexField, InstanceFieldInitializationInfo> instanceFieldAssignmentsPre;
+
+ // Field assignments that happens after the parent constructor call.
+ private final Map<DexField, InstanceFieldInitializationInfo> instanceFieldAssignmentsPost;
+
+ // The parent constructor method and the arguments passed to it.
private final DexMethod parentConstructor;
private final List<InstanceFieldInitializationInfo> parentConstructorArguments;
+ // The constructor parameters, where reference types have been mapped to java.lang.Object, to
+ // ensure we don't group constructors such as <init>(int) and <init>(Object), since this would
+ // lead to type errors.
+ private final DexTypeList relaxedParameters;
+
InstanceInitializerDescription(
- int arity,
- Map<DexField, InstanceFieldInitializationInfo> instanceFieldAssignments,
+ Map<DexField, InstanceFieldInitializationInfo> instanceFieldAssignmentsPre,
+ Map<DexField, InstanceFieldInitializationInfo> instanceFieldAssignmentsPost,
DexMethod parentConstructor,
- List<InstanceFieldInitializationInfo> parentConstructorArguments) {
- this.arity = arity;
- this.instanceFieldAssignments = instanceFieldAssignments;
+ List<InstanceFieldInitializationInfo> parentConstructorArguments,
+ DexTypeList relaxedParameters) {
+ this.instanceFieldAssignmentsPre = instanceFieldAssignmentsPre;
+ this.instanceFieldAssignmentsPost = instanceFieldAssignmentsPost;
this.parentConstructor = parentConstructor;
this.parentConstructorArguments = parentConstructorArguments;
+ this.relaxedParameters = relaxedParameters;
}
- public static Builder builder(ProgramMethod instanceInitializer) {
- return new Builder(instanceInitializer);
+ public static Builder builder(
+ AppView<? extends AppInfoWithClassHierarchy> appView, ProgramMethod instanceInitializer) {
+ return new Builder(appView.dexItemFactory(), instanceInitializer);
+ }
+
+ /**
+ * Transform this description into actual CF code.
+ *
+ * @param newMethodReference the reference of the method that is being synthesized
+ * @param originalMethodReference the original reference of the representative method
+ * @param syntheticMethodReference the original, synthetic reference of the new method reference
+ * ($r8$init$synthetic)
+ */
+ public CfCode createCfCode(
+ DexMethod newMethodReference,
+ DexMethod originalMethodReference,
+ DexMethod syntheticMethodReference,
+ MergeGroup group,
+ boolean hasClassId,
+ int extraNulls) {
+ int[] argumentToLocalIndex =
+ new int[newMethodReference.getParameters().size() + 1 - extraNulls];
+ int maxLocals = 0;
+ argumentToLocalIndex[0] = maxLocals++;
+ for (int i = 1; i < argumentToLocalIndex.length; i++) {
+ argumentToLocalIndex[i] = maxLocals;
+ maxLocals += newMethodReference.getParameter(i - 1).getRequiredRegisters();
+ }
+
+ IntBox maxStack = new IntBox();
+ ImmutableList.Builder<CfInstruction> instructionBuilder = ImmutableList.builder();
+
+ // Set position.
+ Position callerPosition = Position.synthetic(0, syntheticMethodReference, null);
+ Position calleePosition = Position.synthetic(0, originalMethodReference, callerPosition);
+ CfPosition position = new CfPosition(new CfLabel(), calleePosition);
+ instructionBuilder.add(position);
+ instructionBuilder.add(position.getLabel());
+
+ // Assign class id.
+ if (group.hasClassIdField()) {
+ assert hasClassId;
+ int classIdLocalIndex = maxLocals - 1;
+ instructionBuilder.add(new CfLoad(ValueType.OBJECT, 0));
+ instructionBuilder.add(new CfLoad(ValueType.INT, classIdLocalIndex));
+ instructionBuilder.add(new CfFieldInstruction(Opcodes.PUTFIELD, group.getClassIdField()));
+ maxStack.set(2);
+ } else {
+ assert !hasClassId;
+ }
+
+ // Assign each field.
+ addCfInstructionsForInstanceFieldAssignments(
+ instructionBuilder, instanceFieldAssignmentsPre, argumentToLocalIndex, maxStack);
+
+ // Load receiver for parent constructor call.
+ int stackHeightForParentConstructorCall = 1;
+ instructionBuilder.add(new CfLoad(ValueType.OBJECT, 0));
+
+ // Load constructor arguments.
+ int i = 0;
+ for (InstanceFieldInitializationInfo initializationInfo : parentConstructorArguments) {
+ stackHeightForParentConstructorCall +=
+ addCfInstructionsForInitializationInfo(
+ instructionBuilder,
+ initializationInfo,
+ argumentToLocalIndex,
+ parentConstructor.getParameter(i));
+ i++;
+ }
+
+ // Invoke parent constructor.
+ instructionBuilder.add(new CfInvoke(Opcodes.INVOKESPECIAL, parentConstructor, false));
+ maxStack.setMax(stackHeightForParentConstructorCall);
+
+ // Assign each field.
+ addCfInstructionsForInstanceFieldAssignments(
+ instructionBuilder, instanceFieldAssignmentsPost, argumentToLocalIndex, maxStack);
+
+ // Return.
+ instructionBuilder.add(new CfReturnVoid());
+
+ return new HorizontalClassMergerCfCode(
+ newMethodReference.getHolderType(),
+ maxStack.get(),
+ maxLocals,
+ instructionBuilder.build(),
+ ImmutableList.of(),
+ ImmutableList.of());
+ }
+
+ private static void addCfInstructionsForInstanceFieldAssignments(
+ ImmutableList.Builder<CfInstruction> instructionBuilder,
+ Map<DexField, InstanceFieldInitializationInfo> instanceFieldAssignments,
+ int[] argumentToLocalIndex,
+ IntBox maxStack) {
+ instanceFieldAssignments.forEach(
+ (field, initializationInfo) -> {
+ // Load the receiver, the field value, and then set the field.
+ instructionBuilder.add(new CfLoad(ValueType.OBJECT, 0));
+ int stackSizeForInitializationInfo =
+ addCfInstructionsForInitializationInfo(
+ instructionBuilder, initializationInfo, argumentToLocalIndex, field.getType());
+ instructionBuilder.add(new CfFieldInstruction(Opcodes.PUTFIELD, field));
+ maxStack.setMax(stackSizeForInitializationInfo + 1);
+ });
+ }
+
+ private static int addCfInstructionsForInitializationInfo(
+ ImmutableList.Builder<CfInstruction> instructionBuilder,
+ InstanceFieldInitializationInfo initializationInfo,
+ int[] argumentToLocalIndex,
+ DexType type) {
+ if (initializationInfo.isArgumentInitializationInfo()) {
+ int argumentIndex = initializationInfo.asArgumentInitializationInfo().getArgumentIndex();
+ instructionBuilder.add(
+ new CfLoad(ValueType.fromDexType(type), argumentToLocalIndex[argumentIndex]));
+ return type.getRequiredRegisters();
+ }
+
+ assert initializationInfo.isSingleValue();
+ assert initializationInfo.asSingleValue().isSingleConstValue();
+
+ SingleConstValue singleConstValue = initializationInfo.asSingleValue().asSingleConstValue();
+ if (singleConstValue.isSingleConstClassValue()) {
+ instructionBuilder.add(
+ new CfConstClass(singleConstValue.asSingleConstClassValue().getType()));
+ return 1;
+ } else if (singleConstValue.isSingleDexItemBasedStringValue()) {
+ SingleDexItemBasedStringValue dexItemBasedStringValue =
+ singleConstValue.asSingleDexItemBasedStringValue();
+ instructionBuilder.add(
+ new CfDexItemBasedConstString(
+ dexItemBasedStringValue.getItem(), dexItemBasedStringValue.getNameComputationInfo()));
+ return 1;
+ } else if (singleConstValue.isSingleNumberValue()) {
+ if (type.isReferenceType()) {
+ assert singleConstValue.isNull();
+ instructionBuilder.add(new CfConstNull());
+ return 1;
+ } else {
+ instructionBuilder.add(
+ new CfConstNumber(
+ singleConstValue.asSingleNumberValue().getValue(), ValueType.fromDexType(type)));
+ return type.getRequiredRegisters();
+ }
+ } else {
+ assert singleConstValue.isSingleStringValue();
+ instructionBuilder.add(
+ new CfConstString(singleConstValue.asSingleStringValue().getDexString()));
+ return 1;
+ }
+ }
+
+ public DexCode createDexCode(
+ DexMethod newMethodReference,
+ DexMethod originalMethodReference,
+ DexMethod syntheticMethodReference,
+ MergeGroup group,
+ boolean hasClassId,
+ int extraNulls) {
+ assert !hasClassId;
+ assert extraNulls == 0;
+ assert parentConstructorArguments.isEmpty();
+ assert instanceFieldAssignmentsPre.isEmpty();
+ assert instanceFieldAssignmentsPost.isEmpty();
+ Instruction[] instructions = new Instruction[2];
+ instructions[0] = new InvokeDirectRange(0, 1, parentConstructor);
+ instructions[1] = new ReturnVoid();
+ int incomingRegisterSize =
+ 1 + IterableUtils.sumInt(newMethodReference.getParameters(), DexType::getRequiredRegisters);
+ int outgoingRegisterSize = 1;
+ return new DexCode(
+ incomingRegisterSize,
+ incomingRegisterSize,
+ outgoingRegisterSize,
+ instructions,
+ new DexCode.Try[0],
+ new DexCode.TryHandler[0],
+ null);
}
@Override
@@ -45,8 +270,8 @@
return false;
}
InstanceInitializerDescription description = (InstanceInitializerDescription) obj;
- return arity == description.arity
- && instanceFieldAssignments.equals(description.instanceFieldAssignments)
+ return instanceFieldAssignmentsPre.equals(description.instanceFieldAssignmentsPre)
+ && instanceFieldAssignmentsPost.equals(description.instanceFieldAssignmentsPost)
&& parentConstructor == description.parentConstructor
&& parentConstructorArguments.equals(description.parentConstructorArguments);
}
@@ -54,38 +279,67 @@
@Override
public int hashCode() {
return Objects.hash(
- arity, instanceFieldAssignments, parentConstructor, parentConstructorArguments);
+ instanceFieldAssignmentsPre,
+ instanceFieldAssignmentsPost,
+ parentConstructor,
+ parentConstructorArguments,
+ relaxedParameters);
}
public static class Builder {
- private final int arity;
+ private final DexItemFactory dexItemFactory;
+ private final DexTypeList relaxedParameters;
- private Map<DexField, InstanceFieldInitializationInfo> instanceFieldAssignments =
+ private Map<DexField, InstanceFieldInitializationInfo> instanceFieldAssignmentsPre =
+ new LinkedHashMap<>();
+ private Map<DexField, InstanceFieldInitializationInfo> instanceFieldAssignmentsPost =
new LinkedHashMap<>();
private DexMethod parentConstructor;
private List<InstanceFieldInitializationInfo> parentConstructorArguments;
- Builder(ProgramMethod method) {
- this.arity = method.getReference().getArity();
+ Builder(DexItemFactory dexItemFactory, ProgramMethod method) {
+ this.dexItemFactory = dexItemFactory;
+ this.relaxedParameters =
+ method
+ .getParameters()
+ .map(
+ parameter -> parameter.isPrimitiveType() ? parameter : dexItemFactory.objectType);
}
public void addInstancePut(DexField field, InstanceFieldInitializationInfo value) {
- instanceFieldAssignments.put(field, value);
+ // If the parent constructor is java.lang.Object.<init>() then group all the field assignments
+ // before the parent constructor call to allow more sharing.
+ if (parentConstructor == null
+ || parentConstructor == dexItemFactory.objectMembers.constructor) {
+ instanceFieldAssignmentsPre.put(field, value);
+ } else {
+ instanceFieldAssignmentsPost.put(field, value);
+ }
}
- public void addInvokeConstructor(
+ public boolean addInvokeConstructor(
DexMethod method, List<InstanceFieldInitializationInfo> arguments) {
- assert parentConstructor == null;
- assert parentConstructorArguments == null;
- parentConstructor = method;
- parentConstructorArguments = arguments;
+ if (parentConstructor == null) {
+ parentConstructor = method;
+ parentConstructorArguments = arguments;
+ return true;
+ }
+ return false;
}
public InstanceInitializerDescription build() {
- assert parentConstructor != null;
+ assert isValid();
return new InstanceInitializerDescription(
- arity, instanceFieldAssignments, parentConstructor, parentConstructorArguments);
+ instanceFieldAssignmentsPre,
+ instanceFieldAssignmentsPost,
+ parentConstructor,
+ parentConstructorArguments,
+ relaxedParameters);
+ }
+
+ public boolean isValid() {
+ return parentConstructor != null;
}
}
}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMerger.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMerger.java
index b50c71f..862a23c 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMerger.java
@@ -10,11 +10,13 @@
import com.android.tools.r8.dex.Constants;
import com.android.tools.r8.graph.AppInfoWithClassHierarchy;
import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.Code;
import com.android.tools.r8.graph.DexAnnotationSet;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.DexTypeUtils;
import com.android.tools.r8.graph.GenericSignature.MethodTypeSignature;
import com.android.tools.r8.graph.MethodAccessFlags;
import com.android.tools.r8.graph.ParameterAnnotationsList;
@@ -24,61 +26,62 @@
import com.android.tools.r8.ir.conversion.ExtraConstantIntParameter;
import com.android.tools.r8.ir.conversion.ExtraParameter;
import com.android.tools.r8.ir.conversion.ExtraUnusedNullParameter;
-import com.android.tools.r8.ir.optimize.info.MethodOptimizationInfo;
-import com.android.tools.r8.ir.optimize.info.initializer.InstanceInitializerInfo;
+import com.android.tools.r8.utils.ArrayUtils;
+import com.android.tools.r8.utils.BooleanUtils;
import com.android.tools.r8.utils.ListUtils;
import com.android.tools.r8.utils.structural.Ordered;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import it.unimi.dsi.fastutil.ints.Int2ReferenceAVLTreeMap;
import it.unimi.dsi.fastutil.ints.Int2ReferenceSortedMap;
import it.unimi.dsi.fastutil.objects.Reference2IntMap;
import java.util.ArrayList;
import java.util.Collections;
-import java.util.Iterator;
import java.util.List;
public class InstanceInitializerMerger {
- private final AppView<?> appView;
+ private final AppView<? extends AppInfoWithClassHierarchy> appView;
+ private final Reference2IntMap<DexType> classIdentifiers;
private final DexItemFactory dexItemFactory;
private final MergeGroup group;
private final List<ProgramMethod> instanceInitializers;
private final InstanceInitializerDescription instanceInitializerDescription;
+ private final HorizontalClassMergerGraphLens.Builder lensBuilder;
private final Mode mode;
InstanceInitializerMerger(
- AppView<?> appView, MergeGroup group, List<ProgramMethod> instanceInitializers, Mode mode) {
- this(appView, group, instanceInitializers, mode, null);
+ AppView<? extends AppInfoWithClassHierarchy> appView,
+ Reference2IntMap<DexType> classIdentifiers,
+ MergeGroup group,
+ List<ProgramMethod> instanceInitializers,
+ HorizontalClassMergerGraphLens.Builder lensBuilder,
+ Mode mode) {
+ this(appView, classIdentifiers, group, instanceInitializers, lensBuilder, mode, null);
}
InstanceInitializerMerger(
- AppView<?> appView,
+ AppView<? extends AppInfoWithClassHierarchy> appView,
+ Reference2IntMap<DexType> classIdentifiers,
MergeGroup group,
List<ProgramMethod> instanceInitializers,
+ HorizontalClassMergerGraphLens.Builder lensBuilder,
Mode mode,
InstanceInitializerDescription instanceInitializerDescription) {
this.appView = appView;
+ this.classIdentifiers = classIdentifiers;
this.dexItemFactory = appView.dexItemFactory();
this.group = group;
this.instanceInitializers = instanceInitializers;
this.instanceInitializerDescription = instanceInitializerDescription;
+ this.lensBuilder = lensBuilder;
this.mode = mode;
- // Constructors should not be empty and all constructors should have the same prototype.
+ // Constructors should not be empty and all constructors should have the same prototype unless
+ // equivalent.
assert !instanceInitializers.isEmpty();
- assert instanceInitializers.stream().map(ProgramMethod::getProto).distinct().count() == 1;
- }
-
- /**
- * The method reference template describes which arguments the constructor must have, and is used
- * to generate the final reference by appending null arguments until it is fresh.
- */
- private DexMethod generateReferenceMethodTemplate() {
- DexMethod methodTemplate = instanceInitializers.iterator().next().getReference();
- if (instanceInitializers.size() > 1) {
- methodTemplate = dexItemFactory.appendTypeToMethod(methodTemplate, dexItemFactory.intType);
- }
- return methodTemplate.withHolder(group.getTarget(), dexItemFactory);
+ assert instanceInitializers.stream().map(ProgramMethod::getProto).distinct().count() == 1
+ || instanceInitializerDescription != null;
}
public int getArity() {
@@ -89,6 +92,78 @@
return instanceInitializers;
}
+ private CfVersion getNewClassFileVersion() {
+ CfVersion classFileVersion = null;
+ for (ProgramMethod instanceInitializer : instanceInitializers) {
+ if (instanceInitializer.getDefinition().hasClassFileVersion()) {
+ classFileVersion =
+ Ordered.maxIgnoreNull(
+ classFileVersion, instanceInitializer.getDefinition().getClassFileVersion());
+ }
+ }
+ return classFileVersion;
+ }
+
+ private DexMethod getNewMethodReference(ProgramMethod representative) {
+ return getNewMethodReference(representative, false);
+ }
+
+ private DexMethod getNewMethodReference(ProgramMethod representative, boolean needsClassId) {
+ DexType[] oldParameters = representative.getParameters().values;
+ DexType[] newParameters =
+ new DexType[representative.getParameters().size() + BooleanUtils.intValue(needsClassId)];
+ System.arraycopy(oldParameters, 0, newParameters, 0, oldParameters.length);
+ for (int i = 0; i < oldParameters.length; i++) {
+ final int parameterIndex = i;
+ newParameters[i] =
+ DexTypeUtils.computeLeastUpperBound(
+ appView,
+ Iterables.transform(
+ instanceInitializers,
+ instanceInitializer -> instanceInitializer.getParameter(parameterIndex)));
+ }
+ if (needsClassId) {
+ assert ArrayUtils.last(newParameters) == null;
+ newParameters[newParameters.length - 1] = dexItemFactory.intType;
+ }
+ return dexItemFactory.createInstanceInitializer(group.getTarget().getType(), newParameters);
+ }
+
+ private DexMethod getOriginalMethodReference() {
+ return appView.graphLens().getOriginalMethodSignature(getRepresentative().getReference());
+ }
+
+ private ProgramMethod getRepresentative() {
+ return ListUtils.first(instanceInitializers);
+ }
+
+ /**
+ * Returns a special original method signature for the synthesized constructor that did not exist
+ * prior to horizontal class merging. Otherwise we might accidentally think that the synthesized
+ * constructor corresponds to the previous <init>() method on the target class, which could have
+ * unintended side-effects such as leading to unused argument removal being applied to the
+ * synthesized constructor all-though it by construction doesn't have any unused arguments.
+ */
+ private DexMethod getSyntheticMethodReference(
+ ClassMethodsBuilder classMethodsBuilder, ProgramMethod representative) {
+ return dexItemFactory.createFreshMethodNameWithoutHolder(
+ "$r8$init$synthetic",
+ representative.getProto(),
+ representative.getHolderType(),
+ classMethodsBuilder::isFresh);
+ }
+
+ private Int2ReferenceSortedMap<DexMethod> createClassIdToInstanceInitializerMap() {
+ assert !hasInstanceInitializerDescription();
+ Int2ReferenceSortedMap<DexMethod> typeConstructorClassMap = new Int2ReferenceAVLTreeMap<>();
+ for (ProgramMethod instanceInitializer : instanceInitializers) {
+ typeConstructorClassMap.put(
+ classIdentifiers.getInt(instanceInitializer.getHolderType()),
+ lensBuilder.getRenamedMethodSignature(instanceInitializer.getReference()));
+ }
+ return typeConstructorClassMap;
+ }
+
public int size() {
return instanceInitializers.size();
}
@@ -96,12 +171,20 @@
public static class Builder {
private final AppView<? extends AppInfoWithClassHierarchy> appView;
+ private final Reference2IntMap<DexType> classIdentifiers;
private int estimatedDexCodeSize;
private final List<List<ProgramMethod>> instanceInitializerGroups = new ArrayList<>();
+ private final HorizontalClassMergerGraphLens.Builder lensBuilder;
private final Mode mode;
- public Builder(AppView<? extends AppInfoWithClassHierarchy> appView, Mode mode) {
+ public Builder(
+ AppView<? extends AppInfoWithClassHierarchy> appView,
+ Reference2IntMap<DexType> classIdentifiers,
+ HorizontalClassMergerGraphLens.Builder lensBuilder,
+ Mode mode) {
this.appView = appView;
+ this.classIdentifiers = classIdentifiers;
+ this.lensBuilder = lensBuilder;
this.mode = mode;
createNewGroup();
}
@@ -136,7 +219,8 @@
return ListUtils.map(
instanceInitializerGroups,
instanceInitializers ->
- new InstanceInitializerMerger(appView, group, instanceInitializers, mode));
+ new InstanceInitializerMerger(
+ appView, classIdentifiers, group, instanceInitializers, lensBuilder, mode));
}
public InstanceInitializerMerger buildSingle(
@@ -145,56 +229,18 @@
assert instanceInitializerGroups.size() == 1;
List<ProgramMethod> instanceInitializers = ListUtils.first(instanceInitializerGroups);
return new InstanceInitializerMerger(
- appView, group, instanceInitializers, mode, instanceInitializerDescription);
+ appView,
+ classIdentifiers,
+ group,
+ instanceInitializers,
+ lensBuilder,
+ mode,
+ instanceInitializerDescription);
}
}
- // Returns true if we can simply use an existing constructor as the new constructor.
- private boolean isTrivialMerge(ClassMethodsBuilder classMethodsBuilder) {
- if (group.hasClassIdField()) {
- // We need to set the class id field.
- return false;
- }
- DexMethod trivialInstanceInitializerReference =
- ListUtils.first(instanceInitializers)
- .getReference()
- .withHolder(group.getTarget(), dexItemFactory);
- if (!classMethodsBuilder.isFresh(trivialInstanceInitializerReference)) {
- // We need to append null arguments for disambiguation.
- return false;
- }
- return isMergeOfEquivalentInstanceInitializers();
- }
-
- private boolean isMergeOfEquivalentInstanceInitializers() {
- Iterator<ProgramMethod> instanceInitializerIterator = instanceInitializers.iterator();
- ProgramMethod firstInstanceInitializer = instanceInitializerIterator.next();
- if (!instanceInitializerIterator.hasNext()) {
- return true;
- }
- // We need all the constructors to be equivalent.
- InstanceInitializerInfo instanceInitializerInfo =
- firstInstanceInitializer
- .getDefinition()
- .getOptimizationInfo()
- .getContextInsensitiveInstanceInitializerInfo();
- if (!instanceInitializerInfo.hasParent()) {
- // We don't know the parent constructor of the first constructor.
- return false;
- }
- DexMethod parent = instanceInitializerInfo.getParent();
- return Iterables.all(
- instanceInitializers,
- instanceInitializer ->
- isSideEffectFreeInstanceInitializerWithParent(instanceInitializer, parent));
- }
-
- private boolean isSideEffectFreeInstanceInitializerWithParent(
- ProgramMethod instanceInitializer, DexMethod parent) {
- MethodOptimizationInfo optimizationInfo =
- instanceInitializer.getDefinition().getOptimizationInfo();
- return !optimizationInfo.mayHaveSideEffects()
- && optimizationInfo.getContextInsensitiveInstanceInitializerInfo().getParent() == parent;
+ private boolean hasInstanceInitializerDescription() {
+ return instanceInitializerDescription != null;
}
private DexMethod moveInstanceInitializer(
@@ -219,128 +265,118 @@
return method;
}
- private MethodAccessFlags getAccessFlags() {
+ private MethodAccessFlags getNewAccessFlags() {
// TODO(b/164998929): ensure this behaviour is correct, should probably calculate upper bound
return MethodAccessFlags.fromSharedAccessFlags(
Constants.ACC_PUBLIC | Constants.ACC_SYNTHETIC, true);
}
+ private Code getNewCode(
+ DexMethod newMethodReference,
+ DexMethod syntheticMethodReference,
+ boolean needsClassId,
+ int extraNulls) {
+ if (hasInstanceInitializerDescription()) {
+ if (mode.isInitial() || appView.options().isGeneratingClassFiles()) {
+ return instanceInitializerDescription.createCfCode(
+ newMethodReference,
+ getOriginalMethodReference(),
+ syntheticMethodReference,
+ group,
+ needsClassId,
+ extraNulls);
+ }
+ return instanceInitializerDescription.createDexCode(
+ newMethodReference,
+ getOriginalMethodReference(),
+ syntheticMethodReference,
+ group,
+ needsClassId,
+ extraNulls);
+ }
+ if (isSingleton() && !group.hasClassIdField()) {
+ return getRepresentative().getDefinition().getCode();
+ }
+ return new ConstructorEntryPointSynthesizedCode(
+ createClassIdToInstanceInitializerMap(),
+ newMethodReference,
+ group.hasClassIdField() ? group.getClassIdField() : null,
+ syntheticMethodReference);
+ }
+
+ private boolean isSingleton() {
+ return instanceInitializers.size() == 1;
+ }
+
/** Synthesize a new method which selects the constructor based on a parameter type. */
void merge(
ClassMethodsBuilder classMethodsBuilder,
- HorizontalClassMergerGraphLens.Builder lensBuilder,
- Reference2IntMap<DexType> classIdentifiers,
SyntheticArgumentClass syntheticArgumentClass) {
- // TODO(b/189296638): Handle merging of equivalent constructors when
- // `instanceInitializerDescription` is set.
- if (isTrivialMerge(classMethodsBuilder)) {
- mergeTrivial(classMethodsBuilder, lensBuilder);
- return;
- }
-
- assert mode.isInitial();
-
- // Tree map as must be sorted.
- Int2ReferenceSortedMap<DexMethod> typeConstructorClassMap = new Int2ReferenceAVLTreeMap<>();
-
- // Move constructors to target class.
- CfVersion classFileVersion = null;
- for (ProgramMethod instanceInitializer : instanceInitializers) {
- if (instanceInitializer.getDefinition().hasClassFileVersion()) {
- classFileVersion =
- Ordered.maxIgnoreNull(
- classFileVersion, instanceInitializer.getDefinition().getClassFileVersion());
- }
- DexMethod movedInstanceInitializer =
- moveInstanceInitializer(classMethodsBuilder, instanceInitializer);
- lensBuilder.mapMethod(movedInstanceInitializer, movedInstanceInitializer);
- lensBuilder.recordNewMethodSignature(
- instanceInitializer.getReference(), movedInstanceInitializer);
- typeConstructorClassMap.put(
- classIdentifiers.getInt(instanceInitializer.getHolderType()), movedInstanceInitializer);
- }
-
- // Create merged constructor reference.
- DexMethod methodReferenceTemplate = generateReferenceMethodTemplate();
- DexMethod newConstructorReference =
- dexItemFactory.createInstanceInitializerWithFreshProto(
- methodReferenceTemplate,
- syntheticArgumentClass.getArgumentClasses(),
- classMethodsBuilder::isFresh);
- int extraNulls = newConstructorReference.getArity() - methodReferenceTemplate.getArity();
-
ProgramMethod representative = ListUtils.first(instanceInitializers);
- DexMethod originalConstructorReference =
- appView.graphLens().getOriginalMethodSignature(representative.getReference());
- // Create a special original method signature for the synthesized constructor that did not exist
- // prior to horizontal class merging. Otherwise we might accidentally think that the synthesized
- // constructor corresponds to the previous <init>() method on the target class, which could have
- // unintended side-effects such as leading to unused argument removal being applied to the
- // synthesized constructor all-though it by construction doesn't have any unused arguments.
- DexMethod bridgeConstructorReference =
- dexItemFactory.createFreshMethodNameWithoutHolder(
- "$r8$init$bridge",
- originalConstructorReference.getProto(),
- originalConstructorReference.getHolderType(),
+ // Create merged instance initializer reference.
+ boolean needsClassId =
+ instanceInitializers.size() > 1
+ && (!hasInstanceInitializerDescription() || group.hasClassIdField());
+ assert mode.isInitial() || !needsClassId;
+
+ DexMethod newMethodReferenceTemplate = getNewMethodReference(representative, needsClassId);
+ assert mode.isInitial() || classMethodsBuilder.isFresh(newMethodReferenceTemplate);
+
+ DexMethod newMethodReference =
+ dexItemFactory.createInstanceInitializerWithFreshProto(
+ newMethodReferenceTemplate,
+ mode.isInitial() ? syntheticArgumentClass.getArgumentClasses() : ImmutableList.of(),
classMethodsBuilder::isFresh);
+ int extraNulls = newMethodReference.getArity() - newMethodReferenceTemplate.getArity();
- ConstructorEntryPointSynthesizedCode synthesizedCode =
- new ConstructorEntryPointSynthesizedCode(
- typeConstructorClassMap,
- newConstructorReference,
- group.hasClassIdField() ? group.getClassIdField() : null,
- bridgeConstructorReference);
- DexEncodedMethod newConstructor =
- new DexEncodedMethod(
- newConstructorReference,
- getAccessFlags(),
- MethodTypeSignature.noSignature(),
- DexAnnotationSet.empty(),
- ParameterAnnotationsList.empty(),
- synthesizedCode,
- true,
- classFileVersion);
+ // Verify that the merge is a simple renaming in the final round of merging.
+ assert mode.isInitial() || newMethodReference == newMethodReferenceTemplate;
- // Map each old constructor to the newly synthesized constructor in the graph lens.
- for (ProgramMethod oldInstanceInitializer : instanceInitializers) {
+ // Move instance initializers to target class.
+ if (hasInstanceInitializerDescription()) {
+ lensBuilder.moveMethods(instanceInitializers, newMethodReference);
+ } else if (isSingleton() && !group.hasClassIdField()) {
+ lensBuilder.moveMethod(representative.getReference(), newMethodReference, true);
+ } else {
+ for (ProgramMethod instanceInitializer : instanceInitializers) {
+ DexMethod movedInstanceInitializer =
+ moveInstanceInitializer(classMethodsBuilder, instanceInitializer);
+ lensBuilder.mapMethod(movedInstanceInitializer, movedInstanceInitializer);
+ lensBuilder.recordNewMethodSignature(
+ instanceInitializer.getReference(), movedInstanceInitializer);
+ }
+ }
+
+ // Add a mapping from a synthetic name to the synthetic constructor.
+ DexMethod syntheticMethodReference =
+ getSyntheticMethodReference(classMethodsBuilder, representative);
+ if (!isSingleton() || group.hasClassIdField()) {
+ lensBuilder.recordNewMethodSignature(syntheticMethodReference, newMethodReference, true);
+ }
+
+ // Map each of the instance initializers to the new instance initializer in the graph lens.
+ for (ProgramMethod instanceInitializer : instanceInitializers) {
List<ExtraParameter> extraParameters = new ArrayList<>();
- if (instanceInitializers.size() > 1) {
- int classIdentifier = classIdentifiers.getInt(oldInstanceInitializer.getHolderType());
+ if (needsClassId) {
+ int classIdentifier = classIdentifiers.getInt(instanceInitializer.getHolderType());
extraParameters.add(new ExtraConstantIntParameter(classIdentifier));
}
extraParameters.addAll(Collections.nCopies(extraNulls, new ExtraUnusedNullParameter()));
lensBuilder.mapMergedConstructor(
- oldInstanceInitializer.getReference(), newConstructorReference, extraParameters);
+ instanceInitializer.getReference(), newMethodReference, extraParameters);
}
- // Add a mapping from a synthetic name to the synthetic constructor.
- lensBuilder.recordNewMethodSignature(bridgeConstructorReference, newConstructorReference);
-
- classMethodsBuilder.addDirectMethod(newConstructor);
- }
-
- private void mergeTrivial(
- ClassMethodsBuilder classMethodsBuilder, HorizontalClassMergerGraphLens.Builder lensBuilder) {
- ProgramMethod representative = ListUtils.first(instanceInitializers);
- DexMethod newMethodReference =
- representative.getReference().withHolder(group.getTarget(), dexItemFactory);
- lensBuilder.moveMethods(instanceInitializers, newMethodReference, representative);
-
- DexEncodedMethod newMethod =
- representative.getHolder() == group.getTarget()
- ? representative.getDefinition()
- : representative.getDefinition().toTypeSubstitutedMethod(newMethodReference);
- fixupAccessFlagsForTrivialMerge(newMethod.getAccessFlags());
-
- classMethodsBuilder.addDirectMethod(newMethod);
- }
-
- private void fixupAccessFlagsForTrivialMerge(MethodAccessFlags accessFlags) {
- if (!accessFlags.isPublic()) {
- accessFlags.unsetPrivate();
- accessFlags.unsetProtected();
- accessFlags.setPublic();
- }
+ DexEncodedMethod newInstanceInitializer =
+ new DexEncodedMethod(
+ newMethodReference,
+ getNewAccessFlags(),
+ MethodTypeSignature.noSignature(),
+ DexAnnotationSet.empty(),
+ ParameterAnnotationsList.empty(),
+ getNewCode(newMethodReference, syntheticMethodReference, needsClassId, extraNulls),
+ true,
+ getNewClassFileVersion());
+ classMethodsBuilder.addDirectMethod(newInstanceInitializer);
}
}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMergerCollection.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMergerCollection.java
index ed7e099..b5e485b 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMergerCollection.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMergerCollection.java
@@ -10,9 +10,11 @@
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexProto;
+import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.horizontalclassmerging.HorizontalClassMerger.Mode;
import com.android.tools.r8.horizontalclassmerging.InstanceInitializerMerger.Builder;
import com.android.tools.r8.utils.collections.ProgramMethodSet;
+import it.unimi.dsi.fastutil.objects.Reference2IntMap;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.LinkedHashMap;
@@ -30,14 +32,15 @@
List<InstanceInitializerMerger> instanceInitializerMergers,
Map<InstanceInitializerDescription, InstanceInitializerMerger>
equivalentInstanceInitializerMergers) {
- assert equivalentInstanceInitializerMergers.isEmpty();
this.instanceInitializerMergers = instanceInitializerMergers;
this.equivalentInstanceInitializerMergers = equivalentInstanceInitializerMergers;
}
public static InstanceInitializerMergerCollection create(
AppView<? extends AppInfoWithClassHierarchy> appView,
+ Reference2IntMap<DexType> classIdentifiers,
MergeGroup group,
+ ClassInstanceFieldsMerger instanceFieldsMerger,
HorizontalClassMergerGraphLens.Builder lensBuilder,
Mode mode) {
// Create an instance initializer merger for each group of instance initializers that are
@@ -50,12 +53,16 @@
DexEncodedMethod::isInstanceInitializer,
instanceInitializer -> {
InstanceInitializerDescription description =
- InstanceInitializerAnalysis.analyze(instanceInitializer, lensBuilder);
+ InstanceInitializerAnalysis.analyze(
+ appView, group, instanceInitializer, instanceFieldsMerger, mode);
if (description != null) {
buildersByDescription
.computeIfAbsent(
description,
- ignoreKey(() -> new InstanceInitializerMerger.Builder(appView, mode)))
+ ignoreKey(
+ () ->
+ new InstanceInitializerMerger.Builder(
+ appView, classIdentifiers, lensBuilder, mode)))
.addEquivalent(instanceInitializer);
} else {
buildersWithoutDescription.add(instanceInitializer);
@@ -86,7 +93,9 @@
buildersByProto
.computeIfAbsent(
instanceInitializer.getDefinition().getProto(),
- ignore -> new InstanceInitializerMerger.Builder(appView, mode))
+ ignore ->
+ new InstanceInitializerMerger.Builder(
+ appView, classIdentifiers, lensBuilder, mode))
.add(instanceInitializer));
for (InstanceInitializerMerger.Builder builder : buildersByProto.values()) {
instanceInitializerMergers.addAll(builder.build(group));
@@ -95,7 +104,8 @@
buildersWithoutDescription.forEach(
instanceInitializer ->
instanceInitializerMergers.addAll(
- new InstanceInitializerMerger.Builder(appView, mode)
+ new InstanceInitializerMerger.Builder(
+ appView, classIdentifiers, lensBuilder, mode)
.add(instanceInitializer)
.build(group)));
}
@@ -110,5 +120,6 @@
public void forEach(Consumer<InstanceInitializerMerger> consumer) {
instanceInitializerMergers.forEach(consumer);
+ equivalentInstanceInitializerMergers.values().forEach(consumer);
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/value/SingleDexItemBasedStringValue.java b/src/main/java/com/android/tools/r8/ir/analysis/value/SingleDexItemBasedStringValue.java
index f2d8962..13df457 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/value/SingleDexItemBasedStringValue.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/value/SingleDexItemBasedStringValue.java
@@ -34,6 +34,14 @@
this.nameComputationInfo = nameComputationInfo;
}
+ public DexReference getItem() {
+ return item;
+ }
+
+ public NameComputationInfo<?> getNameComputationInfo() {
+ return nameComputationInfo;
+ }
+
@Override
public boolean isSingleDexItemBasedStringValue() {
return true;
diff --git a/src/main/java/com/android/tools/r8/utils/ArrayUtils.java b/src/main/java/com/android/tools/r8/utils/ArrayUtils.java
index aae7abe..4fc6914 100644
--- a/src/main/java/com/android/tools/r8/utils/ArrayUtils.java
+++ b/src/main/java/com/android/tools/r8/utils/ArrayUtils.java
@@ -87,6 +87,10 @@
return true;
}
+ public static <T> T last(T[] array) {
+ return array[array.length - 1];
+ }
+
/**
* Rewrites the input array based on the given function.
*
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithClassIdAfterUnusedArgumentRemovalMergingTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithClassIdAfterUnusedArgumentRemovalMergingTest.java
index 5d9722b..57108ff 100644
--- a/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithClassIdAfterUnusedArgumentRemovalMergingTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithClassIdAfterUnusedArgumentRemovalMergingTest.java
@@ -52,9 +52,8 @@
inspector -> {
ClassSubject aClassSubject = inspector.clazz(A.class);
assertThat(aClassSubject, isPresent());
- // TODO(b/189296638): Should be 1.
assertEquals(
- 2, aClassSubject.allMethods(FoundMethodSubject::isInstanceInitializer).size());
+ 1, aClassSubject.allMethods(FoundMethodSubject::isInstanceInitializer).size());
})
.run(parameters.getRuntime(), Main.class)
.assertSuccessWithOutputLines("C", "D");
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithClassIdAndDifferentArgumentOrderMergingTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithClassIdAndDifferentArgumentOrderMergingTest.java
index a5d7e26..5297a09 100644
--- a/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithClassIdAndDifferentArgumentOrderMergingTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithClassIdAndDifferentArgumentOrderMergingTest.java
@@ -53,8 +53,8 @@
ClassSubject aClassSubject = inspector.clazz(A.class);
assertThat(aClassSubject, isPresent());
- // TODO(b/189296638): Should be 1, but requires changing the order of arguments at
- // constructor call sites.
+ // TODO(b/189296638): Enable constructor merging by changing the constructor
+ // arguments.
assertEquals(
2, aClassSubject.allMethods(FoundMethodSubject::isInstanceInitializer).size());
})
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithClassIdAndDifferentAssignmentOrderMergingTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithClassIdAndDifferentAssignmentOrderMergingTest.java
index bfce2d6..7a7a997 100644
--- a/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithClassIdAndDifferentAssignmentOrderMergingTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithClassIdAndDifferentAssignmentOrderMergingTest.java
@@ -52,9 +52,8 @@
inspector -> {
ClassSubject aClassSubject = inspector.clazz(A.class);
assertThat(aClassSubject, isPresent());
- // TODO(b/189296638): Should be 1.
assertEquals(
- 2, aClassSubject.allMethods(FoundMethodSubject::isInstanceInitializer).size());
+ 1, aClassSubject.allMethods(FoundMethodSubject::isInstanceInitializer).size());
})
.run(parameters.getRuntime(), Main.class)
.assertSuccessWithOutputLines("CC", "DD");
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithClassIdAndExtraNullsMergingTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithClassIdAndExtraNullsMergingTest.java
index 5e144b0..3917d56 100644
--- a/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithClassIdAndExtraNullsMergingTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithClassIdAndExtraNullsMergingTest.java
@@ -16,7 +16,6 @@
import com.android.tools.r8.synthesis.SyntheticItemsTestUtils;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.FoundMethodSubject;
-import org.junit.AssumptionViolatedException;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -52,9 +51,8 @@
inspector -> {
ClassSubject aClassSubject = inspector.clazz(A.class);
assertThat(aClassSubject, isPresent());
- // TODO(b/189296638): Should be 2.
assertEquals(
- 3, aClassSubject.allMethods(FoundMethodSubject::isInstanceInitializer).size());
+ 2, aClassSubject.allMethods(FoundMethodSubject::isInstanceInitializer).size());
ClassSubject nullArgumentClassSubject =
inspector.allClasses().stream()
@@ -63,11 +61,7 @@
SyntheticItemsTestUtils.isHorizontalInitializerTypeArgument(
clazz.getOriginalReference()))
.findFirst()
- // TODO(b/189296638): Should throw RuntimeException.
- .orElseThrow(
- () ->
- new AssumptionViolatedException(
- "Expected Horizontal initializer type argument"));
+ .orElseThrow(RuntimeException::new);
assertThat(
aClassSubject.method("void", "<init>", "java.lang.Object", "int"), isPresent());
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithClassIdMergingTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithClassIdMergingTest.java
index 59756bf..2bbd4e5 100644
--- a/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithClassIdMergingTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithClassIdMergingTest.java
@@ -50,9 +50,8 @@
inspector -> {
ClassSubject aClassSubject = inspector.clazz(A.class);
assertThat(aClassSubject, isPresent());
- // TODO(b/189296638): Should be 1.
assertEquals(
- 2, aClassSubject.allMethods(FoundMethodSubject::isInstanceInitializer).size());
+ 1, aClassSubject.allMethods(FoundMethodSubject::isInstanceInitializer).size());
})
.run(parameters.getRuntime(), Main.class)
.assertSuccessWithOutputLines("C", "D");
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithInterfaceValueToParentTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithInterfaceValueToParentTest.java
index 8785c85..c45274f 100644
--- a/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithInterfaceValueToParentTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithInterfaceValueToParentTest.java
@@ -61,8 +61,6 @@
@NeverClassInline
static class A extends Parent {
- // When merging this initializer with B(L), it is important that we choose the signature AB(J)
- // and not AB(I) or AB(Object), or the program will not verify.
A(K k) {
super(k);
}
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithPrimitiveAndReferencesParametersTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithPrimitiveAndReferencesParametersTest.java
new file mode 100644
index 0000000..48e6535
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithPrimitiveAndReferencesParametersTest.java
@@ -0,0 +1,92 @@
+// Copyright (c) 2021, 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.classmerging.horizontal;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.KeepUnusedArguments;
+import com.android.tools.r8.NeverClassInline;
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.classmerging.horizontal.EquivalentConstructorsWithClassIdAndDifferentAssignmentOrderMergingTest.A;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.FoundMethodSubject;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class EquivalentConstructorsWithPrimitiveAndReferencesParametersTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimes().withAllApiLevelsAlsoForCf().build();
+ }
+
+ public EquivalentConstructorsWithPrimitiveAndReferencesParametersTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepMainRule(Main.class)
+ .addHorizontallyMergedClassesInspector(
+ inspector ->
+ inspector.assertIsCompleteMergeGroup(A.class, B.class).assertNoOtherClassesMerged())
+ .enableInliningAnnotations()
+ .enableNeverClassInliningAnnotations()
+ .enableUnusedArgumentAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .inspect(
+ inspector -> {
+ ClassSubject aClassSubject = inspector.clazz(A.class);
+ assertThat(aClassSubject, isPresent());
+ assertEquals(
+ 2, aClassSubject.allMethods(FoundMethodSubject::isInstanceInitializer).size());
+ })
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines("A", "B");
+ }
+
+ static class Main {
+
+ public static void main(String[] args) {
+ new A(42).m1();
+ new B(new Object()).m2();
+ }
+ }
+
+ @NeverClassInline
+ static class A {
+ @KeepUnusedArguments
+ A(int i) {}
+
+ @NeverInline
+ void m1() {
+ System.out.println("A");
+ }
+ }
+
+ @NeverClassInline
+ static class B {
+ @KeepUnusedArguments
+ B(Object o) {}
+
+ @NeverInline
+ void m2() {
+ System.out.println("B");
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithoutClassIdAfterUnusedArgumentRemovalMergingTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithoutClassIdAfterUnusedArgumentRemovalMergingTest.java
index d429057..4a66189 100644
--- a/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithoutClassIdAfterUnusedArgumentRemovalMergingTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithoutClassIdAfterUnusedArgumentRemovalMergingTest.java
@@ -54,9 +54,8 @@
inspector -> {
ClassSubject aClassSubject = inspector.clazz(A.class);
assertThat(aClassSubject, isPresent());
- // TODO(b/189296638): Should be 1.
assertEquals(
- 2, aClassSubject.allMethods(FoundMethodSubject::isInstanceInitializer).size());
+ 1, aClassSubject.allMethods(FoundMethodSubject::isInstanceInitializer).size());
})
.run(parameters.getRuntime(), Main.class)
.assertSuccessWithOutputLines("C", "D");
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithoutClassIdMergingTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithoutClassIdMergingTest.java
index a8b85e0..090699b 100644
--- a/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithoutClassIdMergingTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithoutClassIdMergingTest.java
@@ -52,9 +52,8 @@
inspector -> {
ClassSubject aClassSubject = inspector.clazz(A.class);
assertThat(aClassSubject, isPresent());
- // TODO(b/189296638): Should be 1.
assertEquals(
- 2, aClassSubject.allMethods(FoundMethodSubject::isInstanceInitializer).size());
+ 1, aClassSubject.allMethods(FoundMethodSubject::isInstanceInitializer).size());
})
.run(parameters.getRuntime(), Main.class)
.assertSuccessWithOutputLines("C", "D");
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/MergedConstructorStackTraceTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/MergedConstructorStackTraceTest.java
index e785238..c39b7b5 100644
--- a/src/test/java/com/android/tools/r8/classmerging/horizontal/MergedConstructorStackTraceTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/MergedConstructorStackTraceTest.java
@@ -44,6 +44,9 @@
.addKeepMainRule(Main.class)
.addKeepAttributeLineNumberTable()
.addKeepAttributeSourceFile()
+ .addHorizontallyMergedClassesInspector(
+ inspector ->
+ inspector.assertIsCompleteMergeGroup(A.class, B.class).assertNoOtherClassesMerged())
.enableNoVerticalClassMergingAnnotations()
.enableNeverClassInliningAnnotations()
.setMinApi(parameters.getApiLevel())
@@ -52,22 +55,22 @@
.inspectStackTrace(
(stackTrace, codeInspector) -> {
assertThat(codeInspector.clazz(A.class), isPresent());
- StackTrace expectedStackTraceWithMergedConstructor =
- StackTrace.builder()
- .add(expectedStackTrace)
- .add(
- 2,
- StackTraceLine.builder()
- .setClassName(A.class.getTypeName())
- // TODO(b/124483578): The synthetic method should not be part of the
- // retraced stack trace.
- .setMethodName("$r8$init$bridge")
- .setFileName(getClass().getSimpleName() + ".java")
- .setLineNumber(0)
- .build())
- .build();
- assertThat(stackTrace, isSame(expectedStackTraceWithMergedConstructor));
- assertThat(codeInspector.clazz(B.class), not(isPresent()));
+ StackTrace expectedStackTraceWithMergedConstructor =
+ StackTrace.builder()
+ .add(expectedStackTrace)
+ .add(
+ 2,
+ StackTraceLine.builder()
+ .setClassName(A.class.getTypeName())
+ // TODO(b/124483578): The synthetic method should not be part of the
+ // retraced stack trace.
+ .setMethodName("$r8$init$synthetic")
+ .setFileName(getClass().getSimpleName() + ".java")
+ .setLineNumber(0)
+ .build())
+ .build();
+ assertThat(stackTrace, isSame(expectedStackTraceWithMergedConstructor));
+ assertThat(codeInspector.clazz(B.class), not(isPresent()));
});
}
@@ -82,12 +85,18 @@
@NeverClassInline
static class A extends Parent {
- A() {}
+ A() {
+ // To avoid constructor equivalence.
+ System.out.println("A");
+ }
}
@NeverClassInline
static class B extends Parent {
- B() {}
+ B() {
+ // To avoid constructor equivalence.
+ System.out.println("B");
+ }
}
static class Main {
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/MergedConstructorWithEquivalenceStackTraceTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/MergedConstructorWithEquivalenceStackTraceTest.java
new file mode 100644
index 0000000..9b5b87d
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/MergedConstructorWithEquivalenceStackTraceTest.java
@@ -0,0 +1,107 @@
+// Copyright (c) 2020, 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.classmerging.horizontal;
+
+import static com.android.tools.r8.naming.retrace.StackTrace.isSame;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.android.tools.r8.NeverClassInline;
+import com.android.tools.r8.NoVerticalClassMerging;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestRuntime.CfRuntime;
+import com.android.tools.r8.classmerging.horizontal.MergedConstructorStackTraceTest.A;
+import com.android.tools.r8.naming.retrace.StackTrace;
+import com.android.tools.r8.naming.retrace.StackTrace.StackTraceLine;
+import org.junit.Before;
+import org.junit.Test;
+
+public class MergedConstructorWithEquivalenceStackTraceTest extends HorizontalClassMergingTestBase {
+
+ public StackTrace expectedStackTrace;
+
+ public MergedConstructorWithEquivalenceStackTraceTest(TestParameters parameters) {
+ super(parameters);
+ }
+
+ @Before
+ public void setup() throws Exception {
+ // Get the expected stack trace by running on the JVM.
+ expectedStackTrace =
+ testForJvm()
+ .addTestClasspath()
+ .run(CfRuntime.getSystemRuntime(), Main.class)
+ .assertFailure()
+ .map(StackTrace::extractFromJvm);
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepMainRule(Main.class)
+ .addKeepAttributeLineNumberTable()
+ .addKeepAttributeSourceFile()
+ .addHorizontallyMergedClassesInspector(
+ inspector ->
+ inspector.assertIsCompleteMergeGroup(A.class, B.class).assertNoOtherClassesMerged())
+ .enableNoVerticalClassMergingAnnotations()
+ .enableNeverClassInliningAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .run(parameters.getRuntime(), Main.class)
+ .inspectStackTrace(
+ (stackTrace, codeInspector) -> {
+ assertThat(codeInspector.clazz(A.class), isPresent());
+ StackTrace expectedStackTraceWithMergedConstructor =
+ StackTrace.builder()
+ .add(expectedStackTrace)
+ // TODO(b/124483578): Stack trace lines from the merging of equivalent
+ // constructors should retrace to the set of lines from each of the
+ // individual source constructors.
+ .map(1, stackTraceLine -> stackTraceLine.builderOf().setLineNumber(0).build())
+ // TODO(b/124483578): The synthetic method should not be part of the retraced
+ // stack trace.
+ .add(
+ 2,
+ StackTraceLine.builder()
+ .setClassName(A.class.getTypeName())
+ .setMethodName("$r8$init$synthetic")
+ .setFileName(getClass().getSimpleName() + ".java")
+ .setLineNumber(0)
+ .build())
+ .build();
+ assertThat(stackTrace, isSame(expectedStackTraceWithMergedConstructor));
+ assertThat(codeInspector.clazz(B.class), not(isPresent()));
+ });
+ }
+
+ @NoVerticalClassMerging
+ static class Parent {
+ Parent() {
+ if (System.currentTimeMillis() >= 0) {
+ throw new RuntimeException();
+ }
+ }
+ }
+
+ @NeverClassInline
+ static class A extends Parent {
+ A() {}
+ }
+
+ @NeverClassInline
+ static class B extends Parent {
+ B() {}
+ }
+
+ static class Main {
+ public static void main(String[] args) {
+ new A();
+ new B();
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/DesugaredLibraryTestBase.java b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/DesugaredLibraryTestBase.java
index fdb7faa..a4c52ad 100644
--- a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/DesugaredLibraryTestBase.java
+++ b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/DesugaredLibraryTestBase.java
@@ -115,15 +115,11 @@
private List<Path> additionalProgramFiles = new ArrayList<>();
private Consumer<InternalOptions> optionsModifier = ConsumerUtils.emptyConsumer();
private Path desugarJDKLibs = ToolHelper.getDesugarJDKLibs();
- private Path desugarJDKLibsConfiguration = ToolHelper.DESUGAR_LIB_CONVERSIONS;
+ private Path desugarJDKLibsConfiguration = null;
private StringResource desugaredLibraryConfiguration =
StringResource.fromFile(ToolHelper.getDesugarLibJsonForTesting());
private List<Path> libraryFiles = new ArrayList<>();
- public static L8TestBuilder builder(AndroidApiLevel apiLevel, TemporaryFolder temp) {
- return new L8TestBuilder(apiLevel, temp);
- }
-
private L8TestBuilder(AndroidApiLevel apiLevel, TemporaryFolder temp) {
this.apiLevel = apiLevel;
this.state = new TestState(temp);
@@ -239,11 +235,11 @@
}
private Collection<Path> getProgramFiles() {
- return ImmutableList.<Path>builder()
- .add(desugarJDKLibs)
- .add(desugarJDKLibsConfiguration)
- .addAll(additionalProgramFiles)
- .build();
+ ImmutableList.Builder<Path> builder = ImmutableList.<Path>builder().add(desugarJDKLibs);
+ if (desugarJDKLibsConfiguration != null) {
+ builder.add(desugarJDKLibsConfiguration);
+ }
+ return builder.addAll(additionalProgramFiles).build();
}
private Collection<Path> getLibraryFiles() {
@@ -300,6 +296,7 @@
},
L8TestBuilder::setDebug)
.addOptionsModifier(optionsModifier)
+ .setDesugarJDKLibsConfiguration(ToolHelper.DESUGAR_LIB_CONVERSIONS)
// If we compile extended library here, it means we use TestNG. TestNG requires
// annotations, hence we disable annotation removal. This implies that extra warnings are
// generated.
diff --git a/src/test/java/com/android/tools/r8/internal/YouTubeV1620Test.java b/src/test/java/com/android/tools/r8/internal/YouTubeV1620Test.java
new file mode 100644
index 0000000..c1182bd
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/internal/YouTubeV1620Test.java
@@ -0,0 +1,191 @@
+// Copyright (c) 2021, 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.internal;
+
+import static com.android.tools.r8.ToolHelper.isLocalDevelopment;
+import static com.android.tools.r8.ToolHelper.shouldRunSlowTests;
+import static com.android.tools.r8.internal.proto.ProtoShrinkingTestBase.assertRewrittenProtoSchemasMatch;
+import static com.android.tools.r8.internal.proto.ProtoShrinkingTestBase.keepAllProtosRule;
+import static com.android.tools.r8.internal.proto.ProtoShrinkingTestBase.keepDynamicMethodSignatureRule;
+import static com.android.tools.r8.internal.proto.ProtoShrinkingTestBase.keepNewMessageInfoSignatureRule;
+import static org.hamcrest.CoreMatchers.anyOf;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assume.assumeTrue;
+
+import com.android.tools.r8.CompilationFailedException;
+import com.android.tools.r8.L8TestCompileResult;
+import com.android.tools.r8.R8FullTestBuilder;
+import com.android.tools.r8.R8TestCompileResult;
+import com.android.tools.r8.ResourceException;
+import com.android.tools.r8.StringConsumer;
+import com.android.tools.r8.StringResource;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.ThrowableConsumer;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.ToolHelper.DexVm.Version;
+import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.Reporter;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.concurrent.ExecutionException;
+import org.junit.After;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class YouTubeV1620Test extends YouTubeCompilationTestBase {
+
+ private static final int MAX_APPLICATION_SIZE = 29750000;
+ private static final int MAX_DESUGARED_LIBRARY_SIZE = 425000;
+
+ private final TestParameters parameters;
+
+ private final Path dumpDirectory = Paths.get("YouTubeV1620-" + System.currentTimeMillis());
+ private final Reporter reporter = new Reporter();
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters()
+ .withDexRuntime(Version.V9_0_0)
+ .withApiLevel(AndroidApiLevel.L)
+ .build();
+ }
+
+ public YouTubeV1620Test(TestParameters parameters) {
+ super(16, 20, parameters.getApiLevel());
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ assumeTrue(isLocalDevelopment());
+ assumeTrue(shouldRunSlowTests());
+
+ KeepRuleConsumer keepRuleConsumer = new PresentKeepRuleConsumer();
+ R8TestCompileResult r8CompileResult = compileApplicationWithR8(keepRuleConsumer);
+ L8TestCompileResult l8CompileResult = compileDesugaredLibraryWithL8(keepRuleConsumer);
+
+ inspect(r8CompileResult, l8CompileResult);
+
+ if (isLocalDevelopment()) {
+ dump(r8CompileResult, l8CompileResult);
+ }
+ }
+
+ @Test
+ public void testProtoRewriting() throws Exception {
+ assumeTrue(shouldRunSlowTests());
+
+ StringConsumer keepRuleConsumer = StringConsumer.emptyConsumer();
+ R8TestCompileResult r8CompileResult =
+ compileApplicationWithR8(
+ keepRuleConsumer,
+ builder ->
+ builder
+ .addKeepRules(
+ keepAllProtosRule(),
+ keepDynamicMethodSignatureRule(),
+ keepNewMessageInfoSignatureRule())
+ .allowCheckDiscardedErrors(true));
+ assertRewrittenProtoSchemasMatch(
+ new CodeInspector(getProgramFiles()), r8CompileResult.inspector());
+ }
+
+ @After
+ public void teardown() {
+ reporter.failIfPendingErrors();
+ }
+
+ private R8TestCompileResult compileApplicationWithR8(StringConsumer keepRuleConsumer)
+ throws IOException, CompilationFailedException {
+ return compileApplicationWithR8(keepRuleConsumer, ThrowableConsumer.empty());
+ }
+
+ private R8TestCompileResult compileApplicationWithR8(
+ StringConsumer keepRuleConsumer, ThrowableConsumer<R8FullTestBuilder> configuration)
+ throws IOException, CompilationFailedException {
+ return testForR8(parameters.getBackend())
+ .addProgramFiles(getProgramFiles())
+ .addLibraryFiles(getLibraryFiles())
+ .addKeepRuleFiles(getKeepRuleFiles())
+ .addDontWarn("android.app.Activity$TranslucentConversionListener")
+ .allowDiagnosticMessages()
+ .allowUnusedDontWarnPatterns()
+ .allowUnusedProguardConfigurationRules()
+ .apply(configuration)
+ .setMinApi(getApiLevel())
+ .enableCoreLibraryDesugaring(
+ getApiLevel(),
+ keepRuleConsumer,
+ StringResource.fromFile(getDesugaredLibraryConfiguration()))
+ .compile()
+ .assertAllInfoMessagesMatch(
+ anyOf(
+ containsString("Ignoring option: -optimizations"),
+ containsString("Proguard configuration rule does not match anything"),
+ containsString("Invalid signature")))
+ .apply(this::printProtoStats);
+ }
+
+ private L8TestCompileResult compileDesugaredLibraryWithL8(KeepRuleConsumer keepRuleConsumer)
+ throws CompilationFailedException, IOException, ExecutionException {
+ return testForL8(getApiLevel())
+ .setDesugaredLibraryConfiguration(getDesugaredLibraryConfiguration())
+ .setDesugarJDKLibs(getDesugaredLibraryJDKLibs())
+ .addGeneratedKeepRules(keepRuleConsumer.get())
+ .addKeepRuleFiles(getDesugaredLibraryKeepRuleFiles())
+ .addLibraryFiles(ToolHelper.getAndroidJar(AndroidApiLevel.P))
+ .compile();
+ }
+
+ private void inspect(R8TestCompileResult r8CompileResult, L8TestCompileResult l8CompileResult)
+ throws IOException, ResourceException {
+ r8CompileResult.runDex2Oat(parameters.getRuntime()).assertNoVerificationErrors();
+ l8CompileResult.runDex2Oat(parameters.getRuntime()).assertNoVerificationErrors();
+
+ int applicationSize = r8CompileResult.getApp().applicationSize();
+ if (applicationSize > MAX_APPLICATION_SIZE) {
+ reporter.error(
+ "Expected application size to be <="
+ + MAX_APPLICATION_SIZE
+ + ", but was "
+ + applicationSize);
+ }
+
+ int desugaredLibrarySize = l8CompileResult.getApp().applicationSize();
+ if (desugaredLibrarySize > MAX_DESUGARED_LIBRARY_SIZE) {
+ reporter.error(
+ "Expected desugared library size to be <="
+ + MAX_DESUGARED_LIBRARY_SIZE
+ + ", but was "
+ + desugaredLibrarySize);
+ }
+
+ if (isLocalDevelopment()) {
+ System.out.println("Dex size (application, excluding desugared library): " + applicationSize);
+ System.out.println("Dex size (desugared library): " + desugaredLibrarySize);
+ System.out.println("Dex size (total): " + (applicationSize + desugaredLibrarySize));
+ }
+ }
+
+ private void dump(R8TestCompileResult r8CompileResult, L8TestCompileResult l8CompileResult)
+ throws IOException {
+ assertTrue(isLocalDevelopment());
+ Files.createDirectories(dumpDirectory);
+ r8CompileResult
+ .writeToDirectory(dumpDirectory)
+ .writeProguardMap(dumpDirectory.resolve("mapping.txt"));
+ l8CompileResult
+ .writeSingleDexOutputToFile(dumpDirectory.resolve("classes5.dex"))
+ .writeGeneratedKeepRules(dumpDirectory.resolve("l8-keep.txt"))
+ .writeProguardMap(dumpDirectory.resolve("l8-mapping.txt"));
+ }
+}
