Merge commit '995cc548cab16a4a51873229d7238b1331771a39' into dev-release
diff --git a/src/main/java/com/android/tools/r8/ExtractMarker.java b/src/main/java/com/android/tools/r8/ExtractMarker.java index c9f2032..bda3597 100644 --- a/src/main/java/com/android/tools/r8/ExtractMarker.java +++ b/src/main/java/com/android/tools/r8/ExtractMarker.java
@@ -101,7 +101,7 @@ options.minApiLevel = AndroidApiLevel.P.getLevel(); DexApplication dexApp = new ApplicationReader(app, options, new Timing("ExtractMarker")).read(); - return dexApp.dexItemFactory.extractMarker(); + return dexApp.dexItemFactory.extractMarkers(); } public static void main(String[] args)
diff --git a/src/main/java/com/android/tools/r8/dex/ApplicationReader.java b/src/main/java/com/android/tools/r8/dex/ApplicationReader.java index 6e95cac..4549d8e 100644 --- a/src/main/java/com/android/tools/r8/dex/ApplicationReader.java +++ b/src/main/java/com/android/tools/r8/dex/ApplicationReader.java
@@ -7,17 +7,14 @@ import static com.android.tools.r8.graph.ClassKind.LIBRARY; import static com.android.tools.r8.graph.ClassKind.PROGRAM; import static com.android.tools.r8.utils.ExceptionUtils.unwrapExecutionException; -import static com.android.tools.r8.utils.InternalOptions.ASM_VERSION; import com.android.tools.r8.ClassFileResourceProvider; -import com.android.tools.r8.DataEntryResource; import com.android.tools.r8.DataResourceProvider; import com.android.tools.r8.ProgramResource; import com.android.tools.r8.ProgramResource.Kind; import com.android.tools.r8.ProgramResourceProvider; import com.android.tools.r8.ResourceException; import com.android.tools.r8.StringResource; -import com.android.tools.r8.Version; import com.android.tools.r8.errors.CompilationError; import com.android.tools.r8.graph.ClassKind; import com.android.tools.r8.graph.DexApplication; @@ -43,38 +40,26 @@ import com.android.tools.r8.utils.StringDiagnostic; import com.android.tools.r8.utils.ThreadUtils; import com.android.tools.r8.utils.Timing; -import com.android.tools.r8.utils.ZipUtils; -import com.google.common.collect.ImmutableSet; -import com.google.common.io.ByteStreams; -import com.google.common.io.Closer; -import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; -import java.nio.file.Files; -import java.nio.file.OpenOption; import java.nio.file.Paths; -import java.nio.file.StandardOpenOption; import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Queue; -import java.util.Set; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.stream.Collectors; -import java.util.zip.ZipEntry; -import java.util.zip.ZipOutputStream; -import org.objectweb.asm.ClassVisitor; public class ApplicationReader { private final InternalOptions options; private final DexItemFactory itemFactory; private final Timing timing; - private final AndroidApp inputApp; + private AndroidApp inputApp; public interface ProgramClassConflictResolver { DexProgramClass resolveClassConflict(DexProgramClass a, DexProgramClass b); @@ -123,7 +108,7 @@ throws IOException, ExecutionException { assert verifyMainDexOptionsCompatible(inputApp, options); if (options.dumpInputToFile != null) { - dumpInputToFile(); + inputApp = dumpInputToFile(inputApp, options); throw options.reporter.fatalError("Dumped compilation inputs to: " + options.dumpInputToFile); } timing.begin("DexApplication.read"); @@ -160,141 +145,9 @@ return builder.build(); } - private void dumpInputToFile() throws IOException { - try { - List<ProgramResourceProvider> programResourceProviders = - inputApp.getProgramResourceProviders(); - Set<DataEntryResource> dataEntryResources = inputApp.getDataEntryResourcesForTesting(); - List<ProgramResource> programResourcesWithDescriptors = new ArrayList<>(); - for (ProgramResourceProvider programResourceProvider : programResourceProviders) { - addProgramResourcesWithDescriptor( - programResourcesWithDescriptors, programResourceProvider.getProgramResources()); - } - - List<ProgramResource> libraryProgramResourcesWithDescriptors = - getProgramResourcesWithDescriptors(inputApp.getLibraryResourceProviders()); - - List<ProgramResource> classpathProgramResourcesWithDescriptors = - getProgramResourcesWithDescriptors(inputApp.getClasspathResourceProviders()); - - OpenOption[] openOptions = - new OpenOption[] {StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING}; - try (Closer closer = Closer.create()) { - try (ZipOutputStream out = - new ZipOutputStream( - Files.newOutputStream(Paths.get(options.dumpInputToFile), openOptions))) { - writeToZip( - dataEntryResources, programResourcesWithDescriptors, closer, out, "program.jar"); - writeToZip( - ImmutableSet.of(), - libraryProgramResourcesWithDescriptors, - closer, - out, - "library.jar"); - writeToZip( - ImmutableSet.of(), - classpathProgramResourcesWithDescriptors, - closer, - out, - "classpath.jar"); - if (options.hasProguardConfiguration()) { - String proguardConfig = options.getProguardConfiguration().getParsedConfiguration(); - ZipUtils.writeToZipStream( - out, "proguard.config", proguardConfig.getBytes(), ZipEntry.DEFLATED); - } - - ZipUtils.writeToZipStream( - out, "r8-version", Version.getVersionString().getBytes(), ZipEntry.DEFLATED); - } - } - } catch (ResourceException e) { - options.reporter.fatalError("Failed to write input:" + e.getMessage()); - } - } - - private static void writeToZip( - Set<DataEntryResource> dataEntryResources, - List<ProgramResource> programResourcesWithDescriptors, - Closer closer, - ZipOutputStream out, - String entry) - throws IOException, ResourceException { - try (ByteArrayOutputStream programByteStream = new ByteArrayOutputStream()) { - try (ZipOutputStream archiveOutputStream = new ZipOutputStream(programByteStream)) { - ZipUtils.writeResourcesToZip( - programResourcesWithDescriptors, dataEntryResources, closer, archiveOutputStream); - } - ZipUtils.writeToZipStream(out, entry, programByteStream.toByteArray(), ZipEntry.DEFLATED); - } - } - - private static List<ProgramResource> getProgramResourcesWithDescriptors( - List<ClassFileResourceProvider> classFileResourceProviders) - throws IOException, ResourceException { - ArrayList<ProgramResource> programResourcesWithDescriptors = new ArrayList<>(); - for (ClassFileResourceProvider libraryResourceProvider : classFileResourceProviders) { - List<ProgramResource> programResources = new ArrayList<>(); - for (String classDescriptor : libraryResourceProvider.getClassDescriptors()) { - programResources.add(libraryResourceProvider.getProgramResource(classDescriptor)); - } - addProgramResourcesWithDescriptor(programResourcesWithDescriptors, programResources); - } - return programResourcesWithDescriptors; - } - - private static void addProgramResourcesWithDescriptor( - List<ProgramResource> programResourcesWithDescriptors, - Collection<ProgramResource> programResources) - throws IOException, ResourceException { - for (ProgramResource programResource : programResources) { - if (programResource.getKind() != Kind.CF) { - continue; - } - try (InputStream inputStream = programResource.getByteStream()) { - byte[] bytes = ByteStreams.toByteArray(inputStream); - String descriptor = extractClassInternalType(bytes); - programResourcesWithDescriptors.add( - ProgramResource.fromBytes( - programResource.getOrigin(), - programResource.getKind(), - bytes, - ImmutableSet.of(descriptor))); - } - } - } - - private static String extractClassInternalType(byte[] bytes) throws IOException { - class ClassNameExtractor extends ClassVisitor { - private String className; - - private ClassNameExtractor() { - super(ASM_VERSION); - } - - @Override - public void visit( - int version, - int access, - String name, - String signature, - String superName, - String[] interfaces) { - className = name; - } - - String getDescriptor() { - return "L" + className + ";"; - } - } - - org.objectweb.asm.ClassReader reader = new org.objectweb.asm.ClassReader(bytes); - ClassNameExtractor extractor = new ClassNameExtractor(); - reader.accept( - extractor, - org.objectweb.asm.ClassReader.SKIP_CODE - | org.objectweb.asm.ClassReader.SKIP_DEBUG - | org.objectweb.asm.ClassReader.SKIP_FRAMES); - return extractor.getDescriptor(); + private static AndroidApp dumpInputToFile(AndroidApp app, InternalOptions options) { + return app.dump( + Paths.get(options.dumpInputToFile), options.getProguardConfiguration(), options.reporter); } private static boolean verifyMainDexOptionsCompatible(
diff --git a/src/main/java/com/android/tools/r8/dex/ApplicationWriter.java b/src/main/java/com/android/tools/r8/dex/ApplicationWriter.java index 24a33b9..5daaccc 100644 --- a/src/main/java/com/android/tools/r8/dex/ApplicationWriter.java +++ b/src/main/java/com/android/tools/r8/dex/ApplicationWriter.java
@@ -246,7 +246,7 @@ application.dexItemFactory.sort(namingLens); assert markers == null || markers.isEmpty() - || application.dexItemFactory.extractMarker() != null; + || application.dexItemFactory.extractMarkers() != null; SortAnnotations sortAnnotations = new SortAnnotations(); application.classes().forEach((clazz) -> clazz.addDependencies(sortAnnotations));
diff --git a/src/main/java/com/android/tools/r8/dex/ClassesChecksum.java b/src/main/java/com/android/tools/r8/dex/ClassesChecksum.java index 07d2176..540502b 100644 --- a/src/main/java/com/android/tools/r8/dex/ClassesChecksum.java +++ b/src/main/java/com/android/tools/r8/dex/ClassesChecksum.java
@@ -7,11 +7,13 @@ import com.google.gson.JsonSyntaxException; import it.unimi.dsi.fastutil.objects.Object2LongMap; import it.unimi.dsi.fastutil.objects.Object2LongOpenHashMap; +import java.io.UTFDataFormatException; import java.util.Comparator; import java.util.Map; public class ClassesChecksum { + private static final String PREFIX = "~~~"; private static final char PREFIX_CHAR0 = '~'; private static final char PREFIX_CHAR1 = '~'; private static final char PREFIX_CHAR2 = '~'; @@ -19,6 +21,10 @@ private Object2LongMap<String> dictionary = null; public ClassesChecksum() { + assert PREFIX.length() == 3; + assert PREFIX.charAt(0) == PREFIX_CHAR0; + assert PREFIX.charAt(1) == PREFIX_CHAR1; + assert PREFIX.charAt(2) == PREFIX_CHAR2; } private void ensureMap() { @@ -71,12 +77,28 @@ } } - public static boolean preceedChecksumMarker(DexString string) { - return string.size < 1 || - string.content[0] < PREFIX_CHAR0 || - string.size < 2 || - string.content[1] < PREFIX_CHAR1 || - string.size < 3 || - string.content[2] < PREFIX_CHAR2; + /** + * Check if this string will definitely preceded the checksum marker. + * + * <p>If true is returned the string passed definitely preceded the checksum marker. If false is + * returned the string passed might still preceded, so this can give false negatives. + * + * @param string String to check if definitely preceded the checksum marker. + * @return If the string passed definitely preceded the checksum marker + */ + public static boolean definitelyPreceedChecksumMarker(DexString string) { + try { + assert PREFIX.length() == 3; + char[] prefix = string.decodePrefix(3); + return prefix.length == 0 + || (prefix.length == 1 && prefix[0] <= PREFIX_CHAR0) + || (prefix.length == 2 && prefix[0] == PREFIX_CHAR0 && prefix[1] <= PREFIX_CHAR1) + || (prefix.length == 3 + && prefix[0] == PREFIX_CHAR0 + && prefix[1] == PREFIX_CHAR1 + && prefix[2] < PREFIX_CHAR2); + } catch (UTFDataFormatException e) { + throw new RuntimeException("Bad format", e); + } } }
diff --git a/src/main/java/com/android/tools/r8/dex/CodeToKeep.java b/src/main/java/com/android/tools/r8/dex/CodeToKeep.java index 2e87405..7a2c325 100644 --- a/src/main/java/com/android/tools/r8/dex/CodeToKeep.java +++ b/src/main/java/com/android/tools/r8/dex/CodeToKeep.java
@@ -12,7 +12,6 @@ import com.android.tools.r8.naming.NamingLens; import com.android.tools.r8.utils.DescriptorUtils; import com.android.tools.r8.utils.InternalOptions; -import com.android.tools.r8.utils.Pair; import com.google.common.collect.Sets; import java.util.Map; import java.util.Set; @@ -35,16 +34,23 @@ abstract void recordClass(DexType type); + abstract void recordClassAllAccesses(DexType type); + abstract boolean isNop(); abstract void generateKeepRules(InternalOptions options); public static class DesugaredLibraryCodeToKeep extends CodeToKeep { + private static class KeepStruct { + Set<DexField> fields = Sets.newConcurrentHashSet(); + Set<DexMethod> methods = Sets.newConcurrentHashSet(); + boolean all = false; + } + private final NamingLens namingLens; private final Set<DexType> emulatedInterfaces = Sets.newIdentityHashSet(); - private final Map<DexType, Pair<Set<DexField>, Set<DexMethod>>> toKeep = - new ConcurrentHashMap<>(); + private final Map<DexType, KeepStruct> toKeep = new ConcurrentHashMap<>(); private final InternalOptions options; public DesugaredLibraryCodeToKeep(NamingLens namingLens, InternalOptions options) { @@ -62,7 +68,7 @@ void recordMethod(DexMethod method) { if (shouldKeep(method.holder)) { keepClass(method.holder); - toKeep.get(method.holder).getSecond().add(method); + toKeep.get(method.holder).methods.add(method); } if (shouldKeep(method.proto.returnType)) { keepClass(method.proto.returnType); @@ -78,7 +84,7 @@ void recordField(DexField field) { if (shouldKeep(field.holder)) { keepClass(field.holder); - toKeep.get(field.holder).getFirst().add(field); + toKeep.get(field.holder).fields.add(field); } if (shouldKeep(field.type)) { keepClass(field.type); @@ -92,10 +98,17 @@ } } + @Override + void recordClassAllAccesses(DexType type) { + if (shouldKeep(type)) { + keepClass(type); + toKeep.get(type).all = true; + } + } + private void keepClass(DexType type) { DexType baseType = type.lookupBaseType(options.itemFactory); - toKeep.putIfAbsent( - baseType, new Pair<>(Sets.newConcurrentHashSet(), Sets.newConcurrentHashSet())); + toKeep.putIfAbsent(baseType, new KeepStruct()); } @Override @@ -115,23 +128,26 @@ StringBuilder sb = new StringBuilder(); String cr = System.lineSeparator(); for (DexType type : toKeep.keySet()) { - Set<DexField> fieldsToKeep = toKeep.get(type).getFirst(); - Set<DexMethod> methodsToKeep = toKeep.get(type).getSecond(); + KeepStruct keepStruct = toKeep.get(type); sb.append("-keep class ").append(convertType(type)); - if (fieldsToKeep.isEmpty() && methodsToKeep.isEmpty()) { + if (keepStruct.all) { + sb.append(" { *; }").append(cr); + continue; + } + if (keepStruct.fields.isEmpty() && keepStruct.methods.isEmpty()) { sb.append(cr); continue; } sb.append(" {").append(cr); - for (DexField field : fieldsToKeep) { + for (DexField field : keepStruct.fields) { sb.append(" ") - .append(convertType(type)) + .append(convertType(field.type)) .append(" ") .append(field.name) .append(";") .append(cr); } - for (DexMethod method : methodsToKeep) { + for (DexMethod method : keepStruct.methods) { sb.append(" ") .append(convertType(method.proto.returnType)) .append(" ") @@ -164,6 +180,9 @@ void recordClass(DexType type) {} @Override + void recordClassAllAccesses(DexType type) {} + + @Override boolean isNop() { return true; }
diff --git a/src/main/java/com/android/tools/r8/dex/DexParser.java b/src/main/java/com/android/tools/r8/dex/DexParser.java index 2f249a7..5989981 100644 --- a/src/main/java/com/android/tools/r8/dex/DexParser.java +++ b/src/main/java/com/android/tools/r8/dex/DexParser.java
@@ -950,7 +950,7 @@ ClassesChecksum parsedChecksums = new ClassesChecksum(); for (int i = stringIDs.length - 1; i >= 0; i--) { DexString value = indexedItems.getString(i); - if (ClassesChecksum.preceedChecksumMarker(value)) { + if (ClassesChecksum.definitelyPreceedChecksumMarker(value)) { break; } parsedChecksums.tryParseAndAppend(value);
diff --git a/src/main/java/com/android/tools/r8/dex/FileWriter.java b/src/main/java/com/android/tools/r8/dex/FileWriter.java index 367ea2e..e77be34 100644 --- a/src/main/java/com/android/tools/r8/dex/FileWriter.java +++ b/src/main/java/com/android/tools/r8/dex/FileWriter.java
@@ -645,8 +645,10 @@ private void writeClassData(DexProgramClass clazz) { assert clazz.hasMethodsOrFields(); - desugaredLibraryCodeToKeep.recordClass(clazz.superType); + desugaredLibraryCodeToKeep.recordClassAllAccesses(clazz.superType); for (DexType itf : clazz.interfaces.values) { + // No need for recordClassAllAccesses here because default methods are desugared away + // with concrete implementations or not supported. desugaredLibraryCodeToKeep.recordClass(itf); } mixedSectionOffsets.setOffsetFor(clazz, dest.position());
diff --git a/src/main/java/com/android/tools/r8/dex/Marker.java b/src/main/java/com/android/tools/r8/dex/Marker.java index 85d3568..7ab2a56 100644 --- a/src/main/java/com/android/tools/r8/dex/Marker.java +++ b/src/main/java/com/android/tools/r8/dex/Marker.java
@@ -21,6 +21,7 @@ public static final String MIN_API = "min-api"; public static final String SHA1 = "sha-1"; public static final String COMPILATION_MODE = "compilation-mode"; + public static final String HAS_CHECKSUMS = "has-checksums"; public static final String PG_MAP_ID = "pg-map-id"; public enum Tool { @@ -107,6 +108,16 @@ return this; } + public boolean getHasChecksums() { + return jsonObject.get(HAS_CHECKSUMS).getAsBoolean(); + } + + public Marker setHasChecksums(boolean hasChecksums) { + assert !jsonObject.has(HAS_CHECKSUMS); + jsonObject.addProperty(HAS_CHECKSUMS, hasChecksums); + return this; + } + public String getPgMapId() { return jsonObject.get(PG_MAP_ID).getAsString(); }
diff --git a/src/main/java/com/android/tools/r8/graph/AppInfo.java b/src/main/java/com/android/tools/r8/graph/AppInfo.java index 773f4ad..d569852 100644 --- a/src/main/java/com/android/tools/r8/graph/AppInfo.java +++ b/src/main/java/com/android/tools/r8/graph/AppInfo.java
@@ -6,8 +6,9 @@ import com.android.tools.r8.graph.ResolutionResult.ArrayCloneMethodResult; import com.android.tools.r8.graph.ResolutionResult.ClassNotFoundResult; import com.android.tools.r8.graph.ResolutionResult.IncompatibleClassResult; -import com.android.tools.r8.graph.ResolutionResult.MultiResult; +import com.android.tools.r8.graph.ResolutionResult.MultiResolutionResult; import com.android.tools.r8.graph.ResolutionResult.NoSuchMethodResult; +import com.android.tools.r8.graph.ResolutionResult.SingleResolutionResult; import com.android.tools.r8.origin.Origin; import com.android.tools.r8.shaking.AppInfoWithLiveness; import com.android.tools.r8.utils.InternalOptions; @@ -190,7 +191,7 @@ public DexEncodedMethod lookupStaticTarget(DexMethod method) { assert checkIfObsolete(); ResolutionResult resolutionResult = resolveMethod(method.holder, method); - DexEncodedMethod target = resolutionResult.asSingleTarget(); + DexEncodedMethod target = resolutionResult.getSingleTarget(); return target == null || target.isStatic() ? target : null; } @@ -215,7 +216,7 @@ // https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-6.html#jvms-6.5.invokespecial, use // the "symbolic reference" if the "symbolic reference" does not name a class. if (definitionFor(method.holder).isInterface()) { - return resolveMethodOnInterface(method.holder, method).asSingleTarget(); + return resolveMethodOnInterface(method.holder, method).getSingleTarget(); } // Then, resume on the search, but this time, starting from the holder of the caller. DexClass contextClass = definitionFor(invocationContext); @@ -223,7 +224,7 @@ return null; } resolutionResult = resolveMethod(contextClass.superType, method); - DexEncodedMethod target = resolutionResult.asSingleTarget(); + DexEncodedMethod target = resolutionResult.getSingleTarget(); return target == null || !target.isStatic() ? target : null; } @@ -238,7 +239,7 @@ public DexEncodedMethod lookupDirectTarget(DexMethod method) { assert checkIfObsolete(); ResolutionResult resolutionResult = resolveMethod(method.holder, method); - DexEncodedMethod target = resolutionResult.asSingleTarget(); + DexEncodedMethod target = resolutionResult.getSingleTarget(); return target == null || target.isDirectMethod() ? target : null; } @@ -252,7 +253,7 @@ assert checkIfObsolete(); assert type.isClassType() || type.isArrayType(); ResolutionResult resolutionResult = resolveMethod(type, method); - DexEncodedMethod target = resolutionResult.asSingleTarget(); + DexEncodedMethod target = resolutionResult.getSingleTarget(); return target == null || target.isVirtualMethod() ? target : null; } @@ -352,7 +353,7 @@ // Step 2: DexEncodedMethod singleTarget = resolveMethodOnClassStep2(clazz, method); if (singleTarget != null) { - return singleTarget; + return new SingleResolutionResult(singleTarget); } // Finally Step 3: return resolveMethodStep3(clazz, method); @@ -414,7 +415,7 @@ return result; } // Return any of the non-default methods. - return anyTarget == null ? NoSuchMethodResult.INSTANCE : anyTarget; + return anyTarget == null ? NoSuchMethodResult.INSTANCE : new SingleResolutionResult(anyTarget); } /** @@ -491,7 +492,7 @@ // Step 2: Look for exact method on interface. DexEncodedMethod result = definition.lookupMethod(desc); if (result != null) { - return result; + return new SingleResolutionResult(result); } // Step 3: Look for matching method on object class. DexClass objectClass = definitionFor(dexItemFactory.objectType); @@ -500,7 +501,7 @@ } result = objectClass.lookupMethod(desc); if (result != null && result.accessFlags.isPublic() && !result.accessFlags.isAbstract()) { - return result; + return new SingleResolutionResult(result); } // Step 3: Look for maximally-specific superinterface methods or any interface definition. // This is the same for classes and interfaces. @@ -583,7 +584,7 @@ */ public DexEncodedMethod dispatchStaticInvoke(ResolutionResult resolvedMethod) { assert checkIfObsolete(); - DexEncodedMethod target = resolvedMethod.asSingleTarget(); + DexEncodedMethod target = resolvedMethod.getSingleTarget(); if (target != null && target.accessFlags.isStatic()) { return target; } @@ -597,7 +598,7 @@ */ public DexEncodedMethod dispatchDirectInvoke(ResolutionResult resolvedMethod) { assert checkIfObsolete(); - DexEncodedMethod target = resolvedMethod.asSingleTarget(); + DexEncodedMethod target = resolvedMethod.getSingleTarget(); if (target != null && !target.accessFlags.isStatic()) { return target; } @@ -669,10 +670,12 @@ ResolutionResult build() { if (builder != null) { - return new MultiResult(builder.build().asList()); - } else { - return singleResult; + return new MultiResolutionResult(builder.build().asList()); } + if (singleResult != null) { + return new SingleResolutionResult(singleResult); + } + return null; } }
diff --git a/src/main/java/com/android/tools/r8/graph/AppInfoWithSubtyping.java b/src/main/java/com/android/tools/r8/graph/AppInfoWithSubtyping.java index 3425946..faa25c2 100644 --- a/src/main/java/com/android/tools/r8/graph/AppInfoWithSubtyping.java +++ b/src/main/java/com/android/tools/r8/graph/AppInfoWithSubtyping.java
@@ -141,6 +141,7 @@ super(previous); missingClasses.addAll(previous.missingClasses); subtypeMap.putAll(previous.subtypeMap); + supertypesForSynthesizedClasses.putAll(previous.supertypesForSynthesizedClasses); typeInfo = new ConcurrentHashMap<>(previous.typeInfo); assert app() instanceof DirectMappedDexApplication; } @@ -746,7 +747,7 @@ if (clazz.isProgramClass()) { if (lookUpwards) { DexEncodedMethod resolutionResult = - resolveMethod(type, dexItemFactory().objectMethods.finalize).asSingleTarget(); + resolveMethod(type, dexItemFactory().objectMethods.finalize).getSingleTarget(); if (resolutionResult != null && resolutionResult.isProgramMethod(this)) { mayHaveFinalizeMethodDirectlyOrIndirectlyCache.put(type, true); return true;
diff --git a/src/main/java/com/android/tools/r8/graph/AppView.java b/src/main/java/com/android/tools/r8/graph/AppView.java index 7eda1e6..3803b2c 100644 --- a/src/main/java/com/android/tools/r8/graph/AppView.java +++ b/src/main/java/com/android/tools/r8/graph/AppView.java
@@ -9,6 +9,7 @@ import com.android.tools.r8.ir.analysis.proto.GeneratedExtensionRegistryShrinker; import com.android.tools.r8.ir.analysis.proto.GeneratedMessageLiteShrinker; import com.android.tools.r8.ir.analysis.proto.ProtoShrinker; +import com.android.tools.r8.ir.analysis.value.AbstractValueFactory; import com.android.tools.r8.ir.desugar.PrefixRewritingMapper; import com.android.tools.r8.ir.optimize.CallSiteOptimizationInfoPropagator; import com.android.tools.r8.shaking.AppInfoWithLiveness; @@ -36,6 +37,7 @@ private GraphLense graphLense; private final InternalOptions options; private RootSet rootSet; + private final AbstractValueFactory abstractValueFactory = new AbstractValueFactory(); // Desugared library prefix rewriter. public final PrefixRewritingMapper rewritePrefix; @@ -107,6 +109,10 @@ return new AppView<>(appInfo, WholeProgramOptimizations.OFF, options, mapper); } + public AbstractValueFactory abstractValueFactory() { + return abstractValueFactory; + } + public T appInfo() { return appInfo; }
diff --git a/src/main/java/com/android/tools/r8/graph/AssemblyWriter.java b/src/main/java/com/android/tools/r8/graph/AssemblyWriter.java index 33c8dde..eafa809 100644 --- a/src/main/java/com/android/tools/r8/graph/AssemblyWriter.java +++ b/src/main/java/com/android/tools/r8/graph/AssemblyWriter.java
@@ -5,6 +5,7 @@ import com.android.tools.r8.ClassFileConsumer; import com.android.tools.r8.ir.conversion.IRConverter; +import com.android.tools.r8.ir.conversion.OneTimeMethodProcessor; import com.android.tools.r8.ir.optimize.info.OptimizationFeedbackIgnore; import com.android.tools.r8.naming.ClassNameMapper; import com.android.tools.r8.naming.MemberNaming.FieldSignature; @@ -121,7 +122,10 @@ private void writeIR(DexEncodedMethod method, PrintStream ps) { CfgPrinter printer = new CfgPrinter(); new IRConverter(appInfo, options, timing, printer) - .processMethod(method, OptimizationFeedbackIgnore.getInstance(), null, null, null); + .processMethod( + method, + OptimizationFeedbackIgnore.getInstance(), + OneTimeMethodProcessor.getInstance()); ps.println(printer.toString()); }
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java index 200240c..776cbaf 100644 --- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java +++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -77,7 +77,7 @@ import java.util.function.IntPredicate; import org.objectweb.asm.Opcodes; -public class DexEncodedMethod extends KeyedDexItem<DexMethod> implements ResolutionResult { +public class DexEncodedMethod extends KeyedDexItem<DexMethod> { public static final String CONFIGURATION_DEBUGGING_PREFIX = "Shaking error: Missing method in "; @@ -1311,46 +1311,4 @@ return result; } } - - @Override - public boolean isValidVirtualTarget(InternalOptions options) { - return options.canUseNestBasedAccess() - ? (!accessFlags.isStatic() && !accessFlags.isConstructor()) - : isVirtualMethod(); - } - - @Override - public boolean isValidVirtualTargetForDynamicDispatch() { - return isVirtualMethod(); - } - - @Override - public DexEncodedMethod asResultOfResolve() { - checkIfObsolete(); - return this; - } - - @Override - public DexEncodedMethod asSingleTarget() { - checkIfObsolete(); - return this; - } - - @Override - public boolean hasSingleTarget() { - checkIfObsolete(); - return true; - } - - @Override - public List<DexEncodedMethod> asListOfTargets() { - checkIfObsolete(); - return Collections.singletonList(this); - } - - @Override - public void forEachTarget(Consumer<DexEncodedMethod> consumer) { - checkIfObsolete(); - consumer.accept(this); - } }
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 ad03301..dbeb35f 100644 --- a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java +++ b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
@@ -516,6 +516,7 @@ // If not, that library method should not be added here because it literally has side effects. public Map<DexMethod, Predicate<InvokeMethod>> libraryMethodsWithoutSideEffects = Streams.<Pair<DexMethod, Predicate<InvokeMethod>>>concat( + Stream.of(new Pair<>(enumMethods.constructor, alwaysTrue())), Stream.of(new Pair<>(objectMethods.constructor, alwaysTrue())), mapToPredicate(classMethods.getNames, alwaysTrue()), mapToPredicate( @@ -550,7 +551,7 @@ .build(); public final Set<DexType> libraryClassesWithoutStaticInitialization = - ImmutableSet.of(objectType, stringBufferType, stringBuilderType); + ImmutableSet.of(enumType, objectType, stringBufferType, stringBuilderType); private boolean skipNameValidationForTesting = false; @@ -750,6 +751,8 @@ public final DexMethod name; public final DexMethod toString; + public final DexMethod constructor = + createMethod(enumType, createProto(voidType, stringType, intType), constructorMethodName); public final DexMethod finalize = createMethod(enumType, createProto(voidType), finalizeMethodName); @@ -1196,19 +1199,6 @@ } // Debugging support to extract marking string. - public synchronized Collection<Marker> extractMarker() { - // This is slow but it is not needed for any production code yet. - List<Marker> markers = new ArrayList<>(); - for (DexString dexString : strings.keySet()) { - Marker result = Marker.parse(dexString); - if (result != null) { - markers.add(result); - } - } - return markers; - } - - // Debugging support to extract marking string. // Find all markers. public synchronized List<Marker> extractMarkers() { // This is slow but it is not needed for any production code yet.
diff --git a/src/main/java/com/android/tools/r8/graph/DexString.java b/src/main/java/com/android/tools/r8/graph/DexString.java index bac6fe3..a6591b4 100644 --- a/src/main/java/com/android/tools/r8/graph/DexString.java +++ b/src/main/java/com/android/tools/r8/graph/DexString.java
@@ -143,6 +143,51 @@ } } + public char[] decodePrefix(int prefixLength) throws UTFDataFormatException { + int s = 0; + int p = 0; + char[] out = new char[prefixLength]; + while (true) { + char a = (char) (content[p++] & 0xff); + if (a == 0) { + if (s == prefixLength) { + return out; + } + char[] result = new char[s]; + System.arraycopy(out, 0, result, 0, s); + return result; + } + out[s] = a; + if (a < '\u0080') { + s++; + if (s == prefixLength) { + return out; + } + } else if ((a & 0xe0) == 0xc0) { + int b = content[p++] & 0xff; + if ((b & 0xC0) != 0x80) { + throw new UTFDataFormatException("bad second byte"); + } + out[s++] = (char) (((a & 0x1F) << 6) | (b & 0x3F)); + if (s == prefixLength) { + return out; + } + } else if ((a & 0xf0) == 0xe0) { + int b = content[p++] & 0xff; + int c = content[p++] & 0xff; + if (((b & 0xC0) != 0x80) || ((c & 0xC0) != 0x80)) { + throw new UTFDataFormatException("bad second or third byte"); + } + out[s++] = (char) (((a & 0x0F) << 12) | ((b & 0x3F) << 6) | (c & 0x3F)); + if (s == prefixLength) { + return out; + } + } else { + throw new UTFDataFormatException("bad byte"); + } + } + } + public int decodedHashCode() throws UTFDataFormatException { if (size == 0) { assert decode().hashCode() == 0;
diff --git a/src/main/java/com/android/tools/r8/graph/ResolutionResult.java b/src/main/java/com/android/tools/r8/graph/ResolutionResult.java index dfd31fb..6a9a22a 100644 --- a/src/main/java/com/android/tools/r8/graph/ResolutionResult.java +++ b/src/main/java/com/android/tools/r8/graph/ResolutionResult.java
@@ -11,30 +11,30 @@ import java.util.Set; import java.util.function.Consumer; -public interface ResolutionResult { +public abstract class ResolutionResult { // TODO(b/140214802): Remove this method as its usage is questionable. - DexEncodedMethod asResultOfResolve(); + public abstract DexEncodedMethod asResultOfResolve(); - DexEncodedMethod asSingleTarget(); + public abstract DexEncodedMethod getSingleTarget(); - boolean hasSingleTarget(); + public abstract boolean hasSingleTarget(); - List<DexEncodedMethod> asListOfTargets(); + public abstract List<DexEncodedMethod> asListOfTargets(); - void forEachTarget(Consumer<DexEncodedMethod> consumer); + public abstract void forEachTarget(Consumer<DexEncodedMethod> consumer); - boolean isValidVirtualTarget(InternalOptions options); + public abstract boolean isValidVirtualTarget(InternalOptions options); - boolean isValidVirtualTargetForDynamicDispatch(); + public abstract boolean isValidVirtualTargetForDynamicDispatch(); - default Set<DexEncodedMethod> lookupVirtualDispatchTargets( + public Set<DexEncodedMethod> lookupVirtualDispatchTargets( boolean isInterface, AppInfoWithSubtyping appInfo) { return isInterface ? lookupInterfaceTargets(appInfo) : lookupVirtualTargets(appInfo); } // TODO(b/140204899): Leverage refined receiver type if available. - default Set<DexEncodedMethod> lookupVirtualTargets(AppInfoWithSubtyping appInfo) { + public Set<DexEncodedMethod> lookupVirtualTargets(AppInfoWithSubtyping appInfo) { assert isValidVirtualTarget(appInfo.app().options); // First add the target for receiver type method.type. Set<DexEncodedMethod> result = Sets.newIdentityHashSet(); @@ -60,7 +60,7 @@ } // TODO(b/140204899): Leverage refined receiver type if available. - default Set<DexEncodedMethod> lookupInterfaceTargets(AppInfoWithSubtyping appInfo) { + public Set<DexEncodedMethod> lookupInterfaceTargets(AppInfoWithSubtyping appInfo) { assert isValidVirtualTarget(appInfo.app().options); Set<DexEncodedMethod> result = Sets.newIdentityHashSet(); if (hasSingleTarget()) { @@ -86,7 +86,7 @@ // public void bar() { } // } // - DexEncodedMethod singleTarget = asSingleTarget(); + DexEncodedMethod singleTarget = getSingleTarget(); if (singleTarget.getCode() != null && appInfo.hasAnyInstantiatedLambdas(singleTarget.method.holder)) { result.add(singleTarget); @@ -128,11 +128,61 @@ return result; } - class MultiResult implements ResolutionResult { + public static class SingleResolutionResult extends ResolutionResult { + final DexEncodedMethod resolutionTarget; + + public static boolean isValidVirtualTarget(InternalOptions options, DexEncodedMethod target) { + return options.canUseNestBasedAccess() + ? (!target.accessFlags.isStatic() && !target.accessFlags.isConstructor()) + : target.isVirtualMethod(); + } + + public SingleResolutionResult(DexEncodedMethod resolutionTarget) { + assert resolutionTarget != null; + this.resolutionTarget = resolutionTarget; + } + + @Override + public boolean isValidVirtualTarget(InternalOptions options) { + return isValidVirtualTarget(options, resolutionTarget); + } + + @Override + public boolean isValidVirtualTargetForDynamicDispatch() { + return resolutionTarget.isVirtualMethod(); + } + + @Override + public DexEncodedMethod asResultOfResolve() { + return resolutionTarget; + } + + @Override + public DexEncodedMethod getSingleTarget() { + return resolutionTarget; + } + + @Override + public boolean hasSingleTarget() { + return true; + } + + @Override + public List<DexEncodedMethod> asListOfTargets() { + return Collections.singletonList(resolutionTarget); + } + + @Override + public void forEachTarget(Consumer<DexEncodedMethod> consumer) { + consumer.accept(resolutionTarget); + } + } + + public static class MultiResolutionResult extends ResolutionResult { private final ImmutableList<DexEncodedMethod> methods; - MultiResult(ImmutableList<DexEncodedMethod> results) { + public MultiResolutionResult(ImmutableList<DexEncodedMethod> results) { assert results.size() > 1; this.methods = results; } @@ -140,7 +190,7 @@ @Override public boolean isValidVirtualTarget(InternalOptions options) { for (DexEncodedMethod method : methods) { - if (!method.isValidVirtualTarget(options)) { + if (!SingleResolutionResult.isValidVirtualTarget(options, method)) { return false; } } @@ -164,7 +214,7 @@ } @Override - public DexEncodedMethod asSingleTarget() { + public DexEncodedMethod getSingleTarget() { // There is no single target that is guaranteed to be called. return null; } @@ -185,7 +235,7 @@ } } - abstract class EmptyResult implements ResolutionResult { + public abstract static class EmptyResult extends ResolutionResult { @Override public DexEncodedMethod asResultOfResolve() { @@ -193,7 +243,7 @@ } @Override - public DexEncodedMethod asSingleTarget() { + public DexEncodedMethod getSingleTarget() { return null; } @@ -223,7 +273,7 @@ } } - class ArrayCloneMethodResult extends EmptyResult { + public static class ArrayCloneMethodResult extends EmptyResult { static final ArrayCloneMethodResult INSTANCE = new ArrayCloneMethodResult(); @@ -242,7 +292,7 @@ } } - abstract class FailedResolutionResult extends EmptyResult { + public abstract static class FailedResolutionResult extends EmptyResult { @Override public boolean isValidVirtualTarget(InternalOptions options) { @@ -255,7 +305,7 @@ } } - class ClassNotFoundResult extends FailedResolutionResult { + public static class ClassNotFoundResult extends FailedResolutionResult { static final ClassNotFoundResult INSTANCE = new ClassNotFoundResult(); private ClassNotFoundResult() { @@ -263,7 +313,7 @@ } } - class IncompatibleClassResult extends FailedResolutionResult { + public static class IncompatibleClassResult extends FailedResolutionResult { static final IncompatibleClassResult INSTANCE = new IncompatibleClassResult(); private IncompatibleClassResult() { @@ -271,7 +321,7 @@ } } - class NoSuchMethodResult extends FailedResolutionResult { + public static class NoSuchMethodResult extends FailedResolutionResult { static final NoSuchMethodResult INSTANCE = new NoSuchMethodResult(); private NoSuchMethodResult() {
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/ClassInitializationAnalysis.java b/src/main/java/com/android/tools/r8/ir/analysis/ClassInitializationAnalysis.java index e8583f6..827529a 100644 --- a/src/main/java/com/android/tools/r8/ir/analysis/ClassInitializationAnalysis.java +++ b/src/main/java/com/android/tools/r8/ir/analysis/ClassInitializationAnalysis.java
@@ -339,7 +339,7 @@ if (!resolutionResult.hasSingleTarget()) { return false; } - DexType holder = resolutionResult.asSingleTarget().method.holder; + DexType holder = resolutionResult.getSingleTarget().method.holder; return appView.isSubtype(holder, type).isTrue(); } @@ -397,7 +397,7 @@ if (!resolutionResult.hasSingleTarget()) { return false; } - DexType holder = resolutionResult.asSingleTarget().method.holder; + DexType holder = resolutionResult.getSingleTarget().method.holder; return appView.isSubtype(holder, type).isTrue(); } @@ -433,7 +433,7 @@ if (!resolutionResult.hasSingleTarget()) { return false; } - DexType holder = resolutionResult.asSingleTarget().method.holder; + DexType holder = resolutionResult.getSingleTarget().method.holder; return appView.isSubtype(holder, type).isTrue(); }
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/ValueMayDependOnEnvironmentAnalysis.java b/src/main/java/com/android/tools/r8/ir/analysis/ValueMayDependOnEnvironmentAnalysis.java index d654c87..96dada1 100644 --- a/src/main/java/com/android/tools/r8/ir/analysis/ValueMayDependOnEnvironmentAnalysis.java +++ b/src/main/java/com/android/tools/r8/ir/analysis/ValueMayDependOnEnvironmentAnalysis.java
@@ -29,6 +29,8 @@ private final IRCode code; private final DexType context; + private final Set<Value> knownNotToDependOnEnvironment = Sets.newIdentityHashSet(); + public ValueMayDependOnEnvironmentAnalysis(AppView<?> appView, IRCode code) { this.appView = appView; this.code = code; @@ -36,20 +38,57 @@ } public boolean valueMayDependOnEnvironment(Value value) { + return valueMayDependOnEnvironment(value, Sets.newIdentityHashSet()); + } + + private boolean valueMayDependOnEnvironment( + Value value, Set<Value> assumedNotToDependOnEnvironment) { Value root = value.getAliasedValue(); + if (assumedNotToDependOnEnvironment.contains(root)) { + return false; + } + if (knownNotToDependOnEnvironment.contains(root)) { + return false; + } if (root.isConstant()) { return false; } - if (isConstantArrayThroughoutMethod(root)) { + if (isConstantArrayThroughoutMethod(root, assumedNotToDependOnEnvironment)) { return false; } - if (isNewInstanceWithoutEnvironmentDependentFields(root)) { + if (isNewInstanceWithoutEnvironmentDependentFields(root, assumedNotToDependOnEnvironment)) { return false; } return true; } + private boolean valueMayNotDependOnEnvironmentAssumingArrayDoesNotDependOnEnvironment( + Value value, Value array, Set<Value> assumedNotToDependOnEnvironment) { + assert !value.hasAliasedValue(); + assert !array.hasAliasedValue(); + + if (assumedNotToDependOnEnvironment.add(array)) { + boolean valueMayDependOnEnvironment = + valueMayDependOnEnvironment(value, assumedNotToDependOnEnvironment); + boolean changed = assumedNotToDependOnEnvironment.remove(array); + assert changed; + return !valueMayDependOnEnvironment; + } + return !valueMayDependOnEnvironment(value, assumedNotToDependOnEnvironment); + } + + /** + * Used to identify if an array is "constant" in the sense that none of the values written into + * the array may depend on the environment. + * + * <p>Examples include {@code new int[] {1,2,3}} and {@code new Object[]{new Object()}}. + */ public boolean isConstantArrayThroughoutMethod(Value value) { + return isConstantArrayThroughoutMethod(value, Sets.newIdentityHashSet()); + } + + private boolean isConstantArrayThroughoutMethod( + Value value, Set<Value> assumedNotToDependOnEnvironment) { Value root = value.getAliasedValue(); if (root.isPhi()) { // Would need to track the aliases, just give up. @@ -96,6 +135,7 @@ // Allow array-put and new-array-filled-data instructions that immediately follow the array // creation. + Set<Value> arrayValues = Sets.newIdentityHashSet(); Set<Instruction> consumedInstructions = Sets.newIdentityHashSet(); for (Instruction instruction : definition.getBlock().instructionsAfter(definition)) { @@ -118,10 +158,19 @@ return false; } - if (!arrayPut.value().isConstant()) { + // Check if the value being written into the array may depend on the environment. + // + // When analyzing if the value may depend on the environment, we assume that the current + // array does not depend on the environment. Otherwise, we would classify the value as + // possibly depending on the environment since it could escape via the array and then + // be mutated indirectly. + Value rhs = arrayPut.value().getAliasedValue(); + if (!valueMayNotDependOnEnvironmentAssumingArrayDoesNotDependOnEnvironment( + rhs, root, assumedNotToDependOnEnvironment)) { return false; } + arrayValues.add(rhs); consumedInstructions.add(arrayPut); continue; } @@ -149,10 +198,21 @@ // Currently, we only allow the array to flow into static-put instructions that are not // followed by an instruction that may have side effects. Instructions that do not have any // side effects are ignored because they cannot mutate the array. - return !valueMayBeMutatedBeforeMethodExit(root, consumedInstructions); + if (valueMayBeMutatedBeforeMethodExit( + root, assumedNotToDependOnEnvironment, consumedInstructions)) { + return false; + } + + if (assumedNotToDependOnEnvironment.isEmpty()) { + knownNotToDependOnEnvironment.add(root); + knownNotToDependOnEnvironment.addAll(arrayValues); + } + + return true; } - private boolean isNewInstanceWithoutEnvironmentDependentFields(Value value) { + private boolean isNewInstanceWithoutEnvironmentDependentFields( + Value value, Set<Value> assumedNotToDependOnEnvironment) { assert !value.hasAliasedValue(); if (value.isPhi() || !value.definition.isNewInstance()) { @@ -203,16 +263,26 @@ // Check that none of the arguments to the constructor depend on the environment. for (int i = 1; i < constructorInvoke.arguments().size(); i++) { Value argument = constructorInvoke.arguments().get(i); - if (valueMayDependOnEnvironment(argument)) { + if (valueMayDependOnEnvironment(argument, assumedNotToDependOnEnvironment)) { return false; } } // Finally, check that the object does not escape. - return !valueMayBeMutatedBeforeMethodExit(value, ImmutableSet.of(constructorInvoke)); + if (valueMayBeMutatedBeforeMethodExit( + value, assumedNotToDependOnEnvironment, ImmutableSet.of(constructorInvoke))) { + return false; + } + + if (assumedNotToDependOnEnvironment.isEmpty()) { + knownNotToDependOnEnvironment.add(value); + } + + return true; } - private boolean valueMayBeMutatedBeforeMethodExit(Value value, Set<Instruction> whitelist) { + private boolean valueMayBeMutatedBeforeMethodExit( + Value value, Set<Value> assumedNotToDependOnEnvironment, Set<Instruction> whitelist) { assert !value.hasAliasedValue(); if (value.numberOfPhiUsers() > 0) { @@ -226,6 +296,14 @@ continue; } + if (user.isArrayPut()) { + ArrayPut arrayPut = user.asArrayPut(); + if (value == arrayPut.value() + && !valueMayDependOnEnvironment(arrayPut.array(), assumedNotToDependOnEnvironment)) { + continue; + } + } + if (user.isStaticPut()) { StaticPut staticPut = user.asStaticPut(); if (visited.contains(staticPut)) {
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/constant/SparseConditionalConstantPropagation.java b/src/main/java/com/android/tools/r8/ir/analysis/constant/SparseConditionalConstantPropagation.java index 79081ee..72ac893 100644 --- a/src/main/java/com/android/tools/r8/ir/analysis/constant/SparseConditionalConstantPropagation.java +++ b/src/main/java/com/android/tools/r8/ir/analysis/constant/SparseConditionalConstantPropagation.java
@@ -88,7 +88,7 @@ if (value.definition != evaluatedConst) { if (value.isPhi()) { // D8 relies on dead code removal to get rid of the dead phi itself. - if (value.numberOfAllUsers() != 0) { + if (value.hasAnyUsers()) { BasicBlock block = value.asPhi().getBlock(); blockToAnalyze.add(block); // Create a new constant, because it can be an existing constant that flow
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/fieldvalueanalysis/FieldValueAnalysis.java b/src/main/java/com/android/tools/r8/ir/analysis/fieldvalueanalysis/FieldValueAnalysis.java index 1ef58fb..5f85ba6 100644 --- a/src/main/java/com/android/tools/r8/ir/analysis/fieldvalueanalysis/FieldValueAnalysis.java +++ b/src/main/java/com/android/tools/r8/ir/analysis/fieldvalueanalysis/FieldValueAnalysis.java
@@ -4,20 +4,29 @@ package com.android.tools.r8.ir.analysis.fieldvalueanalysis; +import static com.android.tools.r8.ir.code.Opcodes.INVOKE_DIRECT; +import static com.android.tools.r8.ir.code.Opcodes.STATIC_PUT; + import com.android.tools.r8.graph.AppView; import com.android.tools.r8.graph.DexEncodedField; import com.android.tools.r8.graph.DexEncodedMethod; import com.android.tools.r8.graph.DexField; +import com.android.tools.r8.graph.DexProgramClass; import com.android.tools.r8.graph.DexType; import com.android.tools.r8.ir.analysis.type.ClassTypeLatticeElement; import com.android.tools.r8.ir.analysis.type.Nullability; import com.android.tools.r8.ir.analysis.type.TypeLatticeElement; +import com.android.tools.r8.ir.analysis.value.AbstractValue; +import com.android.tools.r8.ir.analysis.value.SingleEnumValue; +import com.android.tools.r8.ir.analysis.value.UnknownValue; import com.android.tools.r8.ir.code.BasicBlock; import com.android.tools.r8.ir.code.DominatorTree; import com.android.tools.r8.ir.code.DominatorTree.Assumption; import com.android.tools.r8.ir.code.IRCode; import com.android.tools.r8.ir.code.Instruction; import com.android.tools.r8.ir.code.InstructionIterator; +import com.android.tools.r8.ir.code.InvokeDirect; +import com.android.tools.r8.ir.code.NewInstance; import com.android.tools.r8.ir.code.StaticPut; import com.android.tools.r8.ir.code.Value; import com.android.tools.r8.ir.optimize.info.OptimizationFeedback; @@ -33,6 +42,7 @@ public class FieldValueAnalysis { private final AppView<AppInfoWithLiveness> appView; + private final DexProgramClass clazz; private final IRCode code; private final OptimizationFeedback feedback; private final DexEncodedMethod method; @@ -45,9 +55,11 @@ OptimizationFeedback feedback, DexEncodedMethod method) { this.appView = appView; + this.clazz = appView.definitionFor(method.method.holder).asProgramClass(); this.code = code; this.feedback = feedback; this.method = method; + assert this.clazz != null; } public static void run( @@ -244,16 +256,92 @@ } private void updateFieldOptimizationInfo(DexEncodedField field, Value value) { + // Abstract value. + Value root = value.getAliasedValue(); + AbstractValue abstractValue = computeAbstractValue(root); + if (!abstractValue.isUnknown()) { + feedback.recordFieldHasAbstractValue(field, abstractValue); + } + + // Dynamic upper bound type. TypeLatticeElement fieldType = TypeLatticeElement.fromDexType(field.field.type, Nullability.maybeNull(), appView); TypeLatticeElement dynamicUpperBoundType = value.getDynamicUpperBoundType(appView); if (dynamicUpperBoundType.strictlyLessThan(fieldType, appView)) { feedback.markFieldHasDynamicUpperBoundType(field, dynamicUpperBoundType); } + + // Dynamic lower bound type. ClassTypeLatticeElement dynamicLowerBoundType = value.getDynamicLowerBoundType(appView); if (dynamicLowerBoundType != null) { assert dynamicLowerBoundType.lessThanOrEqual(dynamicUpperBoundType, appView); feedback.markFieldHasDynamicLowerBoundType(field, dynamicLowerBoundType); } } + + private AbstractValue computeAbstractValue(Value value) { + assert !value.hasAliasedValue(); + if (clazz.isEnum()) { + SingleEnumValue singleEnumValue = getSingleEnumValue(value); + if (singleEnumValue != null) { + return singleEnumValue; + } + } + return UnknownValue.getInstance(); + } + + /** + * If {@param value} is defined by a new-instance instruction that instantiates the enclosing enum + * class, and the value is assigned into exactly one static enum field on the enclosing enum + * class, then returns a {@link SingleEnumValue} instance. Otherwise, returns {@code null}. + */ + private SingleEnumValue getSingleEnumValue(Value value) { + assert clazz.isEnum(); + assert !value.hasAliasedValue(); + if (value.isPhi() || !value.definition.isNewInstance()) { + return null; + } + + NewInstance newInstance = value.definition.asNewInstance(); + if (newInstance.clazz != clazz.type) { + return null; + } + + if (value.hasDebugUsers() || value.hasPhiUsers()) { + return null; + } + + DexEncodedField enumField = null; + for (Instruction user : value.uniqueUsers()) { + switch (user.opcode()) { + case INVOKE_DIRECT: + // Check that this is the corresponding constructor call. + InvokeDirect invoke = user.asInvokeDirect(); + if (!appView.dexItemFactory().isConstructor(invoke.getInvokedMethod()) + || invoke.getReceiver() != value) { + return null; + } + break; + + case STATIC_PUT: + DexEncodedField field = clazz.lookupStaticField(user.asStaticPut().getField()); + if (field != null && field.accessFlags.isEnum()) { + if (enumField != null) { + return null; + } + enumField = field; + } + break; + + default: + return null; + } + } + + if (enumField == null) { + return null; + } + + return appView.abstractValueFactory().createSingleEnumValue(enumField.field); + } }
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/proto/GeneratedExtensionRegistryShrinker.java b/src/main/java/com/android/tools/r8/ir/analysis/proto/GeneratedExtensionRegistryShrinker.java index 614c521..624cf90 100644 --- a/src/main/java/com/android/tools/r8/ir/analysis/proto/GeneratedExtensionRegistryShrinker.java +++ b/src/main/java/com/android/tools/r8/ir/analysis/proto/GeneratedExtensionRegistryShrinker.java
@@ -4,7 +4,6 @@ package com.android.tools.r8.ir.analysis.proto; -import static com.google.common.base.Predicates.alwaysFalse; import static com.google.common.base.Predicates.not; import com.android.tools.r8.graph.AppView; @@ -17,9 +16,8 @@ import com.android.tools.r8.graph.DexProgramClass; import com.android.tools.r8.graph.FieldAccessInfo; import com.android.tools.r8.graph.FieldAccessInfoCollection; -import com.android.tools.r8.ir.conversion.CallSiteInformation; import com.android.tools.r8.ir.conversion.IRConverter; -import com.android.tools.r8.ir.optimize.Outliner; +import com.android.tools.r8.ir.conversion.OneTimeMethodProcessor; import com.android.tools.r8.ir.optimize.info.OptimizationFeedbackIgnore; import com.android.tools.r8.logging.Log; import com.android.tools.r8.shaking.AppInfoWithLiveness; @@ -96,9 +94,7 @@ converter.processMethod( method, OptimizationFeedbackIgnore.getInstance(), - alwaysFalse(), - CallSiteInformation.empty(), - Outliner::noProcessing)); + OneTimeMethodProcessor.getInstance())); } private void forEachFindLiteExtensionByNumberMethod(Consumer<DexEncodedMethod> consumer) {
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/proto/GeneratedMessageLiteShrinker.java b/src/main/java/com/android/tools/r8/ir/analysis/proto/GeneratedMessageLiteShrinker.java index 62a9bb5..f2b354f 100644 --- a/src/main/java/com/android/tools/r8/ir/analysis/proto/GeneratedMessageLiteShrinker.java +++ b/src/main/java/com/android/tools/r8/ir/analysis/proto/GeneratedMessageLiteShrinker.java
@@ -7,7 +7,6 @@ import static com.android.tools.r8.ir.analysis.proto.ProtoUtils.getInfoValueFromMessageInfoConstructionInvoke; import static com.android.tools.r8.ir.analysis.proto.ProtoUtils.getObjectsValueFromMessageInfoConstructionInvoke; import static com.android.tools.r8.ir.analysis.proto.ProtoUtils.setObjectsValueForMessageInfoConstructionInvoke; -import static com.google.common.base.Predicates.alwaysFalse; import com.android.tools.r8.graph.AppView; import com.android.tools.r8.graph.DexClass; @@ -28,9 +27,8 @@ import com.android.tools.r8.ir.code.MemberType; import com.android.tools.r8.ir.code.NewArrayEmpty; import com.android.tools.r8.ir.code.Value; -import com.android.tools.r8.ir.conversion.CallSiteInformation; import com.android.tools.r8.ir.conversion.IRConverter; -import com.android.tools.r8.ir.optimize.Outliner; +import com.android.tools.r8.ir.conversion.OneTimeMethodProcessor; import com.android.tools.r8.ir.optimize.info.OptimizationFeedbackIgnore; import com.android.tools.r8.shaking.AppInfoWithLiveness; import java.util.List; @@ -76,9 +74,7 @@ converter.processMethod( method, OptimizationFeedbackIgnore.getInstance(), - alwaysFalse(), - CallSiteInformation.empty(), - Outliner::noProcessing)); + OneTimeMethodProcessor.getInstance())); } private void forEachDynamicMethod(Consumer<DexEncodedMethod> consumer) {
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/value/AbstractValue.java b/src/main/java/com/android/tools/r8/ir/analysis/value/AbstractValue.java new file mode 100644 index 0000000..e7a1e8b --- /dev/null +++ b/src/main/java/com/android/tools/r8/ir/analysis/value/AbstractValue.java
@@ -0,0 +1,34 @@ +// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +package com.android.tools.r8.ir.analysis.value; + +public abstract class AbstractValue { + + /** + * Returns true if this abstract value represents a single concrete value (i.e., the + * concretization of this abstract value has size 1). + */ + public boolean isSingleValue() { + return false; + } + + public boolean isSingleEnumValue() { + return false; + } + + public SingleEnumValue asSingleEnumValue() { + return null; + } + + public boolean isUnknown() { + return false; + } + + @Override + public abstract boolean equals(Object o); + + @Override + public abstract int hashCode(); +}
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/value/AbstractValueFactory.java b/src/main/java/com/android/tools/r8/ir/analysis/value/AbstractValueFactory.java new file mode 100644 index 0000000..c58addb --- /dev/null +++ b/src/main/java/com/android/tools/r8/ir/analysis/value/AbstractValueFactory.java
@@ -0,0 +1,17 @@ +// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +package com.android.tools.r8.ir.analysis.value; + +import com.android.tools.r8.graph.DexField; +import java.util.concurrent.ConcurrentHashMap; + +public class AbstractValueFactory { + + private ConcurrentHashMap<DexField, SingleEnumValue> singleEnumValues = new ConcurrentHashMap<>(); + + public SingleEnumValue createSingleEnumValue(DexField field) { + return singleEnumValues.computeIfAbsent(field, SingleEnumValue::new); + } +}
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/value/SingleEnumValue.java b/src/main/java/com/android/tools/r8/ir/analysis/value/SingleEnumValue.java new file mode 100644 index 0000000..6a3d96b --- /dev/null +++ b/src/main/java/com/android/tools/r8/ir/analysis/value/SingleEnumValue.java
@@ -0,0 +1,42 @@ +// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +package com.android.tools.r8.ir.analysis.value; + +import com.android.tools.r8.graph.DexField; + +public class SingleEnumValue extends AbstractValue { + + private final DexField field; + + /** Intentionally package private, use {@link AbstractValueFactory} instead. */ + SingleEnumValue(DexField field) { + this.field = field; + } + + @Override + public boolean isSingleValue() { + return true; + } + + @Override + public boolean isSingleEnumValue() { + return true; + } + + @Override + public SingleEnumValue asSingleEnumValue() { + return this; + } + + @Override + public boolean equals(Object o) { + return this == o; + } + + @Override + public int hashCode() { + return System.identityHashCode(this); + } +}
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/value/UnknownValue.java b/src/main/java/com/android/tools/r8/ir/analysis/value/UnknownValue.java new file mode 100644 index 0000000..bbae7bd --- /dev/null +++ b/src/main/java/com/android/tools/r8/ir/analysis/value/UnknownValue.java
@@ -0,0 +1,31 @@ +// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +package com.android.tools.r8.ir.analysis.value; + +public class UnknownValue extends AbstractValue { + + private static final UnknownValue INSTANCE = new UnknownValue(); + + private UnknownValue() {} + + public static UnknownValue getInstance() { + return INSTANCE; + } + + @Override + public boolean isUnknown() { + return true; + } + + @Override + public boolean equals(Object o) { + return this == o; + } + + @Override + public int hashCode() { + return System.identityHashCode(this); + } +}
diff --git a/src/main/java/com/android/tools/r8/ir/code/FieldInstruction.java b/src/main/java/com/android/tools/r8/ir/code/FieldInstruction.java index 72e4c9d..0e0c975 100644 --- a/src/main/java/com/android/tools/r8/ir/code/FieldInstruction.java +++ b/src/main/java/com/android/tools/r8/ir/code/FieldInstruction.java
@@ -191,7 +191,7 @@ DexEncodedMethod resolutionResult = appInfo .resolveMethod(clazz.type, dexItemFactory.objectMethods.finalize) - .asSingleTarget(); + .getSingleTarget(); return resolutionResult != null && resolutionResult.isProgramMethod(appInfo); }
diff --git a/src/main/java/com/android/tools/r8/ir/code/IRCode.java b/src/main/java/com/android/tools/r8/ir/code/IRCode.java index 955182b..bd5ea0a 100644 --- a/src/main/java/com/android/tools/r8/ir/code/IRCode.java +++ b/src/main/java/com/android/tools/r8/ir/code/IRCode.java
@@ -878,13 +878,13 @@ private boolean verifyNoValueWithOnlyAssumeInstructionAsUsers() { Predicate<Value> verifyValue = v -> { - assert v.numberOfUsers() == 0 - || v.uniqueUsers().stream().anyMatch(i -> !i.isAssume()) - || (!v.isPhi() && v.definition.isArgument()) - || v.numberOfDebugUsers() == 0 - || v.debugUsers().stream().anyMatch(i -> !i.isAssume()) - || v.numberOfPhiUsers() > 0 - : StringUtils.join(v.uniqueUsers(), System.lineSeparator()); + assert !v.hasUsers() + || v.uniqueUsers().stream().anyMatch(i -> !i.isAssume()) + || (!v.isPhi() && v.definition.isArgument()) + || !v.hasDebugUsers() + || v.debugUsers().stream().anyMatch(i -> !i.isAssume()) + || v.numberOfPhiUsers() > 0 + : StringUtils.join(v.uniqueUsers(), System.lineSeparator()); return true; }; return verifySSATypeLattice(wrapSSAVerifierWithStackValueHandling(verifyValue));
diff --git a/src/main/java/com/android/tools/r8/ir/code/InvokeMethod.java b/src/main/java/com/android/tools/r8/ir/code/InvokeMethod.java index 6c22405..c6f919d 100644 --- a/src/main/java/com/android/tools/r8/ir/code/InvokeMethod.java +++ b/src/main/java/com/android/tools/r8/ir/code/InvokeMethod.java
@@ -109,7 +109,7 @@ ResolutionResult refinedResolution = appView.appInfo().resolveMethod(refinedReceiverType, method); if (refinedResolution.hasSingleTarget()) { - DexEncodedMethod refinedTarget = refinedResolution.asSingleTarget(); + DexEncodedMethod refinedTarget = refinedResolution.getSingleTarget(); Set<DexEncodedMethod> result = Sets.newIdentityHashSet(); for (DexEncodedMethod target : targets) { if (target == refinedTarget
diff --git a/src/main/java/com/android/tools/r8/ir/code/NewInstance.java b/src/main/java/com/android/tools/r8/ir/code/NewInstance.java index 7a38882..d9d65e5 100644 --- a/src/main/java/com/android/tools/r8/ir/code/NewInstance.java +++ b/src/main/java/com/android/tools/r8/ir/code/NewInstance.java
@@ -176,7 +176,7 @@ ResolutionResult finalizeResolutionResult = appView.appInfo().resolveMethod(clazz, dexItemFactory.objectMethods.finalize); if (finalizeResolutionResult.hasSingleTarget()) { - DexMethod finalizeMethod = finalizeResolutionResult.asSingleTarget().method; + DexMethod finalizeMethod = finalizeResolutionResult.getSingleTarget().method; if (finalizeMethod != dexItemFactory.enumMethods.finalize && finalizeMethod != dexItemFactory.objectMethods.finalize) { return true;
diff --git a/src/main/java/com/android/tools/r8/ir/code/Phi.java b/src/main/java/com/android/tools/r8/ir/code/Phi.java index ae145b3..47eb9d4 100644 --- a/src/main/java/com/android/tools/r8/ir/code/Phi.java +++ b/src/main/java/com/android/tools/r8/ir/code/Phi.java
@@ -290,7 +290,7 @@ public void removeDeadPhi() { // First, make sure it is indeed dead, i.e., no non-phi users. - assert numberOfUsers() == 0; + assert !hasUsers(); // Then, manually clean up this from all of the operands. for (Value operand : getOperands()) { operand.removePhiUser(this);
diff --git a/src/main/java/com/android/tools/r8/ir/code/Value.java b/src/main/java/com/android/tools/r8/ir/code/Value.java index 74add42..c7aeff1 100644 --- a/src/main/java/com/android/tools/r8/ir/code/Value.java +++ b/src/main/java/com/android/tools/r8/ir/code/Value.java
@@ -10,12 +10,13 @@ import com.android.tools.r8.graph.DebugLocalInfo; import com.android.tools.r8.graph.DexClass; import com.android.tools.r8.graph.DexEncodedField; -import com.android.tools.r8.graph.DexField; import com.android.tools.r8.graph.DexMethod; import com.android.tools.r8.graph.DexType; import com.android.tools.r8.ir.analysis.type.ClassTypeLatticeElement; import com.android.tools.r8.ir.analysis.type.Nullability; import com.android.tools.r8.ir.analysis.type.TypeLatticeElement; +import com.android.tools.r8.ir.analysis.value.AbstractValue; +import com.android.tools.r8.ir.analysis.value.UnknownValue; import com.android.tools.r8.ir.regalloc.LiveIntervals; import com.android.tools.r8.origin.Origin; import com.android.tools.r8.position.MethodPosition; @@ -466,6 +467,22 @@ return debugData == null ? null : Collections.unmodifiableSet(debugData.users.keySet()); } + public boolean hasAnyUsers() { + return hasUsers() || hasPhiUsers() || hasDebugUsers(); + } + + public boolean hasDebugUsers() { + return debugData != null && !debugData.users.isEmpty(); + } + + public boolean hasPhiUsers() { + return !phiUsers.isEmpty(); + } + + public boolean hasUsers() { + return !users.isEmpty(); + } + public int numberOfUsers() { int size = users.size(); if (size <= 1) { @@ -830,33 +847,25 @@ return definition.isOutConstant() && !hasLocalInfo(); } - public DexEncodedField getEnumField(AppView<?> appView) { + public AbstractValue getAbstractValue(AppView<?> appView) { if (!appView.enableWholeProgramOptimizations()) { - return null; + return UnknownValue.getInstance(); } Value root = getAliasedValue(); - if (root.isPhi() || !root.definition.isStaticGet()) { - return null; + if (root.isPhi()) { + return UnknownValue.getInstance(); } - StaticGet staticGet = root.definition.asStaticGet(); - DexField field = staticGet.getField(); - if (field.type != field.holder) { - return null; + if (root.definition.isFieldGet()) { + FieldInstruction fieldGet = root.definition.asFieldInstruction(); + DexEncodedField field = appView.appInfo().resolveField(fieldGet.getField()); + if (field != null) { + return field.getOptimizationInfo().getAbstractValue(); + } } - DexEncodedField encodedField = appView.definitionFor(field); - if (encodedField == null) { - return null; - } - - DexClass clazz = appView.definitionFor(encodedField.field.holder); - if (clazz == null || !clazz.isEnum()) { - return null; - } - - return encodedField; + return UnknownValue.getInstance(); } public boolean isPhi() { @@ -1007,7 +1016,7 @@ // If the value has debug users we cannot eliminate it since it represents a value in a local // variable that should be visible in the debugger. - if (numberOfDebugUsers() != 0) { + if (hasDebugUsers()) { return false; } // This is a candidate for a dead value. Guard against looping by adding it to the set of
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/CallGraph.java b/src/main/java/com/android/tools/r8/ir/conversion/CallGraph.java index b9f72f7..c4f9072 100644 --- a/src/main/java/com/android/tools/r8/ir/conversion/CallGraph.java +++ b/src/main/java/com/android/tools/r8/ir/conversion/CallGraph.java
@@ -9,11 +9,8 @@ import com.android.tools.r8.ir.conversion.CallGraphBuilderBase.CycleEliminator.CycleEliminationResult; import com.android.tools.r8.ir.conversion.CallSiteInformation.CallGraphBasedCallSiteInformation; import com.android.tools.r8.shaking.AppInfoWithLiveness; -import com.android.tools.r8.utils.Timing; import java.util.Set; import java.util.TreeSet; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.ExecutorService; /** * Call graph representation. @@ -169,13 +166,6 @@ return new CallGraphBuilder(appView); } - static MethodProcessor createMethodProcessor( - AppView<AppInfoWithLiveness> appView, ExecutorService executorService, Timing timing) - throws ExecutionException { - CallGraph callGraph = CallGraph.builder(appView).build(executorService, timing); - return new MethodProcessor(appView, callGraph); - } - CallSiteInformation createCallSiteInformation(AppView<AppInfoWithLiveness> appView) { // Don't leverage single/dual call site information when we are not tree shaking. return appView.options().isShrinking()
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/CodeOptimization.java b/src/main/java/com/android/tools/r8/ir/conversion/CodeOptimization.java new file mode 100644 index 0000000..5d73cd5 --- /dev/null +++ b/src/main/java/com/android/tools/r8/ir/conversion/CodeOptimization.java
@@ -0,0 +1,25 @@ +// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. +package com.android.tools.r8.ir.conversion; + +import com.android.tools.r8.graph.AppView; +import com.android.tools.r8.ir.code.IRCode; +import com.android.tools.r8.ir.optimize.info.OptimizationFeedback; + +/** + * An abstraction of {@link IRCode}-level optimization. + */ +public interface CodeOptimization { + + // TODO(b/140766440): if some other code optimizations require more info to pass, i.e., requires + // a signature change, we may need to turn this interface into an abstract base, instead of + // rewriting every affected optimization. + // Note that a code optimization can be a collection of other code optimizations. + // In that way, IRConverter will serve as the default full processing of all optimizations. + void optimize( + AppView<?> appView, + IRCode code, + OptimizationFeedback feedback, + MethodProcessor methodProcessor); +}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/FieldOptimizationFeedback.java b/src/main/java/com/android/tools/r8/ir/conversion/FieldOptimizationFeedback.java index 46a0cc4..ceaf87e 100644 --- a/src/main/java/com/android/tools/r8/ir/conversion/FieldOptimizationFeedback.java +++ b/src/main/java/com/android/tools/r8/ir/conversion/FieldOptimizationFeedback.java
@@ -7,6 +7,7 @@ import com.android.tools.r8.graph.DexEncodedField; import com.android.tools.r8.ir.analysis.type.ClassTypeLatticeElement; import com.android.tools.r8.ir.analysis.type.TypeLatticeElement; +import com.android.tools.r8.ir.analysis.value.AbstractValue; public interface FieldOptimizationFeedback { @@ -19,4 +20,6 @@ void markFieldHasDynamicUpperBoundType(DexEncodedField field, TypeLatticeElement type); void markFieldBitsRead(DexEncodedField field, int bitsRead); + + void recordFieldHasAbstractValue(DexEncodedField field, AbstractValue abstractValue); }
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java index 80b7e61..358a7fe 100644 --- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java +++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -48,6 +48,7 @@ import com.android.tools.r8.ir.desugar.StringConcatRewriter; import com.android.tools.r8.ir.desugar.TwrCloseResourceRewriter; import com.android.tools.r8.ir.optimize.AliasIntroducer; +import com.android.tools.r8.ir.optimize.Assumer; import com.android.tools.r8.ir.optimize.ClassInitializerDefaultsOptimization; import com.android.tools.r8.ir.optimize.CodeRewriter; import com.android.tools.r8.ir.optimize.ConstantCanonicalizer; @@ -93,9 +94,9 @@ import com.android.tools.r8.utils.StringDiagnostic; import com.android.tools.r8.utils.ThreadUtils; import com.android.tools.r8.utils.Timing; -import com.google.common.base.Predicates; import com.google.common.base.Suppliers; import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ListMultimap; import com.google.common.collect.Sets; import com.google.common.collect.Streams; @@ -109,9 +110,8 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; -import java.util.function.BiConsumer; +import java.util.function.Consumer; import java.util.function.Function; -import java.util.function.Predicate; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -156,13 +156,12 @@ private final ServiceLoaderRewriter serviceLoaderRewriter; // Assumers that will insert Assume instructions. - private final AliasIntroducer aliasIntroducer; + public final Collection<Assumer> assumers = new ArrayList<>(); private final DynamicTypeOptimization dynamicTypeOptimization; - private final NonNullTracker nonNullTracker; final DeadCodeRemover deadCodeRemover; - final MethodOptimizationInfoCollector methodOptimizationInfoCollector; + private final MethodOptimizationInfoCollector methodOptimizationInfoCollector; private final OptimizationFeedbackDelayed delayedOptimizationFeedback = new OptimizationFeedbackDelayed(); @@ -210,7 +209,7 @@ // InterfaceMethodRewriter is needed for emulated interfaces. // LambdaRewriter is needed because if it is missing there are invoke custom on // default/static interface methods, and this is not supported by the compiler. - // The rest is nulled out. In addition the rewriting logic fails without lambda rewritting. + // The rest is nulled out. In addition the rewriting logic fails without lambda rewriting. this.backportedMethodRewriter = new BackportedMethodRewriter(appView, this); this.interfaceMethodRewriter = options.desugaredLibraryConfiguration.getEmulateLibraryInterface().isEmpty() @@ -220,8 +219,6 @@ this.twrCloseResourceRewriter = null; this.lambdaMerger = null; this.covariantReturnTypeAnnotationTransformer = null; - this.aliasIntroducer = null; - this.nonNullTracker = null; this.dynamicTypeOptimization = null; this.classInliner = null; this.classStaticizer = null; @@ -259,9 +256,12 @@ options.processCovariantReturnTypeAnnotations ? new CovariantReturnTypeAnnotationTransformer(this, appView.dexItemFactory()) : null; - this.aliasIntroducer = - options.testing.forceAssumeNoneInsertion ? new AliasIntroducer(appView) : null; - this.nonNullTracker = options.enableNonNullTracking ? new NonNullTracker(appView) : null; + if (options.testing.forceAssumeNoneInsertion) { + assumers.add(new AliasIntroducer(appView)); + } + if (options.enableNonNullTracking) { + assumers.add(new NonNullTracker(appView)); + } this.desugaredLibraryAPIConverter = appView.rewritePrefix.isRewriting() ? new DesugaredLibraryAPIConverter(appView) : null; if (appView.enableWholeProgramOptimizations()) { @@ -278,6 +278,9 @@ options.enableDynamicTypeOptimization ? new DynamicTypeOptimization(appViewWithLiveness) : null; + if (dynamicTypeOptimization != null) { + assumers.add(dynamicTypeOptimization); + } this.fieldBitAccessAnalysis = options.enableFieldBitAccessAnalysis ? new FieldBitAccessAnalysis(appViewWithLiveness) @@ -291,7 +294,7 @@ this.lensCodeRewriter = new LensCodeRewriter(appViewWithLiveness, lambdaRewriter); this.inliner = new Inliner(appViewWithLiveness, mainDexClasses, lambdaMerger, lensCodeRewriter); - this.outliner = new Outliner(appViewWithLiveness, this); + this.outliner = new Outliner(appViewWithLiveness); this.memberValuePropagation = options.enableValuePropagation ? new MemberValuePropagation(appViewWithLiveness) : null; this.methodOptimizationInfoCollector = @@ -590,8 +593,10 @@ if (options.isGeneratingClassFiles() || !(options.passthroughDexCode && method.getCode().isDexCode())) { // We do not process in call graph order, so anything could be a leaf. - rewriteCode(method, simpleOptimizationFeedback, x -> true, CallSiteInformation.empty(), - Outliner::noProcessing); + rewriteCode( + method, + simpleOptimizationFeedback, + OneTimeMethodProcessor.getInstance(ImmutableList.of(method))); } else { assert method.getCode().isDexCode(); } @@ -630,29 +635,26 @@ // reprocessing IR code of methods, e.g., outlining, double-inlining, class staticizer, etc. // - a side effect is candidates for those optimizations are identified. // 2) Revisit DexEncodedMethods for the collected candidates. - // TODO(b/127694949): unified framework to reprocess methods only once. printPhase("Primary optimization pass"); // Process the application identifying outlining candidates. GraphLense graphLenseForIR = appView.graphLense(); OptimizationFeedbackDelayed feedback = delayedOptimizationFeedback; + PostMethodProcessor.Builder postMethodProcessorBuilder = + new PostMethodProcessor.Builder(getOptimizationsForPostIRProcessing()); { timing.begin("Build call graph"); - MethodProcessor methodProcessor = - CallGraph.createMethodProcessor(appView.withLiveness(), executorService, timing); + PrimaryMethodProcessor primaryMethodProcessor = + PrimaryMethodProcessor.create( + appView.withLiveness(), postMethodProcessorBuilder, executorService, timing); timing.end(); timing.begin("IR conversion phase 1"); - BiConsumer<IRCode, DexEncodedMethod> outlineHandler = - outliner == null ? Outliner::noProcessing : outliner.identifyCandidateMethods(); - methodProcessor.forEachMethod( - (method, isProcessedConcurrently) -> - processMethod( - method, - feedback, - isProcessedConcurrently, - methodProcessor.getCallSiteInformation(), - outlineHandler), + if (outliner != null) { + outliner.createOutlineMethodIdentifierGenerator(); + } + primaryMethodProcessor.forEachMethod( + method -> processMethod(method, feedback, primaryMethodProcessor), this::waveStart, this::waveDone, executorService); @@ -666,27 +668,31 @@ libraryMethodOverrideAnalysis.finish(); } - // Second pass for methods whose collected call site information become more precise. + // Post processing: + // 1) Second pass for methods whose collected call site information become more precise. + // 2) Second inlining pass for dealing with double inline callers. + printPhase("Post optimization pass"); if (appView.callSiteOptimizationInfoPropagator() != null) { - printPhase("2nd round of method processing after inter-procedural analysis."); - timing.begin("IR conversion phase 2"); - appView.callSiteOptimizationInfoPropagator().revisitMethods(this, executorService); - feedback.updateVisibleOptimizationInfo(); - timing.end(); + postMethodProcessorBuilder.put(appView.callSiteOptimizationInfoPropagator()); } - - // Second inlining pass for dealing with double inline callers. if (inliner != null) { - printPhase("Double caller inlining"); - assert graphLenseForIR == appView.graphLense(); - inliner.processDoubleInlineCallers(this, executorService); + postMethodProcessorBuilder.put(inliner); + } + timing.begin("IR conversion phase 2"); + assert graphLenseForIR == appView.graphLense(); + PostMethodProcessor postMethodProcessor = + postMethodProcessorBuilder.build(appView.withLiveness(), executorService, timing); + if (postMethodProcessor != null) { + postMethodProcessor.forEachWave(feedback, executorService); feedback.updateVisibleOptimizationInfo(); assert graphLenseForIR == appView.graphLense(); } + timing.end(); // TODO(b/112831361): Implement support for staticizeClasses in CF backend. if (!options.isGeneratingClassFiles()) { printPhase("Class staticizer post processing"); + // TODO(b/127694949): Adapt to PostOptimization. staticizeClasses(feedback, executorService); feedback.updateVisibleOptimizationInfo(); } @@ -707,6 +713,7 @@ handleSynthesizedClassMapping(builder); printPhase("Lambda merging finalization"); + // TODO(b/127694949): Adapt to PostOptimization. finalizeLambdaMerging(application, feedback, builder, executorService); printPhase("Desugared library API Conversion finalization"); @@ -721,24 +728,25 @@ // Update optimization info for all synthesized methods at once. feedback.updateVisibleOptimizationInfo(); + // TODO(b/127694949): Adapt to PostOptimization. if (outliner != null) { printPhase("Outlining"); timing.begin("IR conversion phase 3"); if (outliner.selectMethodsForOutlining()) { forEachSelectedOutliningMethod( - (code, method) -> { + code -> { printMethod(code, "IR before outlining (SSA)", null); - outliner.identifyOutlineSites(code, method); + outliner.identifyOutlineSites(code); }, executorService); DexProgramClass outlineClass = outliner.buildOutlinerClass(computeOutlineClassType()); appView.appInfo().addSynthesizedClass(outlineClass); optimizeSynthesizedClass(outlineClass, executorService); forEachSelectedOutliningMethod( - (code, method) -> { - outliner.applyOutliningCandidate(code, method); + code -> { + outliner.applyOutliningCandidate(code); printMethod(code, "IR after outlining (SSA)", null); - finalizeIR(method, code, OptimizationFeedbackIgnore.getInstance()); + finalizeIR(code.method, code, OptimizationFeedbackIgnore.getInstance()); }, executorService); feedback.updateVisibleOptimizationInfo(); @@ -827,7 +835,7 @@ } private void forEachSelectedOutliningMethod( - BiConsumer<IRCode, DexEncodedMethod> consumer, ExecutorService executorService) + Consumer<IRCode> consumer, ExecutorService executorService) throws ExecutionException { assert !options.skipIR; Set<DexEncodedMethod> methods = outliner.getMethodsSelectedForOutlining(); @@ -844,7 +852,7 @@ codeRewriter.rewriteMoveResult(code); deadCodeRemover.run(code); CodeRewriter.removeAssumeInstructions(appView, code); - consumer.accept(code, method); + consumer.accept(code); }, executorService); } @@ -962,40 +970,41 @@ processMethod( method, delayedOptimizationFeedback, - Predicates.alwaysFalse(), - CallSiteInformation.empty(), - Outliner::noProcessing); + OneTimeMethodProcessor.getInstance()); } } public void processMethodsConcurrently( Collection<DexEncodedMethod> methods, ExecutorService executorService) throws ExecutionException { - ThreadUtils.processItems( - methods, - method -> processMethod( - method, - delayedOptimizationFeedback, - methods::contains, - CallSiteInformation.empty(), - Outliner::noProcessing), - executorService); + OneTimeMethodProcessor processor = OneTimeMethodProcessor.getInstance(methods); + processor.forEachWave( + method -> processMethod(method, delayedOptimizationFeedback, processor), executorService); } private String logCode(InternalOptions options, DexEncodedMethod method) { return options.useSmaliSyntax ? method.toSmaliString(null) : method.codeToString(); } + List<CodeOptimization> getOptimizationsForPrimaryIRProcessing() { + // TODO(b/140766440): Remove unnecessary steps once all sub steps are converted. + return ImmutableList.of(this::optimize); + } + + List<CodeOptimization> getOptimizationsForPostIRProcessing() { + // TODO(b/140766440): Remove unnecessary steps once all sub steps are converted. + return ImmutableList.of(this::optimize); + } + + // TODO(b/140766440): Make this receive a list of CodeOptimizations to conduct. public void processMethod( DexEncodedMethod method, OptimizationFeedback feedback, - Predicate<DexEncodedMethod> isProcessedConcurrently, - CallSiteInformation callSiteInformation, - BiConsumer<IRCode, DexEncodedMethod> outlineHandler) { + MethodProcessor methodProcessor) { Code code = method.getCode(); boolean matchesMethodFilter = options.methodMatchesFilter(method); if (code != null && matchesMethodFilter) { - rewriteCode(method, feedback, isProcessedConcurrently, callSiteInformation, outlineHandler); + rewriteCode(method, feedback, methodProcessor); } else { // Mark abstract methods as processed as well. method.markProcessed(ConstraintWithTarget.NEVER); @@ -1011,15 +1020,10 @@ } private void rewriteCode( - DexEncodedMethod method, - OptimizationFeedback feedback, - Predicate<DexEncodedMethod> isProcessedConcurrently, - CallSiteInformation callSiteInformation, - BiConsumer<IRCode, DexEncodedMethod> outlineHandler) { + DexEncodedMethod method, OptimizationFeedback feedback, MethodProcessor methodProcessor) { Origin origin = appView.appInfo().originFor(method.method.holder); try { - rewriteCodeInternal( - method, feedback, isProcessedConcurrently, callSiteInformation, outlineHandler, origin); + rewriteCodeInternal(method, feedback, methodProcessor, origin); } catch (CompilationError e) { // If rewriting throws a compilation error, attach the origin and method if missing. throw e.withAdditionalOriginAndPositionInfo(origin, new MethodPosition(method.method)); @@ -1035,9 +1039,7 @@ private void rewriteCodeInternal( DexEncodedMethod method, OptimizationFeedback feedback, - Predicate<DexEncodedMethod> isProcessedConcurrently, - CallSiteInformation callSiteInformation, - BiConsumer<IRCode, DexEncodedMethod> outlineHandler, + MethodProcessor methodProcessor, Origin origin) { if (options.verbose) { @@ -1057,6 +1059,17 @@ feedback.markProcessed(method, ConstraintWithTarget.NEVER); return; } + optimize(appView, code, feedback, methodProcessor); + } + + // TODO(b/140766440): Convert all sub steps an implementer of CodeOptimization + private void optimize( + AppView<?> appView, + IRCode code, + OptimizationFeedback feedback, + MethodProcessor methodProcessor) { + DexEncodedMethod method = code.method; + if (Log.ENABLED) { Log.debug(getClass(), "Initial (SSA) flow graph for %s:\n%s", method.toSourceString(), code); } @@ -1137,28 +1150,16 @@ previous = printMethod(code, "IR after disable assertions (SSA)", previous); - if (aliasIntroducer != null) { - aliasIntroducer.insertAssumeInstructions(code); - assert code.isConsistentSSA(); - } + CodeRewriter.insertAssumeInstructions(code, assumers); - if (nonNullTracker != null) { - nonNullTracker.insertAssumeInstructions(code); - assert code.isConsistentSSA(); - } - - if (dynamicTypeOptimization != null) { - assert appView.enableWholeProgramOptimizations(); - dynamicTypeOptimization.insertAssumeInstructions(code); - assert code.isConsistentSSA(); - } + previous = printMethod(code, "IR after inserting assume instructions (SSA)", previous); appView.withGeneratedMessageLiteShrinker(shrinker -> shrinker.run(method, code)); - previous = printMethod(code, "IR after null tracking (SSA)", previous); + previous = printMethod(code, "IR after generated message lite shrinking (SSA)", previous); if (!isDebugMode && options.enableInlining && inliner != null) { - inliner.performInlining(method, code, feedback, isProcessedConcurrently, callSiteInformation); + inliner.performInlining(method, code, feedback, methodProcessor); assert code.verifyTypes(appView); } @@ -1276,15 +1277,14 @@ stringOptimizer, method, code, - isProcessedConcurrently, + methodProcessor::isProcessedConcurrently, inliner, Suppliers.memoize( () -> inliner.createDefaultOracle( method, code, - isProcessedConcurrently, - callSiteInformation, + methodProcessor, options.classInliningInstructionLimit, // Inlining instruction allowance is not needed for the class inliner since it // always uses a force inlining oracle for inlining. @@ -1332,8 +1332,10 @@ previous = printMethod(code, "IR after lambda merger (SSA)", previous); - if (options.outline.enabled) { - outlineHandler.accept(code, method); + // TODO(b/140766440): an ideal solution would be puttting CodeOptimization for this into + // the list for primary processing only. + if (options.outline.enabled && outliner != null && methodProcessor.isPrimary()) { + outliner.getOutlineMethodIdentifierGenerator().accept(code); assert code.isConsistentSSA(); } @@ -1383,7 +1385,7 @@ collectOptimizationInfo(code, feedback); } - if (aliasIntroducer != null || nonNullTracker != null || dynamicTypeOptimization != null) { + if (!assumers.isEmpty()) { CodeRewriter.removeAssumeInstructions(appView, code); assert code.isConsistentSSA(); }
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/MethodProcessor.java b/src/main/java/com/android/tools/r8/ir/conversion/MethodProcessor.java index 6a7633a..b2bf648 100644 --- a/src/main/java/com/android/tools/r8/ir/conversion/MethodProcessor.java +++ b/src/main/java/com/android/tools/r8/ir/conversion/MethodProcessor.java
@@ -1,112 +1,27 @@ // Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. - package com.android.tools.r8.ir.conversion; -import com.android.tools.r8.graph.AppView; import com.android.tools.r8.graph.DexEncodedMethod; -import com.android.tools.r8.ir.conversion.CallGraph.Node; -import com.android.tools.r8.ir.conversion.CallGraphBuilderBase.CycleEliminator; -import com.android.tools.r8.shaking.AppInfoWithLiveness; -import com.android.tools.r8.utils.Action; -import com.android.tools.r8.utils.IROrdering; -import com.android.tools.r8.utils.ThreadUtils; -import com.android.tools.r8.utils.ThrowingBiConsumer; -import com.google.common.collect.Sets; -import java.util.ArrayDeque; -import java.util.Collection; -import java.util.Deque; -import java.util.Iterator; -import java.util.Set; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.ExecutorService; -import java.util.function.Consumer; -import java.util.function.Predicate; -public class MethodProcessor { +public interface MethodProcessor { - private final CallSiteInformation callSiteInformation; - private final Deque<Collection<DexEncodedMethod>> waves; - - MethodProcessor(AppView<AppInfoWithLiveness> appView, CallGraph callGraph) { - this.callSiteInformation = callGraph.createCallSiteInformation(appView); - this.waves = createWaves(appView, callGraph, callSiteInformation); + enum Phase { + ONE_TIME, + PRIMARY, + POST } - public CallSiteInformation getCallSiteInformation() { - return callSiteInformation; + Phase getPhase(); + + default boolean isPrimary() { + return getPhase() == Phase.PRIMARY; } - public static Deque<Collection<DexEncodedMethod>> createWaves( - AppView<?> appView, CallGraph callGraph, CallSiteInformation callSiteInformation) { - IROrdering shuffle = appView.options().testing.irOrdering; - Deque<Collection<DexEncodedMethod>> waves = new ArrayDeque<>(); - - Set<Node> nodes = callGraph.nodes; - Set<DexEncodedMethod> reprocessing = Sets.newIdentityHashSet(); - while (!nodes.isEmpty()) { - Set<DexEncodedMethod> wave = Sets.newIdentityHashSet(); - extractLeaves( - nodes, - leaf -> { - wave.add(leaf.method); - - // Reprocess methods that invoke a method with a single call site. - if (callSiteInformation.hasSingleCallSite(leaf.method.method)) { - callGraph.cycleEliminationResult.forEachRemovedCaller( - leaf, caller -> reprocessing.add(caller.method)); - } - }); - waves.addLast(shuffle.order(wave)); - } - // TODO(b/127694949): Reprocess these methods using a general framework for reprocessing - // methods. - if (!reprocessing.isEmpty()) { - waves.addLast(shuffle.order(reprocessing)); - } - return waves; + default CallSiteInformation getCallSiteInformation() { + return CallSiteInformation.empty(); } - /** - * Extract the next set of leaves (nodes with an outgoing call degree of 0) if any. - * - * <p>All nodes in the graph are extracted if called repeatedly until null is returned. Please - * note that there are no cycles in this graph (see {@link CycleEliminator#breakCycles}). - */ - static void extractLeaves(Set<Node> nodes, Consumer<Node> fn) { - Set<Node> removed = Sets.newIdentityHashSet(); - Iterator<Node> nodeIterator = nodes.iterator(); - while (nodeIterator.hasNext()) { - Node node = nodeIterator.next(); - if (node.isLeaf()) { - fn.accept(node); - nodeIterator.remove(); - removed.add(node); - } - } - removed.forEach(Node::cleanCallersForRemoval); - } - - /** - * Applies the given method to all leaf nodes of the graph. - * - * <p>As second parameter, a predicate that can be used to decide whether another method is - * processed at the same time is passed. This can be used to avoid races in concurrent processing. - */ - public <E extends Exception> void forEachMethod( - ThrowingBiConsumer<DexEncodedMethod, Predicate<DexEncodedMethod>, E> consumer, - Consumer<Collection<DexEncodedMethod>> waveStart, - Action waveDone, - ExecutorService executorService) - throws ExecutionException { - while (!waves.isEmpty()) { - Collection<DexEncodedMethod> wave = waves.removeFirst(); - assert wave.size() > 0; - waveStart.accept(wave); - ThreadUtils.processItems( - wave, method -> consumer.accept(method, wave::contains), executorService); - waveDone.execute(); - } - } + boolean isProcessedConcurrently(DexEncodedMethod method); }
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/OneTimeMethodProcessor.java b/src/main/java/com/android/tools/r8/ir/conversion/OneTimeMethodProcessor.java new file mode 100644 index 0000000..43b801e --- /dev/null +++ b/src/main/java/com/android/tools/r8/ir/conversion/OneTimeMethodProcessor.java
@@ -0,0 +1,49 @@ +// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. +package com.android.tools.r8.ir.conversion; + +import com.android.tools.r8.graph.DexEncodedMethod; +import com.android.tools.r8.utils.ThreadUtils; +import com.android.tools.r8.utils.ThrowingConsumer; +import java.util.Collection; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; + +/** + * A {@link MethodProcessor} that doesn't persist; rather just processes the given methods one-time, + * along with a default abstraction of concurrent processing. + */ +public class OneTimeMethodProcessor implements MethodProcessor { + + private Collection<DexEncodedMethod> wave; + + private OneTimeMethodProcessor(Collection<DexEncodedMethod> methodsToProcess) { + this.wave = methodsToProcess; + } + + public static OneTimeMethodProcessor getInstance() { + return new OneTimeMethodProcessor(null); + } + + static OneTimeMethodProcessor getInstance(Collection<DexEncodedMethod> methodsToProcess) { + return new OneTimeMethodProcessor(methodsToProcess); + } + + @Override + public Phase getPhase() { + return Phase.ONE_TIME; + } + + @Override + public boolean isProcessedConcurrently(DexEncodedMethod method) { + return wave != null && wave.contains(method); + } + + <E extends Exception> void forEachWave( + ThrowingConsumer<DexEncodedMethod, E> consumer, + ExecutorService executorService) + throws ExecutionException { + ThreadUtils.processItems(wave, consumer, executorService); + } +}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/PostMethodProcessor.java b/src/main/java/com/android/tools/r8/ir/conversion/PostMethodProcessor.java index 13566359..5107b18 100644 --- a/src/main/java/com/android/tools/r8/ir/conversion/PostMethodProcessor.java +++ b/src/main/java/com/android/tools/r8/ir/conversion/PostMethodProcessor.java
@@ -4,13 +4,121 @@ package com.android.tools.r8.ir.conversion; +import com.android.tools.r8.graph.AppView; +import com.android.tools.r8.graph.DexEncodedMethod; +import com.android.tools.r8.ir.code.IRCode; import com.android.tools.r8.ir.conversion.CallGraph.Node; +import com.android.tools.r8.ir.optimize.Inliner.ConstraintWithTarget; +import com.android.tools.r8.ir.optimize.info.OptimizationFeedback; +import com.android.tools.r8.logging.Log; +import com.android.tools.r8.origin.Origin; +import com.android.tools.r8.shaking.AppInfoWithLiveness; +import com.android.tools.r8.utils.IROrdering; +import com.android.tools.r8.utils.ThreadUtils; +import com.android.tools.r8.utils.Timing; +import com.google.common.collect.Maps; import com.google.common.collect.Sets; +import java.util.ArrayDeque; +import java.util.Collection; +import java.util.Deque; import java.util.Iterator; +import java.util.LinkedHashSet; +import java.util.Map; import java.util.Set; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; import java.util.function.Consumer; -public class PostMethodProcessor { +class PostMethodProcessor implements MethodProcessor { + + private final AppView<AppInfoWithLiveness> appView; + private final Map<DexEncodedMethod, Collection<CodeOptimization>> methodsMap; + private final Deque<Collection<DexEncodedMethod>> waves; + private Collection<DexEncodedMethod> wave; + + private PostMethodProcessor( + AppView<AppInfoWithLiveness> appView, + Map<DexEncodedMethod, Collection<CodeOptimization>> methodsMap, + CallGraph callGraph) { + this.appView = appView; + this.methodsMap = methodsMap; + this.waves = createWaves(appView, callGraph); + } + + @Override + public Phase getPhase() { + return Phase.POST; + } + + static class Builder { + private final Collection<CodeOptimization> defaultCodeOptimizations; + private final Map<DexEncodedMethod, Collection<CodeOptimization>> methodsMap = + Maps.newIdentityHashMap(); + + Builder(Collection<CodeOptimization> defaultCodeOptimizations) { + this.defaultCodeOptimizations = defaultCodeOptimizations; + } + + private void put( + Set<DexEncodedMethod> methodsToRevisit, Collection<CodeOptimization> codeOptimizations) { + if (codeOptimizations.isEmpty()) { + // Nothing to conduct. + return; + } + for (DexEncodedMethod method : methodsToRevisit) { + methodsMap + .computeIfAbsent( + method, + // Optimization order might matter, hence a collection that preserves orderings. + k -> new LinkedHashSet<>()) + .addAll(codeOptimizations); + } + } + + void put(Set<DexEncodedMethod> methodsToRevisit) { + put(methodsToRevisit, defaultCodeOptimizations); + } + + void put(PostOptimization postOptimization) { + Collection<CodeOptimization> codeOptimizations = + postOptimization.codeOptimizationsForPostProcessing(); + if (codeOptimizations == null) { + codeOptimizations = defaultCodeOptimizations; + } + put(postOptimization.methodsToRevisit(), codeOptimizations); + } + + PostMethodProcessor build( + AppView<AppInfoWithLiveness> appView, ExecutorService executorService, Timing timing) + throws ExecutionException { + if (methodsMap.keySet().isEmpty()) { + // Nothing to revisit. + return null; + } + CallGraph callGraph = + new PartialCallGraphBuilder(appView, methodsMap.keySet()) + .build(executorService, timing); + return new PostMethodProcessor(appView, methodsMap, callGraph); + } + } + + private Deque<Collection<DexEncodedMethod>> createWaves(AppView<?> appView, CallGraph callGraph) { + IROrdering shuffle = appView.options().testing.irOrdering; + Deque<Collection<DexEncodedMethod>> waves = new ArrayDeque<>(); + + Set<Node> nodes = callGraph.nodes; + int waveCount = 1; + while (!nodes.isEmpty()) { + Set<DexEncodedMethod> wave = Sets.newIdentityHashSet(); + extractRoots(nodes, n -> wave.add(n.method)); + waves.addLast(shuffle.order(wave)); + if (Log.ENABLED && Log.isLoggingEnabledFor(PostMethodProcessor.class)) { + Log.info(getClass(), "Wave #%d: %d", waveCount++, wave.size()); + } + } + + return waves; + } /** * Extract the next set of roots (nodes with an incoming call degree of 0) if any. @@ -30,4 +138,47 @@ } removed.forEach(Node::cleanCalleesForRemoval); } + + @Override + public boolean isProcessedConcurrently(DexEncodedMethod method) { + return wave != null && wave.contains(method); + } + + void forEachWave(OptimizationFeedback feedback, ExecutorService executorService) + throws ExecutionException { + while (!waves.isEmpty()) { + wave = waves.removeFirst(); + assert wave.size() > 0; + ThreadUtils.processItems( + wave, + method -> { + Collection<CodeOptimization> codeOptimizations = methodsMap.get(method); + assert codeOptimizations != null && !codeOptimizations.isEmpty(); + forEachMethod(method, codeOptimizations, feedback); + }, + executorService); + } + } + + private void forEachMethod( + DexEncodedMethod method, + Collection<CodeOptimization> codeOptimizations, + OptimizationFeedback feedback) { + // TODO(b/140766440): Make IRConverter#process receive a list of CodeOptimization to conduct. + // Then, we can share IRCode creation there. + Origin origin = appView.appInfo().originFor(method.method.holder); + if (appView.options().skipIR) { + feedback.markProcessed(method, ConstraintWithTarget.NEVER); + return; + } + IRCode code = method.buildIR(appView, origin); + if (code == null) { + feedback.markProcessed(method, ConstraintWithTarget.NEVER); + return; + } + // TODO(b/140768815): Reprocessing may trigger more methods to revisit. Update waves on-the-fly. + for (CodeOptimization codeOptimization : codeOptimizations) { + codeOptimization.optimize(appView, code, feedback, this); + } + } }
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/PostOptimization.java b/src/main/java/com/android/tools/r8/ir/conversion/PostOptimization.java new file mode 100644 index 0000000..5ab18b3 --- /dev/null +++ b/src/main/java/com/android/tools/r8/ir/conversion/PostOptimization.java
@@ -0,0 +1,31 @@ +// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. +package com.android.tools.r8.ir.conversion; + +import com.android.tools.r8.graph.DexEncodedMethod; +import java.util.Collection; +import java.util.Set; + +/** + * An abstraction of optimizations that require post processing of methods. + */ +public interface PostOptimization { + + /** + * @return a set of methods that need post processing. + */ + Set<DexEncodedMethod> methodsToRevisit(); + + // TODO(b/127694949): different CodeOptimization for primary processor v.s. post processor? + // In that way, instead of internal state changes, such as COLLECT v.s. APPLY or REVISIT, + // optimizers that need post processing can return what to do at each processor. + // Collection<CodeOptimization> codeOptimizationsForPrimaryProcessing(); + + /** + * @return specific collection of {@link CodeOptimization}s to conduct during post processing. + * Otherwise, i.e., if the default one---IRConverter's full processing---is okay, + * returns {@code null}. + */ + Collection<CodeOptimization> codeOptimizationsForPostProcessing(); +}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/PrimaryMethodProcessor.java b/src/main/java/com/android/tools/r8/ir/conversion/PrimaryMethodProcessor.java new file mode 100644 index 0000000..1787587 --- /dev/null +++ b/src/main/java/com/android/tools/r8/ir/conversion/PrimaryMethodProcessor.java
@@ -0,0 +1,145 @@ +// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +package com.android.tools.r8.ir.conversion; + +import com.android.tools.r8.graph.AppView; +import com.android.tools.r8.graph.DexEncodedMethod; +import com.android.tools.r8.ir.conversion.CallGraph.Node; +import com.android.tools.r8.ir.conversion.CallGraphBuilderBase.CycleEliminator; +import com.android.tools.r8.logging.Log; +import com.android.tools.r8.shaking.AppInfoWithLiveness; +import com.android.tools.r8.utils.Action; +import com.android.tools.r8.utils.IROrdering; +import com.android.tools.r8.utils.ThreadUtils; +import com.android.tools.r8.utils.ThrowingConsumer; +import com.android.tools.r8.utils.Timing; +import com.google.common.collect.Sets; +import java.util.ArrayDeque; +import java.util.Collection; +import java.util.Deque; +import java.util.Iterator; +import java.util.Set; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.function.Consumer; + +/** + * A {@link MethodProcessor} that processes methods in the whole program in a bottom-up manner, + * i.e., from leaves to roots. + */ +class PrimaryMethodProcessor implements MethodProcessor { + + private final CallSiteInformation callSiteInformation; + private final PostMethodProcessor.Builder postMethodProcessorBuilder; + private final Deque<Collection<DexEncodedMethod>> waves; + private Collection<DexEncodedMethod> wave; + + private PrimaryMethodProcessor( + AppView<AppInfoWithLiveness> appView, + PostMethodProcessor.Builder postMethodProcessorBuilder, + CallGraph callGraph) { + this.callSiteInformation = callGraph.createCallSiteInformation(appView); + this.postMethodProcessorBuilder = postMethodProcessorBuilder; + this.waves = createWaves(appView, callGraph, callSiteInformation); + } + + static PrimaryMethodProcessor create( + AppView<AppInfoWithLiveness> appView, + PostMethodProcessor.Builder postMethodProcessorBuilder, + ExecutorService executorService, + Timing timing) + throws ExecutionException { + CallGraph callGraph = CallGraph.builder(appView).build(executorService, timing); + return new PrimaryMethodProcessor(appView, postMethodProcessorBuilder, callGraph); + } + + @Override + public Phase getPhase() { + return Phase.PRIMARY; + } + + @Override + public CallSiteInformation getCallSiteInformation() { + return callSiteInformation; + } + + private Deque<Collection<DexEncodedMethod>> createWaves( + AppView<?> appView, CallGraph callGraph, CallSiteInformation callSiteInformation) { + IROrdering shuffle = appView.options().testing.irOrdering; + Deque<Collection<DexEncodedMethod>> waves = new ArrayDeque<>(); + + Set<Node> nodes = callGraph.nodes; + Set<DexEncodedMethod> reprocessing = Sets.newIdentityHashSet(); + int waveCount = 1; + while (!nodes.isEmpty()) { + Set<DexEncodedMethod> wave = Sets.newIdentityHashSet(); + extractLeaves( + nodes, + leaf -> { + wave.add(leaf.method); + + // Reprocess methods that invoke a method with a single call site. + if (callSiteInformation.hasSingleCallSite(leaf.method.method)) { + callGraph.cycleEliminationResult.forEachRemovedCaller( + leaf, caller -> reprocessing.add(caller.method)); + } + }); + waves.addLast(shuffle.order(wave)); + if (Log.ENABLED && Log.isLoggingEnabledFor(PrimaryMethodProcessor.class)) { + Log.info(getClass(), "Wave #%d: %d", waveCount++, wave.size()); + } + } + if (!reprocessing.isEmpty()) { + postMethodProcessorBuilder.put(reprocessing); + } + return waves; + } + + /** + * Extract the next set of leaves (nodes with an outgoing call degree of 0) if any. + * + * <p>All nodes in the graph are extracted if called repeatedly until null is returned. Please + * note that there are no cycles in this graph (see {@link CycleEliminator#breakCycles}). + */ + static void extractLeaves(Set<Node> nodes, Consumer<Node> fn) { + Set<Node> removed = Sets.newIdentityHashSet(); + Iterator<Node> nodeIterator = nodes.iterator(); + while (nodeIterator.hasNext()) { + Node node = nodeIterator.next(); + if (node.isLeaf()) { + fn.accept(node); + nodeIterator.remove(); + removed.add(node); + } + } + removed.forEach(Node::cleanCallersForRemoval); + } + + @Override + public boolean isProcessedConcurrently(DexEncodedMethod method) { + return wave != null && wave.contains(method); + } + + /** + * Applies the given method to all leaf nodes of the graph. + * + * <p>As second parameter, a predicate that can be used to decide whether another method is + * processed at the same time is passed. This can be used to avoid races in concurrent processing. + */ + <E extends Exception> void forEachMethod( + ThrowingConsumer<DexEncodedMethod, E> consumer, + Consumer<Collection<DexEncodedMethod>> waveStart, + Action waveDone, + ExecutorService executorService) + throws ExecutionException { + while (!waves.isEmpty()) { + wave = waves.removeFirst(); + assert wave.size() > 0; + waveStart.accept(wave); + ThreadUtils.processItems(wave, consumer, executorService); + waveDone.execute(); + } + } +}
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodRewriter.java b/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodRewriter.java index 899277d..54ac5c0 100644 --- a/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodRewriter.java +++ b/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodRewriter.java
@@ -1063,9 +1063,15 @@ } private void warnMissingType(DexMethod referencedFrom, DexType missing) { - // Companion/Emulated interface classes for desugared library won't be missing, + // Companion/Emulated interface/Conversion classes for desugared library won't be missing, // they are in the desugared library. - if (appView.rewritePrefix.hasRewrittenType(missing)) { + if (appView.rewritePrefix.hasRewrittenType(missing) + || appView + .options() + .desugaredLibraryConfiguration + .getCustomConversions() + .values() + .contains(missing)) { return; } DexMethod method = appView.graphLense().getOriginalMethodSignature(referencedFrom);
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Assumer.java b/src/main/java/com/android/tools/r8/ir/optimize/Assumer.java index 10e7314..dfabec5 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/Assumer.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/Assumer.java
@@ -3,6 +3,7 @@ // BSD-style license that can be found in the LICENSE file. package com.android.tools.r8.ir.optimize; +import com.android.tools.r8.Keep; import com.android.tools.r8.ir.code.Assume; import com.android.tools.r8.ir.code.BasicBlock; import com.android.tools.r8.ir.code.IRCode; @@ -13,10 +14,14 @@ /** * One that assumes. Inherited tracker/optimization insert necessary variants of {@link Assume}. */ -interface Assumer { +// TODO(b/143590191): should not need an explicit keep annotation to prevent the default interface +// method from being shrunk. +@Keep +public interface Assumer { default void insertAssumeInstructions(IRCode code) { insertAssumeInstructionsInBlocks(code, code.listIterator(), Predicates.alwaysTrue()); } + void insertAssumeInstructionsInBlocks( IRCode code, ListIterator<BasicBlock> blockIterator, Predicate<BasicBlock> blockTester); }
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/CallSiteOptimizationInfoPropagator.java b/src/main/java/com/android/tools/r8/ir/optimize/CallSiteOptimizationInfoPropagator.java index 2f6c563..cbedf91 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/CallSiteOptimizationInfoPropagator.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/CallSiteOptimizationInfoPropagator.java
@@ -20,7 +20,8 @@ import com.android.tools.r8.ir.code.InstructionListIterator; import com.android.tools.r8.ir.code.InvokeMethod; import com.android.tools.r8.ir.code.Value; -import com.android.tools.r8.ir.conversion.IRConverter; +import com.android.tools.r8.ir.conversion.CodeOptimization; +import com.android.tools.r8.ir.conversion.PostOptimization; import com.android.tools.r8.ir.optimize.info.CallSiteOptimizationInfo; import com.android.tools.r8.ir.optimize.info.MutableCallSiteOptimizationInfo; import com.android.tools.r8.logging.Log; @@ -30,10 +31,8 @@ import java.util.LinkedList; import java.util.List; import java.util.Set; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.ExecutorService; -public class CallSiteOptimizationInfoPropagator { +public class CallSiteOptimizationInfoPropagator implements PostOptimization { // TODO(b/139246447): should we revisit new targets over and over again? // Maybe piggy-back on MethodProcessor's wave/batch processing? @@ -231,8 +230,9 @@ } } - public void revisitMethods(IRConverter converter, ExecutorService executorService) - throws ExecutionException { + @Override + public Set<DexEncodedMethod> methodsToRevisit() { + mode = Mode.REVISIT; Set<DexEncodedMethod> targetsToRevisit = Sets.newIdentityHashSet(); for (DexProgramClass clazz : appView.appInfo().classes()) { for (DexEncodedMethod method : clazz.methods()) { @@ -248,13 +248,15 @@ } } } - mode = Mode.REVISIT; - if (targetsToRevisit.isEmpty()) { - return; - } if (revisitedMethods != null) { revisitedMethods.addAll(targetsToRevisit); } - converter.processMethodsConcurrently(targetsToRevisit, executorService); + return targetsToRevisit; + } + + @Override + public Collection<CodeOptimization> codeOptimizationsForPostProcessing() { + // Run IRConverter#optimize. + return null; } }
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java index f6fba1d..f2c9031 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
@@ -14,7 +14,6 @@ import com.android.tools.r8.graph.AppView; import com.android.tools.r8.graph.DebugLocalInfo; import com.android.tools.r8.graph.DexClass; -import com.android.tools.r8.graph.DexEncodedField; import com.android.tools.r8.graph.DexEncodedMethod; import com.android.tools.r8.graph.DexField; import com.android.tools.r8.graph.DexItemFactory; @@ -27,6 +26,7 @@ import com.android.tools.r8.ir.analysis.type.Nullability; import com.android.tools.r8.ir.analysis.type.TypeAnalysis; import com.android.tools.r8.ir.analysis.type.TypeLatticeElement; +import com.android.tools.r8.ir.analysis.value.AbstractValue; import com.android.tools.r8.ir.code.AlwaysMaterializingNop; import com.android.tools.r8.ir.code.ArrayLength; import com.android.tools.r8.ir.code.ArrayPut; @@ -159,6 +159,13 @@ this.dexItemFactory = appView.dexItemFactory(); } + public static void insertAssumeInstructions(IRCode code, Collection<Assumer> assumers) { + for (Assumer assumer : assumers) { + assumer.insertAssumeInstructions(code); + assert code.isConsistentSSA(); + } + } + public static void removeAssumeInstructions(AppView<?> appView, IRCode code) { // We need to update the types of all values whose definitions depend on a non-null value. // This is needed to preserve soundness of the types after the Assume<NonNullAssumption> @@ -1086,12 +1093,12 @@ // If the original input to the switch is now unused, remove it too. It is not dead // as it might have side-effects but we ignore these here. Instruction arrayGet = info.arrayGet; - if (arrayGet.outValue().numberOfUsers() == 0) { + if (!arrayGet.outValue().hasUsers()) { arrayGet.inValues().forEach(v -> v.removeUser(arrayGet)); arrayGet.getBlock().removeInstruction(arrayGet); } Instruction staticGet = info.staticGet; - if (staticGet.outValue().numberOfUsers() == 0) { + if (!staticGet.outValue().hasUsers()) { assert staticGet.inValues().isEmpty(); staticGet.getBlock().removeInstruction(staticGet); } @@ -1392,7 +1399,7 @@ while (it.hasNext()) { Instruction current = it.next(); if (current.isCheckCast()) { - boolean hasPhiUsers = current.outValue().numberOfPhiUsers() != 0; + boolean hasPhiUsers = current.outValue().hasPhiUsers(); RemoveCheckCastInstructionIfTrivialResult removeResult = removeCheckCastInstructionIfTrivial(current.asCheckCast(), it, code, affectedValues); if (removeResult != RemoveCheckCastInstructionIfTrivialResult.NO_REMOVALS) { @@ -1402,7 +1409,7 @@ affectedValues.clear(); } } else if (current.isInstanceOf()) { - boolean hasPhiUsers = current.outValue().numberOfPhiUsers() != 0; + boolean hasPhiUsers = current.outValue().hasPhiUsers(); if (removeInstanceOfInstructionIfTrivial(current.asInstanceOf(), it, code)) { needToRemoveTrivialPhis |= hasPhiUsers; } @@ -1749,8 +1756,8 @@ dominatorTreeMemoization, addConstantInBlock, insn -> - (insn.isConstNumber() && insn.outValue().numberOfAllUsers() != 0) - || (insn.isConstString() && insn.outValue().numberOfAllUsers() != 0)); + (insn.isConstNumber() && insn.outValue().hasAnyUsers()) + || (insn.isConstString() && insn.outValue().hasAnyUsers())); } else { // For all following blocks only process ConstString with just one use. shortenLiveRangesInsideBlock( @@ -1778,12 +1785,12 @@ // except if they are used by phi instructions or they are a string constants. assert constants instanceof LinkedHashMap; for (Instruction constantInstruction : constants.values()) { - if (constantInstruction.outValue().numberOfPhiUsers() == 0 + if (!constantInstruction.outValue().hasPhiUsers() && !constantInstruction.isConstString()) { assert constantInstruction.isConstNumber(); ConstNumber constNumber = constantInstruction.asConstNumber(); Value constantValue = constantInstruction.outValue(); - assert constantValue.numberOfUsers() != 0; + assert constantValue.hasUsers(); assert constantValue.numberOfUsers() == constantValue.numberOfAllUsers(); for (Instruction user : constantValue.uniqueUsers()) { ConstNumber newCstNum = ConstNumber.copyOf(code, constNumber); @@ -2532,12 +2539,12 @@ } } } else { - DexEncodedField enumField = lhs.getEnumField(appView); - if (enumField != null) { - DexEncodedField otherEnumField = rhs.getEnumField(appView); - if (enumField == otherEnumField) { + AbstractValue abstractValue = lhs.getAbstractValue(appView); + if (abstractValue.isSingleEnumValue()) { + AbstractValue otherAbstractValue = rhs.getAbstractValue(appView); + if (abstractValue == otherAbstractValue) { simplifyIfWithKnownCondition(code, block, theIf, 0); - } else if (otherEnumField != null) { + } else if (otherAbstractValue.isSingleEnumValue()) { simplifyIfWithKnownCondition(code, block, theIf, 1); } }
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/DeadCodeRemover.java b/src/main/java/com/android/tools/r8/ir/optimize/DeadCodeRemover.java index 8dc27fd..b14a47a 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/DeadCodeRemover.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/DeadCodeRemover.java
@@ -16,9 +16,10 @@ import com.android.tools.r8.ir.code.Value; import com.android.tools.r8.shaking.AppInfoWithLiveness; import com.google.common.collect.ImmutableList; +import java.util.ArrayDeque; import java.util.Collection; +import java.util.Deque; import java.util.Iterator; -import java.util.LinkedList; import java.util.Queue; public class DeadCodeRemover { @@ -33,13 +34,14 @@ public void run(IRCode code) { removeUnneededCatchHandlers(code); - Queue<BasicBlock> worklist = new LinkedList<>(); + Deque<BasicBlock> worklist = new ArrayDeque<>(); // We may encounter unneeded catch handlers again, e.g., if a dead instruction (due to // const-string canonicalization for example) is the only throwing instruction in a block. // Removing unneeded catch handlers can lead to more dead instructions. do { - worklist.addAll(code.blocks); - for (BasicBlock block = worklist.poll(); block != null; block = worklist.poll()) { + worklist.addAll(code.topologicallySortedBlocks()); + while (!worklist.isEmpty()) { + BasicBlock block = worklist.removeLast(); removeDeadInstructions(worklist, code, block); removeDeadPhis(worklist, code, block); }
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java index 893cf2d..fea8d6c 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
@@ -27,6 +27,7 @@ import com.android.tools.r8.ir.code.Monitor; import com.android.tools.r8.ir.code.Value; import com.android.tools.r8.ir.conversion.CallSiteInformation; +import com.android.tools.r8.ir.conversion.MethodProcessor; import com.android.tools.r8.ir.optimize.Inliner.InlineAction; import com.android.tools.r8.ir.optimize.Inliner.InlineeWithReason; import com.android.tools.r8.ir.optimize.Inliner.Reason; @@ -53,6 +54,7 @@ private final Inliner inliner; private final DexEncodedMethod method; private final IRCode code; + private final MethodProcessor methodProcessor; private final CallSiteInformation callSiteInformation; private final Predicate<DexEncodedMethod> isProcessedConcurrently; private final int inliningInstructionLimit; @@ -63,16 +65,16 @@ Inliner inliner, DexEncodedMethod method, IRCode code, - CallSiteInformation callSiteInformation, - Predicate<DexEncodedMethod> isProcessedConcurrently, + MethodProcessor methodProcessor, int inliningInstructionLimit, int inliningInstructionAllowance) { this.appView = appView; this.inliner = inliner; this.method = method; this.code = code; - this.callSiteInformation = callSiteInformation; - this.isProcessedConcurrently = isProcessedConcurrently; + this.methodProcessor = methodProcessor; + this.callSiteInformation = methodProcessor.getCallSiteInformation(); + this.isProcessedConcurrently = methodProcessor::isProcessedConcurrently; this.inliningInstructionLimit = inliningInstructionLimit; this.instructionAllowance = inliningInstructionAllowance; } @@ -428,8 +430,7 @@ if (Log.ENABLED) { Log.verbose(getClass(), "Forcing extra inline on " + target.toSourceString()); } - inliner.performInlining( - target, inlinee, feedback, isProcessedConcurrently, callSiteInformation); + inliner.performInlining(target, inlinee, feedback, methodProcessor); } }
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java index 606a489..70013de 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
@@ -39,8 +39,10 @@ import com.android.tools.r8.ir.code.Value; import com.android.tools.r8.ir.code.ValueNumberGenerator; import com.android.tools.r8.ir.conversion.CallSiteInformation; -import com.android.tools.r8.ir.conversion.IRConverter; +import com.android.tools.r8.ir.conversion.CodeOptimization; import com.android.tools.r8.ir.conversion.LensCodeRewriter; +import com.android.tools.r8.ir.conversion.MethodProcessor; +import com.android.tools.r8.ir.conversion.PostOptimization; import com.android.tools.r8.ir.desugar.TwrCloseResourceRewriter; import com.android.tools.r8.ir.optimize.info.OptimizationFeedback; import com.android.tools.r8.ir.optimize.info.OptimizationFeedbackIgnore; @@ -58,17 +60,15 @@ import com.google.common.collect.Sets; import java.util.ArrayDeque; import java.util.ArrayList; +import java.util.Collection; import java.util.Deque; import java.util.HashMap; import java.util.List; import java.util.ListIterator; import java.util.Map; import java.util.Set; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.ExecutorService; -import java.util.function.Predicate; -public class Inliner { +public class Inliner implements PostOptimization { protected final AppView<AppInfoWithLiveness> appView; private final Set<DexMethod> blacklist; @@ -237,17 +237,16 @@ } } - public void processDoubleInlineCallers( - IRConverter converter, ExecutorService executorService) - throws ExecutionException { - if (doubleInlineCallers.isEmpty()) { - return; - } + @Override + public Set<DexEncodedMethod> methodsToRevisit() { applyDoubleInlining = true; - converter.processMethodsConcurrently(doubleInlineCallers, executorService); - doubleInlineCallers.forEach(method -> { - assert method.isProcessed(); - }); + return doubleInlineCallers; + } + + @Override + public Collection<CodeOptimization> codeOptimizationsForPostProcessing() { + // Run IRConverter#optimize. + return null; // Technically same as return converter.getOptimizationForPostIRProcessing(); } /** @@ -824,15 +823,13 @@ DexEncodedMethod method, IRCode code, OptimizationFeedback feedback, - Predicate<DexEncodedMethod> isProcessedConcurrently, - CallSiteInformation callSiteInformation) { + MethodProcessor methodProcessor) { InternalOptions options = appView.options(); DefaultInliningOracle oracle = createDefaultOracle( method, code, - isProcessedConcurrently, - callSiteInformation, + methodProcessor, options.inliningInstructionLimit, options.inliningInstructionAllowance - numberOfInstructions(code)); performInliningImpl(oracle, oracle, method, code, feedback); @@ -841,8 +838,7 @@ public DefaultInliningOracle createDefaultOracle( DexEncodedMethod method, IRCode code, - Predicate<DexEncodedMethod> isProcessedConcurrently, - CallSiteInformation callSiteInformation, + MethodProcessor methodProcessor, int inliningInstructionLimit, int inliningInstructionAllowance) { return new DefaultInliningOracle( @@ -850,8 +846,7 @@ this, method, code, - callSiteInformation, - isProcessedConcurrently, + methodProcessor, inliningInstructionLimit, inliningInstructionAllowance); }
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java b/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java index cea6258..adc1112 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
@@ -270,7 +270,7 @@ // references that have actual definitions are marked by the root set builder. So, here, we // try again with a resolved target, not the direct definition, which may not exist. DexEncodedMethod resolutionTarget = - appView.appInfo().resolveMethod(invokedHolder, invokedMethod).asSingleTarget(); + appView.appInfo().resolveMethod(invokedHolder, invokedMethod).getSingleTarget(); lookup = lookupMemberRule(resolutionTarget); } boolean invokeReplaced = false;
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Outliner.java b/src/main/java/com/android/tools/r8/ir/optimize/Outliner.java index 0795424..7f055de 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/Outliner.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/Outliner.java
@@ -55,7 +55,6 @@ import com.android.tools.r8.ir.code.ValueType; import com.android.tools.r8.ir.code.ValueTypeConstraint; import com.android.tools.r8.ir.conversion.IRBuilder; -import com.android.tools.r8.ir.conversion.IRConverter; import com.android.tools.r8.ir.conversion.SourceCode; import com.android.tools.r8.ir.optimize.Inliner.ConstraintWithTarget; import com.android.tools.r8.naming.ClassNameMapper; @@ -77,7 +76,7 @@ import java.util.Map.Entry; import java.util.Objects; import java.util.Set; -import java.util.function.BiConsumer; +import java.util.function.Consumer; /** * Support class for implementing outlining (i.e. extracting common code patterns as methods). @@ -86,27 +85,26 @@ * * <ul> * <li>First, all methods are converted to IR and passed to {@link - * Outliner#identifyCandidateMethods()} to identify outlining candidates and the methods - * containing each candidate. IR is converted to the output format (DEX or CF) and thrown away - * along with the outlining candidates; only a list of lists of methods is kept, where each - * list of methods corresponds to methods containing an outlining candidate. + * Outliner#createOutlineMethodIdentifierGenerator()}} to identify outlining candidates and + * the methods containing each candidate. IR is converted to the output format (DEX or CF) and + * thrown away along with the outlining candidates; only a list of lists of methods is kept, + * where each list of methods corresponds to methods containing an outlining candidate. * <li>Second, {@link Outliner#selectMethodsForOutlining()} is called to retain the lists of * methods found in the first step that are large enough (see {@link InternalOptions#outline} * {@link OutlineOptions#threshold}), and the methods to be further analyzed for outlining is * returned by {@link Outliner#getMethodsSelectedForOutlining}. Each selected method is then - * converted back to IR and passed to {@link Outliner#identifyOutlineSites(IRCode, - * DexEncodedMethod)}, which then stores concrete outlining candidates in {@link - * Outliner#outlineSites}. + * converted back to IR and passed to {@link Outliner#identifyOutlineSites(IRCode)}, which + * then stores concrete outlining candidates in {@link Outliner#outlineSites}. * <li>Third, {@link Outliner#buildOutlinerClass(DexType)} is called to construct the <em>outline * support class</em> containing a static helper method for each outline candidate that occurs * frequently enough. Each selected method is then converted to IR, passed to {@link - * Outliner#applyOutliningCandidate(IRCode, DexEncodedMethod)} to perform the outlining, and + * Outliner#applyOutliningCandidate(IRCode)} to perform the outlining, and * converted back to the output format (DEX or CF). * </ul> */ public class Outliner { - /** Result of first step (see {@link Outliner#identifyCandidateMethods()}. */ + /** Result of first step (see {@link Outliner#createOutlineMethodIdentifierGenerator()}. */ private final List<List<DexEncodedMethod>> candidateMethodLists = new ArrayList<>(); /** Result of second step (see {@link Outliner#selectMethodsForOutlining()}. */ private final Set<DexEncodedMethod> methodsSelectedForOutlining = Sets.newIdentityHashSet(); @@ -1188,9 +1186,11 @@ int argumentsMapIndex; OutlineRewriter( - DexEncodedMethod method, IRCode code, - ListIterator<BasicBlock> blocksIterator, BasicBlock block, List<Integer> toRemove) { - super(method, block); + IRCode code, + ListIterator<BasicBlock> blocksIterator, + BasicBlock block, + List<Integer> toRemove) { + super(code.method, block); this.code = code; this.blocksIterator = blocksIterator; this.toRemove = toRemove; @@ -1271,30 +1271,39 @@ } } - public Outliner(AppView<AppInfoWithLiveness> appView, IRConverter converter) { + public Outliner(AppView<AppInfoWithLiveness> appView) { this.appView = appView; this.inliningConstraints = new InliningConstraints(appView, GraphLense.getIdentityLense()); } - public BiConsumer<IRCode, DexEncodedMethod> identifyCandidateMethods() { + public void createOutlineMethodIdentifierGenerator() { // Since optimizations may change the map identity of Outline objects (e.g. by setting the // out-value of invokes to null), this map must not be used except for identifying methods // potentially relevant to outlining. OutlineMethodIdentifier will add method lists to // candidateMethodLists whenever it adds an entry to candidateMap. Map<Outline, List<DexEncodedMethod>> candidateMap = new HashMap<>(); assert candidateMethodLists.isEmpty(); - return (code, method) -> { - assert !(method.getCode() instanceof OutlineCode); - for (BasicBlock block : code.blocks) { - new OutlineMethodIdentifier(method, block, candidateMap).process(); - } - }; + assert outlineMethodIdentifierGenerator == null; + outlineMethodIdentifierGenerator = + code -> { + assert !code.method.getCode().isOutlineCode(); + for (BasicBlock block : code.blocks) { + new OutlineMethodIdentifier(code.method, block, candidateMap).process(); + } + }; } - public void identifyOutlineSites(IRCode code, DexEncodedMethod method) { - assert !(method.getCode() instanceof OutlineCode); + private Consumer<IRCode> outlineMethodIdentifierGenerator; + + public Consumer<IRCode> getOutlineMethodIdentifierGenerator() { + assert outlineMethodIdentifierGenerator != null; + return outlineMethodIdentifierGenerator; + } + + public void identifyOutlineSites(IRCode code) { + assert !code.method.getCode().isOutlineCode(); for (BasicBlock block : code.blocks) { - new OutlineSiteIdentifier(method, block).process(); + new OutlineSiteIdentifier(code.method, block).process(); } } @@ -1391,13 +1400,13 @@ return result; } - public void applyOutliningCandidate(IRCode code, DexEncodedMethod method) { - assert !(method.getCode() instanceof OutlineCode); + public void applyOutliningCandidate(IRCode code) { + assert !code.method.getCode().isOutlineCode(); ListIterator<BasicBlock> blocksIterator = code.listIterator(); while (blocksIterator.hasNext()) { BasicBlock block = blocksIterator.next(); List<Integer> toRemove = new ArrayList<>(); - new OutlineRewriter(method, code, blocksIterator, block, toRemove).process(); + new OutlineRewriter(code, blocksIterator, block, toRemove).process(); block.removeInstructions(toRemove); } } @@ -1409,10 +1418,6 @@ return true; } - static public void noProcessing(IRCode code, DexEncodedMethod method) { - // No operation. - } - private class OutlineSourceCode implements SourceCode { final private Outline outline;
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/ServiceLoaderRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/ServiceLoaderRewriter.java index 451788f..29ad438 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/ServiceLoaderRewriter.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/ServiceLoaderRewriter.java
@@ -97,8 +97,7 @@ InvokeStatic serviceLoaderLoad = instruction.asInvokeStatic(); Value serviceLoaderLoadOut = serviceLoaderLoad.outValue(); - if (serviceLoaderLoadOut.numberOfAllUsers() != 1 - || serviceLoaderLoadOut.numberOfPhiUsers() != 0) { + if (serviceLoaderLoadOut.numberOfAllUsers() != 1 || serviceLoaderLoadOut.hasPhiUsers()) { continue; }
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java index 1203a35..40cb024 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
@@ -447,7 +447,7 @@ Assume<?> assumeInstruction = user.asAssume(); Value src = assumeInstruction.src(); Value dest = assumeInstruction.outValue(); - assert dest.numberOfPhiUsers() == 0; + assert !dest.hasPhiUsers(); dest.replaceUsers(src); removeInstruction(user); } @@ -550,7 +550,7 @@ for (FieldValueHelper fieldValueHelper : fieldHelpers.values()) { fieldValueHelper.replaceValue(value, newValue); } - assert value.numberOfAllUsers() == 0; + assert !value.hasAnyUsers(); // `newValue` could be a phi introduced by FieldValueHelper. Its initial type is set as the // type of read field, but it could be more precise than that due to (multiple) inlining. // In addition to values affected by `newValue`, it's necessary to revisit `newValue` itself. @@ -655,7 +655,7 @@ Set<Instruction> indirectUsers) { if (!eligibility.returnsReceiver || invoke.outValue() == null - || invoke.outValue().numberOfAllUsers() == 0) { + || !invoke.outValue().hasAnyUsers()) { return true; } // For CF we no longer perform the code-rewrite in CodeRewriter.rewriteMoveResult that removes @@ -733,7 +733,7 @@ // signature of the invocation resolves to a private or static method. ResolutionResult resolutionResult = appView.appInfo().resolveMethod(callee.holder, callee); if (resolutionResult.hasSingleTarget() - && !resolutionResult.asSingleTarget().isVirtualMethod()) { + && !resolutionResult.getSingleTarget().isVirtualMethod()) { return null; } @@ -885,7 +885,7 @@ } if (parameterUsage.isReturned) { - if (!(invoke.outValue() == null || invoke.outValue().numberOfAllUsers() == 0)) { + if (invoke.outValue() != null && invoke.outValue().hasAnyUsers()) { // Used as return value which is not ignored. return false; }
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/DefaultFieldOptimizationInfo.java b/src/main/java/com/android/tools/r8/ir/optimize/info/DefaultFieldOptimizationInfo.java index e82afb7..a30c24f 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/info/DefaultFieldOptimizationInfo.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/info/DefaultFieldOptimizationInfo.java
@@ -6,6 +6,8 @@ import com.android.tools.r8.ir.analysis.type.ClassTypeLatticeElement; import com.android.tools.r8.ir.analysis.type.TypeLatticeElement; +import com.android.tools.r8.ir.analysis.value.AbstractValue; +import com.android.tools.r8.ir.analysis.value.UnknownValue; public class DefaultFieldOptimizationInfo extends FieldOptimizationInfo { @@ -28,6 +30,11 @@ } @Override + public AbstractValue getAbstractValue() { + return UnknownValue.getInstance(); + } + + @Override public int getReadBits() { return BitAccessInfo.getNoBitsReadValue(); }
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/FieldOptimizationInfo.java b/src/main/java/com/android/tools/r8/ir/optimize/info/FieldOptimizationInfo.java index cc89f84..9103459 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/info/FieldOptimizationInfo.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/info/FieldOptimizationInfo.java
@@ -6,6 +6,7 @@ import com.android.tools.r8.ir.analysis.type.ClassTypeLatticeElement; import com.android.tools.r8.ir.analysis.type.TypeLatticeElement; +import com.android.tools.r8.ir.analysis.value.AbstractValue; public abstract class FieldOptimizationInfo { @@ -13,6 +14,8 @@ public abstract boolean cannotBeKept(); + public abstract AbstractValue getAbstractValue(); + /** * This should only be used once all methods in the program have been processed. Until then the * value returned by this method may not be sound.
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java b/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java index fb442d1..e002c13 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java
@@ -58,7 +58,6 @@ import java.util.List; import java.util.Set; import java.util.function.BiFunction; -import java.util.function.Function; public class MethodOptimizationInfoCollector { private final AppView<AppInfoWithLiveness> appView; @@ -291,7 +290,7 @@ feedback.setTrivialInitializer( method, method.isInstanceInitializer() - ? computeInstanceInitializerInfo(code, clazz, appView::definitionFor) + ? computeInstanceInitializerInfo(code, clazz) : computeClassInitializerInfo(code, clazz)); } @@ -389,8 +388,7 @@ // ** Assigns arguments or non-throwing constants to fields of this class. // // (Note that this initializer does not have to have zero arguments.) - private TrivialInitializer computeInstanceInitializerInfo( - IRCode code, DexClass clazz, Function<DexType, DexClass> typeToClass) { + private TrivialInitializer computeInstanceInitializerInfo(IRCode code, DexClass clazz) { if (clazz.definesFinalizer(options.itemFactory)) { // Defining a finalize method can observe the side-effect of Object.<init> GC registration. return null; @@ -423,11 +421,12 @@ if (invokedMethod.holder != clazz.superType) { return null; } - // java.lang.Object.<init>() is considered trivial. - if (invokedMethod == dexItemFactory.objectMethods.constructor) { + // java.lang.Enum.<init>() and java.lang.Object.<init>() are considered trivial. + if (invokedMethod == dexItemFactory.enumMethods.constructor + || invokedMethod == dexItemFactory.objectMethods.constructor) { continue; } - DexClass holder = typeToClass.apply(invokedMethod.holder); + DexClass holder = appView.definitionFor(invokedMethod.holder); if (holder == null) { return null; }
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/MutableFieldOptimizationInfo.java b/src/main/java/com/android/tools/r8/ir/optimize/info/MutableFieldOptimizationInfo.java index 9d3bb4a..91aa098 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/info/MutableFieldOptimizationInfo.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/info/MutableFieldOptimizationInfo.java
@@ -9,6 +9,8 @@ import com.android.tools.r8.graph.DexType; import com.android.tools.r8.ir.analysis.type.ClassTypeLatticeElement; import com.android.tools.r8.ir.analysis.type.TypeLatticeElement; +import com.android.tools.r8.ir.analysis.value.AbstractValue; +import com.android.tools.r8.ir.analysis.value.UnknownValue; import java.util.function.Function; /** @@ -20,6 +22,7 @@ */ public class MutableFieldOptimizationInfo extends FieldOptimizationInfo { + private AbstractValue abstractValue = UnknownValue.getInstance(); private int readBits = 0; private boolean cannotBeKept = false; private boolean valueHasBeenPropagated = false; @@ -45,6 +48,15 @@ } @Override + public AbstractValue getAbstractValue() { + return abstractValue; + } + + public void setAbstractValue(AbstractValue abstractValue) { + this.abstractValue = abstractValue; + } + + @Override public int getReadBits() { return readBits; }
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackDelayed.java b/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackDelayed.java index 2a80ce4..63f1095 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackDelayed.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackDelayed.java
@@ -13,6 +13,7 @@ import com.android.tools.r8.graph.DexType; import com.android.tools.r8.ir.analysis.type.ClassTypeLatticeElement; import com.android.tools.r8.ir.analysis.type.TypeLatticeElement; +import com.android.tools.r8.ir.analysis.value.AbstractValue; import com.android.tools.r8.ir.optimize.Inliner.ConstraintWithTarget; import com.android.tools.r8.utils.IteratorUtils; import com.android.tools.r8.utils.StringUtils; @@ -121,6 +122,11 @@ getFieldOptimizationInfoForUpdating(field).joinReadBits(bitsRead); } + @Override + public void recordFieldHasAbstractValue(DexEncodedField field, AbstractValue abstractValue) { + getFieldOptimizationInfoForUpdating(field).setAbstractValue(abstractValue); + } + // METHOD OPTIMIZATION INFO: @Override
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackIgnore.java b/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackIgnore.java index 9b5c988..b42b588 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackIgnore.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackIgnore.java
@@ -13,6 +13,7 @@ import com.android.tools.r8.graph.DexType; import com.android.tools.r8.ir.analysis.type.ClassTypeLatticeElement; import com.android.tools.r8.ir.analysis.type.TypeLatticeElement; +import com.android.tools.r8.ir.analysis.value.AbstractValue; import com.android.tools.r8.ir.optimize.Inliner.ConstraintWithTarget; import java.util.BitSet; import java.util.Set; @@ -45,6 +46,9 @@ @Override public void markFieldBitsRead(DexEncodedField field, int bitsRead) {} + @Override + public void recordFieldHasAbstractValue(DexEncodedField field, AbstractValue abstractValue) {} + // METHOD OPTIMIZATION INFO: @Override
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackSimple.java b/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackSimple.java index 474a963..8e0dd11 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackSimple.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackSimple.java
@@ -13,6 +13,7 @@ import com.android.tools.r8.graph.DexType; import com.android.tools.r8.ir.analysis.type.ClassTypeLatticeElement; import com.android.tools.r8.ir.analysis.type.TypeLatticeElement; +import com.android.tools.r8.ir.analysis.value.AbstractValue; import com.android.tools.r8.ir.optimize.Inliner.ConstraintWithTarget; import java.util.BitSet; import java.util.Set; @@ -55,6 +56,11 @@ // Ignored. } + @Override + public void recordFieldHasAbstractValue(DexEncodedField field, AbstractValue abstractValue) { + // Ignored. + } + // METHOD OPTIMIZATION INFO. @Override
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/staticizer/StaticizingProcessor.java b/src/main/java/com/android/tools/r8/ir/optimize/staticizer/StaticizingProcessor.java index 158fd9c..7625078 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/staticizer/StaticizingProcessor.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/staticizer/StaticizingProcessor.java
@@ -176,7 +176,7 @@ trivialPhis.clear(); boolean onlyHasTrivialPhis = testAndCollectPhisComposedOfThis( visited, thisValue.uniquePhiUsers(), thisValue, trivialPhis); - if (thisValue.numberOfPhiUsers() != 0 && !onlyHasTrivialPhis) { + if (thisValue.hasPhiUsers() && !onlyHasTrivialPhis) { fixableThisPointer = false; break; } @@ -204,7 +204,7 @@ trivialPhis.clear(); boolean onlyHasTrivialPhis = testAndCollectPhisComposedOfSameFieldRead( visited, dest.uniquePhiUsers(), read.getField(), trivialPhis); - if (dest.numberOfPhiUsers() != 0 && !onlyHasTrivialPhis) { + if (dest.hasPhiUsers() && !onlyHasTrivialPhis) { fixableFieldReadsPerUsage = false; break; } @@ -289,6 +289,7 @@ Origin origin = appView.appInfo().originFor(method.method.holder); IRCode code = method.buildIR(appView, origin); codeOptimizations.forEach(codeOptimization -> codeOptimization.accept(code)); + CodeRewriter.insertAssumeInstructions(code, converter.assumers); converter.collectOptimizationInfo(code, feedback); CodeRewriter.removeAssumeInstructions(appView, code); converter.finalizeIR(method, code, feedback); @@ -387,7 +388,7 @@ Set<Phi> trivialPhis = Sets.newIdentityHashSet(); boolean onlyHasTrivialPhis = testAndCollectPhisComposedOfThis( Sets.newIdentityHashSet(), thisValue.uniquePhiUsers(), thisValue, trivialPhis); - assert thisValue.numberOfPhiUsers() == 0 || onlyHasTrivialPhis; + assert !thisValue.hasPhiUsers() || onlyHasTrivialPhis; assert trivialPhis.isEmpty() || onlyHasTrivialPhis; Set<Instruction> users = SetUtils.newIdentityHashSet(thisValue.aliasedUsers()); @@ -402,7 +403,7 @@ trivialPhis.forEach(Phi::removeDeadPhi); // No matter what, number of phi users should be zero too. - assert thisValue.numberOfUsers() == 0 && thisValue.numberOfPhiUsers() == 0; + assert !thisValue.hasUsers() && !thisValue.hasPhiUsers(); } // Re-processing finalized code may create slightly different IR code than what the examining @@ -478,7 +479,7 @@ Set<Phi> trivialPhis = Sets.newIdentityHashSet(); boolean onlyHasTrivialPhis = testAndCollectPhisComposedOfSameFieldRead( Sets.newIdentityHashSet(), dest.uniquePhiUsers(), field, trivialPhis); - assert dest.numberOfPhiUsers() == 0 || onlyHasTrivialPhis; + assert !dest.hasPhiUsers() || onlyHasTrivialPhis; assert trivialPhis.isEmpty() || onlyHasTrivialPhis; Set<Instruction> users = SetUtils.newIdentityHashSet(dest.aliasedUsers()); @@ -493,7 +494,7 @@ trivialPhis.forEach(Phi::removeDeadPhi); // No matter what, number of phi users should be zero too. - assert dest.numberOfUsers() == 0 && dest.numberOfPhiUsers() == 0; + assert !dest.hasUsers() && !dest.hasPhiUsers(); } private void fixupStaticizedValueUsers(IRCode code, Set<Instruction> users) { @@ -681,8 +682,12 @@ DexEncodedMethod newMethod = method.toTypeSubstitutedMethod( factory().createMethod(hostType, method.method.proto, method.method.name)); newMethods.add(newMethod); - staticizedMethods.add(newMethod); - staticizedMethods.remove(method); + // If the old method from the candidate class has been staticized, + if (staticizedMethods.remove(method)) { + // Properly update staticized methods to reprocess, i.e., add the corresponding one that + // has just been migrated to the host class. + staticizedMethods.add(newMethod); + } DexMethod originalMethod = methodMapping.inverse().get(method.method); if (originalMethod == null) { methodMapping.put(method.method, newMethod.method);
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/string/StringOptimizer.java b/src/main/java/com/android/tools/r8/ir/optimize/string/StringOptimizer.java index 2751153..41e1264 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/string/StringOptimizer.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/string/StringOptimizer.java
@@ -320,7 +320,7 @@ Value out = invoke.outValue(); // Skip the call if the computed name is already discarded or not used anywhere. - if (out == null || out.numberOfAllUsers() == 0) { + if (out == null || !out.hasAnyUsers()) { continue; }
diff --git a/src/main/java/com/android/tools/r8/optimize/VisibilityBridgeRemover.java b/src/main/java/com/android/tools/r8/optimize/VisibilityBridgeRemover.java index 8101b6b..942cb91 100644 --- a/src/main/java/com/android/tools/r8/optimize/VisibilityBridgeRemover.java +++ b/src/main/java/com/android/tools/r8/optimize/VisibilityBridgeRemover.java
@@ -85,7 +85,7 @@ if (kind == InvokeKind.SUPER) { // This is a visibility forward, so check for the direct target. DexEncodedMethod targetMethod = - appView.appInfo().resolveMethod(target.holder, target).asSingleTarget(); + appView.appInfo().resolveMethod(target.holder, target).getSingleTarget(); if (targetMethod != null && targetMethod.accessFlags.isPublic()) { if (Log.ENABLED) { Log.info(
diff --git a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java index 829c227..11cc117 100644 --- a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java +++ b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
@@ -26,6 +26,7 @@ import com.android.tools.r8.graph.GraphLense; import com.android.tools.r8.graph.PresortedComparable; import com.android.tools.r8.graph.ResolutionResult; +import com.android.tools.r8.graph.ResolutionResult.SingleResolutionResult; import com.android.tools.r8.ir.analysis.type.ClassTypeLatticeElement; import com.android.tools.r8.ir.code.Invoke.Type; import com.android.tools.r8.ir.optimize.NestUtils; @@ -354,7 +355,47 @@ } private AppInfoWithLiveness(AppInfoWithLiveness previous) { - this(previous, previous.app(), null, null); + this( + previous, + previous.liveTypes, + previous.instantiatedAnnotationTypes, + previous.instantiatedAppServices, + previous.instantiatedTypes, + previous.targetedMethods, + previous.bootstrapMethods, + previous.methodsTargetedByInvokeDynamic, + previous.virtualMethodsTargetedByInvokeDirect, + previous.liveMethods, + previous.fieldAccessInfoCollection, + previous.instanceFieldsWrittenOnlyInEnclosingInstanceInitializers, + previous.staticFieldsWrittenOnlyInEnclosingStaticInitializer, + previous.virtualInvokes, + previous.interfaceInvokes, + previous.superInvokes, + previous.directInvokes, + previous.staticInvokes, + previous.callSites, + previous.brokenSuperInvokes, + previous.pinnedItems, + previous.mayHaveSideEffects, + previous.noSideEffects, + previous.assumedValues, + previous.alwaysInline, + previous.forceInline, + previous.neverInline, + previous.whyAreYouNotInlining, + previous.keepConstantArguments, + previous.keepUnusedArguments, + previous.neverClassInline, + previous.neverMerge, + previous.neverPropagateValue, + previous.identifierNameStrings, + previous.prunedTypes, + previous.switchMaps, + previous.enumValueInfoMaps, + previous.instantiatedLambdas, + previous.constClassReferences); + copyMetadataFromPrevious(previous); } private AppInfoWithLiveness( @@ -904,7 +945,7 @@ private DexEncodedMethod validateSingleVirtualTarget( DexEncodedMethod singleTarget, DexEncodedMethod resolutionResult) { - assert resolutionResult.isValidVirtualTarget(options()); + assert SingleResolutionResult.isValidVirtualTarget(options(), resolutionResult); if (singleTarget == null || singleTarget == DexEncodedMethod.SENTINEL) { return null; @@ -921,7 +962,7 @@ private boolean isInvalidSingleVirtualTarget( DexEncodedMethod singleTarget, DexEncodedMethod resolutionResult) { - assert resolutionResult.isValidVirtualTarget(options()); + assert SingleResolutionResult.isValidVirtualTarget(options(), resolutionResult); // Art978_virtual_interfaceTest correctly expects an IncompatibleClassChangeError exception // at runtime. return !singleTarget.accessFlags.isAtLeastAsVisibleAs(resolutionResult.accessFlags); @@ -956,7 +997,7 @@ if (refinedResolutionResult.hasSingleTarget() && refinedResolutionResult.isValidVirtualTargetForDynamicDispatch()) { return validateSingleVirtualTarget( - refinedResolutionResult.asSingleTarget(), resolutionResult.asSingleTarget()); + refinedResolutionResult.getSingleTarget(), resolutionResult.getSingleTarget()); } } return null; @@ -998,7 +1039,7 @@ // Now, resolve the target with the refined receiver type. ResolutionResult refinedResolutionResult = refinedReceiverIsStrictSubType ? resolveMethodOnClass(refinedHolder, method) : topMethod; - DexEncodedMethod topSingleTarget = refinedResolutionResult.asSingleTarget(); + DexEncodedMethod topSingleTarget = refinedResolutionResult.getSingleTarget(); DexClass topHolder = definitionFor(topSingleTarget.method.holder); // We need to know whether the top method is from an interface, as that would allow it to be // shadowed by a default method from an interface further down. @@ -1012,7 +1053,7 @@ topSingleTarget, !refinedHolder.accessFlags.isAbstract(), topIsFromInterface), - topMethod.asSingleTarget()); + topMethod.getSingleTarget()); assert result != DexEncodedMethod.SENTINEL; method.setSingleVirtualMethodCache(refinedReceiverType, result); return result; @@ -1154,7 +1195,7 @@ if (refinedResolutionResult.hasSingleTarget() && refinedResolutionResult.isValidVirtualTargetForDynamicDispatch()) { return validateSingleVirtualTarget( - refinedResolutionResult.asSingleTarget(), resolutionResult.asSingleTarget()); + refinedResolutionResult.getSingleTarget(), resolutionResult.getSingleTarget()); } } return null; @@ -1170,8 +1211,8 @@ } // First check that there is a target for this invoke-interface to hit. If there is none, // this will fail at runtime. - DexEncodedMethod topTarget = resolveMethodOnInterface(holder, method).asSingleTarget(); - if (topTarget == null || !topTarget.isValidVirtualTarget(options())) { + DexEncodedMethod topTarget = resolveMethodOnInterface(holder, method).getSingleTarget(); + if (topTarget == null || !SingleResolutionResult.isValidVirtualTarget(options(), topTarget)) { return null; } // For kept types we cannot ensure a single target. @@ -1200,7 +1241,7 @@ // override them, so we ignore interface methods here. Otherwise, we would look up // default methods that are factually never used. } else if (!clazz.accessFlags.isAbstract()) { - DexEncodedMethod resolutionResult = resolveMethodOnClass(clazz, method).asSingleTarget(); + DexEncodedMethod resolutionResult = resolveMethodOnClass(clazz, method).getSingleTarget(); if (resolutionResult == null || isInvalidSingleVirtualTarget(resolutionResult, topTarget)) { // This will fail at runtime. return null;
diff --git a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java index 2306b28..183151c 100644 --- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java +++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -39,6 +39,7 @@ import com.android.tools.r8.graph.KeyedDexItem; import com.android.tools.r8.graph.PresortedComparable; import com.android.tools.r8.graph.ResolutionResult; +import com.android.tools.r8.graph.ResolutionResult.SingleResolutionResult; import com.android.tools.r8.graph.analysis.EnqueuerAnalysis; import com.android.tools.r8.ir.analysis.proto.schema.ProtoEnqueuerExtension; import com.android.tools.r8.ir.code.ArrayPut; @@ -1305,7 +1306,7 @@ reportMissingMethod(method); return; } - DexEncodedMethod encodedMethod = resolutionResult.asSingleTarget(); + DexEncodedMethod encodedMethod = resolutionResult.getSingleTarget(); if (encodedMethod == null) { // Note: should this be reported too? Or is this unreachable? return; @@ -1927,7 +1928,8 @@ // Otherwise, the resolution target is marked and cached, and all possible targets identified. resolution = findAndMarkResolutionTarget(method, interfaceInvoke, reason); virtualTargetsMarkedAsReachable.put(method, resolution); - if (resolution.isUnresolved() || !resolution.method.isValidVirtualTarget(options)) { + if (resolution.isUnresolved() + || !SingleResolutionResult.isValidVirtualTarget(options, resolution.method)) { // There is no valid resolution, so any call will lead to a runtime exception. return; } @@ -1940,7 +1942,9 @@ assert interfaceInvoke == holder.isInterface(); Set<DexEncodedMethod> possibleTargets = - resolution.method.lookupVirtualDispatchTargets(interfaceInvoke, appInfo); + // TODO(b/140214802): Call on the resolution once proper resolution and lookup is resolved. + new SingleResolutionResult(resolution.method) + .lookupVirtualDispatchTargets(interfaceInvoke, appInfo); if (possibleTargets == null || possibleTargets.isEmpty()) { return; } @@ -2036,56 +2040,11 @@ // invoke. This also ensures preserving the errors detailed below. if (resolutionTargetClass.isProgramClass()) { markMethodAsTargeted(resolutionTargetClass.asProgramClass(), resolutionTarget, reason); - - // If the method of an invoke-virtual instruction resolves to a private or static method, then - // the invoke fails with an IllegalAccessError or IncompatibleClassChangeError, respectively. - // - // Unfortunately the above is not always the case: - // Some Art VMs do not fail with an IllegalAccessError or IncompatibleClassChangeError if the - // method of an invoke-virtual instruction resolves to a private or static method, but instead - // ignores private and static methods during resolution (see also NonVirtualOverrideTest). - // Therefore, we need to continue resolution from the super type until we find a virtual - // method. - if (resolutionTarget.isPrivateMethod() || resolutionTarget.isStatic()) { - assert !interfaceInvoke || resolutionTargetClass.isInterface(); - MarkedResolutionTarget possiblyValidTarget = - markPossiblyValidTarget( - method, reason, resolutionTarget, resolutionTargetClass.asProgramClass()); - if (!possiblyValidTarget.isUnresolved()) { - // Since some Art runtimes may actually end up targeting this method, it is returned as - // the basis of lookup for the enqueuing of virtual dispatches. Not doing so may cause it - // to be marked abstract, thus leading to an AbstractMethodError on said Art runtimes. - return possiblyValidTarget; - } - } } return new MarkedResolutionTarget(resolutionTargetClass, resolutionTarget); } - private MarkedResolutionTarget markPossiblyValidTarget( - DexMethod method, - KeepReason reason, - DexEncodedMethod resolutionTarget, - DexProgramClass resolutionTargetClass) { - while (resolutionTarget.isPrivateMethod() || resolutionTarget.isStatic()) { - resolutionTarget = - appInfo - .resolveMethod( - resolutionTargetClass.superType, method, resolutionTargetClass.isInterface()) - .asResultOfResolve(); - if (resolutionTarget == null) { - return MarkedResolutionTarget.unresolved(); - } - resolutionTargetClass = getProgramClassOrNull(resolutionTarget.method.holder); - if (resolutionTargetClass == null) { - return MarkedResolutionTarget.unresolved(); - } - } - markMethodAsTargeted(resolutionTargetClass, resolutionTarget, reason); - return new MarkedResolutionTarget(resolutionTargetClass, resolutionTarget); - } - private DexMethod generatedEnumValuesMethod(DexClass enumClass) { DexType arrayOfEnumClass = appView @@ -2644,11 +2603,9 @@ if (keepClass) { markInstantiated(clazz, null, KeepReason.reflectiveUseIn(method)); } - markFieldAsKept(clazz, encodedField, KeepReason.reflectiveUseIn(method)); - // Fields accessed by reflection is marked as both read and written. - registerFieldRead(encodedField.field, method); - registerFieldWrite(encodedField.field, method); - + if (pinnedItems.add(encodedField.field)) { + markFieldAsKept(clazz, encodedField, KeepReason.reflectiveUseIn(method)); + } } else { assert identifierItem.isDexMethod(); DexMethod targetedMethod = identifierItem.asDexMethod();
diff --git a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java index 85da345..a9ec234 100644 --- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java +++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -1283,7 +1283,7 @@ abortMerge = true; return null; } - DexEncodedMethod actual = resolutionResult.asSingleTarget(); + DexEncodedMethod actual = resolutionResult.getSingleTarget(); if (actual != method) { assert actual.isVirtualMethod() == method.isVirtualMethod(); return actual;
diff --git a/src/main/java/com/android/tools/r8/utils/AndroidApp.java b/src/main/java/com/android/tools/r8/utils/AndroidApp.java index 1c3af7a..14f705e 100644 --- a/src/main/java/com/android/tools/r8/utils/AndroidApp.java +++ b/src/main/java/com/android/tools/r8/utils/AndroidApp.java
@@ -7,6 +7,8 @@ import static com.android.tools.r8.utils.FileUtils.isArchive; import static com.android.tools.r8.utils.FileUtils.isClassFile; import static com.android.tools.r8.utils.FileUtils.isDexFile; +import static com.android.tools.r8.utils.InternalOptions.ASM_VERSION; +import static com.android.tools.r8.utils.ZipUtils.writeToZipStream; import com.android.tools.r8.ClassFileConsumer; import com.android.tools.r8.ClassFileResourceProvider; @@ -25,28 +27,39 @@ import com.android.tools.r8.Resource; import com.android.tools.r8.ResourceException; import com.android.tools.r8.StringResource; +import com.android.tools.r8.Version; import com.android.tools.r8.errors.CompilationError; import com.android.tools.r8.errors.InternalCompilerError; import com.android.tools.r8.errors.Unreachable; import com.android.tools.r8.origin.Origin; import com.android.tools.r8.origin.PathOrigin; import com.android.tools.r8.shaking.FilteredClassPath; +import com.android.tools.r8.shaking.ProguardConfiguration; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.io.ByteStreams; +import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.InputStream; import java.nio.file.Files; import java.nio.file.NoSuchFileException; +import java.nio.file.OpenOption; import java.nio.file.Path; +import java.nio.file.StandardOpenOption; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.Comparator; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; import java.util.TreeSet; +import java.util.zip.ZipEntry; +import java.util.zip.ZipOutputStream; +import org.objectweb.asm.ClassVisitor; /** * Collection of program files needed for processing. @@ -382,6 +395,154 @@ return programResourcesMainDescriptor.get(resource); } + public AndroidApp dump(Path output, ProguardConfiguration configuration, Reporter reporter) { + int nextDexIndex = 0; + Builder builder = AndroidApp.builder(reporter); + OpenOption[] openOptions = + new OpenOption[] {StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING}; + try (ZipOutputStream out = new ZipOutputStream(Files.newOutputStream(output, openOptions))) { + writeToZipStream(out, "r8-version", Version.getVersionString().getBytes(), ZipEntry.DEFLATED); + if (configuration != null) { + String proguardConfig = configuration.getParsedConfiguration(); + writeToZipStream(out, "proguard.config", proguardConfig.getBytes(), ZipEntry.DEFLATED); + } + nextDexIndex = dumpProgramResources("program.jar", nextDexIndex, out, builder); + nextDexIndex = + dumpClassFileResources( + "classpath.jar", nextDexIndex, out, builder, classpathResourceProviders); + nextDexIndex = + dumpClassFileResources( + "library.jar", nextDexIndex, out, builder, libraryResourceProviders); + } catch (IOException | ResourceException e) { + reporter.fatalError(new StringDiagnostic("Failed to dump inputs"), e); + } + return builder.build(); + } + + private int dumpProgramResources( + String archiveName, int nextDexIndex, ZipOutputStream out, Builder builder) + throws IOException, ResourceException { + try (ByteArrayOutputStream archiveByteStream = new ByteArrayOutputStream()) { + try (ZipOutputStream archiveOutputStream = new ZipOutputStream(archiveByteStream)) { + Set<String> seen = new HashSet<>(); + Set<DataEntryResource> dataEntries = getDataEntryResourcesForTesting(); + for (DataEntryResource dataResource : dataEntries) { + builder.addDataResources(dataResource); + String entryName = dataResource.getName(); + try (InputStream dataStream = dataResource.getByteStream()) { + byte[] bytes = ByteStreams.toByteArray(dataStream); + writeToZipStream(out, entryName, bytes, ZipEntry.DEFLATED); + } + } + for (ProgramResourceProvider provider : programResourceProviders) { + for (ProgramResource programResource : provider.getProgramResources()) { + nextDexIndex = + dumpProgramResource( + builder, seen, nextDexIndex, archiveOutputStream, programResource); + } + } + } + writeToZipStream(out, archiveName, archiveByteStream.toByteArray(), ZipEntry.DEFLATED); + } + return nextDexIndex; + } + + private static int dumpClassFileResources( + String archiveName, + int nextDexIndex, + ZipOutputStream out, + Builder builder, + ImmutableList<ClassFileResourceProvider> classpathResourceProviders) + throws IOException, ResourceException { + try (ByteArrayOutputStream archiveByteStream = new ByteArrayOutputStream()) { + try (ZipOutputStream archiveOutputStream = new ZipOutputStream(archiveByteStream)) { + Set<String> seen = new HashSet<>(); + for (ClassFileResourceProvider provider : classpathResourceProviders) { + for (String descriptor : provider.getClassDescriptors()) { + ProgramResource programResource = provider.getProgramResource(descriptor); + int oldDexIndex = nextDexIndex; + nextDexIndex = + dumpProgramResource( + builder, seen, nextDexIndex, archiveOutputStream, programResource); + assert nextDexIndex == oldDexIndex; + } + } + } + writeToZipStream(out, archiveName, archiveByteStream.toByteArray(), ZipEntry.DEFLATED); + } + return nextDexIndex; + } + + private static int dumpProgramResource( + Builder builder, + Set<String> seen, + int nextDexIndex, + ZipOutputStream archiveOutputStream, + ProgramResource programResource) + throws ResourceException, IOException { + byte[] bytes = ByteStreams.toByteArray(programResource.getByteStream()); + Set<String> classDescriptors = programResource.getClassDescriptors(); + if (classDescriptors.size() != 1 && programResource.getKind() == Kind.CF) { + classDescriptors = Collections.singleton(extractClassDescriptor(bytes)); + } + if (programResource instanceof OneShotByteResource) { + builder.addProgramResources( + OneShotByteResource.create( + programResource.getKind(), programResource.getOrigin(), bytes, classDescriptors)); + + } else { + builder.addProgramResources(programResource); + } + String entryName; + if (programResource.getKind() == Kind.CF) { + String classDescriptor = + classDescriptors.size() == 1 + ? classDescriptors.iterator().next() + : extractClassDescriptor(bytes); + String classFileName = DescriptorUtils.getClassFileName(classDescriptor); + entryName = seen.add(classDescriptor) ? classFileName : (classFileName + ".dup"); + } else { + assert programResource.getKind() == Kind.DEX; + entryName = "classes" + nextDexIndex++ + ".dex"; + } + writeToZipStream(archiveOutputStream, entryName, bytes, ZipEntry.DEFLATED); + return nextDexIndex; + } + + private static String extractClassDescriptor(byte[] bytes) { + class ClassNameExtractor extends ClassVisitor { + private String className; + + private ClassNameExtractor() { + super(ASM_VERSION); + } + + @Override + public void visit( + int version, + int access, + String name, + String signature, + String superName, + String[] interfaces) { + className = name; + } + + String getDescriptor() { + return "L" + className + ";"; + } + } + + org.objectweb.asm.ClassReader reader = new org.objectweb.asm.ClassReader(bytes); + ClassNameExtractor extractor = new ClassNameExtractor(); + reader.accept( + extractor, + org.objectweb.asm.ClassReader.SKIP_CODE + | org.objectweb.asm.ClassReader.SKIP_DEBUG + | org.objectweb.asm.ClassReader.SKIP_FRAMES); + return extractor.getDescriptor(); + } + /** * Builder interface for constructing an AndroidApp. */
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java index 12c444f..828e8d2 100644 --- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java +++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -68,6 +68,7 @@ // Set to true to run compilation in a single thread and without randomly shuffling the input. // This makes life easier when running R8 in a debugger. public static final boolean DETERMINISTIC_DEBUGGING = false; + public enum LineNumberOptimization { OFF, ON @@ -287,7 +288,8 @@ Marker marker = new Marker(tool) .setVersion(Version.LABEL) - .setCompilationMode(debug ? CompilationMode.DEBUG : CompilationMode.RELEASE); + .setCompilationMode(debug ? CompilationMode.DEBUG : CompilationMode.RELEASE) + .setHasChecksums(encodeChecksums); if (!isGeneratingClassFiles()) { marker.setMinApi(minApiLevel); }
diff --git a/src/main/java/com/android/tools/r8/utils/OneShotByteResource.java b/src/main/java/com/android/tools/r8/utils/OneShotByteResource.java index da958c3..1dfbc64 100644 --- a/src/main/java/com/android/tools/r8/utils/OneShotByteResource.java +++ b/src/main/java/com/android/tools/r8/utils/OneShotByteResource.java
@@ -17,7 +17,7 @@ private byte[] bytes; private final Set<String> classDescriptors; - static ProgramResource create( + public static OneShotByteResource create( Kind kind, Origin origin, byte[] bytes, Set<String> classDescriptors) { return new OneShotByteResource(origin, kind, bytes, classDescriptors); }
diff --git a/src/test/java/com/android/tools/r8/D8TestCompileResult.java b/src/test/java/com/android/tools/r8/D8TestCompileResult.java index e4ea07c..117ca7b 100644 --- a/src/test/java/com/android/tools/r8/D8TestCompileResult.java +++ b/src/test/java/com/android/tools/r8/D8TestCompileResult.java
@@ -24,7 +24,7 @@ } @Override - public D8TestRunResult createRunResult(ProcessResult result) { - return new D8TestRunResult(app, result); + public D8TestRunResult createRunResult(TestRuntime runtime, ProcessResult result) { + return new D8TestRunResult(app, runtime, result); } }
diff --git a/src/test/java/com/android/tools/r8/D8TestRunResult.java b/src/test/java/com/android/tools/r8/D8TestRunResult.java index 1aa095a..cd4b663 100644 --- a/src/test/java/com/android/tools/r8/D8TestRunResult.java +++ b/src/test/java/com/android/tools/r8/D8TestRunResult.java
@@ -9,8 +9,8 @@ public class D8TestRunResult extends TestRunResult<D8TestRunResult> { - public D8TestRunResult(AndroidApp app, ProcessResult result) { - super(app, result); + public D8TestRunResult(AndroidApp app, TestRuntime runtime, ProcessResult result) { + super(app, runtime, result); } @Override
diff --git a/src/test/java/com/android/tools/r8/DXTestCompileResult.java b/src/test/java/com/android/tools/r8/DXTestCompileResult.java index 4313211..3cf9b73 100644 --- a/src/test/java/com/android/tools/r8/DXTestCompileResult.java +++ b/src/test/java/com/android/tools/r8/DXTestCompileResult.java
@@ -23,7 +23,7 @@ } @Override - public DXTestRunResult createRunResult(ProcessResult result) { - return new DXTestRunResult(app, result); + public DXTestRunResult createRunResult(TestRuntime runtime, ProcessResult result) { + return new DXTestRunResult(app, runtime, result); } }
diff --git a/src/test/java/com/android/tools/r8/DXTestRunResult.java b/src/test/java/com/android/tools/r8/DXTestRunResult.java index 830cbd8..bcec2f3 100644 --- a/src/test/java/com/android/tools/r8/DXTestRunResult.java +++ b/src/test/java/com/android/tools/r8/DXTestRunResult.java
@@ -9,8 +9,8 @@ public class DXTestRunResult extends TestRunResult<DXTestRunResult> { - public DXTestRunResult(AndroidApp app, ProcessResult result) { - super(app, result); + public DXTestRunResult(AndroidApp app, TestRuntime runtime, ProcessResult result) { + super(app, runtime, result); } @Override
diff --git a/src/test/java/com/android/tools/r8/Dex2OatTestRunResult.java b/src/test/java/com/android/tools/r8/Dex2OatTestRunResult.java index 1a4e0ec..f364232 100644 --- a/src/test/java/com/android/tools/r8/Dex2OatTestRunResult.java +++ b/src/test/java/com/android/tools/r8/Dex2OatTestRunResult.java
@@ -13,8 +13,8 @@ public class Dex2OatTestRunResult extends TestRunResult<Dex2OatTestRunResult> { - public Dex2OatTestRunResult(AndroidApp app, ProcessResult result) { - super(app, result); + public Dex2OatTestRunResult(AndroidApp app, TestRuntime runtime, ProcessResult result) { + super(app, runtime, result); } @Override
diff --git a/src/test/java/com/android/tools/r8/DumpInputsTest.java b/src/test/java/com/android/tools/r8/DumpInputsTest.java new file mode 100644 index 0000000..3c2d9b3 --- /dev/null +++ b/src/test/java/com/android/tools/r8/DumpInputsTest.java
@@ -0,0 +1,72 @@ +// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. +package com.android.tools.r8; + +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import com.android.tools.r8.TestRuntime.CfVm; +import com.android.tools.r8.utils.DescriptorUtils; +import com.android.tools.r8.utils.StringUtils; +import com.android.tools.r8.utils.ZipUtils; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.HashSet; +import java.util.Set; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +@RunWith(Parameterized.class) +public class DumpInputsTest extends TestBase { + + static final String EXPECTED = StringUtils.lines("Hello, world"); + + private final TestParameters parameters; + + @Parameterized.Parameters(name = "{0}") + public static TestParametersCollection data() { + return getTestParameters().withCfRuntime(CfVm.JDK9).build(); + } + + public DumpInputsTest(TestParameters parameters) { + this.parameters = parameters; + } + + @Test + public void test() throws Exception { + Path dump = temp.newFolder().toPath().resolve("dump.zip"); + try { + testForExternalR8(parameters.getBackend()) + .useExternalJDK(parameters.getRuntime().asCf().getVm()) + .addJvmFlag("-Dcom.android.tools.r8.dumpinputtofile=" + dump) + .addProgramClasses(TestClass.class) + .compile(); + } catch (AssertionError e) { + assertTrue(Files.exists(dump)); + Path unzipped = temp.newFolder().toPath(); + ZipUtils.unzip(dump.toString(), unzipped.toFile()); + assertTrue(Files.exists(unzipped.resolve("program.jar"))); + assertTrue(Files.exists(unzipped.resolve("classpath.jar"))); + assertTrue(Files.exists(unzipped.resolve("proguard.config"))); + assertTrue(Files.exists(unzipped.resolve("r8-version"))); + Set<String> entries = new HashSet<>(); + ZipUtils.iter( + unzipped.resolve("program.jar").toString(), + (entry, input) -> entries.add(entry.getName())); + assertTrue( + entries.contains( + DescriptorUtils.getClassFileName( + DescriptorUtils.javaTypeToDescriptor(TestClass.class.getTypeName())))); + return; + } + fail("Expected external compilation to exit"); + } + + static class TestClass { + public static void main(String[] args) { + System.out.println("Hello, world"); + } + } +}
diff --git a/src/test/java/com/android/tools/r8/ExternalR8TestBuilder.java b/src/test/java/com/android/tools/r8/ExternalR8TestBuilder.java index 0328184..ae4a36c 100644 --- a/src/test/java/com/android/tools/r8/ExternalR8TestBuilder.java +++ b/src/test/java/com/android/tools/r8/ExternalR8TestBuilder.java
@@ -57,6 +57,8 @@ private boolean addR8ExternalDeps = false; + private List<String> jvmFlags = new ArrayList<>(); + private ExternalR8TestBuilder(TestState state, Builder builder, Backend backend) { super(state, builder, backend); } @@ -72,7 +74,7 @@ public ExternalR8TestBuilder useExternalJDK(CfVm externalJDK) { this.externalJDK = externalJDK; - return this; + return self(); } private String getJDKToRun() { @@ -82,6 +84,11 @@ return getJavaExecutable(externalJDK); } + public ExternalR8TestBuilder addJvmFlag(String flag) { + jvmFlags.add(flag); + return self(); + } + @Override ExternalR8TestCompileResult internalCompile( Builder builder, Consumer<InternalOptions> optionsConsumer, Supplier<AndroidApp> app) @@ -99,9 +106,12 @@ : r8jar.toAbsolutePath().toString(); List<String> command = new ArrayList<>(); + Collections.addAll(command, getJDKToRun()); + + command.addAll(jvmFlags); + Collections.addAll( command, - getJDKToRun(), "-ea", "-cp", classPath,
diff --git a/src/test/java/com/android/tools/r8/ExternalR8TestCompileResult.java b/src/test/java/com/android/tools/r8/ExternalR8TestCompileResult.java index 290f6ac..04a37fa 100644 --- a/src/test/java/com/android/tools/r8/ExternalR8TestCompileResult.java +++ b/src/test/java/com/android/tools/r8/ExternalR8TestCompileResult.java
@@ -63,7 +63,7 @@ } @Override - protected ExternalR8TestRunResult createRunResult(ProcessResult result) { - return new ExternalR8TestRunResult(app, outputJar, proguardMap, result); + protected ExternalR8TestRunResult createRunResult(TestRuntime runtime, ProcessResult result) { + return new ExternalR8TestRunResult(app, outputJar, proguardMap, runtime, result); } }
diff --git a/src/test/java/com/android/tools/r8/ExternalR8TestRunResult.java b/src/test/java/com/android/tools/r8/ExternalR8TestRunResult.java index f301b78..28e2835 100644 --- a/src/test/java/com/android/tools/r8/ExternalR8TestRunResult.java +++ b/src/test/java/com/android/tools/r8/ExternalR8TestRunResult.java
@@ -19,8 +19,12 @@ private final String proguardMap; public ExternalR8TestRunResult( - AndroidApp app, Path outputJar, String proguardMap, ProcessResult result) { - super(app, result); + AndroidApp app, + Path outputJar, + String proguardMap, + TestRuntime runtime, + ProcessResult result) { + super(app, runtime, result); this.outputJar = outputJar; this.proguardMap = proguardMap; }
diff --git a/src/test/java/com/android/tools/r8/ExtractMarkerTest.java b/src/test/java/com/android/tools/r8/ExtractMarkerTest.java index 948ab96..008ff1f 100644 --- a/src/test/java/com/android/tools/r8/ExtractMarkerTest.java +++ b/src/test/java/com/android/tools/r8/ExtractMarkerTest.java
@@ -4,32 +4,57 @@ package com.android.tools.r8; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import com.android.tools.r8.dex.Marker; import com.android.tools.r8.dex.Marker.Tool; +import com.android.tools.r8.utils.BooleanUtils; import java.nio.file.Paths; import java.util.Collection; import java.util.Set; +import org.junit.Assume; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; -public class ExtractMarkerTest { +@RunWith(Parameterized.class) +public class ExtractMarkerTest extends TestBase { private static final String CLASS_FILE = ToolHelper.EXAMPLES_BUILD_DIR + "classes/trivial/Trivial.class"; - private static void verifyMarker(Marker marker, Tool tool) { + private final TestParameters parameters; + private boolean includeClassesChecksum; + + @Parameterized.Parameters(name = "{0}, includeClassesChecksum: {1}") + public static Collection<Object[]> data() { + return buildParameters( + getTestParameters().withAllRuntimes().withAllApiLevels().build(), BooleanUtils.values()); + } + + public ExtractMarkerTest(TestParameters parameters, boolean includeClassesChecksum) { + this.parameters = parameters; + this.includeClassesChecksum = includeClassesChecksum; + } + + private void verifyMarkerDex(Marker marker, Tool tool) { assertEquals(tool, marker.getTool()); assertEquals(Version.LABEL, marker.getVersion()); assertEquals(CompilationMode.DEBUG.toString().toLowerCase(), marker.getCompilationMode()); + assertEquals(parameters.getApiLevel().getLevel(), marker.getMinApi().intValue()); + assertEquals(includeClassesChecksum, marker.getHasChecksums()); } @Test public void extractMarkerTestDex() throws CompilationFailedException { - boolean[] testExecuted = {false}; + Assume.assumeTrue(parameters.getRuntime().isDex()); + boolean[] testExecuted = {false}; D8.run( D8Command.builder() .addProgramFiles(Paths.get(CLASS_FILE)) + .setMinApiLevel(parameters.getApiLevel().getLevel()) + .setIncludeClassesChecksum(includeClassesChecksum) .setProgramConsumer( new DexIndexedConsumer.ForwardingConsumer(null) { @Override @@ -47,7 +72,7 @@ } catch (Exception e) { throw new RuntimeException(e); } - verifyMarker(marker, Tool.D8); + verifyMarkerDex(marker, Tool.D8); testExecuted[0] = true; } }) @@ -55,8 +80,18 @@ assertTrue(testExecuted[0]); } + private static void verifyMarkerCf(Marker marker, Tool tool) { + assertEquals(tool, marker.getTool()); + assertEquals(Version.LABEL, marker.getVersion()); + assertEquals(CompilationMode.DEBUG.toString().toLowerCase(), marker.getCompilationMode()); + assertFalse(marker.getHasChecksums()); + } + @Test public void extractMarkerTestCf() throws CompilationFailedException { + Assume.assumeTrue(parameters.getRuntime().isCf()); + Assume.assumeFalse(includeClassesChecksum); + boolean[] testExecuted = {false}; R8.run( R8Command.builder() @@ -78,7 +113,7 @@ } catch (Exception e) { throw new RuntimeException(e); } - verifyMarker(marker, Tool.R8); + verifyMarkerCf(marker, Tool.R8); testExecuted[0] = true; } })
diff --git a/src/test/java/com/android/tools/r8/GenerateMainDexListRunResult.java b/src/test/java/com/android/tools/r8/GenerateMainDexListRunResult.java index b9842f2..166feeb 100644 --- a/src/test/java/com/android/tools/r8/GenerateMainDexListRunResult.java +++ b/src/test/java/com/android/tools/r8/GenerateMainDexListRunResult.java
@@ -11,7 +11,7 @@ List<String> mainDexList; public GenerateMainDexListRunResult(List<String> mainDexList) { - super(null, null); + super(null, null, null); this.mainDexList = mainDexList; }
diff --git a/src/test/java/com/android/tools/r8/JvmTestBuilder.java b/src/test/java/com/android/tools/r8/JvmTestBuilder.java index c6a5026..06f97ce 100644 --- a/src/test/java/com/android/tools/r8/JvmTestBuilder.java +++ b/src/test/java/com/android/tools/r8/JvmTestBuilder.java
@@ -39,9 +39,9 @@ } @Override + @Deprecated public JvmTestRunResult run(String mainClass) throws IOException { - ProcessResult result = ToolHelper.runJava(classpath, mainClass); - return new JvmTestRunResult(builder.build(), result); + return run(TestRuntime.getDefaultJavaRuntime(), mainClass); } public JvmTestRunResult run(TestRuntime runtime, String mainClass, String... args) @@ -49,7 +49,7 @@ assert runtime.isCf(); ProcessResult result = ToolHelper.runJava(runtime.asCf().getVm(), classpath, ObjectArrays.concat(mainClass, args)); - return new JvmTestRunResult(builder.build(), result); + return new JvmTestRunResult(builder.build(), runtime, result); } @Override
diff --git a/src/test/java/com/android/tools/r8/JvmTestRunResult.java b/src/test/java/com/android/tools/r8/JvmTestRunResult.java index b7a71fd..a94fd38 100644 --- a/src/test/java/com/android/tools/r8/JvmTestRunResult.java +++ b/src/test/java/com/android/tools/r8/JvmTestRunResult.java
@@ -9,8 +9,8 @@ public class JvmTestRunResult extends TestRunResult<JvmTestRunResult> { - public JvmTestRunResult(AndroidApp app, ProcessResult result) { - super(app, result); + public JvmTestRunResult(AndroidApp app, TestRuntime runtime, ProcessResult result) { + super(app, runtime, result); } @Override
diff --git a/src/test/java/com/android/tools/r8/ProguardTestCompileResult.java b/src/test/java/com/android/tools/r8/ProguardTestCompileResult.java index 0ed5425..fbb8282 100644 --- a/src/test/java/com/android/tools/r8/ProguardTestCompileResult.java +++ b/src/test/java/com/android/tools/r8/ProguardTestCompileResult.java
@@ -42,7 +42,7 @@ } @Override - public ProguardTestRunResult createRunResult(ProcessResult result) { - return new ProguardTestRunResult(app, result, proguardMap); + public ProguardTestRunResult createRunResult(TestRuntime runtime, ProcessResult result) { + return new ProguardTestRunResult(app, runtime, result, proguardMap); } }
diff --git a/src/test/java/com/android/tools/r8/ProguardTestRunResult.java b/src/test/java/com/android/tools/r8/ProguardTestRunResult.java index e242a38..5cfce33 100644 --- a/src/test/java/com/android/tools/r8/ProguardTestRunResult.java +++ b/src/test/java/com/android/tools/r8/ProguardTestRunResult.java
@@ -7,7 +7,9 @@ import static org.junit.Assert.assertNotNull; import com.android.tools.r8.ToolHelper.ProcessResult; +import com.android.tools.r8.naming.retrace.StackTrace; import com.android.tools.r8.utils.AndroidApp; +import com.android.tools.r8.utils.ThrowingConsumer; import com.android.tools.r8.utils.codeinspector.CodeInspector; import java.io.IOException; import java.util.concurrent.ExecutionException; @@ -16,8 +18,9 @@ private final String proguardMap; - public ProguardTestRunResult(AndroidApp app, ProcessResult result, String proguardMap) { - super(app, result); + public ProguardTestRunResult( + AndroidApp app, TestRuntime runtime, ProcessResult result, String proguardMap) { + super(app, runtime, result); this.proguardMap = proguardMap; } @@ -27,10 +30,25 @@ } @Override + public StackTrace getStackTrace() { + return super.getStackTrace().retrace(proguardMap); + } + + public StackTrace getOriginalStackTrace() { + return super.getStackTrace(); + } + + @Override public CodeInspector inspector() throws IOException, ExecutionException { // See comment in base class. assertSuccess(); assertNotNull(app); return new CodeInspector(app, proguardMap); } + + public <E extends Throwable> ProguardTestRunResult inspectOriginalStackTrace( + ThrowingConsumer<StackTrace, E> consumer) throws E { + consumer.accept(getOriginalStackTrace()); + return self(); + } }
diff --git a/src/test/java/com/android/tools/r8/R8EntryPointTests.java b/src/test/java/com/android/tools/r8/R8EntryPointTests.java index 553ff0e..0bbc7bc 100644 --- a/src/test/java/com/android/tools/r8/R8EntryPointTests.java +++ b/src/test/java/com/android/tools/r8/R8EntryPointTests.java
@@ -9,20 +9,13 @@ import com.android.tools.r8.ToolHelper.ProcessResult; import com.android.tools.r8.utils.FileUtils; -import com.android.tools.r8.utils.ZipUtils; -import com.android.tools.r8.utils.ZipUtils.OnEntryHandler; import com.google.common.collect.ImmutableList; import java.io.IOException; -import java.io.InputStream; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.ArrayList; -import java.util.Collection; -import java.util.List; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; -import java.util.zip.ZipEntry; import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -180,53 +173,6 @@ } @Test - public void testDumpInputs() throws IOException { - Path out = temp.newFile("dex.zip").toPath(); - Path dump = temp.newFile("dump.zip").toPath(); - ProcessResult r8 = - ToolHelper.forkR8WithJavaOptions( - Paths.get("."), - ImmutableList.of("-Dcom.android.tools.r8.dumpinputtofile=" + dump.toString()), - "--lib", - ToolHelper.getDefaultAndroidJar().toString(), - "--dex", - "--output", - out.toString(), - "--pg-conf", - PROGUARD_FLAGS.toString(), - "--pg-conf", - testFlags.toString(), - INPUT_JAR.toString()); - - List<ZipEntry> entries = new ArrayList<>(); - ZipUtils.iter( - dump.toString(), - new OnEntryHandler() { - @Override - public void onEntry(ZipEntry entry, InputStream input) throws IOException { - entries.add(entry); - } - }); - Assert.assertTrue(hasEntry(entries, "program.jar")); - Assert.assertTrue(hasEntry(entries, "library.jar")); - Assert.assertTrue(hasEntry(entries, "classpath.jar")); - Assert.assertTrue(hasEntry(entries, "proguard.config")); - Assert.assertTrue(hasEntry(entries, "r8-version")); - // When dumping the inputs we throw an error in the program to exit early. - Assert.assertNotEquals(0, r8.exitCode); - } - - private boolean hasEntry(Collection<ZipEntry> entries, String name) { - for (ZipEntry entry : entries) { - if (entry.getName().equals(name)) { - return true; - } - } - - return false; - } - - @Test public void testSpecifyDexAndClassfileNotAllowed() throws IOException, InterruptedException { Path out = temp.newFile("dex.zip").toPath(); ProcessResult r8 =
diff --git a/src/test/java/com/android/tools/r8/R8TestCompileResult.java b/src/test/java/com/android/tools/r8/R8TestCompileResult.java index 8e63f34..c4ec2aa 100644 --- a/src/test/java/com/android/tools/r8/R8TestCompileResult.java +++ b/src/test/java/com/android/tools/r8/R8TestCompileResult.java
@@ -83,8 +83,8 @@ } @Override - public R8TestRunResult createRunResult(ProcessResult result) { - return new R8TestRunResult(app, result, proguardMap, this::graphInspector); + public R8TestRunResult createRunResult(TestRuntime runtime, ProcessResult result) { + return new R8TestRunResult(app, runtime, result, proguardMap, this::graphInspector); } public String getProguardMap() {
diff --git a/src/test/java/com/android/tools/r8/R8TestRunResult.java b/src/test/java/com/android/tools/r8/R8TestRunResult.java index 835c959..415abb1 100644 --- a/src/test/java/com/android/tools/r8/R8TestRunResult.java +++ b/src/test/java/com/android/tools/r8/R8TestRunResult.java
@@ -7,10 +7,12 @@ import static org.junit.Assert.assertNotNull; import com.android.tools.r8.ToolHelper.ProcessResult; +import com.android.tools.r8.naming.retrace.StackTrace; import com.android.tools.r8.retrace.Retrace; import com.android.tools.r8.retrace.RetraceCommand; import com.android.tools.r8.utils.AndroidApp; import com.android.tools.r8.utils.StringUtils; +import com.android.tools.r8.utils.ThrowingConsumer; import com.android.tools.r8.utils.codeinspector.CodeInspector; import com.android.tools.r8.utils.graphinspector.GraphInspector; import java.io.IOException; @@ -29,10 +31,11 @@ public R8TestRunResult( AndroidApp app, + TestRuntime runtime, ProcessResult result, String proguardMap, GraphInspectorSupplier graphInspector) { - super(app, result); + super(app, runtime, result); this.proguardMap = proguardMap; this.graphInspector = graphInspector; } @@ -43,6 +46,15 @@ } @Override + public StackTrace getStackTrace() { + return super.getStackTrace().retrace(proguardMap); + } + + public StackTrace getOriginalStackTrace() { + return super.getStackTrace(); + } + + @Override public CodeInspector inspector() throws IOException, ExecutionException { // See comment in base class. assertSuccess(); @@ -50,6 +62,12 @@ return new CodeInspector(app, proguardMap); } + public <E extends Throwable> R8TestRunResult inspectOriginalStackTrace( + ThrowingConsumer<StackTrace, E> consumer) throws E { + consumer.accept(getOriginalStackTrace()); + return self(); + } + public GraphInspector graphInspector() throws IOException, ExecutionException { assertSuccess(); return graphInspector.get();
diff --git a/src/test/java/com/android/tools/r8/TestCompileResult.java b/src/test/java/com/android/tools/r8/TestCompileResult.java index 4c6f535..8c643ff 100644 --- a/src/test/java/com/android/tools/r8/TestCompileResult.java +++ b/src/test/java/com/android/tools/r8/TestCompileResult.java
@@ -9,6 +9,7 @@ import com.android.tools.r8.ClassFileConsumer.ArchiveConsumer; import com.android.tools.r8.TestBase.Backend; +import com.android.tools.r8.TestRuntime.DexRuntime; import com.android.tools.r8.ToolHelper.ArtCommandBuilder; import com.android.tools.r8.ToolHelper.DexVm; import com.android.tools.r8.ToolHelper.ProcessResult; @@ -81,7 +82,7 @@ return outputMode; } - protected abstract RR createRunResult(ProcessResult result); + protected abstract RR createRunResult(TestRuntime runtime, ProcessResult result); @Deprecated public RR run(Class<?> mainClass) throws ExecutionException, IOException { @@ -94,7 +95,10 @@ assertThat(mainClassSubject, isPresent()); switch (getBackend()) { case DEX: - return runArt(null, additionalRunClassPath, mainClassSubject.getFinalName()); + return runArt( + new DexRuntime(ToolHelper.getDexVm()), + additionalRunClassPath, + mainClassSubject.getFinalName()); case CF: return runJava( TestRuntime.getDefaultJavaRuntime(), @@ -124,8 +128,7 @@ ClassSubject mainClassSubject = inspector().clazz(mainClass); assertThat("Did you forget a keep rule for the main method?", mainClassSubject, isPresent()); if (runtime.isDex()) { - return runArt( - runtime.asDex().getVm(), additionalRunClassPath, mainClassSubject.getFinalName(), args); + return runArt(runtime, additionalRunClassPath, mainClassSubject.getFinalName(), args); } assert runtime.isCf(); return runJava( @@ -293,11 +296,13 @@ .build(); ProcessResult result = ToolHelper.runJava(runtime.asCf().getVm(), vmArguments, classPath, arguments); - return createRunResult(result); + return createRunResult(runtime, result); } - private RR runArt(DexVm vm, List<Path> additionalClassPath, String mainClass, String... arguments) + private RR runArt( + TestRuntime runtime, List<Path> additionalClassPath, String mainClass, String... arguments) throws IOException { + DexVm vm = runtime.asDex().getVm(); // TODO(b/127785410): Always assume a non-null runtime. Path out = state.getNewTempFolder().resolve("out.zip"); app.writeToZip(out, OutputMode.DexIndexed); @@ -312,24 +317,16 @@ ProcessResult result = ToolHelper.runArtRaw( classPath, mainClass, commandConsumer, vm, withArtFrameworks, arguments); - return createRunResult(result); - } - - @Deprecated - public Dex2OatTestRunResult runDex2Oat() throws IOException { - return runDex2Oat(ToolHelper.getDexVm()); + return createRunResult(runtime, result); } public Dex2OatTestRunResult runDex2Oat(TestRuntime runtime) throws IOException { - return runDex2Oat(runtime.asDex().getVm()); - } - - public Dex2OatTestRunResult runDex2Oat(DexVm vm) throws IOException { assert getBackend() == DEX; + DexVm vm = runtime.asDex().getVm(); Path tmp = state.getNewTempFolder(); Path jarFile = tmp.resolve("out.jar"); Path oatFile = tmp.resolve("out.oat"); app.writeToZip(jarFile, OutputMode.DexIndexed); - return new Dex2OatTestRunResult(app, ToolHelper.runDex2OatRaw(jarFile, oatFile, vm)); + return new Dex2OatTestRunResult(app, runtime, ToolHelper.runDex2OatRaw(jarFile, oatFile, vm)); } }
diff --git a/src/test/java/com/android/tools/r8/TestRunResult.java b/src/test/java/com/android/tools/r8/TestRunResult.java index 4dd10a1..8e47b69 100644 --- a/src/test/java/com/android/tools/r8/TestRunResult.java +++ b/src/test/java/com/android/tools/r8/TestRunResult.java
@@ -9,8 +9,10 @@ import static org.junit.Assert.assertNotNull; import com.android.tools.r8.ToolHelper.ProcessResult; +import com.android.tools.r8.naming.retrace.StackTrace; import com.android.tools.r8.utils.AndroidApp; import com.android.tools.r8.utils.StringUtils; +import com.android.tools.r8.utils.ThrowingConsumer; import com.android.tools.r8.utils.codeinspector.CodeInspector; import java.io.IOException; import java.io.PrintStream; @@ -21,10 +23,12 @@ public abstract class TestRunResult<RR extends TestRunResult<?>> { protected final AndroidApp app; + private final TestRuntime runtime; private final ProcessResult result; - public TestRunResult(AndroidApp app, ProcessResult result) { + public TestRunResult(AndroidApp app, TestRuntime runtime, ProcessResult result) { this.app = app; + this.runtime = runtime; this.result = result; } @@ -51,6 +55,14 @@ return result.stderr; } + public StackTrace getStackTrace() { + if (runtime.isDex()) { + return StackTrace.extractFromArt(getStdErr(), runtime.asDex().getVm()); + } else { + return StackTrace.extractFromJvm(getStdErr()); + } + } + public int getExitCode() { return result.exitCode; } @@ -113,6 +125,12 @@ return self(); } + public <E extends Throwable> RR inspectStackTrace(ThrowingConsumer<StackTrace, E> consumer) + throws E { + consumer.accept(getStackTrace()); + return self(); + } + public RR disassemble(PrintStream ps) throws IOException, ExecutionException { ToolHelper.disassemble(app, ps); return self();
diff --git a/src/test/java/com/android/tools/r8/d8/D8FrameworkDexPassthroughMarkerTest.java b/src/test/java/com/android/tools/r8/d8/D8FrameworkDexPassthroughMarkerTest.java index 93c0323..866dcf5 100644 --- a/src/test/java/com/android/tools/r8/d8/D8FrameworkDexPassthroughMarkerTest.java +++ b/src/test/java/com/android/tools/r8/d8/D8FrameworkDexPassthroughMarkerTest.java
@@ -71,7 +71,7 @@ new ApplicationReader( app, options, new Timing("D8FrameworkDexPassthroughMarkerTest")) .read(); - Collection<Marker> markers = dexApp.dexItemFactory.extractMarker(); + Collection<Marker> markers = dexApp.dexItemFactory.extractMarkers(); assertEquals(1, markers.size()); Marker readMarker = markers.iterator().next(); assertEquals(marker, readMarker);
diff --git a/src/test/java/com/android/tools/r8/desugar/corelib/conversionTests/APIConversionTest.java b/src/test/java/com/android/tools/r8/desugar/corelib/conversionTests/APIConversionTest.java index 6bbf8a1..8fadf97 100644 --- a/src/test/java/com/android/tools/r8/desugar/corelib/conversionTests/APIConversionTest.java +++ b/src/test/java/com/android/tools/r8/desugar/corelib/conversionTests/APIConversionTest.java
@@ -5,14 +5,19 @@ import static org.hamcrest.CoreMatchers.endsWith; import static org.hamcrest.core.StringContains.containsString; +import static org.junit.Assert.assertFalse; +import com.android.tools.r8.D8TestCompileResult; +import com.android.tools.r8.Diagnostic; import com.android.tools.r8.TestParameters; import com.android.tools.r8.TestParametersCollection; import com.android.tools.r8.ToolHelper.DexVm.Version; import com.android.tools.r8.desugar.corelib.CoreLibDesugarTestBase; +import com.android.tools.r8.errors.InterfaceDesugarMissingTypeDiagnostic; import com.android.tools.r8.utils.AndroidApiLevel; import com.android.tools.r8.utils.StringUtils; import java.util.Arrays; +import java.util.List; import java.util.Random; import java.util.function.IntUnaryOperator; import java.util.stream.IntStream; @@ -60,6 +65,7 @@ .setMinApi(parameters.getApiLevel()) .enableCoreLibraryDesugaring(parameters.getApiLevel()) .compile() + .assertOnlyInfos() // No warnings. .addDesugaredCoreLibraryRunClassPath(this::buildDesugaredLibrary, parameters.getApiLevel()) .run(parameters.getRuntime(), Executor.class) .assertSuccessWithOutput(
diff --git a/src/test/java/com/android/tools/r8/desugar/corelib/EnumSetTest.java b/src/test/java/com/android/tools/r8/desugar/corelib/shrinkingtests/EnumSetTest.java similarity index 94% rename from src/test/java/com/android/tools/r8/desugar/corelib/EnumSetTest.java rename to src/test/java/com/android/tools/r8/desugar/corelib/shrinkingtests/EnumSetTest.java index d38ac77..db9206f 100644 --- a/src/test/java/com/android/tools/r8/desugar/corelib/EnumSetTest.java +++ b/src/test/java/com/android/tools/r8/desugar/corelib/shrinkingtests/EnumSetTest.java
@@ -2,9 +2,10 @@ // 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.desugar.corelib; +package com.android.tools.r8.desugar.corelib.shrinkingtests; import com.android.tools.r8.TestParameters; +import com.android.tools.r8.desugar.corelib.CoreLibDesugarTestBase; import com.android.tools.r8.utils.BooleanUtils; import com.android.tools.r8.utils.StringUtils; import java.time.DayOfWeek;
diff --git a/src/test/java/com/android/tools/r8/desugar/corelib/EnumSetTest.java b/src/test/java/com/android/tools/r8/desugar/corelib/shrinkingtests/FieldAccessTest.java similarity index 62% copy from src/test/java/com/android/tools/r8/desugar/corelib/EnumSetTest.java copy to src/test/java/com/android/tools/r8/desugar/corelib/shrinkingtests/FieldAccessTest.java index d38ac77..29d1554 100644 --- a/src/test/java/com/android/tools/r8/desugar/corelib/EnumSetTest.java +++ b/src/test/java/com/android/tools/r8/desugar/corelib/shrinkingtests/FieldAccessTest.java
@@ -2,15 +2,13 @@ // 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.desugar.corelib; +package com.android.tools.r8.desugar.corelib.shrinkingtests; import com.android.tools.r8.TestParameters; +import com.android.tools.r8.desugar.corelib.CoreLibDesugarTestBase; import com.android.tools.r8.utils.BooleanUtils; import com.android.tools.r8.utils.StringUtils; -import java.time.DayOfWeek; -import java.time.chrono.IsoEra; -import java.time.chrono.MinguoEra; -import java.util.EnumSet; +import java.time.ZoneId; import java.util.List; import org.junit.Test; import org.junit.runner.RunWith; @@ -18,7 +16,7 @@ import org.junit.runners.Parameterized.Parameters; @RunWith(Parameterized.class) -public class EnumSetTest extends CoreLibDesugarTestBase { +public class FieldAccessTest extends CoreLibDesugarTestBase { private final TestParameters parameters; private final boolean shrinkDesugaredLibrary; @@ -29,16 +27,16 @@ BooleanUtils.values(), getTestParameters().withDexRuntimes().withAllApiLevels().build()); } - public EnumSetTest(boolean shrinkDesugaredLibrary, TestParameters parameters) { + public FieldAccessTest(boolean shrinkDesugaredLibrary, TestParameters parameters) { this.shrinkDesugaredLibrary = shrinkDesugaredLibrary; this.parameters = parameters; } @Test - public void testEnum() throws Exception { + public void testField() throws Exception { KeepRuleConsumer keepRuleConsumer = createKeepRuleConsumer(parameters); testForD8() - .addProgramClasses(EnumSetUser.class) + .addProgramClasses(Executor.class) .setMinApi(parameters.getApiLevel()) .enableCoreLibraryDesugaring(parameters.getApiLevel(), keepRuleConsumer) .compile() @@ -47,19 +45,14 @@ parameters.getApiLevel(), keepRuleConsumer.get(), shrinkDesugaredLibrary) - .run(parameters.getRuntime(), EnumSetUser.class) - .assertSuccessWithOutput(StringUtils.lines("2", "0", "1")); + .run(parameters.getRuntime(), Executor.class) + .assertSuccessWithOutput(StringUtils.lines("Australia/Darwin")); } - static class EnumSetUser { + static class Executor { public static void main(String[] args) { - EnumSet<IsoEra> isoEras = EnumSet.allOf(IsoEra.class); - System.out.println(isoEras.size()); - EnumSet<DayOfWeek> dayOfWeeks = EnumSet.noneOf(DayOfWeek.class); - System.out.println(dayOfWeeks.size()); - EnumSet<MinguoEra> minguoEras = EnumSet.of(MinguoEra.BEFORE_ROC); - System.out.println(minguoEras.size()); + System.out.println(ZoneId.SHORT_IDS.get("ACT")); } } }
diff --git a/src/test/java/com/android/tools/r8/desugar/corelib/EnumSetTest.java b/src/test/java/com/android/tools/r8/desugar/corelib/shrinkingtests/InheritanceTest.java similarity index 61% copy from src/test/java/com/android/tools/r8/desugar/corelib/EnumSetTest.java copy to src/test/java/com/android/tools/r8/desugar/corelib/shrinkingtests/InheritanceTest.java index d38ac77..24b6b6e 100644 --- a/src/test/java/com/android/tools/r8/desugar/corelib/EnumSetTest.java +++ b/src/test/java/com/android/tools/r8/desugar/corelib/shrinkingtests/InheritanceTest.java
@@ -2,15 +2,15 @@ // 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.desugar.corelib; +package com.android.tools.r8.desugar.corelib.shrinkingtests; import com.android.tools.r8.TestParameters; +import com.android.tools.r8.desugar.corelib.CoreLibDesugarTestBase; import com.android.tools.r8.utils.BooleanUtils; import com.android.tools.r8.utils.StringUtils; -import java.time.DayOfWeek; -import java.time.chrono.IsoEra; -import java.time.chrono.MinguoEra; -import java.util.EnumSet; +import java.time.Clock; +import java.time.Instant; +import java.time.ZoneId; import java.util.List; import org.junit.Test; import org.junit.runner.RunWith; @@ -18,7 +18,7 @@ import org.junit.runners.Parameterized.Parameters; @RunWith(Parameterized.class) -public class EnumSetTest extends CoreLibDesugarTestBase { +public class InheritanceTest extends CoreLibDesugarTestBase { private final TestParameters parameters; private final boolean shrinkDesugaredLibrary; @@ -29,16 +29,16 @@ BooleanUtils.values(), getTestParameters().withDexRuntimes().withAllApiLevels().build()); } - public EnumSetTest(boolean shrinkDesugaredLibrary, TestParameters parameters) { + public InheritanceTest(boolean shrinkDesugaredLibrary, TestParameters parameters) { this.shrinkDesugaredLibrary = shrinkDesugaredLibrary; this.parameters = parameters; } @Test - public void testEnum() throws Exception { + public void testInheritance() throws Exception { KeepRuleConsumer keepRuleConsumer = createKeepRuleConsumer(parameters); testForD8() - .addProgramClasses(EnumSetUser.class) + .addProgramClasses(Impl.class) .setMinApi(parameters.getApiLevel()) .enableCoreLibraryDesugaring(parameters.getApiLevel(), keepRuleConsumer) .compile() @@ -47,19 +47,30 @@ parameters.getApiLevel(), keepRuleConsumer.get(), shrinkDesugaredLibrary) - .run(parameters.getRuntime(), EnumSetUser.class) - .assertSuccessWithOutput(StringUtils.lines("2", "0", "1")); + .run(parameters.getRuntime(), Impl.class) + .assertSuccessWithOutput(StringUtils.lines("123456")); } - static class EnumSetUser { + static class Impl extends Clock { public static void main(String[] args) { - EnumSet<IsoEra> isoEras = EnumSet.allOf(IsoEra.class); - System.out.println(isoEras.size()); - EnumSet<DayOfWeek> dayOfWeeks = EnumSet.noneOf(DayOfWeek.class); - System.out.println(dayOfWeeks.size()); - EnumSet<MinguoEra> minguoEras = EnumSet.of(MinguoEra.BEFORE_ROC); - System.out.println(minguoEras.size()); + Impl impl = new Impl(); + System.out.println(impl.millis()); + } + + @Override + public ZoneId getZone() { + return null; + } + + @Override + public Clock withZone(ZoneId zone) { + return null; + } + + @Override + public Instant instant() { + return Instant.ofEpochMilli(123456); } } }
diff --git a/src/test/java/com/android/tools/r8/desugar/corelib/KeepRuleShrinkTest.java b/src/test/java/com/android/tools/r8/desugar/corelib/shrinkingtests/KeepRuleShrinkTest.java similarity index 94% rename from src/test/java/com/android/tools/r8/desugar/corelib/KeepRuleShrinkTest.java rename to src/test/java/com/android/tools/r8/desugar/corelib/shrinkingtests/KeepRuleShrinkTest.java index 843e154..b20abee 100644 --- a/src/test/java/com/android/tools/r8/desugar/corelib/KeepRuleShrinkTest.java +++ b/src/test/java/com/android/tools/r8/desugar/corelib/shrinkingtests/KeepRuleShrinkTest.java
@@ -2,17 +2,17 @@ // 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.desugar.corelib; +package com.android.tools.r8.desugar.corelib.shrinkingtests; import com.android.tools.r8.D8TestRunResult; import com.android.tools.r8.TestParameters; +import com.android.tools.r8.desugar.corelib.CoreLibDesugarTestBase; import com.android.tools.r8.utils.BooleanUtils; import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ConcurrentSkipListMap; -import org.junit.Assume; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @@ -37,7 +37,6 @@ @Test public void testMapProblem() throws Exception { - Assume.assumeFalse("TODO(b/134732760): Fix shrinking of core library", shrinkDesugaredLibrary); KeepRuleConsumer keepRuleConsumer = createKeepRuleConsumer(parameters); D8TestRunResult d8TestRunResult = testForD8()
diff --git a/src/test/java/com/android/tools/r8/dexfilemerger/DexMergeChecksumsHighSortingStrings.java b/src/test/java/com/android/tools/r8/dexfilemerger/DexMergeChecksumsHighSortingStrings.java new file mode 100644 index 0000000..e6a90ff --- /dev/null +++ b/src/test/java/com/android/tools/r8/dexfilemerger/DexMergeChecksumsHighSortingStrings.java
@@ -0,0 +1,127 @@ +// Copyright (c) 2019, 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.dexfilemerger; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import com.android.tools.r8.CompilationMode; +import com.android.tools.r8.TestBase; +import com.android.tools.r8.TestParameters; +import com.android.tools.r8.TestParametersCollection; +import com.android.tools.r8.dex.ClassesChecksum; +import com.android.tools.r8.graph.DexItemFactory; +import java.nio.file.Path; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +@RunWith(Parameterized.class) +public class DexMergeChecksumsHighSortingStrings extends TestBase { + + private final TestParameters parameters; + + @Parameterized.Parameters(name = "{0}") + public static TestParametersCollection data() { + return getTestParameters().withDexRuntimes().withAllApiLevels().build(); + } + + public DexMergeChecksumsHighSortingStrings(TestParameters parameters) { + this.parameters = parameters; + } + + @Test + public void testChecksumWhitHighSortingStrings() throws Exception { + Path dexArchive1 = + testForD8() + .addProgramClasses(TestClass1.class) + .setMinApi(parameters.getApiLevel()) + .setMode(CompilationMode.DEBUG) + .setIncludeClassesChecksum(true) + .compile() + .writeToZip(); + + Path dexArchive2 = + testForD8() + .addProgramClasses(TestClass2.class) + .setMinApi(parameters.getApiLevel()) + .setMode(CompilationMode.DEBUG) + .setIncludeClassesChecksum(true) + .compile() + .writeToZip(); + + testForD8() + .setMinApi(parameters.getApiLevel()) + .addProgramFiles(dexArchive1) + .setIncludeClassesChecksum(true) + .run(parameters.getRuntime(), TestClass1.class) + .assertSuccessWithOutputLines("Hello, \uDB3F\uDFFD"); + + testForD8() + .setMinApi(parameters.getApiLevel()) + .addProgramFiles(dexArchive2) + .setIncludeClassesChecksum(true) + .run(parameters.getRuntime(), TestClass2.class) + .assertSuccessWithOutputLines("Hello, ~~~\u007f"); + + testForD8() + .setMinApi(parameters.getApiLevel()) + .addProgramFiles(dexArchive1, dexArchive2) + .setIncludeClassesChecksum(true) + .run(parameters.getRuntime(), TestClass2.class) + .assertSuccessWithOutputLines("Hello, ~~~\u007f"); + } + + static class TestClass1 { + // U+DFFFD which is very end of unassigned plane. + private static final String STRING_SORTING_ABOVE_CHECKSUM_STRING = "\uDB3F\uDFFD"; + + public static void main(String[] args) { + System.out.println("Hello, " + STRING_SORTING_ABOVE_CHECKSUM_STRING); + } + } + + static class TestClass2 { + private static final String STRING_SORTING_ABOVE_CHECKSUM_STRING = "~~~\u007f"; + + public static void main(String[] args) { + System.out.println("Hello, " + STRING_SORTING_ABOVE_CHECKSUM_STRING); + } + } + + @Test + public void testChecksumMarkerComparison() { + // z { | } ~ <DEL> + // 0x7a 0x7b 0x7c 0x7d 0x7e 0x7f + assertTrue('{' - 1 == 'z'); + assertTrue('{' + 1 == '|'); + assertTrue('~' - 1 == '}'); + + DexItemFactory factory = new DexItemFactory(); + assertFalse( + ClassesChecksum.definitelyPreceedChecksumMarker(factory.createString("\uDB3F\uDFFD"))); + assertFalse( + ClassesChecksum.definitelyPreceedChecksumMarker(factory.createString("~\uDB3F\uDFFD\""))); + assertFalse( + ClassesChecksum.definitelyPreceedChecksumMarker(factory.createString("~~\uDB3F\uDFFD"))); + assertFalse( + ClassesChecksum.definitelyPreceedChecksumMarker(factory.createString("~~~\uDB3F\uDFFD"))); + assertFalse(ClassesChecksum.definitelyPreceedChecksumMarker(factory.createString("\u007f"))); + assertFalse(ClassesChecksum.definitelyPreceedChecksumMarker(factory.createString("~\u007f"))); + assertFalse(ClassesChecksum.definitelyPreceedChecksumMarker(factory.createString("~~\u007f"))); + assertFalse(ClassesChecksum.definitelyPreceedChecksumMarker(factory.createString("~~~\u007f"))); + assertFalse(ClassesChecksum.definitelyPreceedChecksumMarker(factory.createString("~~~|"))); + + assertTrue(ClassesChecksum.definitelyPreceedChecksumMarker(factory.createString("~~}"))); + assertTrue(ClassesChecksum.definitelyPreceedChecksumMarker(factory.createString("~~"))); + assertTrue(ClassesChecksum.definitelyPreceedChecksumMarker(factory.createString("~}"))); + assertTrue(ClassesChecksum.definitelyPreceedChecksumMarker(factory.createString("~"))); + assertTrue(ClassesChecksum.definitelyPreceedChecksumMarker(factory.createString("}"))); + + // False negatives. + assertFalse(ClassesChecksum.definitelyPreceedChecksumMarker(factory.createString("~~~"))); + assertFalse(ClassesChecksum.definitelyPreceedChecksumMarker(factory.createString("~~~z"))); + } +}
diff --git a/src/test/java/com/android/tools/r8/ir/analysis/sideeffect/ArrayOfObjectsCreationCanBePostponedTest.java b/src/test/java/com/android/tools/r8/ir/analysis/sideeffect/ArrayOfObjectsCreationCanBePostponedTest.java new file mode 100644 index 0000000..065e069 --- /dev/null +++ b/src/test/java/com/android/tools/r8/ir/analysis/sideeffect/ArrayOfObjectsCreationCanBePostponedTest.java
@@ -0,0 +1,81 @@ +// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +package com.android.tools.r8.ir.analysis.sideeffect; + +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.TestBase; +import com.android.tools.r8.TestParameters; +import com.android.tools.r8.TestParametersCollection; +import com.android.tools.r8.utils.codeinspector.ClassSubject; +import com.android.tools.r8.utils.codeinspector.CodeInspector; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; + +@RunWith(Parameterized.class) +public class ArrayOfObjectsCreationCanBePostponedTest extends TestBase { + + private final TestParameters parameters; + + @Parameters(name = "{0}") + public static TestParametersCollection data() { + return getTestParameters().withAllRuntimesAndApiLevels().build(); + } + + public ArrayOfObjectsCreationCanBePostponedTest(TestParameters parameters) { + this.parameters = parameters; + } + + @Test + public void test() throws Exception { + testForR8(parameters.getBackend()) + .addInnerClasses(ArrayOfObjectsCreationCanBePostponedTest.class) + .addKeepMainRule(TestClass.class) + .setMinApi(parameters.getApiLevel()) + .compile() + .inspect(this::inspect) + .run(parameters.getRuntime(), TestClass.class) + .assertSuccessWithOutputLines("Hello world!"); + } + + private void inspect(CodeInspector inspector) { + ClassSubject classSubject = inspector.clazz(A.class); + assertThat(classSubject, isPresent()); + + // We should have been able to inline A.inlineable() since A.<clinit>() can safely be postponed. + assertThat(classSubject.uniqueMethodWithName("inlineable"), not(isPresent())); + } + + static class TestClass { + + public static void main(String[] args) { + A.inlineable(); + System.out.println(A.values[0].getMessage()); + } + } + + static class A { + + static A[] values = new A[] {new A(" world!")}; + + String message; + + A(String message) { + this.message = message; + } + + static void inlineable() { + System.out.print("Hello"); + } + + String getMessage() { + return message; + } + } +}
diff --git a/src/test/java/com/android/tools/r8/ir/analysis/type/ConstrainedPrimitiveTypeTest.java b/src/test/java/com/android/tools/r8/ir/analysis/type/ConstrainedPrimitiveTypeTest.java index fd6ada0..dd0b5e2 100644 --- a/src/test/java/com/android/tools/r8/ir/analysis/type/ConstrainedPrimitiveTypeTest.java +++ b/src/test/java/com/android/tools/r8/ir/analysis/type/ConstrainedPrimitiveTypeTest.java
@@ -29,7 +29,7 @@ @Parameterized.Parameters(name = "{0}") public static TestParametersCollection data() { - return getTestParameters().withAllRuntimes().build(); + return getTestParameters().withAllRuntimes().withAllApiLevels().build(); } public ConstrainedPrimitiveTypeTest(TestParameters parameters) throws Exception { @@ -47,6 +47,7 @@ .addInnerClasses(ConstrainedPrimitiveTypeTest.class) .addKeepMainRule(TestClass.class) .enableInliningAnnotations() + .setMinApi(parameters.getApiLevel()) .run(parameters.getRuntime(), TestClass.class) .assertSuccessWithOutput(expectedOutput); }
diff --git a/src/test/java/com/android/tools/r8/ir/conversion/NodeExtractionTest.java b/src/test/java/com/android/tools/r8/ir/conversion/NodeExtractionTest.java index 2cde812..b35254e 100644 --- a/src/test/java/com/android/tools/r8/ir/conversion/NodeExtractionTest.java +++ b/src/test/java/com/android/tools/r8/ir/conversion/NodeExtractionTest.java
@@ -51,20 +51,20 @@ Set<Node> wave = Sets.newIdentityHashSet(); - MethodProcessor.extractLeaves(nodes, wave::add); + PrimaryMethodProcessor.extractLeaves(nodes, wave::add); assertEquals(3, wave.size()); assertThat(wave, hasItem(n3)); assertThat(wave, hasItem(n4)); assertThat(wave, hasItem(n6)); wave.clear(); - MethodProcessor.extractLeaves(nodes, wave::add); + PrimaryMethodProcessor.extractLeaves(nodes, wave::add); assertEquals(2, wave.size()); assertThat(wave, hasItem(n2)); assertThat(wave, hasItem(n5)); wave.clear(); - MethodProcessor.extractLeaves(nodes, wave::add); + PrimaryMethodProcessor.extractLeaves(nodes, wave::add); assertEquals(1, wave.size()); assertThat(wave, hasItem(n1)); assertTrue(nodes.isEmpty()); @@ -103,20 +103,20 @@ Set<Node> wave = Sets.newIdentityHashSet(); - MethodProcessor.extractLeaves(nodes, wave::add); + PrimaryMethodProcessor.extractLeaves(nodes, wave::add); assertEquals(3, wave.size()); assertThat(wave, hasItem(n3)); assertThat(wave, hasItem(n4)); assertThat(wave, hasItem(n6)); wave.clear(); - MethodProcessor.extractLeaves(nodes, wave::add); + PrimaryMethodProcessor.extractLeaves(nodes, wave::add); assertEquals(2, wave.size()); assertThat(wave, hasItem(n2)); assertThat(wave, hasItem(n5)); wave.clear(); - MethodProcessor.extractLeaves(nodes, wave::add); + PrimaryMethodProcessor.extractLeaves(nodes, wave::add); assertEquals(1, wave.size()); assertThat(wave, hasItem(n1)); assertTrue(nodes.isEmpty());
diff --git a/src/test/java/com/android/tools/r8/ir/conversion/PartialCallGraphTest.java b/src/test/java/com/android/tools/r8/ir/conversion/PartialCallGraphTest.java index 769515c..880139f 100644 --- a/src/test/java/com/android/tools/r8/ir/conversion/PartialCallGraphTest.java +++ b/src/test/java/com/android/tools/r8/ir/conversion/PartialCallGraphTest.java
@@ -83,20 +83,20 @@ Set<Node> wave = Sets.newIdentityHashSet(); - MethodProcessor.extractLeaves(cg.nodes, wave::add); + PrimaryMethodProcessor.extractLeaves(cg.nodes, wave::add); assertEquals(4, wave.size()); // including <init> assertThat(wave, hasItem(m3)); assertThat(wave, hasItem(m4)); assertThat(wave, hasItem(m6)); wave.clear(); - MethodProcessor.extractLeaves(cg.nodes, wave::add); + PrimaryMethodProcessor.extractLeaves(cg.nodes, wave::add); assertEquals(2, wave.size()); assertThat(wave, hasItem(m2)); assertThat(wave, hasItem(m5)); wave.clear(); - MethodProcessor.extractLeaves(cg.nodes, wave::add); + PrimaryMethodProcessor.extractLeaves(cg.nodes, wave::add); assertEquals(1, wave.size()); assertThat(wave, hasItem(m1)); assertTrue(cg.nodes.isEmpty());
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/DefaultInterfaceIssue143628636Test.java b/src/test/java/com/android/tools/r8/ir/optimize/DefaultInterfaceIssue143628636Test.java new file mode 100644 index 0000000..945e1af --- /dev/null +++ b/src/test/java/com/android/tools/r8/ir/optimize/DefaultInterfaceIssue143628636Test.java
@@ -0,0 +1,133 @@ +// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. +package com.android.tools.r8.ir.optimize; + +import com.android.tools.r8.NeverClassInline; +import com.android.tools.r8.NeverInline; +import com.android.tools.r8.NeverMerge; +import com.android.tools.r8.TestBase; +import com.android.tools.r8.TestParameters; +import com.android.tools.r8.TestParametersCollection; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +/** + * Reproduction for issue b/143628636. + * + * <p>There are a lot of conditions that need to be in place to trigger this issue. The root issue + * is resolution incorrectly pruning out a valid candidate. However, mostly code will not even get + * to that place as a single target is typically found prior to it, and then, if the incorrect + * pruning of the lookup targets takes place, that needs to further cause invalid call site + * information to be propagated, which in turn will cause inlining to inline a method where it + * should not. It is exceedingly unlikely that this test will continue to be a regression test as + * any one of the above aspects could and likely will be changed in the code base. + */ +@RunWith(Parameterized.class) +public class DefaultInterfaceIssue143628636Test extends TestBase { + + private final TestParameters parameters; + + @Parameterized.Parameters(name = "{0}") + public static TestParametersCollection data() { + return getTestParameters().withAllRuntimes().withAllApiLevels().build(); + } + + public DefaultInterfaceIssue143628636Test(TestParameters parameters) { + this.parameters = parameters; + } + + @Test + public void test() throws Exception { + testForR8(parameters.getBackend()) + .enableInliningAnnotations() + .enableClassInliningAnnotations() + .enableMergeAnnotations() + .addInnerClasses(DefaultInterfaceIssue143628636Test.class) + .addKeepMainRule(TestClass.class) + .addKeepClassRules(I.class) + .setMinApi(parameters.getApiLevel()) + .run(parameters.getRuntime(), TestClass.class) + .assertSuccessWithOutputLines("2", "5"); + } + + @NeverMerge + @NeverClassInline + public interface A { + + @NeverInline + default void f( + // This parameter is needed otherwise the info recording bails out. (See b/143684659). + int z) { + // In the end the issue will manifest as a class-cast exception because B.g() is inlined here + // due to the incorrect conclusion that the only possible receiver type is B. + System.out.println(g() + z); + } + + int g(); + } + + // This intermediate interface is needed to cause lookupInterfaceTargets to return null as I + // is pinned (why does it return null for a pinned holder is questionable, filed b/143686005). + // The return of null, will cause the outer lookup to hit a different lookup case where the + // refined receiver (here I) will cause the non-subtype method A.f to be pruned. + public interface I extends A {} + + // Make sure this class and the call to h() are never eliminated. It is the *partial* info + // propagated from h() to f() that results in incorrect call-site optimization info. + @NeverMerge + @NeverClassInline + public static class B implements A { + public final int x; + + public B(int x) { + this.x = x; + } + + @Override + public int g() { + return x; + } + + @NeverInline + public void h() { + // This will lookup A.f and propagate that the receiver is known to be B. + f(x); + } + } + + // Make sure this class and the call to h() are never eliminated. It is the *missing* info + // propagated from h() to f() that results in incorrect call-site optimization info. + @NeverMerge + @NeverClassInline + public static class C implements I { + public final I i; + public final int y; + + public C(I i, int y) { + this.i = i; + this.y = y; + } + + @Override + public int g() { + return y; + } + + @NeverInline + public void h() { + // Due to the refined receiver type I used here, the lookup targets will omit A.f ! + // Thus the info propagation does not propagate that I (and thus C) may be a receiver type. + i.f(y); + } + } + + static class TestClass { + + public static void main(String[] args) { + new B(args.length + 1).h(); + new C(args.length == 42 ? null : new C(null, args.length + 2), args.length + 3).h(); + } + } +}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/ifs/EnumAliasComparisonTest.java b/src/test/java/com/android/tools/r8/ir/optimize/ifs/EnumAliasComparisonTest.java new file mode 100644 index 0000000..5c0f75b --- /dev/null +++ b/src/test/java/com/android/tools/r8/ir/optimize/ifs/EnumAliasComparisonTest.java
@@ -0,0 +1,81 @@ +// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +package com.android.tools.r8.ir.optimize.ifs; + +import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent; +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertTrue; + +import com.android.tools.r8.TestBase; +import com.android.tools.r8.TestParameters; +import com.android.tools.r8.TestParametersCollection; +import com.android.tools.r8.utils.codeinspector.ClassSubject; +import com.android.tools.r8.utils.codeinspector.CodeInspector; +import com.android.tools.r8.utils.codeinspector.InstructionSubject; +import com.android.tools.r8.utils.codeinspector.MethodSubject; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; + +@RunWith(Parameterized.class) +public class EnumAliasComparisonTest extends TestBase { + + private final TestParameters parameters; + + @Parameters(name = "{0}") + public static TestParametersCollection data() { + return getTestParameters().withAllRuntimesAndApiLevels().build(); + } + + public EnumAliasComparisonTest(TestParameters parameters) { + this.parameters = parameters; + } + + @Test + public void test() throws Exception { + testForR8(parameters.getBackend()) + .addInnerClasses(EnumAliasComparisonTest.class) + .addKeepMainRule(TestClass.class) + .setMinApi(parameters.getApiLevel()) + .compile() + .inspect(this::inspect) + .run(parameters.getRuntime(), TestClass.class) + .assertSuccessWithOutputLines("true"); + } + + private void inspect(CodeInspector inspector) { + ClassSubject classSubject = inspector.clazz(TestClass.class); + assertThat(classSubject, isPresent()); + + MethodSubject mainMethod = classSubject.mainMethod(); + assertThat(mainMethod, isPresent()); + assertTrue( + mainMethod + .streamInstructions() + .filter(InstructionSubject::isStaticGet) + .map(InstructionSubject::getField) + .map(field -> field.name.toSourceString()) + .allMatch("out"::equals)); + + assertThat(inspector.clazz(MyEnum.class), not(isPresent())); + } + + static class TestClass { + + public static void main(String[] args) { + // Should print "true" since MyEnum.B is an alias of MyEnum.A. + System.out.println(MyEnum.A == MyEnum.B); + } + } + + enum MyEnum { + A; + + // Introduce an alias of MyEnum.A. + static MyEnum B = A; + } +}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/membervaluepropagation/B143686595.java b/src/test/java/com/android/tools/r8/ir/optimize/membervaluepropagation/B143686595.java new file mode 100644 index 0000000..036bde7 --- /dev/null +++ b/src/test/java/com/android/tools/r8/ir/optimize/membervaluepropagation/B143686595.java
@@ -0,0 +1,77 @@ +// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. +package com.android.tools.r8.ir.optimize.membervaluepropagation; + +import com.android.tools.r8.NeverInline; +import com.android.tools.r8.R8TestCompileResult; +import com.android.tools.r8.TestBase; +import com.android.tools.r8.TestParameters; +import com.android.tools.r8.TestParametersCollection; +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 B143686595 extends TestBase { + private final TestParameters parameters; + + @Parameters(name = "{0}") + public static TestParametersCollection data() { + return getTestParameters().withAllRuntimesAndApiLevels().build(); + } + + public B143686595(TestParameters parameters) { + this.parameters = parameters; + } + + @Test + public void test() throws Exception { + R8TestCompileResult libraryResult = + testForR8(parameters.getBackend()) + .addProgramClasses(I.class) + .addKeepClassAndMembersRules(I.class) + .setMinApi(parameters.getApiLevel()) + .compile(); + + testForR8(parameters.getBackend()) + .addProgramClasses(TestClass.class) + .addClasspathClasses(I.class) + .addKeepMainRule(TestClass.class) + .setMinApi(parameters.getApiLevel()) + .compile() + .addRunClasspathFiles(libraryResult.writeToZip()) + .run(parameters.getRuntime(), TestClass.class) + .assertSuccessWithOutputLines("0.4"); + } + + interface I { + float foo(float v); + } + + static class TestClass { + static final String UNNECESSARY_PUT; + + static { + // Will trigger AppInfoWithLiveness#withoutStaticFieldsWrites that should have copied caches + // of relations between synthesized classes and their super types. + UNNECESSARY_PUT = "DEAD"; + } + + static final I DOUBLER = t -> t * 2.0f; + + @NeverInline + static I wrap(I previous, float base) { + // Interface desugaring + return t -> previous.foo(t / base); + } + + public static void main(String... args) { + I i = wrap(DOUBLER, 10f); + // Without such consistent cache, this will fail with an assertion error that lower/upper + // bound types of receiver are mismatching. + System.out.println(i.foo(2f)); + } + } +}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/staticizer/CompanionAsArgumentTest.java b/src/test/java/com/android/tools/r8/ir/optimize/staticizer/CompanionAsArgumentTest.java index cd52ef0..92cc700 100644 --- a/src/test/java/com/android/tools/r8/ir/optimize/staticizer/CompanionAsArgumentTest.java +++ b/src/test/java/com/android/tools/r8/ir/optimize/staticizer/CompanionAsArgumentTest.java
@@ -27,7 +27,7 @@ @Parameterized.Parameters(name = "{0}") public static TestParametersCollection data() { // TODO(b/112831361): support for class staticizer in CF backend. - return getTestParameters().withDexRuntimes().build(); + return getTestParameters().withDexRuntimes().withAllApiLevels().build(); } private final TestParameters parameters; @@ -43,7 +43,7 @@ .addKeepMainRule(MAIN) .enableInliningAnnotations() .enableClassInliningAnnotations() - .setMinApi(parameters.getRuntime()) + .setMinApi(parameters.getApiLevel()) .run(parameters.getRuntime(), MAIN) .assertSuccessWithOutputLines("Companion#foo(true)") .inspect(this::inspect); @@ -55,9 +55,10 @@ assertThat(companion, isPresent()); MethodSubject foo = companion.uniqueMethodWithName("foo"); assertThat(foo, isPresent()); - assertTrue(foo.streamInstructions().anyMatch( - i -> i.isInvokeVirtual() - && i.getMethod().toSourceString().contains("PrintStream.println"))); + assertTrue( + foo.streamInstructions().anyMatch( + i -> i.isInvokeVirtual() + && i.getMethod().toSourceString().contains("PrintStream.println"))); // Nothing migrated from Companion to Host. ClassSubject host = inspector.clazz(Host.class);
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/staticizer/InstanceInsideCompanionTest.java b/src/test/java/com/android/tools/r8/ir/optimize/staticizer/InstanceInsideCompanionTest.java new file mode 100644 index 0000000..fe55066 --- /dev/null +++ b/src/test/java/com/android/tools/r8/ir/optimize/staticizer/InstanceInsideCompanionTest.java
@@ -0,0 +1,94 @@ +// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. +package com.android.tools.r8.ir.optimize.staticizer; + +import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent; +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertTrue; + +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.utils.codeinspector.ClassSubject; +import com.android.tools.r8.utils.codeinspector.CodeInspector; +import com.android.tools.r8.utils.codeinspector.FieldSubject; +import com.android.tools.r8.utils.codeinspector.MethodSubject; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +@RunWith(Parameterized.class) +public class InstanceInsideCompanionTest extends TestBase { + private static final Class<?> MAIN = Main.class; + + @Parameterized.Parameters(name = "{0}") + public static TestParametersCollection data() { + // TODO(b/112831361): support for class staticizer in CF backend. + return getTestParameters().withDexRuntimes().withAllApiLevels().build(); + } + + private final TestParameters parameters; + + public InstanceInsideCompanionTest(TestParameters parameters) { + this.parameters = parameters; + } + + @Test + public void b143684491() throws Exception { + testForR8(parameters.getBackend()) + .addInnerClasses(InstanceInsideCompanionTest.class) + .addKeepMainRule(MAIN) + .enableInliningAnnotations() + .enableClassInliningAnnotations() + .setMinApi(parameters.getApiLevel()) + .run(parameters.getRuntime(), MAIN) + .assertSuccessWithOutputLines("Candidate#foo(false)") + .inspect(this::inspect); + } + + private void inspect(CodeInspector inspector) { + // Check if the instance is gone. + ClassSubject host = inspector.clazz(Candidate.Host.class); + assertThat(host, isPresent()); + FieldSubject instance = host.uniqueFieldWithName("INSTANCE"); + assertThat(instance, not(isPresent())); + + ClassSubject candidate = inspector.clazz(Candidate.class); + assertThat(candidate, not(isPresent())); + + // Check if the candidate method is staticized and migrated. + MethodSubject foo = host.uniqueMethodWithName("foo"); + assertThat(foo, isPresent()); + assertTrue(foo.isStatic()); + assertTrue( + foo.streamInstructions().anyMatch( + i -> i.isInvokeVirtual() + && i.getMethod().toSourceString().contains("PrintStream.println"))); + } + + @NeverClassInline + static class Candidate { + private static class Host { + static final Candidate INSTANCE = new Candidate(); + } + + public static Candidate getInstance() { + return Host.INSTANCE; + } + + @NeverInline + public void foo(Object arg) { + System.out.println("Candidate#foo(" + (arg != null) + ")"); + } + } + + static class Main { + public static void main(String[] args) { + Candidate.getInstance().foo(null); + } + } +}
diff --git a/src/test/java/com/android/tools/r8/maindexlist/MainDexWithSynthesizedClassesTest.java b/src/test/java/com/android/tools/r8/maindexlist/MainDexWithSynthesizedClassesTest.java index 94e2a9c..d102004 100644 --- a/src/test/java/com/android/tools/r8/maindexlist/MainDexWithSynthesizedClassesTest.java +++ b/src/test/java/com/android/tools/r8/maindexlist/MainDexWithSynthesizedClassesTest.java
@@ -81,9 +81,7 @@ private void checkCompilationResult(D8TestCompileResult compileResult) throws Exception { if (parameters.getRuntime().asDex().getMinApiLevel().getLevel() < nativeMultiDexLevel.getLevel()) { - compileResult - .runDex2Oat(parameters.getRuntime().asDex().getVm()) - .assertNoVerificationErrors(); + compileResult.runDex2Oat(parameters.getRuntime()).assertNoVerificationErrors(); } else { compileResult.run(parameters.getRuntime(), TestClass.class).assertSuccessWithOutput(EXPECTED); }
diff --git a/src/test/java/com/android/tools/r8/naming/retrace/StackTrace.java b/src/test/java/com/android/tools/r8/naming/retrace/StackTrace.java index ce4df9d..d4b758e 100644 --- a/src/test/java/com/android/tools/r8/naming/retrace/StackTrace.java +++ b/src/test/java/com/android/tools/r8/naming/retrace/StackTrace.java
@@ -14,7 +14,6 @@ import com.android.tools.r8.retrace.RetraceCommand; import com.android.tools.r8.utils.StringUtils; import com.google.common.base.Equivalence; -import java.io.IOException; import java.util.ArrayList; import java.util.List; import java.util.function.Predicate; @@ -23,12 +22,82 @@ import org.hamcrest.Matcher; import org.hamcrest.TypeSafeMatcher; -class StackTrace { +public class StackTrace { public static String AT_PREFIX = "at "; public static String TAB_AT_PREFIX = "\t" + AT_PREFIX; - static class StackTraceLine { + public static class Builder { + + private List<StackTraceLine> stackTraceLines = new ArrayList<>(); + + private Builder() {} + + public Builder add(StackTraceLine line) { + stackTraceLines.add(line); + return this; + } + + public Builder addWithoutFileNameAndLineNumber(Class<?> clazz, String methodName) { + return addWithoutFileNameAndLineNumber(clazz.getTypeName(), methodName); + } + + public Builder addWithoutFileNameAndLineNumber(String className, String methodName) { + stackTraceLines.add( + StackTraceLine.builder().setClassName(className).setMethodName(methodName).build()); + return this; + } + + public StackTrace build() { + return new StackTrace( + stackTraceLines, + StringUtils.join( + stackTraceLines.stream().map(StackTraceLine::toString).collect(Collectors.toList()), + "\n")); + } + } + + public static Builder builder() { + return new Builder(); + } + + public static class StackTraceLine { + + public static class Builder { + private String className; + private String methodName; + private String fileName; + private int lineNumber = -1; + + private Builder() {} + + public Builder setClassName(String className) { + this.className = className; + return this; + } + + public Builder setMethodName(String methodName) { + this.methodName = methodName; + return this; + } + + public Builder setFileName(String fileName) { + this.fileName = fileName; + return this; + } + + public Builder setLineNumber(int lineNumber) { + this.lineNumber = lineNumber; + return this; + } + + public StackTraceLine build() { + String lineNumberPart = lineNumber >= 0 ? ":" + lineNumber : ""; + String originalLine = className + '.' + methodName + '(' + fileName + lineNumberPart + ')'; + return new StackTraceLine(originalLine, className, methodName, fileName, lineNumber); + } + } + public final String originalLine; public final String className; public final String methodName; @@ -44,6 +113,10 @@ this.lineNumber = lineNumber; } + public static Builder builder() { + return new Builder(); + } + public boolean hasLineNumber() { return lineNumber >= 0; } @@ -202,7 +275,7 @@ return extractFromJvm(result.getStdErr()); } - public StackTrace retrace(String map) throws IOException { + public StackTrace retrace(String map) { class Box { List<String> result; }
diff --git a/src/test/java/com/android/tools/r8/regress/b118075510/Regress118075510Runner.java b/src/test/java/com/android/tools/r8/regress/b118075510/Regress118075510Runner.java index 12bd452..e562b4f 100644 --- a/src/test/java/com/android/tools/r8/regress/b118075510/Regress118075510Runner.java +++ b/src/test/java/com/android/tools/r8/regress/b118075510/Regress118075510Runner.java
@@ -5,21 +5,39 @@ import com.android.tools.r8.AsmTestBase; import com.android.tools.r8.CompilationFailedException; +import com.android.tools.r8.CompilationMode; import com.android.tools.r8.D8TestCompileResult; +import com.android.tools.r8.TestParameters; import com.android.tools.r8.utils.AndroidApiLevel; import com.android.tools.r8.utils.StringUtils; import com.android.tools.r8.utils.codeinspector.CodeInspector; import com.android.tools.r8.utils.codeinspector.MethodSubject; import java.io.IOException; +import java.util.List; import java.util.concurrent.ExecutionException; import org.junit.Assert; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +@RunWith(Parameterized.class) public class Regress118075510Runner extends AsmTestBase { public static final Class<?> CLASS = Regress118075510Test.class; public static final String EXPECTED = StringUtils.lines("0", "0"); + private final TestParameters parameters; + + @Parameterized.Parameters(name = "{0}, {1}") + public static List<Object[]> data() { + return buildParameters( + getTestParameters().withDexRuntimes().withAllApiLevels().build(), CompilationMode.values()); + } + + public Regress118075510Runner(TestParameters parameters, CompilationMode mode) { + this.parameters = parameters; + } + @Test public void test() throws CompilationFailedException, IOException, ExecutionException, NoSuchMethodException { @@ -32,9 +50,9 @@ checkMethodContainsLongSignum(inspector, "fooNoTryCatch"); checkMethodContainsLongSignum(inspector, "fooWithTryCatch"); // Check the program runs on ART/Dalvik - d8Result.run(CLASS).assertSuccessWithOutput(EXPECTED); + d8Result.run(parameters.getRuntime(), CLASS).assertSuccessWithOutput(EXPECTED); // Check the program can be dex2oat compiled to arm64. This will diverge without the fixup. - d8Result.runDex2Oat().assertSuccess(); + d8Result.runDex2Oat(parameters.getRuntime()).assertSuccess(); } private void checkMethodContainsLongSignum(CodeInspector inspector, String methodName)
diff --git a/src/test/java/com/android/tools/r8/regress/b77842465/Regress77842465.java b/src/test/java/com/android/tools/r8/regress/b77842465/Regress77842465.java index 3ab3467..196ca52 100644 --- a/src/test/java/com/android/tools/r8/regress/b77842465/Regress77842465.java +++ b/src/test/java/com/android/tools/r8/regress/b77842465/Regress77842465.java
@@ -5,6 +5,8 @@ import com.android.tools.r8.AsmTestBase; import com.android.tools.r8.CompilationFailedException; +import com.android.tools.r8.TestRuntime.DexRuntime; +import com.android.tools.r8.ToolHelper; import com.android.tools.r8.utils.AndroidApiLevel; import java.io.IOException; import org.junit.Test; @@ -18,7 +20,7 @@ .noDesugaring() .setMinApi(AndroidApiLevel.M) .compile() - .runDex2Oat() + .runDex2Oat(new DexRuntime(ToolHelper.getDexVm())) .assertSuccess(); } }
diff --git a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfPrivateStaticMethodTest.java b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfPrivateStaticMethodTest.java index e914a7b..ccdb49b 100644 --- a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfPrivateStaticMethodTest.java +++ b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfPrivateStaticMethodTest.java
@@ -84,7 +84,7 @@ public void lookupSingleTarget() { ResolutionResult resolutionResult = appInfo.resolveMethodOnClass(methodOnB.holder, methodOnB); assertFalse(resolutionResult.isValidVirtualTarget(appInfo.app().options)); - DexEncodedMethod resolved = resolutionResult.asSingleTarget(); + DexEncodedMethod resolved = resolutionResult.getSingleTarget(); assertEquals(methodOnA, resolved.method); DexEncodedMethod singleVirtualTarget = appInfo.lookupSingleVirtualTarget(methodOnB, methodOnB.holder); @@ -94,7 +94,7 @@ @Test public void lookupVirtualTargets() { ResolutionResult resolutionResult = appInfo.resolveMethodOnClass(methodOnB.holder, methodOnB); - DexEncodedMethod resolved = resolutionResult.asSingleTarget(); + DexEncodedMethod resolved = resolutionResult.getSingleTarget(); assertEquals(methodOnA, resolved.method); assertFalse(resolutionResult.isValidVirtualTarget(appInfo.app().options)); }
diff --git a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfPrivateStaticMethodWithVirtualParentTest.java b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfPrivateStaticMethodWithVirtualParentTest.java index 8d867d8..6e87ef3 100644 --- a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfPrivateStaticMethodWithVirtualParentTest.java +++ b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfPrivateStaticMethodWithVirtualParentTest.java
@@ -143,7 +143,7 @@ @Test public void lookupVirtualTargets() { ResolutionResult resolutionResult = appInfo.resolveMethodOnClass(methodOnB.holder, methodOnB); - DexEncodedMethod resolved = resolutionResult.asSingleTarget(); + DexEncodedMethod resolved = resolutionResult.getSingleTarget(); assertEquals(methodOnA, resolved.method); assertFalse(resolutionResult.isValidVirtualTarget(appInfo.app().options)); } @@ -165,7 +165,7 @@ .setMinApi(parameters.getApiLevel()) .run(parameters.getRuntime(), Main.class); } - checkResult(runResult); + checkResult(runResult, false); } @Test @@ -177,11 +177,11 @@ .addKeepMainRule(Main.class) .setMinApi(parameters.getApiLevel()) .run(parameters.getRuntime(), Main.class); - checkResult(runResult); + checkResult(runResult, true); } - private void checkResult(TestRunResult<?> runResult) { - if (expectedToIncorrectlyRun(parameters.getRuntime())) { + private void checkResult(TestRunResult<?> runResult, boolean isCorrectedByR8) { + if (expectedToIncorrectlyRun(parameters.getRuntime(), isCorrectedByR8)) { // Do to incorrect resolution, some Art VMs will resolve to Base.f (ignoring A.f) and thus // virtual dispatch to C.f. See b/140013075. runResult.assertSuccessWithOutputLines("Called C.f"); @@ -190,8 +190,9 @@ } } - private boolean expectedToIncorrectlyRun(TestRuntime runtime) { - return runtime.isDex() + private boolean expectedToIncorrectlyRun(TestRuntime runtime, boolean isCorrectedByR8) { + return !isCorrectedByR8 + && runtime.isDex() && runtime.asDex().getVm().isNewerThan(DexVm.ART_4_4_4_HOST) && runtime.asDex().getVm().isOlderThanOrEqual(DexVm.ART_7_0_0_HOST); }
diff --git a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentInterfaceTest.java b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentInterfaceTest.java index f968c03..7c435d3 100644 --- a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentInterfaceTest.java +++ b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentInterfaceTest.java
@@ -121,7 +121,7 @@ public void lookupSingleTarget() { ResolutionResult resolutionResult = appInfo.resolveMethodOnInterface(methodOnB.holder, methodOnB); - DexEncodedMethod resolved = resolutionResult.asSingleTarget(); + DexEncodedMethod resolved = resolutionResult.getSingleTarget(); assertEquals(methodOnB, resolved.method); assertFalse(resolutionResult.isValidVirtualTarget(appInfo.app().options)); DexEncodedMethod singleVirtualTarget = @@ -132,7 +132,7 @@ @Test public void lookupVirtualTargets() { ResolutionResult resolutionResult = appInfo.resolveMethodOnInterface(methodOnB.holder, methodOnB); - DexEncodedMethod resolved = resolutionResult.asSingleTarget(); + DexEncodedMethod resolved = resolutionResult.getSingleTarget(); assertEquals(methodOnB, resolved.method); assertFalse(resolutionResult.isValidVirtualTarget(appInfo.app().options)); }
diff --git a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentTest.java b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentTest.java index 20c3cab..968f21e 100644 --- a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentTest.java +++ b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentTest.java
@@ -167,7 +167,7 @@ @Test public void lookupSingleTarget() { ResolutionResult resolutionResult = appInfo.resolveMethodOnClass(methodOnB.holder, methodOnB); - DexEncodedMethod resolved = resolutionResult.asSingleTarget(); + DexEncodedMethod resolved = resolutionResult.getSingleTarget(); assertEquals(methodOnA, resolved.method); assertFalse(resolutionResult.isValidVirtualTarget(appInfo.app().options)); DexEncodedMethod singleVirtualTarget = @@ -200,7 +200,7 @@ .setMinApi(parameters.getApiLevel()) .run(parameters.getRuntime(), Main.class); } - checkResult(runResult); + checkResult(runResult, false); } @Test @@ -212,11 +212,11 @@ .addKeepMainRule(Main.class) .setMinApi(parameters.getApiLevel()) .run(parameters.getRuntime(), Main.class); - checkResult(runResult); + checkResult(runResult, true); } - private void checkResult(TestRunResult<?> runResult) { - if (expectedToIncorrectlyRun(parameters.getRuntime())) { + private void checkResult(TestRunResult<?> runResult, boolean isCorrectedByR8) { + if (expectedToIncorrectlyRun(parameters.getRuntime(), isCorrectedByR8)) { // Do to incorrect resolution, some Art VMs will resolve to Base.f (ignoring A.f) and thus // virtual dispatch to C.f. See b/140013075. runResult.assertSuccessWithOutputLines("Called C.f"); @@ -225,8 +225,9 @@ } } - private boolean expectedToIncorrectlyRun(TestRuntime runtime) { - return runtime.isDex() + private boolean expectedToIncorrectlyRun(TestRuntime runtime, boolean isCorrectedByR8) { + return !isCorrectedByR8 + && runtime.isDex() && runtime.asDex().getVm().isNewerThan(DexVm.ART_4_4_4_HOST) && runtime.asDex().getVm().isOlderThanOrEqual(DexVm.ART_7_0_0_HOST); }
diff --git a/src/test/java/com/android/tools/r8/retrace/stacktraces/InlineWithoutNullCheck.java b/src/test/java/com/android/tools/r8/retrace/stacktraces/InlineWithoutNullCheck.java new file mode 100644 index 0000000..6126c4b --- /dev/null +++ b/src/test/java/com/android/tools/r8/retrace/stacktraces/InlineWithoutNullCheck.java
@@ -0,0 +1,267 @@ +// Copyright (c) 2019, 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.retrace.stacktraces; + +import static com.android.tools.r8.naming.retrace.StackTrace.isSame; +import static com.android.tools.r8.naming.retrace.StackTrace.isSameExceptForFileNameAndLineNumber; +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.AlwaysInline; +import com.android.tools.r8.NeverInline; +import com.android.tools.r8.NeverPropagateValue; +import com.android.tools.r8.TestBase; +import com.android.tools.r8.TestParameters; +import com.android.tools.r8.TestParametersCollection; +import com.android.tools.r8.naming.retrace.StackTrace; +import com.android.tools.r8.utils.codeinspector.ClassSubject; +import com.android.tools.r8.utils.codeinspector.CodeInspector; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +@RunWith(Parameterized.class) +public class InlineWithoutNullCheck extends TestBase { + + private final TestParameters parameters; + + @Parameterized.Parameters(name = "{0}") + public static TestParametersCollection data() { + return getTestParameters().withAllRuntimes().withAllApiLevels().build(); + } + + public InlineWithoutNullCheck(TestParameters parameters) { + this.parameters = parameters; + } + + public StackTrace expectedStackTraceForInlineMethod; + public StackTrace expectedStackTraceForInlineField; + public StackTrace expectedStackTraceForInlineStaticField; + + @Before + public void setup() throws Exception { + // Get the expected stack traces by running on the runtime to test. + expectedStackTraceForInlineMethod = + testForRuntime(parameters.getRuntime(), parameters.getApiLevel()) + .addInnerClasses(InlineWithoutNullCheck.class) + .run(parameters.getRuntime(), TestClassForInlineMethod.class) + .writeProcessResult(System.out) + .assertFailure() + .getStackTrace(); + expectedStackTraceForInlineField = + testForRuntime(parameters.getRuntime(), parameters.getApiLevel()) + .addInnerClasses(InlineWithoutNullCheck.class) + .run(parameters.getRuntime(), TestClassForInlineField.class) + .writeProcessResult(System.out) + .assertFailure() + .getStackTrace(); + expectedStackTraceForInlineStaticField = + testForRuntime(parameters.getRuntime(), parameters.getApiLevel()) + .addInnerClasses(InlineWithoutNullCheck.class) + .run(parameters.getRuntime(), TestClassForInlineStaticField.class) + .writeProcessResult(System.out) + .assertFailure() + .getStackTrace(); + + // Check the expected stack traces from running on the runtime to test. + assertThat( + expectedStackTraceForInlineMethod, + isSameExceptForFileNameAndLineNumber( + StackTrace.builder() + .addWithoutFileNameAndLineNumber(A.class, "inlineMethodWhichAccessInstanceMethod") + .addWithoutFileNameAndLineNumber(TestClassForInlineMethod.class, "main") + .build())); + assertThat( + expectedStackTraceForInlineField, + isSameExceptForFileNameAndLineNumber( + StackTrace.builder() + .addWithoutFileNameAndLineNumber(A.class, "inlineMethodWhichAccessInstanceField") + .addWithoutFileNameAndLineNumber(TestClassForInlineField.class, "main") + .build())); + assertThat( + expectedStackTraceForInlineStaticField, + isSameExceptForFileNameAndLineNumber( + StackTrace.builder() + .addWithoutFileNameAndLineNumber(A.class, "inlineMethodWhichAccessStaticField") + .addWithoutFileNameAndLineNumber(TestClassForInlineStaticField.class, "main") + .build())); + } + + private void checkSomething(CodeInspector inspector) { + ClassSubject classSubject = inspector.clazz(Result.class); + assertThat(classSubject, isPresent()); + assertThat( + classSubject.uniqueMethodWithName("methodWhichAccessInstanceMethod"), not(isPresent())); + assertThat( + classSubject.uniqueMethodWithName("methodWhichAccessInstanceField"), not(isPresent())); + assertThat(classSubject.uniqueMethodWithName("methodWhichAccessStaticField"), not(isPresent())); + } + + @Test + public void testInlineMethodWhichChecksNullReceiverBeforeAnySideEffectMethod() throws Exception { + testForR8(parameters.getBackend()) + .addInnerClasses(InlineWithoutNullCheck.class) + .addKeepMainRule(TestClassForInlineMethod.class) + .enableInliningAnnotations() + .setMinApi(parameters.getApiLevel()) + .compile() + .inspect(this::checkSomething) + .run(parameters.getRuntime(), TestClassForInlineMethod.class) + .assertFailure() + // TODO(b/143607166): The stack trace has one more frame on the top than expected. + .inspectStackTrace( + stackTrace -> assertThat(expectedStackTraceForInlineMethod, not(isSame(stackTrace)))) + .inspectStackTrace( + stackTrace -> + assertThat( + stackTrace, + isSameExceptForFileNameAndLineNumber( + StackTrace.builder() + .addWithoutFileNameAndLineNumber( + Result.class, "methodWhichAccessInstanceMethod") + .addWithoutFileNameAndLineNumber( + A.class, "inlineMethodWhichAccessInstanceMethod") + .addWithoutFileNameAndLineNumber(TestClassForInlineMethod.class, "main") + .build()))); + } + + @Test + public void testInlineMethodWhichChecksNullReceiverBeforeAnySideEffectField() throws Exception { + testForR8(parameters.getBackend()) + .addInnerClasses(InlineWithoutNullCheck.class) + .addKeepMainRule(TestClassForInlineField.class) + .enableInliningAnnotations() + .setMinApi(parameters.getApiLevel()) + .compile() + .inspect(this::checkSomething) + .run(parameters.getRuntime(), TestClassForInlineField.class) + .assertFailure() + // TODO(b/143607166): The stack trace has one more frame on the top than expected. + .inspectStackTrace( + stackTrace -> assertThat(expectedStackTraceForInlineField, not(isSame(stackTrace)))) + .inspectStackTrace( + stackTrace -> + assertThat( + stackTrace, + isSameExceptForFileNameAndLineNumber( + StackTrace.builder() + .addWithoutFileNameAndLineNumber( + Result.class, "methodWhichAccessInstanceField") + .addWithoutFileNameAndLineNumber( + A.class, "inlineMethodWhichAccessInstanceField") + .addWithoutFileNameAndLineNumber(TestClassForInlineField.class, "main") + .build()))); + } + + @Test + public void testInlineMethodWhichChecksNullReceiverBeforeAnySideEffectStaticField() + throws Exception { + testForR8(parameters.getBackend()) + .addInnerClasses(InlineWithoutNullCheck.class) + .addKeepMainRule(TestClassForInlineStaticField.class) + .enableInliningAnnotations() + .setMinApi(parameters.getApiLevel()) + .compile() + .inspect(this::checkSomething) + .run(parameters.getRuntime(), TestClassForInlineStaticField.class) + .assertFailure() + // TODO(b/143607166): The stack trace has one more frame on the top than expected. + .inspectStackTrace( + stackTrace -> + assertThat(expectedStackTraceForInlineStaticField, not(isSame(stackTrace)))) + .inspectStackTrace( + stackTrace -> + assertThat( + stackTrace, + isSameExceptForFileNameAndLineNumber( + StackTrace.builder() + .addWithoutFileNameAndLineNumber( + Result.class, "methodWhichAccessStaticField") + .addWithoutFileNameAndLineNumber( + A.class, "inlineMethodWhichAccessStaticField") + .addWithoutFileNameAndLineNumber( + TestClassForInlineStaticField.class, "main") + .build()))); + } + + static class TestClassForInlineMethod { + + public static void main(String[] args) { + A a = new A(System.currentTimeMillis() > 0 ? null : new Result()); + System.out.print(a.inlineMethodWhichAccessInstanceMethod()); + } + } + + static class TestClassForInlineField { + + public static void main(String[] args) { + A a = new A(System.currentTimeMillis() > 0 ? null : new Result()); + System.out.print(a.inlineMethodWhichAccessInstanceField()); + } + } + + static class TestClassForInlineStaticField { + + public static void main(String[] args) { + A a = new A(System.currentTimeMillis() > 0 ? null : new Result()); + System.out.print(a.inlineMethodWhichAccessStaticField()); + } + } + + static class A { + @NeverPropagateValue Result result; + + A(Result result) { + this.result = result; + } + + @NeverInline + Result get() { + return result; + } + + @NeverInline + int inlineMethodWhichAccessInstanceMethod() { + return get().methodWhichAccessInstanceMethod(); + } + + @NeverInline + int inlineMethodWhichAccessInstanceField() { + return get().methodWhichAccessInstanceField(); + } + + @NeverInline + int inlineMethodWhichAccessStaticField() { + return get().methodWhichAccessStaticField(); + } + } + + static class Result { + @NeverPropagateValue int x = 1; + @NeverPropagateValue static int y = 1; + + @AlwaysInline + int methodWhichAccessInstanceMethod() { + return privateMethod(); + } + + @AlwaysInline + int methodWhichAccessInstanceField() { + return x; + } + + @AlwaysInline + int methodWhichAccessStaticField() { + return Result.y; + } + + @NeverInline + private int privateMethod() { + return 0; + } + } +}
diff --git a/src/test/java/com/android/tools/r8/rewrite/enums/EnumOptimizationTest.java b/src/test/java/com/android/tools/r8/rewrite/enums/EnumOptimizationTest.java index 73c42d0..3369fb4 100644 --- a/src/test/java/com/android/tools/r8/rewrite/enums/EnumOptimizationTest.java +++ b/src/test/java/com/android/tools/r8/rewrite/enums/EnumOptimizationTest.java
@@ -3,7 +3,6 @@ // BSD-style license that can be found in the LICENSE file. package com.android.tools.r8.rewrite.enums; -import static com.android.tools.r8.ToolHelper.getDefaultAndroidJar; import static java.util.Collections.emptyList; import static java.util.stream.Collectors.toList; import static org.junit.Assert.assertEquals; @@ -30,12 +29,14 @@ */ @RunWith(Parameterized.class) public class EnumOptimizationTest extends TestBase { + private final boolean enableOptimization; private final TestParameters parameters; @Parameters(name = "{1}, enable enum optimization: {0}") public static List<Object[]> data() { - return buildParameters(BooleanUtils.values(), getTestParameters().withAllRuntimes().build()); + return buildParameters( + BooleanUtils.values(), getTestParameters().withAllRuntimesAndApiLevels().build()); } public EnumOptimizationTest(boolean enableOptimization, TestParameters parameters) { @@ -50,12 +51,11 @@ @Test public void ordinals() throws Exception { testForR8(parameters.getBackend()) - .addLibraryFiles(getDefaultAndroidJar()) .addProgramClassesAndInnerClasses(Ordinals.class) .addKeepMainRule(Ordinals.class) .enableInliningAnnotations() .addOptionsModification(this::configure) - .setMinApi(parameters.getRuntime()) + .setMinApi(parameters.getApiLevel()) .compile() .inspect(this::inspectOrdinals) .run(parameters.getRuntime(), Ordinals.class) @@ -96,12 +96,11 @@ @Test public void names() throws Exception { testForR8(parameters.getBackend()) - .addLibraryFiles(getDefaultAndroidJar()) .addProgramClassesAndInnerClasses(Names.class) .addKeepMainRule(Names.class) .enableInliningAnnotations() .addOptionsModification(this::configure) - .setMinApi(parameters.getRuntime()) + .setMinApi(parameters.getApiLevel()) .compile() .inspect(this::inspectNames) .run(parameters.getRuntime(), Names.class) @@ -139,17 +138,17 @@ @Test public void toStrings() throws Exception { testForR8(parameters.getBackend()) - .addLibraryFiles(getDefaultAndroidJar()) .addProgramClassesAndInnerClasses(ToStrings.class) .addKeepMainRule(ToStrings.class) .enableInliningAnnotations() .addOptionsModification(this::configure) - .setMinApi(parameters.getRuntime()) + .setMinApi(parameters.getApiLevel()) .compile() .inspect(this::inspectToStrings) .run(parameters.getRuntime(), ToStrings.class) - .assertSuccessWithOutputLines("one", "one", "TWO", "TWO", "TWO", "1TWO", "TWO", "SECONDS", - "DOWN", "TWO", "TWO", "TWO"); + .assertSuccessWithOutputLines( + "one", "one", "TWO", "TWO", "TWO", "1TWO", "TWO", "SECONDS", "DOWN", "TWO", "TWO", + "TWO"); } private void inspectToStrings(CodeInspector inspector) { @@ -157,7 +156,11 @@ assertTrue(clazz.isPresent()); assertToStringWasNotReplaced(clazz.uniqueMethodWithName("typeToString")); - assertToStringWasNotReplaced(clazz.uniqueMethodWithName("valueWithToString")); + if (parameters.isCfRuntime()) { + assertToStringWasNotReplaced(clazz.uniqueMethodWithName("valueWithToString")); + } else { + assertToStringReplacedWithConst(clazz.uniqueMethodWithName("valueWithToString"), "one"); + } assertToStringWasNotReplaced(clazz.uniqueMethodWithName("valueWithoutToString")); if (enableOptimization) {
diff --git a/src/test/java/com/android/tools/r8/shaking/b136195382/AbstractBridgeInheritTest.java b/src/test/java/com/android/tools/r8/shaking/b136195382/AbstractBridgeInheritTest.java index 2514381..f1fc012 100644 --- a/src/test/java/com/android/tools/r8/shaking/b136195382/AbstractBridgeInheritTest.java +++ b/src/test/java/com/android/tools/r8/shaking/b136195382/AbstractBridgeInheritTest.java
@@ -27,7 +27,7 @@ @Parameters(name = "{0}") public static TestParametersCollection data() { - return getTestParameters().withAllRuntimes().build(); + return getTestParameters().withAllRuntimesAndApiLevels().build(); } public AbstractBridgeInheritTest(TestParameters parameters) { @@ -42,6 +42,7 @@ Service.class, Factory.class, SubService.class, SubFactory.class, Main.class) .addKeepMainRule(Main.class) .enableInliningAnnotations() + .setMinApi(parameters.getApiLevel()) .run(parameters.getRuntime(), Main.class) .assertSuccessWithOutputLines("Hello World!") .inspector();
diff --git a/src/test/java/com/android/tools/r8/shaking/reflection/GetDeclaredFieldShouldMarkFieldAsWrittenTest.java b/src/test/java/com/android/tools/r8/shaking/reflection/GetDeclaredFieldShouldMarkFieldAsWrittenTest.java new file mode 100644 index 0000000..0efa7fa --- /dev/null +++ b/src/test/java/com/android/tools/r8/shaking/reflection/GetDeclaredFieldShouldMarkFieldAsWrittenTest.java
@@ -0,0 +1,48 @@ +// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +package com.android.tools.r8.shaking.reflection; + +import com.android.tools.r8.TestBase; +import com.android.tools.r8.TestParameters; +import com.android.tools.r8.TestParametersCollection; +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 GetDeclaredFieldShouldMarkFieldAsWrittenTest extends TestBase { + + private final TestParameters parameters; + + @Parameters(name = "{0}") + public static TestParametersCollection data() { + return getTestParameters().withAllRuntimesAndApiLevels().build(); + } + + public GetDeclaredFieldShouldMarkFieldAsWrittenTest(TestParameters parameters) { + this.parameters = parameters; + } + + @Test + public void test() throws Exception { + testForR8(parameters.getBackend()) + .addProgramClasses(TestClass.class) + .addKeepMainRule(TestClass.class) + .setMinApi(parameters.getApiLevel()) + .run(parameters.getRuntime(), TestClass.class) + .assertSuccessWithOutputLines("42"); + } + + static class TestClass { + + static int f = -1; + + public static void main(String[] args) throws Exception { + TestClass.class.getDeclaredField("f").set(null, 42); + System.out.println(f); + } + } +}
diff --git a/src/test/java/com/android/tools/r8/testing/StackTraceTest.java b/src/test/java/com/android/tools/r8/testing/StackTraceTest.java new file mode 100644 index 0000000..33915ef --- /dev/null +++ b/src/test/java/com/android/tools/r8/testing/StackTraceTest.java
@@ -0,0 +1,224 @@ +// Copyright (c) 2019, 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.testing; + +import static com.android.tools.r8.naming.retrace.StackTrace.isSame; +import static com.android.tools.r8.naming.retrace.StackTrace.isSameExceptForFileName; +import static com.android.tools.r8.naming.retrace.StackTrace.isSameExceptForFileNameAndLineNumber; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assume.assumeTrue; + +import com.android.tools.r8.TestBase; +import com.android.tools.r8.TestParameters; +import com.android.tools.r8.TestParametersCollection; +import com.android.tools.r8.ToolHelper.DexVm; +import com.android.tools.r8.naming.retrace.StackTrace; +import com.android.tools.r8.naming.retrace.StackTrace.StackTraceLine; +import com.android.tools.r8.utils.StringUtils; +import com.google.common.collect.ImmutableList; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +@RunWith(Parameterized.class) +public class StackTraceTest extends TestBase { + + private final TestParameters parameters; + + @Parameterized.Parameters(name = "{0}") + public static TestParametersCollection data() { + return getTestParameters().withAllRuntimes().withAllApiLevels().build(); + } + + public StackTraceTest(TestParameters parameters) { + this.parameters = parameters; + } + + @Test + public void testJvmStackTrace() throws Exception { + String stderr = + StringUtils.join( + ImmutableList.of( + "Exception in thread \"main\" java.lang.RuntimeException", + "\tat com.example.A.method2(Test.java:30)", + "\tat com.example.A.method1(Test.java:20)", + "\tat com.example.Main.main(Test.java:10)"), + "\n"); + checkStackTrace(StackTrace.extractFromJvm(stderr)); + } + + @Test + public void testArtStackTrace1() { + String stderr = + StringUtils.join( + "\n", + "dex2oat I 10-30 11:41:40 232588 232588 dex2oat.cc:3108]" + + " /usr/local/prj/r8-public/ws1/tools/linux/art/bin/../bin/dex2oat" + + " --instruction-set=x86_64" + + " --instruction-set-features=ssse3,sse4.1,sse4.2,-avx,-avx2,popcnt --runtime-arg" + + " -Xnorelocate --host" + + " --boot-image=/usr/local/prj/r8-public/ws1/tools/linux/art/bin/../framework/core.art" + + " --dex-file=/tmp/junit5857866495600860150/junit14865793933637294589/out.zip" + + " --output-vdex-fd=7 --oat-fd=8" + + " --oat-location=/tmp/junit5857866495600860150/junit14865793933637294589/oat/x86_64/out.odex" + + " --compiler-filter=quicken --class-loader-context=PCL[]\n", + "dex2oat I 10-30 11:41:40 232588 232588 dex2oat.cc:2808] dex2oat took 94.860ms" + + " (71.941ms cpu) (threads: 72) arena alloc=3KB (3312B) java alloc=32KB (32800B)" + + " native alloc=440KB (450720B) free=9MB (9539424B)", + "Exception in thread \"main\" java.lang.RuntimeException", + "\tat com.example.A.method2(Test.java:30)", + "\tat com.example.A.method1(Test.java:20)", + "\tat com.example.Main.main(Test.java:10)"); + checkStackTrace(StackTrace.extractFromArt(stderr, DexVm.ART_5_1_1_HOST)); + } + + @Test + public void testArtStackTrace2() { + String stderr = + StringUtils.join( + "\n", + "dex2oat I 10-30 11:41:40 232588 232588 dex2oat.cc:3108]" + + " /usr/local/prj/r8-public/ws1/tools/linux/art/bin/../bin/dex2oat" + + " --instruction-set=x86_64" + + " --instruction-set-features=ssse3,sse4.1,sse4.2,-avx,-avx2,popcnt --runtime-arg" + + " -Xnorelocate --host" + + " --boot-image=/usr/local/prj/r8-public/ws1/tools/linux/art/bin/../framework/core.art" + + " --dex-file=/tmp/junit5857866495600860150/junit14865793933637294589/out.zip" + + " --output-vdex-fd=7 --oat-fd=8" + + " --oat-location=/tmp/junit5857866495600860150/junit14865793933637294589/oat/x86_64/out.odex" + + " --compiler-filter=quicken --class-loader-context=PCL[]\n", + "Exception in thread \"main\" java.lang.RuntimeException", + "\tat com.example.A.method2(Test.java:30)", + "\tat com.example.A.method1(Test.java:20)", + "\tat com.example.Main.main(Test.java:10)", + "dex2oat I 10-30 11:41:40 232588 232588 dex2oat.cc:2808] dex2oat took 94.860ms" + + " (71.941ms cpu) (threads: 72) arena alloc=3KB (3312B) java alloc=32KB (32800B)" + + " native alloc=440KB (450720B) free=9MB (9539424B)" + ); + checkStackTrace(StackTrace.extractFromArt(stderr, DexVm.ART_5_1_1_HOST)); + } + + private void checkStackTrace(StackTrace stackTrace) { + assertThat( + stackTrace, + isSame( + StackTrace.builder() + .add( + StackTraceLine.builder() + .setClassName("com.example.A") + .setMethodName("method2") + .setFileName("Test.java") + .setLineNumber(30) + .build()) + .add( + StackTraceLine.builder() + .setClassName("com.example.A") + .setMethodName("method1") + .setFileName("Test.java") + .setLineNumber(20) + .build()) + .add( + StackTraceLine.builder() + .setClassName("com.example.Main") + .setMethodName("main") + .setFileName("Test.java") + .setLineNumber(10) + .build()) + .build())); + + assertThat( + stackTrace, + isSameExceptForFileName( + StackTrace.builder() + .add( + StackTraceLine.builder() + .setClassName("com.example.A") + .setMethodName("method2") + .setLineNumber(30) + .build()) + .add( + StackTraceLine.builder() + .setClassName("com.example.A") + .setMethodName("method1") + .setLineNumber(20) + .build()) + .add( + StackTraceLine.builder() + .setClassName("com.example.Main") + .setMethodName("main") + .setLineNumber(10) + .build()) + .build())); + + assertThat( + stackTrace, + isSameExceptForFileNameAndLineNumber( + StackTrace.builder() + .add( + StackTraceLine.builder() + .setClassName("com.example.A") + .setMethodName("method2") + .build()) + .add( + StackTraceLine.builder() + .setClassName("com.example.A") + .setMethodName("method1") + .build()) + .add( + StackTraceLine.builder() + .setClassName("com.example.Main") + .setMethodName("main") + .build()) + .build())); + } + + @Test + public void testJvmStackTraceFromRunning() throws Exception { + assumeTrue(parameters.isCfRuntime()); + testForJvm() + .addInnerClasses(StackTraceTest.class) + .run(parameters.getRuntime(), Main.class) + .assertFailure() + .inspectStackTrace(this::checkStackTraceFromRunning); + } + + @Test + public void testArtStackTraceFromRunning() throws Exception { + assumeTrue(parameters.isDexRuntime()); + testForD8() + .addInnerClasses(StackTraceTest.class) + .setMinApi(parameters.getApiLevel()) + .run(parameters.getRuntime(), Main.class) + .assertFailure() + .inspectStackTrace(this::checkStackTraceFromRunning); + } + + private void checkStackTraceFromRunning(StackTrace stackTrace) { + assertThat( + stackTrace, + isSameExceptForFileNameAndLineNumber( + StackTrace.builder() + .addWithoutFileNameAndLineNumber(A.class, "method2") + .addWithoutFileNameAndLineNumber(A.class, "method1") + .addWithoutFileNameAndLineNumber(Main.class, "main") + .build())); + } + + public static class Main { + public static void main(String[] args) { + new A().method1(); + } + } + + static class A { + public void method1() { + method2(); + } + + public void method2() { + throw new RuntimeException(); + } + } +}
diff --git a/tools/internal_test.py b/tools/internal_test.py index 9d4bc2b..182ccf8 100755 --- a/tools/internal_test.py +++ b/tools/internal_test.py
@@ -79,6 +79,9 @@ 'find-xmx-max': 1200, 'find-xmx-range': 32, 'oom-threshold': 1037, + # TODO(b/143431825): Youtube can OOM randomly in memory configurations + # that should work. + 'skip-find-xmx-max': True, }, { 'app': 'iosched', @@ -108,7 +111,7 @@ '--find-min-xmx-archive'] def compile_with_memory_max_command(record): - return [ + return [] if 'skip-find-xmx-max' in record else [ 'tools/run_on_app.py', '--compiler=r8', '--compiler-build=lib', @@ -144,7 +147,6 @@ + map(compile_with_memory_max_command, BENCHMARK_APPS) + map(compile_with_memory_min_command, BENCHMARK_APPS)) - # Command timeout, in seconds. RUN_TIMEOUT = 3600 * 6 BOT_RUN_TIMEOUT = RUN_TIMEOUT * len(TEST_COMMANDS) @@ -349,6 +351,9 @@ print 'stdout: %s' % f.read() def execute(cmd, archive, env=None): + if cmd == []: + return + utils.PrintCmd(cmd) with utils.TempDir() as temp: try:
diff --git a/tools/r8_release.py b/tools/r8_release.py index 3575edf..0048d52 100755 --- a/tools/r8_release.py +++ b/tools/r8_release.py
@@ -75,16 +75,25 @@ sed(old_version, version, R8_VERSION_FILE) subprocess.check_call([ - 'git', 'commit', '-a', '-m', '"Version %s"' % version]) + 'git', 'commit', '-a', '-m', 'Version %s' % version]) version_diff_output = subprocess.check_output([ 'git', 'diff', '%s..HEAD' % commithash]) invalid = version_change_diff(version_diff_output, "master", version) if invalid: - print "Unexpected diff content for line:" - print invalid - sys.exit(1) + print "Unexpected diff:" + print "=" * 80 + print version_diff_output + print "=" * 80 + accept_string = 'THE DIFF IS OK!' + input = raw_input( + "Accept the additonal diff as part of the release? " + "Type '%s' to accept: " % accept_string) + if input != accept_string: + print "You did not type '%s'" % accept_string + print 'Aborting dev release for %s' % version + sys.exit(1) # Double check that we want to push the release. if not args.dry_run:
diff --git a/tools/run_on_as_app.py b/tools/run_on_as_app.py index e4ab909..8d7264b 100755 --- a/tools/run_on_as_app.py +++ b/tools/run_on_as_app.py
@@ -402,6 +402,18 @@ }) ] + +class EnsureNoGradleAlive(object): + def __init__(self, active): + self.active = active + + def __enter__(self): + if self.active: + print 'Running with wrapper that will kill java after' + + def __exit__(self, *_): + subprocess.call(['pkill', 'java']) + def GetAllApps(): apps = [] for repo in APP_REPOSITORIES: @@ -783,6 +795,9 @@ profile_dest_dir = os.path.join(out_dir, 'profile') as_utils.MoveProfileReportTo(profile_dest_dir, stdout, quiet=options.quiet) + # Ensure that the gradle daemon is stopped if we are running with it. + if options.use_daemon: + utils.RunGradlew(['--stop', '-g=' + os.path.join(checkout_dir, GRADLE_USER_HOME)]) return (apk_dest, profile_dest_dir, proguard_config_dest) @@ -1289,13 +1304,14 @@ shutil.copyfile(utils.R8LIB_JAR, os.path.join(temp_dir, 'r8lib.jar')) result_per_shrinker_per_app = [] - - for (app, repo) in options.apps: - if app.skip: - continue - result_per_shrinker_per_app.append( - (app, GetResultsForApp(app, repo, options, temp_dir))) - + # If we are running on golem we kill all java processes after the run + # to ensure no hanging gradle daemons. + with EnsureNoGradleAlive(options.golem): + for (app, repo) in options.apps: + if app.skip: + continue + result_per_shrinker_per_app.append( + (app, GetResultsForApp(app, repo, options, temp_dir))) return LogResultsForApps(result_per_shrinker_per_app, options) def success(message):