Revert "Merge equivalent instance initializers"
This reverts commit f1a7dfd4e7ace989f8be1dfa20c2dff89f004848.
Reason for revert: Failures
Change-Id: I4528bdd771c900f2a9306b294ff10559a979dd50
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 cd8d6b7..592ee31 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,10 +42,6 @@
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 a42388a..646c9a0 100644
--- a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
+++ b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
@@ -2238,10 +2238,6 @@
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);
@@ -2262,6 +2258,8 @@
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();
@@ -2278,7 +2276,6 @@
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 96c3084..a38770a 100644
--- a/src/main/java/com/android/tools/r8/graph/DexType.java
+++ b/src/main/java/com/android/tools/r8/graph/DexType.java
@@ -93,11 +93,6 @@
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
deleted file mode 100644
index b6d9757..0000000
--- a/src/main/java/com/android/tools/r8/graph/DexTypeUtils.java
+++ /dev/null
@@ -1,89 +0,0 @@
-// 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 3927d11..999869c 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java
@@ -62,18 +62,13 @@
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<>();
-
- // Field mergers.
- private final ClassInstanceFieldsMerger classInstanceFieldsMerger;
private final ClassStaticFieldsMerger classStaticFieldsMerger;
-
- // Method mergers.
- private final ClassInitializerMerger classInitializerMerger;
- private final InstanceInitializerMergerCollection instanceInitializerMergers;
+ private final ClassInstanceFieldsMerger classInstanceFieldsMerger;
private final Collection<VirtualMethodMerger> virtualMethodMergers;
private ClassMerger(
@@ -87,18 +82,10 @@
this.group = group;
this.lensBuilder = lensBuilder;
this.mode = mode;
-
- // Field mergers.
+ this.virtualMethodMergers = virtualMethodMergers;
+ this.classInitializerMerger = ClassInitializerMerger.create(group);
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();
}
@@ -188,8 +175,12 @@
}
void mergeInstanceInitializers(SyntheticArgumentClass syntheticArgumentClass) {
+ InstanceInitializerMergerCollection instanceInitializerMergers =
+ InstanceInitializerMergerCollection.create(appView, group, lensBuilder, mode);
instanceInitializerMergers.forEach(
- merger -> merger.merge(classMethodsBuilder, syntheticArgumentClass));
+ merger ->
+ merger.merge(
+ classMethodsBuilder, lensBuilder, classIdentifiers, 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 4ad6f55..c05af00 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java
@@ -212,9 +212,6 @@
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
deleted file mode 100644
index 839b6d5..0000000
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMergerCfCode.java
+++ /dev/null
@@ -1,33 +0,0 @@
-// 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 20eb5a3..abadbc4 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMergerGraphLens.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMergerGraphLens.java
@@ -120,11 +120,6 @@
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 9a6390a..febdbe6 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerAnalysis.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerAnalysis.java
@@ -4,204 +4,14 @@
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(
- 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() {
+ 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.
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 b67c652..41f2c4e 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerDescription.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerDescription.java
@@ -4,44 +4,14 @@
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
@@ -49,219 +19,24 @@
*/
public class InstanceInitializerDescription {
- // 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 int arity;
+ private final Map<DexField, InstanceFieldInitializationInfo> instanceFieldAssignments;
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(
- Map<DexField, InstanceFieldInitializationInfo> instanceFieldAssignmentsPre,
- Map<DexField, InstanceFieldInitializationInfo> instanceFieldAssignmentsPost,
+ int arity,
+ Map<DexField, InstanceFieldInitializationInfo> instanceFieldAssignments,
DexMethod parentConstructor,
- List<InstanceFieldInitializationInfo> parentConstructorArguments,
- DexTypeList relaxedParameters) {
- this.instanceFieldAssignmentsPre = instanceFieldAssignmentsPre;
- this.instanceFieldAssignmentsPost = instanceFieldAssignmentsPost;
+ List<InstanceFieldInitializationInfo> parentConstructorArguments) {
+ this.arity = arity;
+ this.instanceFieldAssignments = instanceFieldAssignments;
this.parentConstructor = parentConstructor;
this.parentConstructorArguments = parentConstructorArguments;
- this.relaxedParameters = relaxedParameters;
}
- 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);
+ public static Builder builder(ProgramMethod instanceInitializer) {
+ return new Builder(instanceInitializer);
}
@Override
@@ -270,8 +45,8 @@
return false;
}
InstanceInitializerDescription description = (InstanceInitializerDescription) obj;
- return instanceFieldAssignmentsPre.equals(description.instanceFieldAssignmentsPre)
- && instanceFieldAssignmentsPost.equals(description.instanceFieldAssignmentsPost)
+ return arity == description.arity
+ && instanceFieldAssignments.equals(description.instanceFieldAssignments)
&& parentConstructor == description.parentConstructor
&& parentConstructorArguments.equals(description.parentConstructorArguments);
}
@@ -279,67 +54,38 @@
@Override
public int hashCode() {
return Objects.hash(
- instanceFieldAssignmentsPre,
- instanceFieldAssignmentsPost,
- parentConstructor,
- parentConstructorArguments,
- relaxedParameters);
+ arity, instanceFieldAssignments, parentConstructor, parentConstructorArguments);
}
public static class Builder {
- private final DexItemFactory dexItemFactory;
- private final DexTypeList relaxedParameters;
+ private final int arity;
- private Map<DexField, InstanceFieldInitializationInfo> instanceFieldAssignmentsPre =
- new LinkedHashMap<>();
- private Map<DexField, InstanceFieldInitializationInfo> instanceFieldAssignmentsPost =
+ private Map<DexField, InstanceFieldInitializationInfo> instanceFieldAssignments =
new LinkedHashMap<>();
private DexMethod parentConstructor;
private List<InstanceFieldInitializationInfo> parentConstructorArguments;
- Builder(DexItemFactory dexItemFactory, ProgramMethod method) {
- this.dexItemFactory = dexItemFactory;
- this.relaxedParameters =
- method
- .getParameters()
- .map(
- parameter -> parameter.isPrimitiveType() ? parameter : dexItemFactory.objectType);
+ Builder(ProgramMethod method) {
+ this.arity = method.getReference().getArity();
}
public void addInstancePut(DexField field, InstanceFieldInitializationInfo 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);
- }
+ instanceFieldAssignments.put(field, value);
}
- public boolean addInvokeConstructor(
+ public void addInvokeConstructor(
DexMethod method, List<InstanceFieldInitializationInfo> arguments) {
- if (parentConstructor == null) {
- parentConstructor = method;
- parentConstructorArguments = arguments;
- return true;
- }
- return false;
+ assert parentConstructor == null;
+ assert parentConstructorArguments == null;
+ parentConstructor = method;
+ parentConstructorArguments = arguments;
}
public InstanceInitializerDescription build() {
- assert isValid();
+ assert parentConstructor != null;
return new InstanceInitializerDescription(
- instanceFieldAssignmentsPre,
- instanceFieldAssignmentsPost,
- parentConstructor,
- parentConstructorArguments,
- relaxedParameters);
- }
-
- public boolean isValid() {
- return parentConstructor != null;
+ arity, instanceFieldAssignments, parentConstructor, parentConstructorArguments);
}
}
}
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 862a23c..b50c71f 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMerger.java
@@ -10,13 +10,11 @@
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;
@@ -26,62 +24,61 @@
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.utils.ArrayUtils;
-import com.android.tools.r8.utils.BooleanUtils;
+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.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<? extends AppInfoWithClassHierarchy> appView;
- private final Reference2IntMap<DexType> classIdentifiers;
+ private final AppView<?> appView;
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<? extends AppInfoWithClassHierarchy> appView,
- Reference2IntMap<DexType> classIdentifiers,
- MergeGroup group,
- List<ProgramMethod> instanceInitializers,
- HorizontalClassMergerGraphLens.Builder lensBuilder,
- Mode mode) {
- this(appView, classIdentifiers, group, instanceInitializers, lensBuilder, mode, null);
+ AppView<?> appView, MergeGroup group, List<ProgramMethod> instanceInitializers, Mode mode) {
+ this(appView, group, instanceInitializers, mode, null);
}
InstanceInitializerMerger(
- AppView<? extends AppInfoWithClassHierarchy> appView,
- Reference2IntMap<DexType> classIdentifiers,
+ AppView<?> appView,
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 unless
- // equivalent.
+ // Constructors should not be empty and all constructors should have the same prototype.
assert !instanceInitializers.isEmpty();
- assert instanceInitializers.stream().map(ProgramMethod::getProto).distinct().count() == 1
- || instanceInitializerDescription != null;
+ 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);
}
public int getArity() {
@@ -92,78 +89,6 @@
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();
}
@@ -171,20 +96,12 @@
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,
- Reference2IntMap<DexType> classIdentifiers,
- HorizontalClassMergerGraphLens.Builder lensBuilder,
- Mode mode) {
+ public Builder(AppView<? extends AppInfoWithClassHierarchy> appView, Mode mode) {
this.appView = appView;
- this.classIdentifiers = classIdentifiers;
- this.lensBuilder = lensBuilder;
this.mode = mode;
createNewGroup();
}
@@ -219,8 +136,7 @@
return ListUtils.map(
instanceInitializerGroups,
instanceInitializers ->
- new InstanceInitializerMerger(
- appView, classIdentifiers, group, instanceInitializers, lensBuilder, mode));
+ new InstanceInitializerMerger(appView, group, instanceInitializers, mode));
}
public InstanceInitializerMerger buildSingle(
@@ -229,18 +145,56 @@
assert instanceInitializerGroups.size() == 1;
List<ProgramMethod> instanceInitializers = ListUtils.first(instanceInitializerGroups);
return new InstanceInitializerMerger(
- appView,
- classIdentifiers,
- group,
- instanceInitializers,
- lensBuilder,
- mode,
- instanceInitializerDescription);
+ appView, group, instanceInitializers, mode, instanceInitializerDescription);
}
}
- private boolean hasInstanceInitializerDescription() {
- return instanceInitializerDescription != null;
+ // 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 DexMethod moveInstanceInitializer(
@@ -265,118 +219,128 @@
return method;
}
- private MethodAccessFlags getNewAccessFlags() {
+ private MethodAccessFlags getAccessFlags() {
// 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) {
- ProgramMethod representative = ListUtils.first(instanceInitializers);
-
- // 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();
-
- // Verify that the merge is a simple renaming in the final round of merging.
- assert mode.isInitial() || newMethodReference == newMethodReferenceTemplate;
-
- // 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);
- }
+ // TODO(b/189296638): Handle merging of equivalent constructors when
+ // `instanceInitializerDescription` is set.
+ if (isTrivialMerge(classMethodsBuilder)) {
+ mergeTrivial(classMethodsBuilder, lensBuilder);
+ return;
}
- // 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);
- }
+ assert mode.isInitial();
- // Map each of the instance initializers to the new instance initializer in the graph lens.
+ // 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(),
+ classMethodsBuilder::isFresh);
+
+ 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);
+
+ // Map each old constructor to the newly synthesized constructor in the graph lens.
+ for (ProgramMethod oldInstanceInitializer : instanceInitializers) {
List<ExtraParameter> extraParameters = new ArrayList<>();
- if (needsClassId) {
- int classIdentifier = classIdentifiers.getInt(instanceInitializer.getHolderType());
+ if (instanceInitializers.size() > 1) {
+ int classIdentifier = classIdentifiers.getInt(oldInstanceInitializer.getHolderType());
extraParameters.add(new ExtraConstantIntParameter(classIdentifier));
}
extraParameters.addAll(Collections.nCopies(extraNulls, new ExtraUnusedNullParameter()));
lensBuilder.mapMergedConstructor(
- instanceInitializer.getReference(), newMethodReference, extraParameters);
+ oldInstanceInitializer.getReference(), newConstructorReference, extraParameters);
}
- DexEncodedMethod newInstanceInitializer =
- new DexEncodedMethod(
- newMethodReference,
- getNewAccessFlags(),
- MethodTypeSignature.noSignature(),
- DexAnnotationSet.empty(),
- ParameterAnnotationsList.empty(),
- getNewCode(newMethodReference, syntheticMethodReference, needsClassId, extraNulls),
- true,
- getNewClassFileVersion());
- classMethodsBuilder.addDirectMethod(newInstanceInitializer);
+ // 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();
+ }
}
}
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 b5e485b..ed7e099 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMergerCollection.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMergerCollection.java
@@ -10,11 +10,9 @@
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;
@@ -32,15 +30,14 @@
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
@@ -53,16 +50,12 @@
DexEncodedMethod::isInstanceInitializer,
instanceInitializer -> {
InstanceInitializerDescription description =
- InstanceInitializerAnalysis.analyze(
- appView, group, instanceInitializer, instanceFieldsMerger, mode);
+ InstanceInitializerAnalysis.analyze(instanceInitializer, lensBuilder);
if (description != null) {
buildersByDescription
.computeIfAbsent(
description,
- ignoreKey(
- () ->
- new InstanceInitializerMerger.Builder(
- appView, classIdentifiers, lensBuilder, mode)))
+ ignoreKey(() -> new InstanceInitializerMerger.Builder(appView, mode)))
.addEquivalent(instanceInitializer);
} else {
buildersWithoutDescription.add(instanceInitializer);
@@ -93,9 +86,7 @@
buildersByProto
.computeIfAbsent(
instanceInitializer.getDefinition().getProto(),
- ignore ->
- new InstanceInitializerMerger.Builder(
- appView, classIdentifiers, lensBuilder, mode))
+ ignore -> new InstanceInitializerMerger.Builder(appView, mode))
.add(instanceInitializer));
for (InstanceInitializerMerger.Builder builder : buildersByProto.values()) {
instanceInitializerMergers.addAll(builder.build(group));
@@ -104,8 +95,7 @@
buildersWithoutDescription.forEach(
instanceInitializer ->
instanceInitializerMergers.addAll(
- new InstanceInitializerMerger.Builder(
- appView, classIdentifiers, lensBuilder, mode)
+ new InstanceInitializerMerger.Builder(appView, mode)
.add(instanceInitializer)
.build(group)));
}
@@ -120,6 +110,5 @@
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 13df457..f2d8962 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,14 +34,6 @@
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 4fc6914..aae7abe 100644
--- a/src/main/java/com/android/tools/r8/utils/ArrayUtils.java
+++ b/src/main/java/com/android/tools/r8/utils/ArrayUtils.java
@@ -87,10 +87,6 @@
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 57108ff..5d9722b 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,8 +52,9 @@
inspector -> {
ClassSubject aClassSubject = inspector.clazz(A.class);
assertThat(aClassSubject, isPresent());
+ // TODO(b/189296638): Should be 1.
assertEquals(
- 1, aClassSubject.allMethods(FoundMethodSubject::isInstanceInitializer).size());
+ 2, 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 5297a09..a5d7e26 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): Enable constructor merging by changing the constructor
- // arguments.
+ // TODO(b/189296638): Should be 1, but requires changing the order of arguments at
+ // constructor call sites.
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 7a7a997..bfce2d6 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,8 +52,9 @@
inspector -> {
ClassSubject aClassSubject = inspector.clazz(A.class);
assertThat(aClassSubject, isPresent());
+ // TODO(b/189296638): Should be 1.
assertEquals(
- 1, aClassSubject.allMethods(FoundMethodSubject::isInstanceInitializer).size());
+ 2, 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 3917d56..5e144b0 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,6 +16,7 @@
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;
@@ -51,8 +52,9 @@
inspector -> {
ClassSubject aClassSubject = inspector.clazz(A.class);
assertThat(aClassSubject, isPresent());
+ // TODO(b/189296638): Should be 2.
assertEquals(
- 2, aClassSubject.allMethods(FoundMethodSubject::isInstanceInitializer).size());
+ 3, aClassSubject.allMethods(FoundMethodSubject::isInstanceInitializer).size());
ClassSubject nullArgumentClassSubject =
inspector.allClasses().stream()
@@ -61,7 +63,11 @@
SyntheticItemsTestUtils.isHorizontalInitializerTypeArgument(
clazz.getOriginalReference()))
.findFirst()
- .orElseThrow(RuntimeException::new);
+ // TODO(b/189296638): Should throw RuntimeException.
+ .orElseThrow(
+ () ->
+ new AssumptionViolatedException(
+ "Expected Horizontal initializer type argument"));
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 2bbd4e5..59756bf 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,8 +50,9 @@
inspector -> {
ClassSubject aClassSubject = inspector.clazz(A.class);
assertThat(aClassSubject, isPresent());
+ // TODO(b/189296638): Should be 1.
assertEquals(
- 1, aClassSubject.allMethods(FoundMethodSubject::isInstanceInitializer).size());
+ 2, 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 c45274f..8785c85 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,6 +61,8 @@
@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
deleted file mode 100644
index 48e6535..0000000
--- a/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentConstructorsWithPrimitiveAndReferencesParametersTest.java
+++ /dev/null
@@ -1,92 +0,0 @@
-// 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 4a66189..d429057 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,8 +54,9 @@
inspector -> {
ClassSubject aClassSubject = inspector.clazz(A.class);
assertThat(aClassSubject, isPresent());
+ // TODO(b/189296638): Should be 1.
assertEquals(
- 1, aClassSubject.allMethods(FoundMethodSubject::isInstanceInitializer).size());
+ 2, 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 090699b..a8b85e0 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,8 +52,9 @@
inspector -> {
ClassSubject aClassSubject = inspector.clazz(A.class);
assertThat(aClassSubject, isPresent());
+ // TODO(b/189296638): Should be 1.
assertEquals(
- 1, aClassSubject.allMethods(FoundMethodSubject::isInstanceInitializer).size());
+ 2, 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 c39b7b5..e785238 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,9 +44,6 @@
.addKeepMainRule(Main.class)
.addKeepAttributeLineNumberTable()
.addKeepAttributeSourceFile()
- .addHorizontallyMergedClassesInspector(
- inspector ->
- inspector.assertIsCompleteMergeGroup(A.class, B.class).assertNoOtherClassesMerged())
.enableNoVerticalClassMergingAnnotations()
.enableNeverClassInliningAnnotations()
.setMinApi(parameters.getApiLevel())
@@ -55,22 +52,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$synthetic")
- .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$bridge")
+ .setFileName(getClass().getSimpleName() + ".java")
+ .setLineNumber(0)
+ .build())
+ .build();
+ assertThat(stackTrace, isSame(expectedStackTraceWithMergedConstructor));
+ assertThat(codeInspector.clazz(B.class), not(isPresent()));
});
}
@@ -85,18 +82,12 @@
@NeverClassInline
static class A extends Parent {
- A() {
- // To avoid constructor equivalence.
- System.out.println("A");
- }
+ A() {}
}
@NeverClassInline
static class B extends Parent {
- B() {
- // To avoid constructor equivalence.
- System.out.println("B");
- }
+ 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
deleted file mode 100644
index 9b5b87d..0000000
--- a/src/test/java/com/android/tools/r8/classmerging/horizontal/MergedConstructorWithEquivalenceStackTraceTest.java
+++ /dev/null
@@ -1,107 +0,0 @@
-// 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 a4c52ad..fdb7faa 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,11 +115,15 @@
private List<Path> additionalProgramFiles = new ArrayList<>();
private Consumer<InternalOptions> optionsModifier = ConsumerUtils.emptyConsumer();
private Path desugarJDKLibs = ToolHelper.getDesugarJDKLibs();
- private Path desugarJDKLibsConfiguration = null;
+ private Path desugarJDKLibsConfiguration = ToolHelper.DESUGAR_LIB_CONVERSIONS;
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);
@@ -235,11 +239,11 @@
}
private Collection<Path> getProgramFiles() {
- ImmutableList.Builder<Path> builder = ImmutableList.<Path>builder().add(desugarJDKLibs);
- if (desugarJDKLibsConfiguration != null) {
- builder.add(desugarJDKLibsConfiguration);
- }
- return builder.addAll(additionalProgramFiles).build();
+ return ImmutableList.<Path>builder()
+ .add(desugarJDKLibs)
+ .add(desugarJDKLibsConfiguration)
+ .addAll(additionalProgramFiles)
+ .build();
}
private Collection<Path> getLibraryFiles() {
@@ -296,7 +300,6 @@
},
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
deleted file mode 100644
index c1182bd..0000000
--- a/src/test/java/com/android/tools/r8/internal/YouTubeV1620Test.java
+++ /dev/null
@@ -1,191 +0,0 @@
-// 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"));
- }
-}
