Merge commit '9e7ce5ac7cde29d9c9e806143cc45b1bd9c0302b' into dev-release
diff --git a/LIBRARY-LICENSE b/LIBRARY-LICENSE index adf291d..2cedeee 100644 --- a/LIBRARY-LICENSE +++ b/LIBRARY-LICENSE
@@ -26,11 +26,6 @@ license: Apache License, Version 2.0 licenseUrl: http://www.apache.org/licenses/LICENSE-2.0.html url: http://fasutil.di.unimi.it/ -- artifact: net.sf.jopt-simple:jopt-simple:+ - name: JOpt Simple - license: The MIT License - licenseUrl: http://www.opensource.org/licenses/mit-license.php - url: http://pholser.github.com/jopt-simple - artifact: org.ow2.asm:asm-commons:+ name: ASM Commons copyrightHolder: INRIA, France Telecom
diff --git a/PRESUBMIT.py b/PRESUBMIT.py index b916f47..a329c0f 100644 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py
@@ -33,10 +33,11 @@ continue diff = check_output( ['git', 'diff', '--no-prefix', '-U0', branch, '--', path]) + proc = Popen(FMT_CMD, stdin=PIPE, stdout=PIPE, stderr=STDOUT) (stdout, stderr) = proc.communicate(input=diff) if len(stdout) > 0: - results.append(output_api.PresubmitError(stdout)) + results.append(output_api.PresubmitError(stdout.decode('utf-8'))) if len(results) > 0: results.append(output_api.PresubmitError( """Please fix the formatting by running: @@ -69,7 +70,7 @@ if not path.endswith('InternalOptions.java'): continue diff = check_output( - ['git', 'diff', '--no-prefix', '-U0', branch, '--', path]) + ['git', 'diff', '--no-prefix', '-U0', branch, '--', path]).decode('utf-8') if 'DETERMINISTIC_DEBUGGING' in diff: return [output_api.PresubmitError(diff)] return []
diff --git a/build.gradle b/build.gradle index 5df3bb4..35cc8df 100644 --- a/build.gradle +++ b/build.gradle
@@ -11,14 +11,6 @@ import tasks.DownloadDependency import tasks.GetJarsFromConfiguration -buildscript { - repositories { - google() - mavenCentral() - gradlePluginPortal() - } -} - plugins { id "net.ltgt.errorprone" version "2.0.2" } @@ -27,6 +19,8 @@ apply plugin: 'idea' ext { + // When updating dependencies also update and run + // tools/create_local_maven_with_dependencies.py androidSupportVersion = '25.4.0' asmVersion = '9.5' // When updating update tools/asmifier.py, build.src and Toolhelper as well. javassistVersion = '3.29.2-GA' @@ -46,8 +40,9 @@ } repositories { - google() - mavenCentral() + maven { + url uri('file:third_party/dependencies') + } } if (project.hasProperty('with_code_coverage')) {
diff --git a/buildSrc/build.gradle b/buildSrc/build.gradle index 85815bd..dc6f3be 100644 --- a/buildSrc/build.gradle +++ b/buildSrc/build.gradle
@@ -5,8 +5,9 @@ apply plugin: 'idea' repositories { - google() - mavenCentral() + maven { + url uri('file:../third_party/dependencies') + } } ext {
diff --git a/commonBuildSrc/settings.gradle.kts b/commonBuildSrc/settings.gradle.kts index 75c6675..f346f85 100644 --- a/commonBuildSrc/settings.gradle.kts +++ b/commonBuildSrc/settings.gradle.kts
@@ -4,15 +4,23 @@ pluginManagement { repositories { - gradlePluginPortal() + maven { + url = uri("file:../third_party/dependencies_new") + } + maven { + url = uri("file:../third_party/dependencies") + } } } dependencyResolutionManagement { repositories { - google() - mavenCentral() - gradlePluginPortal() + maven { + url = uri("file:../third_party/dependencies_new") + } + maven { + url = uri("file:../third_party/dependencies") + } } }
diff --git a/d8_r8/keepanno/settings.gradle.kts b/d8_r8/keepanno/settings.gradle.kts index 7b4d459..c214cca 100644 --- a/d8_r8/keepanno/settings.gradle.kts +++ b/d8_r8/keepanno/settings.gradle.kts
@@ -4,15 +4,24 @@ pluginManagement { repositories { - gradlePluginPortal() + maven { + url = uri("file:../../third_party/dependencies") + } + maven { + url = uri("file:../../third_party/dependencies_new") + } } } dependencyResolutionManagement { - repositories { - mavenCentral() - gradlePluginPortal() + repositories { + maven { + url= uri("file:../../third_party/dependencies") } + maven { + url= uri("file:../../third_party/dependencies_new") + } + } } rootProject.name = "keepanno"
diff --git a/d8_r8/main/settings.gradle.kts b/d8_r8/main/settings.gradle.kts index 2d727f0..347b7a4 100644 --- a/d8_r8/main/settings.gradle.kts +++ b/d8_r8/main/settings.gradle.kts
@@ -4,15 +4,24 @@ pluginManagement { repositories { - gradlePluginPortal() + maven { + url = uri("file:../../third_party/dependencies") + } + maven { + url = uri("file:../../third_party/dependencies_new") + } } } dependencyResolutionManagement { - repositories { - mavenCentral() - gradlePluginPortal() + repositories { + maven { + url= uri("file:../../third_party/dependencies") } + maven { + url= uri("file:../../third_party/dependencies_new") + } + } } rootProject.name = "r8"
diff --git a/d8_r8/settings.gradle.kts b/d8_r8/settings.gradle.kts index 2feb93c..9d2dfa1 100644 --- a/d8_r8/settings.gradle.kts +++ b/d8_r8/settings.gradle.kts
@@ -6,19 +6,67 @@ pluginManagement { repositories { - gradlePluginPortal() + maven { + url = uri("file:../../third_party/dependencies") + } + maven { + url = uri("file:../../third_party/dependencies_new") + } } } dependencyResolutionManagement { repositories { - mavenCentral() - gradlePluginPortal() + maven { + url= uri("file:../third_party/dependencies") + } + maven { + url= uri("file:../third_party/dependencies_new") + } } } rootProject.name = "d8-r8" +// Bootstrap building by downloading dependencies. +fun String.execute() = + org.codehaus.groovy.runtime.ProcessGroovyMethods.execute(this) + +fun Process.out() = + String( + this.getInputStream().readAllBytes(), + java.nio.charset.StandardCharsets.UTF_8) +fun Process.err() = + String( + this.getErrorStream().readAllBytes(), + java.nio.charset.StandardCharsets.UTF_8) + +val dependencies_bucket = "r8-deps" +val dependencies_sha1_file = "third_party/dependencies.tar.gz.sha1" +var cmd = + ("download_from_google_storage.py --extract" + + " --bucket ${dependencies_bucket}" + + " --sha1_file ${dependencies_sha1_file}") +var process = cmd.execute() +process.waitFor() +if (process.exitValue() != 0) { + throw GradleException( + "Bootstrapping dependencies download failed:" + + "\n${process.err()}\n${process.out()}") +} +val dependencies_new_sha1_file = "third_party/dependencies_new.tar.gz.sha1" +cmd = + ("download_from_google_storage.py --extract" + + " --bucket ${dependencies_bucket}" + + " --sha1_file ${dependencies_new_sha1_file}") +process = cmd.execute() +process.waitFor() +if (process.exitValue() != 0) { + throw GradleException( + "Bootstrapping dependencies_new download failed:" + + "\n${process.err()}\n${process.out()}") +} + // This project is temporarily located in d8_r8. When moved to root, the parent // folder should just be removed. includeBuild(rootProject.projectDir.parentFile.resolve("commonBuildSrc"))
diff --git a/d8_r8/test/settings.gradle.kts b/d8_r8/test/settings.gradle.kts index 3138d52..5946a12 100644 --- a/d8_r8/test/settings.gradle.kts +++ b/d8_r8/test/settings.gradle.kts
@@ -2,13 +2,28 @@ // 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. -dependencyResolutionManagement { +pluginManagement { repositories { - mavenCentral() - gradlePluginPortal() + maven { + url = uri("file:../../third_party/dependencies") + } + maven { + url = uri("file:../../third_party/dependencies_new") + } } } +dependencyResolutionManagement { + repositories { + maven { + url= uri("file:../third_party/dependencies") + } + maven { + url= uri("file:../third_party/dependencies_new") + } + } +} + rootProject.name = "r8-tests" val root = rootProject.projectDir.parentFile
diff --git a/d8_r8/test_modules/tests_java_8/settings.gradle.kts b/d8_r8/test_modules/tests_java_8/settings.gradle.kts index 290b820..6ab7fac 100644 --- a/d8_r8/test_modules/tests_java_8/settings.gradle.kts +++ b/d8_r8/test_modules/tests_java_8/settings.gradle.kts
@@ -3,15 +3,24 @@ // BSD-style license that can be found in the LICENSE file. pluginManagement { - repositories { - gradlePluginPortal() - } + repositories { + maven { + url = uri("file:../../../third_party/dependencies") + } + maven { + url = uri("file:../../../third_party/dependencies_new") + } + } } dependencyResolutionManagement { repositories { - mavenCentral() - gradlePluginPortal() + maven { + url= uri("file:../third_party/dependencies") + } + maven { + url= uri("file:../third_party/dependencies_new") + } } }
diff --git a/library-licensing/checkerframework.txt b/library-licensing/checkerframework.txt new file mode 100644 index 0000000..9837c6b --- /dev/null +++ b/library-licensing/checkerframework.txt
@@ -0,0 +1,22 @@ +Checker Framework qualifiers +Copyright 2004-present by the Checker Framework developers + +MIT License: + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE.
diff --git a/library-licensing/jopt-simple.txt b/library-licensing/jopt-simple.txt deleted file mode 100644 index 2077257..0000000 --- a/library-licensing/jopt-simple.txt +++ /dev/null
@@ -1,22 +0,0 @@ - The MIT License - - Copyright (c) 2004-2016 Paul R. Holser, Jr. - - Permission is hereby granted, free of charge, to any person obtaining - a copy of this software and associated documentation files (the - "Software"), to deal in the Software without restriction, including - without limitation the rights to use, copy, modify, merge, publish, - distribute, sublicense, and/or sell copies of the Software, and to - permit persons to whom the Software is furnished to do so, subject to - the following conditions: - - The above copyright notice and this permission notice shall be - included in all copies or substantial portions of the Software. - - THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, - EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF - MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND - NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE - LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION - OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION - WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
diff --git a/settings.gradle b/settings.gradle index 9924d92..969cb9c 100644 --- a/settings.gradle +++ b/settings.gradle
@@ -2,5 +2,24 @@ // 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. +import org.gradle.internal.os.OperatingSystem + rootProject.name = 'r8' +// Bootstrap building by downloading dependencies. +def dependencies_bucket = "r8-deps" +def dependencies_sha1_file = "third_party/dependencies.tar.gz.sha1" +def extension = OperatingSystem.current().isWindows() ? ".bat" : ".py" + +def cmd = + ("download_from_google_storage${extension} --extract" + + " --bucket ${dependencies_bucket}" + + " --sha1_file ${dependencies_sha1_file}") +def process = cmd.execute() +process.waitFor() +if (process.exitValue() != 0) { + throw new GradleException( + "Bootstrapping dependencies download failed:\n" + + "STDOUT:\n${process.in.text}\n" + + "STDERR:\n${process.err.text}") +} \ No newline at end of file
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 8f9378e..7636cbd 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
@@ -30,7 +30,7 @@ import com.android.tools.r8.ir.conversion.passes.BinopRewriter; import com.android.tools.r8.ir.conversion.passes.CommonSubexpressionElimination; import com.android.tools.r8.ir.conversion.passes.ParentConstructorHoistingCodeRewriter; -import com.android.tools.r8.ir.conversion.passes.SplitBranchOnKnownBoolean; +import com.android.tools.r8.ir.conversion.passes.SplitBranch; import com.android.tools.r8.ir.desugar.CfInstructionDesugaringCollection; import com.android.tools.r8.ir.desugar.CovariantReturnTypeAnnotationTransformer; import com.android.tools.r8.ir.optimize.AssertionErrorTwoArgsConstructorRewriter; @@ -115,7 +115,7 @@ protected final InternalOptions options; public final CodeRewriter codeRewriter; public final CommonSubexpressionElimination commonSubexpressionElimination; - private final SplitBranchOnKnownBoolean splitBranchOnKnownBoolean; + private final SplitBranch splitBranch; public final AssertionErrorTwoArgsConstructorRewriter assertionErrorTwoArgsConstructorRewriter; private final NaturalIntLoopRemover naturalIntLoopRemover = new NaturalIntLoopRemover(); public final MemberValuePropagation<?> memberValuePropagation; @@ -155,6 +155,7 @@ // Use AtomicBoolean to satisfy TSAN checking (see b/153714743). AtomicBoolean seenNotNeverMergePrefix = new AtomicBoolean(); AtomicBoolean seenNeverMergePrefix = new AtomicBoolean(); + String conflictingPrefixesErrorMessage = null; /** * The argument `appView` is used to determine if whole program optimizations are allowed or not @@ -167,7 +168,7 @@ this.options = appView.options(); this.codeRewriter = new CodeRewriter(appView); this.commonSubexpressionElimination = new CommonSubexpressionElimination(appView); - this.splitBranchOnKnownBoolean = new SplitBranchOnKnownBoolean(appView); + this.splitBranch = new SplitBranch(appView); this.assertionErrorTwoArgsConstructorRewriter = appView.options().desugarState.isOn() ? new AssertionErrorTwoArgsConstructorRewriter(appView) @@ -777,7 +778,7 @@ timing.end(); } timing.end(); - splitBranchOnKnownBoolean.run(code.context(), code, timing); + splitBranch.run(code.context(), code, timing); if (options.enableRedundantConstNumberOptimization) { timing.begin("Remove const numbers"); codeRewriter.redundantConstNumberRemoval(code);
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/PrimaryD8L8IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/PrimaryD8L8IRConverter.java index 532c129..172e90a 100644 --- a/src/main/java/com/android/tools/r8/ir/conversion/PrimaryD8L8IRConverter.java +++ b/src/main/java/com/android/tools/r8/ir/conversion/PrimaryD8L8IRConverter.java
@@ -195,64 +195,68 @@ // TODO(b/168001352): Consider requiring that no 'never merge' prefix is ever seen as a // passthrough object. if (seenNeverMergePrefix.get() && seenNotNeverMergePrefix.get()) { - StringBuilder message = new StringBuilder(); - message - .append("Merging DEX file containing classes with prefix") - .append(neverMerge.getPrefixes().size() > 1 ? "es " : " "); - for (int i = 0; i < neverMerge.getPrefixes().size(); i++) { - message - .append("'") - .append(neverMerge.getPrefixes().get(i).toString().substring(1).replace('/', '.')) - .append("'") - .append(i < neverMerge.getPrefixes().size() - 1 ? ", " : ""); - } - if (!neverMerge.getExceptionPrefixes().isEmpty()) { - message - .append(" with other classes, except classes with prefix") - .append(neverMerge.getExceptionPrefixes().size() > 1 ? "es " : " "); - for (int i = 0; i < neverMerge.getExceptionPrefixes().size(); i++) { - message - .append("'") - .append( - neverMerge - .getExceptionPrefixes() - .get(i) - .toString() - .substring(1) - .replace('/', '.')) - .append("'") - .append(i < neverMerge.getExceptionPrefixes().size() - 1 ? ", " : ""); - } - message.append(","); - } else { - message.append(" with classes with any other prefixes"); - } - message.append(" is not allowed: "); - boolean first = true; - int limit = 11; - for (DexProgramClass clazz : appView.appInfo().classesWithDeterministicOrder()) { - if (!clazz.type.descriptor.startsWith(neverMergePrefix)) { - if (hasExceptionPrefix(clazz)) { - continue; - } - if (limit-- < 0) { - message.append(".."); - break; - } - if (first) { - first = false; - } else { - message.append(", "); - } - message.append(clazz.type); - } - } - message.append("."); - throw new CompilationError(message.toString()); + throw new CompilationError(getOrComputeConflictingPrefixesErrorMessage(neverMergePrefix)); } } } + private synchronized String getOrComputeConflictingPrefixesErrorMessage( + DexString neverMergePrefix) { + if (conflictingPrefixesErrorMessage != null) { + return conflictingPrefixesErrorMessage; + } + StringBuilder message = new StringBuilder(); + message + .append("Merging DEX file containing classes with prefix") + .append(neverMerge.getPrefixes().size() > 1 ? "es " : " "); + for (int i = 0; i < neverMerge.getPrefixes().size(); i++) { + message + .append("'") + .append(neverMerge.getPrefixes().get(i).toString().substring(1).replace('/', '.')) + .append("'") + .append(i < neverMerge.getPrefixes().size() - 1 ? ", " : ""); + } + if (!neverMerge.getExceptionPrefixes().isEmpty()) { + message + .append(" with other classes, except classes with prefix") + .append(neverMerge.getExceptionPrefixes().size() > 1 ? "es " : " "); + for (int i = 0; i < neverMerge.getExceptionPrefixes().size(); i++) { + message + .append("'") + .append( + neverMerge.getExceptionPrefixes().get(i).toString().substring(1).replace('/', '.')) + .append("'") + .append(i < neverMerge.getExceptionPrefixes().size() - 1 ? ", " : ""); + } + message.append(","); + } else { + message.append(" with classes with any other prefixes"); + } + message.append(" is not allowed: "); + boolean first = true; + int limit = 11; + for (DexProgramClass clazz : appView.appInfo().classesWithDeterministicOrder()) { + if (!clazz.type.descriptor.startsWith(neverMergePrefix)) { + if (hasExceptionPrefix(clazz)) { + continue; + } + if (limit-- < 0) { + message.append(".."); + break; + } + if (first) { + first = false; + } else { + message.append(", "); + } + message.append(clazz.type); + } + } + message.append("."); + conflictingPrefixesErrorMessage = message.toString(); + return conflictingPrefixesErrorMessage; + } + private boolean hasExceptionPrefix(DexProgramClass clazz) { for (DexString exceptionPrefix : neverMerge.getExceptionPrefixes()) { if (clazz.type.descriptor.startsWith(exceptionPrefix)) {
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/passes/SplitBranchOnKnownBoolean.java b/src/main/java/com/android/tools/r8/ir/conversion/passes/SplitBranch.java similarity index 68% rename from src/main/java/com/android/tools/r8/ir/conversion/passes/SplitBranchOnKnownBoolean.java rename to src/main/java/com/android/tools/r8/ir/conversion/passes/SplitBranch.java index 134d2bc..9a3a024 100644 --- a/src/main/java/com/android/tools/r8/ir/conversion/passes/SplitBranchOnKnownBoolean.java +++ b/src/main/java/com/android/tools/r8/ir/conversion/passes/SplitBranch.java
@@ -9,6 +9,7 @@ import com.android.tools.r8.graph.ProgramMethod; import com.android.tools.r8.ir.analysis.type.TypeAnalysis; import com.android.tools.r8.ir.code.BasicBlock; +import com.android.tools.r8.ir.code.ConstNumber; import com.android.tools.r8.ir.code.Goto; import com.android.tools.r8.ir.code.IRCode; import com.android.tools.r8.ir.code.If; @@ -23,17 +24,17 @@ import java.util.Map; import java.util.Set; -public class SplitBranchOnKnownBoolean extends CodeRewriterPass<AppInfo> { +public class SplitBranch extends CodeRewriterPass<AppInfo> { private static final boolean ALLOW_PARTIAL_REWRITE = true; - public SplitBranchOnKnownBoolean(AppView<?> appView) { + public SplitBranch(AppView<?> appView) { super(appView); } @Override String getTimingId() { - return "SplitBranchOnKnownBoolean"; + return "SplitBranch"; } @Override @@ -89,17 +90,31 @@ private Map<Goto, BasicBlock> findGotosToRetarget(List<BasicBlock> candidates) { Map<Goto, BasicBlock> newTargets = new LinkedHashMap<>(); for (BasicBlock block : candidates) { - // We need to verify any instruction in between the if and the chain of phis is empty (we - // could duplicate instruction, but the common case is empty). + // We need to verify any instruction in between the if and the chain of phis is empty or just + // a constant used in the If instruction (we could duplicate instruction, but the common case + // is empty). // Then we can redirect any known value. This can lead to dead code. If theIf = block.exit().asIf(); - Set<Phi> allowedPhis = getAllowedPhis(theIf.lhs().asPhi()); + Set<Phi> allowedPhis = getAllowedPhis(nonConstNumberOperand(theIf).asPhi()); Set<Phi> foundPhis = Sets.newIdentityHashSet(); WorkList.newIdentityWorkList(block) .process( (current, workList) -> { if (current.getInstructions().size() > 1) { - return; + // We allow a single instruction, which is the constant used exclusively in the + // if. This is run before constant canonicalization. + if (theIf.isZeroTest() + || current.getInstructions().size() != 2 + || !current.entry().isConstNumber()) { + return; + } + Value value = current.entry().outValue(); + if (value.hasPhiUsers() + || value.uniqueUsers().size() > 1 + || (value.uniqueUsers().size() == 1 + && value.uniqueUsers().iterator().next() != theIf)) { + return; + } } if (current != block && !current.exit().isGoto()) { return; @@ -135,30 +150,61 @@ return newTargets; } + private boolean isNumberAgainstConstNumberIf(If theIf) { + if (!(theIf.lhs().getType().isInt() || theIf.lhs().getType().isFloat())) { + return false; + } + if (theIf.isZeroTest()) { + return true; + } + assert theIf.lhs().getType() == theIf.rhs().getType(); + return theIf.lhs().isConstNumber() || theIf.rhs().isConstNumber(); + } + + private Value nonConstNumberOperand(If theIf) { + return theIf.isZeroTest() + ? theIf.lhs() + : (theIf.lhs().isConstNumber() ? theIf.rhs() : theIf.lhs()); + } + private List<BasicBlock> computeCandidates(IRCode code) { List<BasicBlock> candidates = new ArrayList<>(); - for (BasicBlock block : ListUtils.filter(code.blocks, block -> block.entry().isIf())) { + for (BasicBlock block : ListUtils.filter(code.blocks, block -> block.exit().isIf())) { If theIf = block.exit().asIf(); - if (theIf.isZeroTest() - && theIf.lhs().getType().isInt() - && theIf.lhs().isPhi() - && theIf.lhs().hasSingleUniqueUser() - && !theIf.lhs().hasPhiUsers()) { + if (!isNumberAgainstConstNumberIf(theIf)) { + continue; + } + Value nonConstNumberOperand = nonConstNumberOperand(theIf); + if (isNumberAgainstConstNumberIf(theIf) + && nonConstNumberOperand.isPhi() + && nonConstNumberOperand.hasSingleUniqueUser() + && !nonConstNumberOperand.hasPhiUsers()) { candidates.add(block); } } return candidates; } + private BasicBlock targetFromCondition(If theIf, ConstNumber constForPhi) { + if (theIf.isZeroTest()) { + return theIf.targetFromCondition(constForPhi); + } + if (theIf.lhs().isConstNumber()) { + return theIf.targetFromCondition( + theIf.lhs().getConstInstruction().asConstNumber(), constForPhi); + } + assert theIf.rhs().isConstNumber(); + return theIf.targetFromCondition( + constForPhi, theIf.rhs().getConstInstruction().asConstNumber()); + } + private void recordNewTargetForGoto( Value value, BasicBlock basicBlock, If theIf, Map<Goto, BasicBlock> newTargets) { // The GoTo at the end of basicBlock should target the phiBlock, and should target instead // the correct if destination. assert basicBlock.exit().isGoto(); assert value.isConstant(); - assert value.getType().isInt(); - assert theIf.isZeroTest(); - BasicBlock newTarget = theIf.targetFromCondition(value.getConstInstruction().asConstNumber()); + BasicBlock newTarget = targetFromCondition(theIf, value.getConstInstruction().asConstNumber()); Goto aGoto = basicBlock.exit().asGoto(); newTargets.put(aGoto, newTarget); }
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 c0d3e6b..26da1f1 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
@@ -36,6 +36,7 @@ import com.android.tools.r8.graph.ProgramMethod; import com.android.tools.r8.horizontalclassmerging.HorizontalClassMergerUtils; import com.android.tools.r8.ir.analysis.equivalence.BasicBlockBehavioralSubsumption; +import com.android.tools.r8.ir.analysis.type.ArrayTypeElement; import com.android.tools.r8.ir.analysis.type.DynamicTypeWithUpperBound; import com.android.tools.r8.ir.analysis.type.Nullability; import com.android.tools.r8.ir.analysis.type.TypeAnalysis; @@ -2263,7 +2264,7 @@ } } - private FilledArrayCandidate computeFilledArrayCandiate( + private FilledArrayCandidate computeFilledArrayCandidate( Instruction instruction, RewriteArrayOptions options) { NewArrayEmpty newArrayEmpty = instruction.asNewArrayEmpty(); if (newArrayEmpty == null) { @@ -2284,9 +2285,57 @@ if (!encodeAsFilledNewArray && !canUseFilledArrayData(arrayType, size, options)) { return null; } + // Check that all arguments to the array is the array type or that the array is type Object[]. + if (!options.canUseSubTypesInFilledNewArray() + && arrayType != dexItemFactory.objectArrayType + && !arrayType.isPrimitiveArrayType()) { + DexType elementType = arrayType.toArrayElementType(dexItemFactory); + for (Instruction uniqueUser : newArrayEmpty.outValue().uniqueUsers()) { + if (uniqueUser.isArrayPut() + && uniqueUser.asArrayPut().array() == newArrayEmpty.outValue() + && !checkTypeOfArrayPut(uniqueUser.asArrayPut(), elementType)) { + return null; + } + } + } return new FilledArrayCandidate(newArrayEmpty, size, encodeAsFilledNewArray); } + private boolean checkTypeOfArrayPut(ArrayPut arrayPut, DexType elementType) { + TypeElement valueType = arrayPut.value().getType(); + if (!valueType.isPrimitiveType() && elementType == dexItemFactory.objectType) { + return true; + } + if (valueType.isNullType() && !elementType.isPrimitiveType()) { + return true; + } + if (elementType.isArrayType()) { + if (valueType.isNullType()) { + return true; + } + ArrayTypeElement arrayTypeElement = valueType.asArrayType(); + if (arrayTypeElement == null + || arrayTypeElement.getNesting() != elementType.getNumberOfLeadingSquareBrackets()) { + return false; + } + valueType = arrayTypeElement.getBaseType(); + elementType = elementType.toBaseType(dexItemFactory); + } + assert !valueType.isArrayType(); + assert !elementType.isArrayType(); + if (valueType.isPrimitiveType() && !elementType.isPrimitiveType()) { + return false; + } + if (valueType.isPrimitiveType()) { + return true; + } + DexClass clazz = appView.definitionFor(elementType); + if (clazz == null) { + return false; + } + return clazz.isInterface() || valueType.isClassType(elementType); + } + private boolean canUseFilledNewArray(DexType arrayType, int size, RewriteArrayOptions options) { if (size < options.minSizeForFilledNewArray) { return false; @@ -2393,7 +2442,7 @@ RewriteArrayOptions rewriteOptions = options.rewriteArrayOptions(); InstructionListIterator it = block.listIterator(code); while (it.hasNext()) { - FilledArrayCandidate candidate = computeFilledArrayCandiate(it.next(), rewriteOptions); + FilledArrayCandidate candidate = computeFilledArrayCandidate(it.next(), rewriteOptions); if (candidate == null) { continue; }
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 2f66e6f..b2ce050 100644 --- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java +++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -1519,6 +1519,13 @@ return hasFeaturePresentFrom(AndroidApiLevel.N); } + // When adding support for emitting filled-new-array for sub-types, ART 13 (Api-level 33) had + // issues. See b/283715197. + public boolean canUseSubTypesInFilledNewArray() { + assert isGeneratingDex(); + return !canHaveBugPresentUntil(AndroidApiLevel.U); + } + // Dalvik doesn't handle new-filled-array with arrays as values. It fails with: // W(629880) VFY: [Ljava/lang/Integer; is not instance of Ljava/lang/Integer; (dalvikvm) public boolean canUseFilledNewArrayOfArrays() {
diff --git a/src/test/java/com/android/tools/r8/TestParameters.java b/src/test/java/com/android/tools/r8/TestParameters.java index 7e64f85..c792710 100644 --- a/src/test/java/com/android/tools/r8/TestParameters.java +++ b/src/test/java/com/android/tools/r8/TestParameters.java
@@ -112,6 +112,14 @@ return false; } + public boolean canUseFilledNewArrayOnNonStringObjects() { + return isDexRuntime() && getApiLevel().isGreaterThanOrEqualTo(AndroidApiLevel.N); + } + + public boolean canUseSubTypesInFilledNewArray() { + return isDexRuntime() && getApiLevel().isGreaterThanOrEqualTo(AndroidApiLevel.U); + } + public boolean runtimeWithClassValue() { assert isCfRuntime() || isDexRuntime(); return isCfRuntime() || getDexRuntimeVersion().isNewerThanOrEqual(DexVm.Version.V14_0_0);
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/ifs/DoubleDiamondCstTest.java b/src/test/java/com/android/tools/r8/ir/optimize/ifs/DoubleDiamondCstTest.java new file mode 100644 index 0000000..7c5ed88 --- /dev/null +++ b/src/test/java/com/android/tools/r8/ir/optimize/ifs/DoubleDiamondCstTest.java
@@ -0,0 +1,159 @@ +// Copyright (c) 2023, 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 org.junit.Assert.assertEquals; + +import com.android.tools.r8.AlwaysInline; +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.CodeInspector; +import com.android.tools.r8.utils.codeinspector.FoundMethodSubject; +import com.android.tools.r8.utils.codeinspector.InstructionSubject; +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 DoubleDiamondCstTest extends TestBase { + + private final TestParameters parameters; + + @Parameters(name = "{0}") + public static TestParametersCollection data() { + return getTestParameters().withAllRuntimesAndApiLevels().build(); + } + + public DoubleDiamondCstTest(TestParameters parameters) { + this.parameters = parameters; + } + + @Test + public void test() throws Exception { + testForR8(parameters.getBackend()) + .addInnerClasses(getClass()) + .addKeepMainRule(Main.class) + .enableInliningAnnotations() + .enableAlwaysInliningAnnotations() + .setMinApi(parameters) + .compile() + .inspect(this::inspect) + .run(parameters.getRuntime(), Main.class) + .assertSuccessWithOutputLines( + "1", "5", "5", "1", "1", "5", "5", "1", "5", "5", "1", "1", "1", "5"); + } + + private void inspect(CodeInspector inspector) { + for (FoundMethodSubject method : inspector.clazz(Main.class).allMethods()) { + if (!method.getOriginalName().equals("main")) { + long count = method.streamInstructions().filter(InstructionSubject::isIf).count(); + assertEquals(method.getOriginalName().contains("Double") ? 2 : 1, count); + } + } + } + + public static class Main { + + public static void main(String[] args) { + System.out.println(indirectTest(2, 6)); + System.out.println(indirectTest(3, 3)); + + System.out.println(indirectTestNegated(2, 6)); + System.out.println(indirectTestNegated(3, 3)); + + System.out.println(indirectCmp(2, 6)); + System.out.println(indirectCmp(7, 3)); + + System.out.println(indirectCmpNegated(2, 6)); + System.out.println(indirectCmpNegated(7, 3)); + + System.out.println(indirectDoubleTest(2, 6, 6)); + System.out.println(indirectDoubleTest(7, 7, 3)); + System.out.println(indirectDoubleTest(1, 1, 1)); + + System.out.println(indirectDoubleTestNegated(2, 6, 6)); + System.out.println(indirectDoubleTestNegated(2, 2, 6)); + System.out.println(indirectDoubleTestNegated(7, 7, 7)); + } + + @AlwaysInline + public static int doubleTest(int i, int j, int k) { + if (i != j) { + return 1; + } + if (j == k) { + return 2; + } + return 3; + } + + @NeverInline + public static int indirectDoubleTest(int i, int j, int k) { + if (doubleTest(i, j, k) == 2) { + return 1; + } else { + return 5; + } + } + + @NeverInline + public static int indirectDoubleTestNegated(int i, int j, int k) { + if (doubleTest(i, j, k) != 2) { + return 1; + } else { + return 5; + } + } + + @AlwaysInline + public static int test(int i, int j) { + return i == j ? 1 : 2; + } + + @NeverInline + public static int indirectTest(int i, int j) { + if (test(i, j) == 2) { + return 1; + } else { + return 5; + } + } + + @NeverInline + public static int indirectTestNegated(int i, int j) { + if (test(i, j) != 2) { + return 1; + } else { + return 5; + } + } + + @AlwaysInline + public static int cmp(int i, int j) { + return i <= j ? 1 : 2; + } + + @NeverInline + public static int indirectCmp(int i, int j) { + if (cmp(i, j) < 2) { + return 1; + } else { + return 5; + } + } + + @NeverInline + public static int indirectCmpNegated(int i, int j) { + if (cmp(i, j) > 1) { + return 1; + } else { + return 5; + } + } + } +}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/ifs/DoubleDiamondFloatTest.java b/src/test/java/com/android/tools/r8/ir/optimize/ifs/DoubleDiamondFloatTest.java new file mode 100644 index 0000000..b433e5b --- /dev/null +++ b/src/test/java/com/android/tools/r8/ir/optimize/ifs/DoubleDiamondFloatTest.java
@@ -0,0 +1,158 @@ +// Copyright (c) 2023, 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 org.junit.Assert.assertEquals; + +import com.android.tools.r8.AlwaysInline; +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.CodeInspector; +import com.android.tools.r8.utils.codeinspector.FoundMethodSubject; +import com.android.tools.r8.utils.codeinspector.InstructionSubject; +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 DoubleDiamondFloatTest extends TestBase { + + private final TestParameters parameters; + + @Parameters(name = "{0}") + public static TestParametersCollection data() { + return getTestParameters().withAllRuntimesAndApiLevels().build(); + } + + public DoubleDiamondFloatTest(TestParameters parameters) { + this.parameters = parameters; + } + + @Test + public void test() throws Exception { + testForR8(parameters.getBackend()) + .addInnerClasses(getClass()) + .addKeepMainRule(Main.class) + .enableInliningAnnotations() + .enableAlwaysInliningAnnotations() + .setMinApi(parameters) + .compile() + .inspect(this::inspect) + .run(parameters.getRuntime(), Main.class) + .assertSuccessWithOutputLines( + "5", "1", "1", "5", "5", "5", "1", "1", "1", "5", "5", "5", "1", "1", "1", "5"); + } + + private void inspect(CodeInspector inspector) { + for (FoundMethodSubject method : inspector.clazz(Main.class).allMethods()) { + if (!method.getOriginalName().equals("main")) { + long count = method.streamInstructions().filter(InstructionSubject::isIf).count(); + assertEquals(method.getOriginalName().contains("Double") ? 2 : 1, count); + } + } + } + + public static class Main { + + public static void main(String[] args) { + System.out.println(indirectEquals(2.0f, 6.0f)); + System.out.println(indirectEquals(3.0f, 3.0f)); + + System.out.println(indirectEqualsNegated(2.0f, 6.0f)); + System.out.println(indirectEqualsNegated(3.0f, 3.0f)); + + System.out.println(indirectDoubleEquals(2.0f, 6.0f, 6.0f)); + System.out.println(indirectDoubleEquals(7.0f, 7.0f, 3.0f)); + System.out.println(indirectDoubleEquals(1.0f, 1.0f, 1.0f)); + + System.out.println(indirectDoubleEqualsNegated(2.0f, 6.0f, 6.0f)); + System.out.println(indirectDoubleEqualsNegated(2.0f, 2.0f, 6.0f)); + System.out.println(indirectDoubleEqualsNegated(7.0f, 7.0f, 7.0f)); + + System.out.println(indirectDoubleEqualsSplit(2.0f, 6.0f, 6.0f)); + System.out.println(indirectDoubleEqualsSplit(7.0f, 7.0f, 3.0f)); + System.out.println(indirectDoubleEqualsSplit(1.0f, 1.0f, 1.0f)); + + System.out.println(indirectDoubleEqualsSplitNegated(2.0f, 6.0f, 6.0f)); + System.out.println(indirectDoubleEqualsSplitNegated(2.0f, 2.0f, 6.0f)); + System.out.println(indirectDoubleEqualsSplitNegated(7.0f, 7.0f, 7.0f)); + } + + @AlwaysInline + public static boolean doubleEqualsSplit(float i, float j, float k) { + if (i != j) { + return false; + } + return j == k; + } + + @NeverInline + public static int indirectDoubleEqualsSplit(float i, float j, float k) { + if (doubleEqualsSplit(i, j, k)) { + return 1; + } else { + return 5; + } + } + + @NeverInline + public static int indirectDoubleEqualsSplitNegated(float i, float j, float k) { + if (!doubleEqualsSplit(i, j, k)) { + return 1; + } else { + return 5; + } + } + + @AlwaysInline + public static boolean doubleEquals(float i, float j, float k) { + return i == j && j == k; + } + + @NeverInline + public static int indirectDoubleEquals(float i, float j, float k) { + if (doubleEquals(i, j, k)) { + return 1; + } else { + return 5; + } + } + + @NeverInline + public static int indirectDoubleEqualsNegated(float i, float j, float k) { + if (!doubleEquals(i, j, k)) { + return 1; + } else { + return 5; + } + } + + @AlwaysInline + public static boolean equals(float i, float j) { + return i == j; + } + + @NeverInline + public static int indirectEquals(float i, float j) { + if (equals(i, j)) { + return 1; + } else { + return 5; + } + } + + @NeverInline + public static int indirectEqualsNegated(float i, float j) { + if (!equals(i, j)) { + return 1; + } else { + return 5; + } + } + } +}
diff --git a/src/test/java/com/android/tools/r8/proguard/RemovedAndroidApiTest.java b/src/test/java/com/android/tools/r8/proguard/RemovedAndroidApiTest.java new file mode 100644 index 0000000..7e4a3b2 --- /dev/null +++ b/src/test/java/com/android/tools/r8/proguard/RemovedAndroidApiTest.java
@@ -0,0 +1,98 @@ +// Copyright (c) 2023, 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.proguard; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.MatcherAssert.assertThat; + +import com.android.tools.r8.CompilationFailedException; +import com.android.tools.r8.ProguardVersion; +import com.android.tools.r8.TestBase; +import com.android.tools.r8.TestParameters; +import com.android.tools.r8.TestParametersCollection; +import com.android.tools.r8.ToolHelper; +import com.android.tools.r8.utils.AndroidApiLevel; +import java.io.IOException; +import org.hamcrest.CoreMatchers; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; + +// See b/37324358 for reference. +@RunWith(Parameterized.class) +public class RemovedAndroidApiTest extends TestBase { + + @Parameter() public TestParameters parameters; + + @Parameters(name = "{0}") + public static TestParametersCollection data() { + return getTestParameters().withDefaultDexRuntime().withApiLevel(AndroidApiLevel.B).build(); + } + + @Test + public void testR8() throws Exception { + parameters.assumeR8TestParameters(); + testForR8(parameters.getBackend()) + .addLibraryFiles(ToolHelper.getAndroidJar(AndroidApiLevel.LATEST)) + .addProgramClassFileData(getClassUsingRemovedApi()) + .addKeepMainRule(TestClass.class) + .setMinApi(parameters) + .run(parameters.getRuntime(), TestClass.class) + .assertFailureWithErrorThatMatches( + CoreMatchers.anyOf( + containsString("java.lang.ClassNotFoundException: android.util.FloatMath"), + containsString( + "java.lang.ClassNotFoundException: Didn't find class" + + " \"android.util.FloatMath\""))); + } + + @Test + public void testProguard() throws Exception { + parameters.assumeR8TestParameters(); + try { + testForProguard(ProguardVersion.V7_0_0) + .addProgramClassFileData(getClassUsingRemovedApi()) + .addLibraryFiles(ToolHelper.getAndroidJar(AndroidApiLevel.LATEST)) + .addKeepMainRule(TestClass.class) + .compile(); + } catch (CompilationFailedException e) { + assertThat( + e.getMessage(), + containsString( + "can't find referenced method 'float floor(float)'" + + " in library class android.util.FloatMath")); + } + } + + private byte[] getClassUsingRemovedApi() throws IOException { + return transformer(TestClass.class) + .transformMethodInsnInMethod( + "main", + (opcode, owner, name, descriptor, isInterface, visitor) -> { + if (name.equals("floor")) { + // The class android.util.FloatMath is still in android.jar, but without any + // methods. + // Methods where removed from API level 23. + visitor.visitMethodInsn( + opcode, "android/util/FloatMath", "floor", descriptor, isInterface); + } else { + visitor.visitMethodInsn(opcode, owner, name, descriptor, isInterface); + } + }) + .transform(); + } + + static class TestClass { + public static float floor(float f) { + throw new RuntimeException("Stub"); + } + + public static void main(String[] args) { + System.out.println(floor(1.234f)); + } + } +}
diff --git a/src/test/java/com/android/tools/r8/rewrite/arrays/FilledArrayDataRemoveCheckCastTest.java b/src/test/java/com/android/tools/r8/rewrite/arrays/FilledArrayDataRemoveCheckCastTest.java new file mode 100644 index 0000000..71639f7 --- /dev/null +++ b/src/test/java/com/android/tools/r8/rewrite/arrays/FilledArrayDataRemoveCheckCastTest.java
@@ -0,0 +1,122 @@ +// Copyright (c) 2023, 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.rewrite.arrays; + +import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertEquals; + +import com.android.tools.r8.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.InstructionSubject; +import com.android.tools.r8.utils.codeinspector.MethodSubject; +import java.util.Optional; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; + +/** + * This is a reproduction of b/283715197. It actually does not reproduce on our headless VMs, so we + * cannot assert an error - only the existences of the instruction that causes verification error. + */ +@RunWith(Parameterized.class) +public class FilledArrayDataRemoveCheckCastTest extends TestBase { + + @Parameter() public TestParameters parameters; + + @Parameters(name = "{0}") + public static TestParametersCollection data() { + return getTestParameters().withAllRuntimesAndApiLevels().build(); + } + + @Test + public void testR8() throws Exception { + testForR8(parameters.getBackend()) + .addInnerClasses(getClass()) + .setMinApi(parameters) + .addKeepClassAndMembersRules(Main.class) + .addKeepClassAndMembersRules(Base.class) + .addKeepClassRulesWithAllowObfuscation(Sub1.class, Sub2.class) + .run(parameters.getRuntime(), Main.class) + .assertSuccessWithOutputLines("Hello", "World", "Hello", "World") + .inspect( + inspector -> { + ClassSubject mainClass = inspector.clazz(Main.class); + assertThat(mainClass, isPresent()); + MethodSubject iterateSubClasses = + mainClass.uniqueMethodWithOriginalName("iterateSubClasses"); + assertThat(iterateSubClasses, isPresent()); + Optional<InstructionSubject> filledNewArrayInIterateSubClasses = + iterateSubClasses + .streamInstructions() + .filter(InstructionSubject::isFilledNewArray) + .findFirst(); + MethodSubject iterateBaseClasses = + mainClass.uniqueMethodWithOriginalName("iterateBaseClasses"); + assertThat(iterateBaseClasses, isPresent()); + Optional<InstructionSubject> filledNewArrayInIterateBaseClasses = + iterateBaseClasses + .streamInstructions() + .filter(InstructionSubject::isFilledNewArray) + .findFirst(); + assertEquals( + parameters.canUseFilledNewArrayOnNonStringObjects(), + filledNewArrayInIterateBaseClasses.isPresent()); + assertEquals( + parameters.canUseFilledNewArrayOnNonStringObjects() + && parameters.canUseSubTypesInFilledNewArray(), + filledNewArrayInIterateSubClasses.isPresent()); + }); + } + + public abstract static class Base { + + public abstract String toString(); + } + + public static class Sub1 extends Base { + + @Override + public String toString() { + return "Hello"; + } + } + + public static class Sub2 extends Base { + + @Override + public String toString() { + return "World"; + } + } + + public static class Main { + + public static void main(String[] args) { + iterateBaseClasses(new Sub1(), new Sub2()); + iterateSubClasses(); + } + + public static void iterateBaseClasses(Base b1, Base b2) { + Base[] arr = new Base[] {b1, b2}; + iterate(arr); + } + + public static void iterateSubClasses() { + Base[] arr = new Base[] {new Sub1(), new Sub2()}; + iterate(arr); + } + + public static void iterate(Object[] arr) { + for (Object b : arr) { + System.out.println(b.toString()); + } + } + } +}
diff --git a/src/test/java/com/android/tools/r8/rewrite/arrays/SimplifyArrayConstructionTest.java b/src/test/java/com/android/tools/r8/rewrite/arrays/SimplifyArrayConstructionTest.java index 38870bd..76fcda9 100644 --- a/src/test/java/com/android/tools/r8/rewrite/arrays/SimplifyArrayConstructionTest.java +++ b/src/test/java/com/android/tools/r8/rewrite/arrays/SimplifyArrayConstructionTest.java
@@ -170,6 +170,8 @@ mainClass.uniqueMethodWithOriginalName("referenceArraysNoCasts"); MethodSubject referenceArraysWithSubclasses = mainClass.uniqueMethodWithOriginalName("referenceArraysWithSubclasses"); + MethodSubject referenceArraysWithInterfaceImplementations = + mainClass.uniqueMethodWithOriginalName("referenceArraysWithInterfaceImplementations"); MethodSubject interfaceArrayWithRawObject = mainClass.uniqueMethodWithOriginalName("interfaceArrayWithRawObject"); @@ -220,18 +222,23 @@ if (parameters.getApiLevel().isLessThan(AndroidApiLevel.N)) { assertArrayTypes(referenceArraysNoCasts, DexNewArray.class); assertArrayTypes(referenceArraysWithSubclasses, DexNewArray.class); + assertArrayTypes(referenceArraysWithInterfaceImplementations, DexNewArray.class); assertArrayTypes(phiFilledNewArray, DexNewArray.class); assertArrayTypes(objectArraysFilledNewArrayRange, DexNewArray.class); assertArrayTypes(twoDimensionalArrays, DexNewArray.class); assertArrayTypes(assumedValues, DexNewArray.class); } else { assertArrayTypes(referenceArraysNoCasts, DexFilledNewArray.class); - // TODO(b/246971330): Add support for arrays with subtypes. - if (isR8) { + if (isR8 && parameters.canUseSubTypesInFilledNewArray()) { assertArrayTypes(referenceArraysWithSubclasses, DexFilledNewArray.class); } else { assertArrayTypes(referenceArraysWithSubclasses, DexNewArray.class); } + if (isR8) { + assertArrayTypes(referenceArraysWithInterfaceImplementations, DexFilledNewArray.class); + } else { + assertArrayTypes(referenceArraysWithInterfaceImplementations, DexNewArray.class); + } // TODO(b/246971330): Add support for arrays whose values have conditionals. // assertArrayTypes(phiFilledNewArray, DexFilledNewArray.class); @@ -298,6 +305,7 @@ stringArrays(); referenceArraysNoCasts(); referenceArraysWithSubclasses(); + referenceArraysWithInterfaceImplementations(); interfaceArrayWithRawObject(); phiFilledNewArray(); intsThatUseFilledNewArray(); @@ -338,9 +346,13 @@ } @NeverInline - private static void referenceArraysWithSubclasses() { + private static void referenceArraysWithInterfaceImplementations() { Serializable[] interfaceArr = {1, null, 2}; System.out.println(Arrays.toString(interfaceArr)); + } + + @NeverInline + private static void referenceArraysWithSubclasses() { Number[] objArray = {1, null, 2}; System.out.println(Arrays.toString(objArray)); }
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/CfInstructionSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/CfInstructionSubject.java index 23684e1..45d6202 100644 --- a/src/test/java/com/android/tools/r8/utils/codeinspector/CfInstructionSubject.java +++ b/src/test/java/com/android/tools/r8/utils/codeinspector/CfInstructionSubject.java
@@ -380,6 +380,11 @@ } @Override + public boolean isFilledNewArray() { + return false; + } + + @Override public boolean isNewArray() { return instruction instanceof CfNewArray; }
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/DexInstructionSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/DexInstructionSubject.java index 16a41b1..38421a2 100644 --- a/src/test/java/com/android/tools/r8/utils/codeinspector/DexInstructionSubject.java +++ b/src/test/java/com/android/tools/r8/utils/codeinspector/DexInstructionSubject.java
@@ -49,6 +49,7 @@ import com.android.tools.r8.dex.code.DexDivIntLit8; import com.android.tools.r8.dex.code.DexDivLong; import com.android.tools.r8.dex.code.DexDivLong2Addr; +import com.android.tools.r8.dex.code.DexFilledNewArray; import com.android.tools.r8.dex.code.DexGoto; import com.android.tools.r8.dex.code.DexIfEq; import com.android.tools.r8.dex.code.DexIfEqz; @@ -624,6 +625,11 @@ } @Override + public boolean isFilledNewArray() { + return instruction instanceof DexFilledNewArray; + } + + @Override public int size() { return instruction.getSize(); }
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java index 7fd166e..6d1bd94 100644 --- a/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java +++ b/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java
@@ -158,6 +158,8 @@ boolean isMonitorExit(); + boolean isFilledNewArray(); + int size(); InstructionOffsetSubject getOffset(MethodSubject methodSubject);
diff --git a/third_party/dependencies.tar.gz.sha1 b/third_party/dependencies.tar.gz.sha1 new file mode 100644 index 0000000..cf12fc2 --- /dev/null +++ b/third_party/dependencies.tar.gz.sha1
@@ -0,0 +1 @@ +76c51489d87c284cea0e73646c5cc45a9ffc3665 \ No newline at end of file
diff --git a/third_party/dependencies_new.tar.gz.sha1 b/third_party/dependencies_new.tar.gz.sha1 new file mode 100644 index 0000000..54a7c72 --- /dev/null +++ b/third_party/dependencies_new.tar.gz.sha1
@@ -0,0 +1 @@ +08dbd497e182be658628252f0bb890894cc88dcc \ No newline at end of file
diff --git a/tools/create_local_maven_with_dependencies.py b/tools/create_local_maven_with_dependencies.py new file mode 100755 index 0000000..296cdf0 --- /dev/null +++ b/tools/create_local_maven_with_dependencies.py
@@ -0,0 +1,143 @@ +#!/usr/bin/env python3 +# Copyright (c) 2023, 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. + +import argparse +import os.path +import subprocess +import shutil +import sys + +import utils + +REPOSITORIES = [ + 'Maven Central=https://repo1.maven.org/maven2/', + 'Google=https://maven.google.com/', + "Gradle Plugins=https://plugins.gradle.org/m2/", +] + +ANDRDID_SUPPORT_VERSION = '25.4.0' +ASM_VERSION = '9.5' +ESPRESSO_VERSION = '3.0.0' +FASTUTIL_VERSION = '7.2.1' +KOTLIN_METADATA_VERSION = '0.6.0' +KOTLIN_VERSION = '1.8.0' +GUAVA_VERSION = '31.1-jre' +GSON_VERSION = '2.7' +JAVASSIST_VERSION = '3.29.2-GA' +JUNIT_VERSION = '4.13-beta-2' +MOCKITO_VERSION = '2.10.0' +SMALI_VERSION = '3.0.3' +ERROR_PRONE_VERSION = '2.18.0' +TESTNG_VERSION = '6.10' + + + +BUILD_DEPENDENCIES = [ + 'com.google.code.gson:gson:{version}'.format(version = GSON_VERSION), + 'com.google.guava:guava:{version}'.format(version = GUAVA_VERSION), + 'it.unimi.dsi:fastutil:{version}'.format(version = FASTUTIL_VERSION), + 'org.jetbrains.kotlinx:kotlinx-metadata-jvm:{version}'.format(version = KOTLIN_METADATA_VERSION), + 'org.ow2.asm:asm:{version}'.format(version = ASM_VERSION), + 'org.ow2.asm:asm-util:{version}'.format(version = ASM_VERSION), + 'org.ow2.asm:asm-commons:{version}'.format(version = ASM_VERSION), +] + +TEST_DEPENDENCIES = [ + 'junit:junit:{version}'.format(version = JUNIT_VERSION), + 'com.android.support:support-v4:{version}'.format(version = ANDRDID_SUPPORT_VERSION), + 'com.android.support.test.espresso:espresso-core:{version}'.format(version = ESPRESSO_VERSION), + 'com.android.tools.smali:smali:{version}'.format(version = SMALI_VERSION), + 'com.google.errorprone:error_prone_core:{version}'.format(version = ERROR_PRONE_VERSION), + 'org.javassist:javassist:{version}'.format(version = JAVASSIST_VERSION), + 'org.jetbrains.kotlin:kotlin-stdlib:{version}'.format(version = KOTLIN_VERSION), + 'org.jetbrains.kotlin:kotlin-reflect:{version}'.format(version = KOTLIN_VERSION), + 'org.mockito:mockito-core:{version}'.format(version = MOCKITO_VERSION), + 'org.testng:testng:{version}'.format(version = TESTNG_VERSION), +] + +NEW_DEPENDENCIES = [ + 'org.gradle.kotlin.kotlin-dsl:org.gradle.kotlin.kotlin-dsl.gradle.plugin:4.0.6', + 'org.jetbrains.kotlin:kotlin-gradle-plugin-api:1.8.10', + 'org.jetbrains.kotlin:kotlin-gradle-plugin-idea:1.8.10', + 'org.jetbrains.kotlin:kotlin-reflect:1.6.10', + 'org.jetbrains.kotlin:kotlin-reflect:1.8.10', + 'org.jetbrains.kotlin:kotlin-script-runtime:1.8.10', + 'org.jetbrains.kotlin:kotlin-tooling-core:1.8.10', + 'net.ltgt.errorprone:net.ltgt.errorprone.gradle.plugin:3.0.1' +] + +def dependencies_tar(dependencies_path): + return os.path.join( + os.path.dirname(dependencies_path), + os.path.basename(dependencies_path) + '.tar.gz') + +def dependencies_tar_sha1(dependencies_path): + return os.path.join( + os.path.dirname(dependencies_path), + os.path.basename(dependencies_path) + '.tar.gz.sha1') + +def remove_local_maven_repository(dependencies_path): + if os.path.exists(dependencies_path): + shutil.rmtree(dependencies_path) + tar = dependencies_tar(dependencies_path) + if os.path.exists(tar): + os.remove(tar) + sha1 = dependencies_tar_sha1(dependencies_path) + if os.path.exists(sha1): + os.remove(sha1) + +def create_local_maven_repository(args, dependencies_path, repositories, dependencies): + with utils.ChangedWorkingDirectory(args.studio): + cmd = [ + os.path.join('tools', 'base', 'bazel', 'bazel'), + 'run', + '//tools/base/bazel:local_maven_repository_generator_cli', + '--', + '--repo-path', + dependencies_path, + '--fetch'] + for repository in repositories: + cmd.extend(['--remote-repo', repository]) + for dependency in dependencies: + cmd.append(dependency) + subprocess.check_call(cmd) + +def parse_options(): + result = argparse.ArgumentParser( + description='Create local Maven repository woth dependencies') + result.add_argument('--studio', + metavar=('<path>'), + required=True, + help='Path to a studio-main checkout (to get the tool ' + '//tools/base/bazel:local_maven_repository_generator_cli)') + return result.parse_args() + + +def main(): + args = parse_options() + + dependencies_path = os.path.join(utils.THIRD_PARTY, 'dependencies') + print("Downloading to " + dependencies_path) + remove_local_maven_repository(dependencies_path) + create_local_maven_repository( + args, dependencies_path, REPOSITORIES, BUILD_DEPENDENCIES + TEST_DEPENDENCIES) + + dependencies_new_path = os.path.join(utils.THIRD_PARTY, 'dependencies_new') + print("Downloading to " + dependencies_new_path) + remove_local_maven_repository(dependencies_new_path) + create_local_maven_repository( + args, dependencies_new_path, REPOSITORIES, NEW_DEPENDENCIES) + + print('Now run') + print(' (cd {third_party};' + ' upload_to_google_storage.py -a --bucket r8-deps {dependencies};' + ' upload_to_google_storage.py -a --bucket r8-deps {dependencies_new})' + .format( + third_party = utils.THIRD_PARTY, + dependencies = 'dependencies', + dependencies_new = 'dependencies_new')) + +if __name__ == '__main__': + sys.exit(main())
diff --git a/tools/r8_release.py b/tools/r8_release.py index 0e32525..dc47fa2 100755 --- a/tools/r8_release.py +++ b/tools/r8_release.py
@@ -161,13 +161,16 @@ return subprocess.check_call(cmd) -def update_prebuilds(r8_checkout, version, checkout): +def update_prebuilds(r8_checkout, version, checkout, keepanno=False): path = os.path.join(r8_checkout, 'tools', 'update_prebuilds_in_android.py') commit_arg = '--commit_hash=' if len(version) == 40 else '--version=' - subprocess.check_call([path, '--targets=lib', '--maps', commit_arg + version, checkout]) + cmd = [path, '--targets=lib', '--maps', commit_arg + version, checkout] + if keepanno: + cmd.append("--keepanno") + subprocess.check_call(cmd) -def release_studio_or_aosp(r8_checkout, path, options, git_message): +def release_studio_or_aosp(r8_checkout, path, options, git_message, keepanno=False): with utils.ChangedWorkingDirectory(path): if not options.use_existing_work_branch: subprocess.call(['repo', 'abandon', 'update-r8']) @@ -180,7 +183,7 @@ with utils.ChangedWorkingDirectory(prebuilts_r8): subprocess.check_call(['repo', 'start', 'update-r8']) - update_prebuilds(r8_checkout, options.version, path) + update_prebuilds(r8_checkout, options.version, path, keepanno) with utils.ChangedWorkingDirectory(prebuilts_r8): if not options.use_existing_work_branch: @@ -193,7 +196,8 @@ # Don't upload if requested not to, or if changes are not committed due # to --use-existing-work-branch if not options.no_upload and not options.use_existing_work_branch: - process = subprocess.Popen(['repo', 'upload', '.', '--verify'], + process = subprocess.Popen(['repo', 'upload', '.', '--verify', + '--current-branch'], stdin=subprocess.PIPE) return process.communicate(input=b'y\n')[0] @@ -217,7 +221,7 @@ Test: TARGET_PRODUCT=aosp_arm64 m -j core-oj""" % (args.version, args.version, args.version)) return release_studio_or_aosp( - utils.REPO_ROOT, args.aosp, options, git_message) + utils.REPO_ROOT, args.aosp, options, git_message, keepanno=True) return release_aosp
diff --git a/tools/update_prebuilds_in_android.py b/tools/update_prebuilds_in_android.py index 6f8da19..4dfbb7d 100755 --- a/tools/update_prebuilds_in_android.py +++ b/tools/update_prebuilds_in_android.py
@@ -21,6 +21,7 @@ } OTHER_TARGETS = ["LICENSE"] +KEEPANNO_JAR = 'keepanno-annotations.jar' def parse_arguments(): parser = argparse.ArgumentParser( @@ -42,6 +43,11 @@ help="Download proguard maps for jars, use only with '--target lib'.", ) parser.add_argument( + '--keepanno', + action='store_true', + help="Download keepanno-annotations library.", + ) + parser.add_argument( '--java-max-memory-size', '--java_max_memory_size', help='Use a custom max memory size for the gradle java instance, eg, 4g') @@ -86,7 +92,7 @@ print('Downloading: ' + url + ' -> ' + download_path) utils.download_file_from_cloud_storage(url, download_path, quiet=quiet) -def main_download(hash, maps, targets, target_root, version): +def main_download(hash, maps, targets, target_root, version, keepanno=False): jar_targets = JAR_TARGETS_MAP[targets] final_targets = list(map((lambda t: t[0] + '.jar'), jar_targets)) + OTHER_TARGETS with utils.TempDir() as root: @@ -95,13 +101,19 @@ download_hash(root, hash, target) if maps and target not in OTHER_TARGETS: download_hash(root, hash, target + '.map') + if keepanno: + download_hash(root, hash, KEEPANNO_JAR) else: assert version download_version(root, version, target) if maps and target not in OTHER_TARGETS: download_version(root, version, target + '.map') + if keepanno: + download_version(root, version, KEEPANNO_JAR) copy_jar_targets(root, target_root, jar_targets, maps) copy_other_targets(root, target_root) + if keepanno: + copy_targets(root, target_root, [KEEPANNO_JAR], [KEEPANNO_JAR]) def main_build(maps, max_memory_size, targets, target_root): jar_targets = JAR_TARGETS_MAP[targets] @@ -120,7 +132,8 @@ main_build(args.maps, args.java_max_memory_size, args.targets, target_root) else: assert args.commit_hash == None or args.version == None - main_download(args.commit_hash, args.maps, args.targets, target_root, args.version) + main_download( + args.commit_hash, args.maps, args.targets, target_root, args.version, args.keepanno) if __name__ == '__main__': sys.exit(main(parse_arguments()))