Merge commit 'fd460d2100e349d16a6a95dc5d1b11db54177751' into dev-release
Change-Id: I516d2653ef1b33df8ff42a229f58a12422879fb7
diff --git a/src/main/java/com/android/tools/r8/AndroidResourceInput.java b/src/main/java/com/android/tools/r8/AndroidResourceInput.java
index 9403c75..dc2602e 100644
--- a/src/main/java/com/android/tools/r8/AndroidResourceInput.java
+++ b/src/main/java/com/android/tools/r8/AndroidResourceInput.java
@@ -23,11 +23,13 @@
MANIFEST,
// The resource table, in proto format.
RESOURCE_TABLE,
- // An xml file within the res folder, in proto format if not inside res/raw, otherwise
- // in UTF-8 format.
+ // An xml file within the res folder, in proto format if not inside res/raw.
XML_FILE,
// Any other binary file withing the res folder.
RES_FOLDER_FILE,
+ // A xml file in text format which should be parsed for keep/discard rules. The files is not
+ // required to include rules. The files will NOT be copied to the output provider.
+ KEEP_RULE_FILE,
// Other files are ignored, but copied through
UNKNOWN
}
diff --git a/src/main/java/com/android/tools/r8/ArchiveProtoAndroidResourceProvider.java b/src/main/java/com/android/tools/r8/ArchiveProtoAndroidResourceProvider.java
index 7af6e19..e67c735 100644
--- a/src/main/java/com/android/tools/r8/ArchiveProtoAndroidResourceProvider.java
+++ b/src/main/java/com/android/tools/r8/ArchiveProtoAndroidResourceProvider.java
@@ -35,6 +35,7 @@
private static final String MANIFEST_NAME = "AndroidManifest.xml";
private static final String RESOURCE_TABLE = "resources.pb";
private static final String RES_FOLDER = "res/";
+ private static final String RES_RAW_FOLDER = RES_FOLDER + "raw/";
private static final String XML_SUFFIX = ".xml";
/**
@@ -56,13 +57,24 @@
while (entries.hasMoreElements()) {
ZipEntry entry = entries.nextElement();
String name = entry.getName();
+ Kind kind = getKindFromName(name);
ByteAndroidResourceInput resource =
new ByteAndroidResourceInput(
name,
- getKindFromName(name),
+ kind,
ByteStreams.toByteArray(zipFile.getInputStream(entry)),
new ArchiveEntryOrigin(name, origin));
resources.add(resource);
+ // We explicitly add any res/raw folder xml file also as a keep rule file.
+ if (kind == Kind.XML_FILE && name.startsWith(RES_RAW_FOLDER)) {
+ ByteAndroidResourceInput keepResource =
+ new ByteAndroidResourceInput(
+ name,
+ Kind.KEEP_RULE_FILE,
+ ByteStreams.toByteArray(zipFile.getInputStream(entry)),
+ new ArchiveEntryOrigin(name, origin));
+ resources.add(keepResource);
+ }
}
return resources;
} catch (IOException e) {
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index f84fd22..da49f2e 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -114,6 +114,7 @@
import com.android.tools.r8.utils.ExceptionUtils;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.Reporter;
+import com.android.tools.r8.utils.ResourceShrinkerUtils;
import com.android.tools.r8.utils.SelfRetraceTest;
import com.android.tools.r8.utils.StringDiagnostic;
import com.android.tools.r8.utils.StringUtils;
@@ -968,6 +969,9 @@
if (options.androidResourceProguardMapStrings != null) {
resourceShrinkerBuilder.setProguardMapStrings(options.androidResourceProguardMapStrings);
}
+ resourceShrinkerBuilder.setShrinkerDebugReporter(
+ ResourceShrinkerUtils.shrinkerDebugReporterFromStringConsumer(
+ options.resourceShrinkerConfiguration.getDebugConsumer(), reporter));
LegacyResourceShrinker shrinker = resourceShrinkerBuilder.build();
ShrinkerResult shrinkerResult;
if (options.resourceShrinkerConfiguration.isOptimizedShrinking()) {
@@ -1024,6 +1028,9 @@
shrinkerResult.getResourceTableInProtoFormat(featureSplit)),
reporter);
break;
+ case KEEP_RULE_FILE:
+ // Intentionally not written
+ break;
case RES_FOLDER_FILE:
case XML_FILE:
if (toKeep.contains(androidResource.getPath().location())) {
@@ -1059,6 +1066,9 @@
case XML_FILE:
resourceShrinkerBuilder.addXmlInput(path, bytes);
break;
+ case KEEP_RULE_FILE:
+ resourceShrinkerBuilder.addKeepRuleInput(bytes);
+ break;
case UNKNOWN:
break;
}
diff --git a/src/main/java/com/android/tools/r8/ResourceShrinkerConfiguration.java b/src/main/java/com/android/tools/r8/ResourceShrinkerConfiguration.java
index 1a5b781..82137e2 100644
--- a/src/main/java/com/android/tools/r8/ResourceShrinkerConfiguration.java
+++ b/src/main/java/com/android/tools/r8/ResourceShrinkerConfiguration.java
@@ -30,14 +30,17 @@
@KeepForApi
public class ResourceShrinkerConfiguration {
public static ResourceShrinkerConfiguration DEFAULT_CONFIGURATION =
- new ResourceShrinkerConfiguration(false, true);
+ new ResourceShrinkerConfiguration(false, true, null);
private final boolean optimizedShrinking;
private final boolean preciseShrinking;
+ private final StringConsumer debugConsumer;
- private ResourceShrinkerConfiguration(boolean optimizedShrinking, boolean preciseShrinking) {
+ private ResourceShrinkerConfiguration(
+ boolean optimizedShrinking, boolean preciseShrinking, StringConsumer debugConsumer) {
this.optimizedShrinking = optimizedShrinking;
this.preciseShrinking = preciseShrinking;
+ this.debugConsumer = debugConsumer;
}
public static Builder builder(DiagnosticsHandler handler) {
@@ -52,6 +55,10 @@
return preciseShrinking;
}
+ public StringConsumer getDebugConsumer() {
+ return debugConsumer;
+ }
+
/**
* Builder for constructing a ResourceShrinkerConfiguration.
*
@@ -63,6 +70,7 @@
private boolean optimizedShrinking = false;
private boolean preciseShrinking = true;
+ private StringConsumer debugConsumer;
private Builder() {}
@@ -82,6 +90,11 @@
return this;
}
+ public Builder setDebugConsumer(StringConsumer consumer) {
+ this.debugConsumer = consumer;
+ return this;
+ }
+
/**
* Disable precise shrinking.
*
@@ -97,7 +110,7 @@
/** Build and return the {@link ResourceShrinkerConfiguration} */
public ResourceShrinkerConfiguration build() {
- return new ResourceShrinkerConfiguration(optimizedShrinking, preciseShrinking);
+ return new ResourceShrinkerConfiguration(optimizedShrinking, preciseShrinking, debugConsumer);
}
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/inlining/SimpleInliningConstraint.java b/src/main/java/com/android/tools/r8/ir/analysis/inlining/SimpleInliningConstraint.java
index cbeb329..e32cc71 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/inlining/SimpleInliningConstraint.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/inlining/SimpleInliningConstraint.java
@@ -61,7 +61,7 @@
}
assert isArgumentConstraint() || isDisjunction();
assert other.isArgumentConstraint() || other.isDisjunction();
- return new SimpleInliningConstraintConjunction(ImmutableList.of(this, other));
+ return SimpleInliningConstraintConjunction.create(ImmutableList.of(this, other));
}
public final SimpleInliningConstraintWithDepth lazyMeet(
@@ -90,7 +90,7 @@
}
assert isArgumentConstraint() || isConjunction();
assert other.isArgumentConstraint() || other.isConjunction();
- return new SimpleInliningConstraintDisjunction(ImmutableList.of(this, other));
+ return SimpleInliningConstraintDisjunction.create(ImmutableList.of(this, other));
}
public abstract SimpleInliningConstraint fixupAfterParametersChanged(
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/inlining/SimpleInliningConstraintAnalysis.java b/src/main/java/com/android/tools/r8/ir/analysis/inlining/SimpleInliningConstraintAnalysis.java
index 41e9bb0..de956ca 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/inlining/SimpleInliningConstraintAnalysis.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/inlining/SimpleInliningConstraintAnalysis.java
@@ -46,6 +46,8 @@
*/
public class SimpleInliningConstraintAnalysis {
+ private static final int MAX_BRANCH_DEPTH = 3;
+
private final SimpleInliningConstraintFactory constraintFactory;
private final DexItemFactory dexItemFactory;
private final ProgramMethod method;
@@ -78,18 +80,20 @@
// returns.
InstructionIterator instructionIterator =
code.entryBlock().iterator(code.getNumberOfArguments());
- return analyzeInstructionsInBlock(code.entryBlock(), 0, instructionIterator);
+ return analyzeInstructionsInBlock(code.entryBlock(), 0, 0, instructionIterator);
}
private SimpleInliningConstraintWithDepth analyzeInstructionsInBlock(
- BasicBlock block, int depth) {
- return analyzeInstructionsInBlock(block, depth, block.iterator());
+ BasicBlock block, int branchDepth, int instructionDepth) {
+ return analyzeInstructionsInBlock(block, branchDepth, instructionDepth, block.iterator());
}
private SimpleInliningConstraintWithDepth analyzeInstructionsInBlock(
- BasicBlock block, int instructionDepth, InstructionIterator instructionIterator) {
- // If we reach a block that has already been seen, or one that has catch handlers, then give up.
- if (!seen.add(block) || block.hasCatchHandlers()) {
+ BasicBlock block,
+ int branchDepth,
+ int instructionDepth,
+ InstructionIterator instructionIterator) {
+ if (!seen.add(block) || block.hasCatchHandlers() || branchDepth > MAX_BRANCH_DEPTH) {
return SimpleInliningConstraintWithDepth.getNever();
}
@@ -119,7 +123,8 @@
}
SimpleInliningConstraintWithDepth jumpConstraint =
- computeConstraintForJumpInstruction(instruction.asJumpInstruction(), instructionDepth);
+ computeConstraintForJumpInstruction(
+ instruction.asJumpInstruction(), branchDepth, instructionDepth);
return jumpConstraint.meet(blockConstraint);
}
@@ -154,79 +159,94 @@
}
private SimpleInliningConstraintWithDepth computeConstraintForJumpInstruction(
- JumpInstruction instruction, int instructionDepth) {
+ JumpInstruction instruction, int branchDepth, int instructionDepth) {
switch (instruction.opcode()) {
case IF:
- If ifInstruction = instruction.asIf();
- Value singleArgumentOperand = getSingleArgumentOperand(ifInstruction);
- if (singleArgumentOperand == null || singleArgumentOperand.isThis()) {
- break;
+ {
+ If ifInstruction = instruction.asIf();
+ Value singleArgumentOperand = getSingleArgumentOperand(ifInstruction);
+ if (singleArgumentOperand == null || singleArgumentOperand.isThis()) {
+ break;
+ }
+
+ Value otherOperand =
+ ifInstruction.isZeroTest()
+ ? null
+ : ifInstruction.getOperand(
+ 1 - ifInstruction.inValues().indexOf(singleArgumentOperand));
+
+ int argumentIndex =
+ singleArgumentOperand.getAliasedValue().getDefinition().asArgument().getIndex();
+ DexType argumentType = method.getDefinition().getArgumentType(argumentIndex);
+ int currentBranchDepth = branchDepth;
+ int currentInstructionDepth = instructionDepth;
+
+ // Compute the constraint for which paths through the true target are guaranteed to exit
+ // early.
+ int newBranchDepth = currentBranchDepth + 1;
+ SimpleInliningConstraintWithDepth trueTargetConstraint =
+ computeConstraintFromIfTest(
+ argumentIndex, argumentType, otherOperand, ifInstruction.getType())
+ // Only recurse into the true target if the constraint from the if-instruction
+ // is not 'never'.
+ .lazyMeet(
+ () ->
+ analyzeInstructionsInBlock(
+ ifInstruction.getTrueTarget(),
+ newBranchDepth,
+ currentInstructionDepth));
+
+ // Compute the constraint for which paths through the false target are guaranteed to
+ // exit early.
+ SimpleInliningConstraintWithDepth fallthroughTargetConstraint =
+ computeConstraintFromIfTest(
+ argumentIndex, argumentType, otherOperand, ifInstruction.getType().inverted())
+ // Only recurse into the false target if the constraint from the if-instruction
+ // is not 'never'.
+ .lazyMeet(
+ () ->
+ analyzeInstructionsInBlock(
+ ifInstruction.fallthroughBlock(),
+ newBranchDepth,
+ currentInstructionDepth));
+
+ // Paths going through this basic block are guaranteed to exit early if the true target
+ // is guaranteed to exit early or the false target is.
+ return trueTargetConstraint.join(fallthroughTargetConstraint);
}
- Value otherOperand =
- ifInstruction.isZeroTest()
- ? null
- : ifInstruction.getOperand(
- 1 - ifInstruction.inValues().indexOf(singleArgumentOperand));
-
- int argumentIndex =
- singleArgumentOperand.getAliasedValue().getDefinition().asArgument().getIndex();
- DexType argumentType = method.getDefinition().getArgumentType(argumentIndex);
- int currentDepth = instructionDepth;
-
- // Compute the constraint for which paths through the true target are guaranteed to exit
- // early.
- SimpleInliningConstraintWithDepth trueTargetConstraint =
- computeConstraintFromIfTest(
- argumentIndex, argumentType, otherOperand, ifInstruction.getType())
- // Only recurse into the true target if the constraint from the if-instruction
- // is not 'never'.
- .lazyMeet(
- () -> analyzeInstructionsInBlock(ifInstruction.getTrueTarget(), currentDepth));
-
- // Compute the constraint for which paths through the false target are guaranteed to
- // exit early.
- SimpleInliningConstraintWithDepth fallthroughTargetConstraint =
- computeConstraintFromIfTest(
- argumentIndex, argumentType, otherOperand, ifInstruction.getType().inverted())
- // Only recurse into the false target if the constraint from the if-instruction
- // is not 'never'.
- .lazyMeet(
- () ->
- analyzeInstructionsInBlock(ifInstruction.fallthroughBlock(), currentDepth));
-
- // Paths going through this basic block are guaranteed to exit early if the true target
- // is guaranteed to exit early or the false target is.
- return trueTargetConstraint.join(fallthroughTargetConstraint);
-
case GOTO:
- return analyzeInstructionsInBlock(instruction.asGoto().getTarget(), instructionDepth);
+ return analyzeInstructionsInBlock(
+ instruction.asGoto().getTarget(), branchDepth, instructionDepth);
case RETURN:
return AlwaysSimpleInliningConstraint.getInstance().withDepth(instructionDepth);
case STRING_SWITCH:
- // Require that all cases including the default case are simple. In that case we can
- // guarantee simpleness by requiring that the switch value is constant.
- StringSwitch stringSwitch = instruction.asStringSwitch();
- Value valueRoot = stringSwitch.value().getAliasedValue();
- if (!valueRoot.isDefinedByInstructionSatisfying(Instruction::isArgument)) {
- return SimpleInliningConstraintWithDepth.getNever();
- }
- int maxInstructionDepth = instructionDepth;
- for (BasicBlock successor : stringSwitch.getBlock().getNormalSuccessors()) {
- SimpleInliningConstraintWithDepth successorConstraintWithDepth =
- analyzeInstructionsInBlock(successor, instructionDepth);
- if (!successorConstraintWithDepth.getConstraint().isAlways()) {
+ {
+ // Require that all cases including the default case are simple. In that case we can
+ // guarantee simpleness by requiring that the switch value is constant.
+ StringSwitch stringSwitch = instruction.asStringSwitch();
+ Value valueRoot = stringSwitch.value().getAliasedValue();
+ if (!valueRoot.isDefinedByInstructionSatisfying(Instruction::isArgument)) {
return SimpleInliningConstraintWithDepth.getNever();
}
- maxInstructionDepth =
- Math.max(maxInstructionDepth, successorConstraintWithDepth.getInstructionDepth());
+ int newBranchDepth = branchDepth + 1;
+ int maxInstructionDepth = instructionDepth;
+ for (BasicBlock successor : stringSwitch.getBlock().getNormalSuccessors()) {
+ SimpleInliningConstraintWithDepth successorConstraintWithDepth =
+ analyzeInstructionsInBlock(successor, newBranchDepth, instructionDepth);
+ if (!successorConstraintWithDepth.getConstraint().isAlways()) {
+ return SimpleInliningConstraintWithDepth.getNever();
+ }
+ maxInstructionDepth =
+ Math.max(maxInstructionDepth, successorConstraintWithDepth.getInstructionDepth());
+ }
+ Argument argument = valueRoot.getDefinition().asArgument();
+ ConstSimpleInliningConstraint simpleConstraint =
+ constraintFactory.createConstConstraint(argument.getIndex());
+ return simpleConstraint.withDepth(maxInstructionDepth);
}
- Argument argument = valueRoot.getDefinition().asArgument();
- ConstSimpleInliningConstraint simpleConstraint =
- constraintFactory.createConstConstraint(argument.getIndex());
- return simpleConstraint.withDepth(maxInstructionDepth);
case THROW:
return instruction.getBlock().hasCatchHandlers()
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/inlining/SimpleInliningConstraintConjunction.java b/src/main/java/com/android/tools/r8/ir/analysis/inlining/SimpleInliningConstraintConjunction.java
index 6147cd8..a282039 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/inlining/SimpleInliningConstraintConjunction.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/inlining/SimpleInliningConstraintConjunction.java
@@ -15,9 +15,11 @@
public class SimpleInliningConstraintConjunction extends SimpleInliningConstraint {
+ private static final int MAX_SIZE = 3;
+
private final List<SimpleInliningConstraint> constraints;
- public SimpleInliningConstraintConjunction(List<SimpleInliningConstraint> constraints) {
+ private SimpleInliningConstraintConjunction(List<SimpleInliningConstraint> constraints) {
assert constraints.size() > 1;
assert constraints.stream().noneMatch(SimpleInliningConstraint::isAlways);
assert constraints.stream().noneMatch(SimpleInliningConstraint::isConjunction);
@@ -25,6 +27,12 @@
this.constraints = constraints;
}
+ public static SimpleInliningConstraint create(List<SimpleInliningConstraint> constraints) {
+ return constraints.size() <= MAX_SIZE
+ ? new SimpleInliningConstraintConjunction(constraints)
+ : NeverSimpleInliningConstraint.getInstance();
+ }
+
SimpleInliningConstraint add(SimpleInliningConstraint constraint) {
assert !constraint.isAlways();
assert !constraint.isNever();
@@ -32,16 +40,15 @@
return addAll(constraint.asConjunction());
}
assert constraint.isArgumentConstraint() || constraint.isDisjunction();
- return new SimpleInliningConstraintConjunction(
+ return create(
ImmutableList.<SimpleInliningConstraint>builder()
.addAll(constraints)
.add(constraint)
.build());
}
- public SimpleInliningConstraintConjunction addAll(
- SimpleInliningConstraintConjunction conjunction) {
- return new SimpleInliningConstraintConjunction(
+ public SimpleInliningConstraint addAll(SimpleInliningConstraintConjunction conjunction) {
+ return create(
ImmutableList.<SimpleInliningConstraint>builder()
.addAll(constraints)
.addAll(conjunction.constraints)
@@ -103,6 +110,6 @@
return NeverSimpleInliningConstraint.getInstance();
}
- return new SimpleInliningConstraintConjunction(rewrittenConstraints);
+ return create(rewrittenConstraints);
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/inlining/SimpleInliningConstraintDisjunction.java b/src/main/java/com/android/tools/r8/ir/analysis/inlining/SimpleInliningConstraintDisjunction.java
index 02c0b01..660aa3b 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/inlining/SimpleInliningConstraintDisjunction.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/inlining/SimpleInliningConstraintDisjunction.java
@@ -15,9 +15,11 @@
public class SimpleInliningConstraintDisjunction extends SimpleInliningConstraint {
+ private static final int MAX_SIZE = 3;
+
private final List<SimpleInliningConstraint> constraints;
- public SimpleInliningConstraintDisjunction(List<SimpleInliningConstraint> constraints) {
+ private SimpleInliningConstraintDisjunction(List<SimpleInliningConstraint> constraints) {
assert constraints.size() > 1;
assert constraints.stream().noneMatch(SimpleInliningConstraint::isAlways);
assert constraints.stream().noneMatch(SimpleInliningConstraint::isDisjunction);
@@ -25,6 +27,12 @@
this.constraints = constraints;
}
+ public static SimpleInliningConstraint create(List<SimpleInliningConstraint> constraints) {
+ return constraints.size() <= MAX_SIZE
+ ? new SimpleInliningConstraintDisjunction(constraints)
+ : NeverSimpleInliningConstraint.getInstance();
+ }
+
SimpleInliningConstraint add(SimpleInliningConstraint constraint) {
assert !constraint.isAlways();
assert !constraint.isNever();
@@ -32,16 +40,15 @@
return addAll(constraint.asDisjunction());
}
assert constraint.isArgumentConstraint() || constraint.isConjunction();
- return new SimpleInliningConstraintDisjunction(
+ return create(
ImmutableList.<SimpleInliningConstraint>builder()
.addAll(constraints)
.add(constraint)
.build());
}
- public SimpleInliningConstraintDisjunction addAll(
- SimpleInliningConstraintDisjunction disjunction) {
- return new SimpleInliningConstraintDisjunction(
+ public SimpleInliningConstraint addAll(SimpleInliningConstraintDisjunction disjunction) {
+ return create(
ImmutableList.<SimpleInliningConstraint>builder()
.addAll(constraints)
.addAll(disjunction.constraints)
@@ -103,6 +110,6 @@
return AlwaysSimpleInliningConstraint.getInstance();
}
- return new SimpleInliningConstraintDisjunction(rewrittenConstraints);
+ return create(rewrittenConstraints);
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/path/PathConstraintAnalysisTransferFunction.java b/src/main/java/com/android/tools/r8/ir/analysis/path/PathConstraintAnalysisTransferFunction.java
index 11fe070..c61df2a 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/path/PathConstraintAnalysisTransferFunction.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/path/PathConstraintAnalysisTransferFunction.java
@@ -7,6 +7,7 @@
import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.ir.analysis.framework.intraprocedural.AbstractTransferFunction;
import com.android.tools.r8.ir.analysis.framework.intraprocedural.TransferFunctionResult;
+import com.android.tools.r8.ir.analysis.path.state.ConcretePathConstraintAnalysisState;
import com.android.tools.r8.ir.analysis.path.state.PathConstraintAnalysisState;
import com.android.tools.r8.ir.analysis.value.AbstractValueFactory;
import com.android.tools.r8.ir.code.BasicBlock;
@@ -41,6 +42,14 @@
}
@Override
+ public PathConstraintAnalysisState computeInitialState(
+ BasicBlock entryBlock, PathConstraintAnalysisState bottom) {
+ // Intentionally returns an empty state instead of BOTTOM, as BOTTOM is used to represent the
+ // path constraint for unreachable program points.
+ return new ConcretePathConstraintAnalysisState();
+ }
+
+ @Override
public PathConstraintAnalysisState computeBlockEntryState(
BasicBlock block, BasicBlock predecessor, PathConstraintAnalysisState predecessorExitState) {
if (predecessorExitState.isUnknown()) {
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/path/PathConstraintSupplier.java b/src/main/java/com/android/tools/r8/ir/analysis/path/PathConstraintSupplier.java
new file mode 100644
index 0000000..cb81605
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/ir/analysis/path/PathConstraintSupplier.java
@@ -0,0 +1,41 @@
+// Copyright (c) 2024, 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.path;
+
+import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.ir.analysis.framework.intraprocedural.DataflowAnalysisResult.SuccessfulDataflowAnalysisResult;
+import com.android.tools.r8.ir.analysis.path.state.PathConstraintAnalysisState;
+import com.android.tools.r8.ir.code.BasicBlock;
+import com.android.tools.r8.ir.code.IRCode;
+import com.android.tools.r8.optimize.argumentpropagation.codescanner.MethodParameterFactory;
+import com.android.tools.r8.shaking.AppInfoWithLiveness;
+
+public class PathConstraintSupplier {
+
+ private final AppView<AppInfoWithLiveness> appView;
+ private final IRCode code;
+ private final MethodParameterFactory methodParameterFactory;
+
+ private SuccessfulDataflowAnalysisResult<BasicBlock, PathConstraintAnalysisState>
+ pathConstraintAnalysisResult;
+
+ public PathConstraintSupplier(
+ AppView<AppInfoWithLiveness> appView,
+ IRCode code,
+ MethodParameterFactory methodParameterFactory) {
+ this.appView = appView;
+ this.code = code;
+ this.methodParameterFactory = methodParameterFactory;
+ }
+
+ public PathConstraintAnalysisState getPathConstraint(BasicBlock block) {
+ if (pathConstraintAnalysisResult == null) {
+ PathConstraintAnalysis analysis =
+ new PathConstraintAnalysis(appView, code, methodParameterFactory);
+ pathConstraintAnalysisResult = analysis.run(code.entryBlock()).asSuccessfulAnalysisResult();
+ assert pathConstraintAnalysisResult != null;
+ }
+ return pathConstraintAnalysisResult.getBlockExitState(block);
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/path/state/ConcretePathConstraintAnalysisState.java b/src/main/java/com/android/tools/r8/ir/analysis/path/state/ConcretePathConstraintAnalysisState.java
index fca5f48..7bcb650 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/path/state/ConcretePathConstraintAnalysisState.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/path/state/ConcretePathConstraintAnalysisState.java
@@ -3,11 +3,12 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.ir.analysis.path.state;
+import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.optimize.argumentpropagation.computation.ComputationTreeNode;
import java.util.Collections;
-import java.util.HashSet;
-import java.util.Objects;
-import java.util.Set;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Map.Entry;
/**
* Represents a non-trivial (neither bottom nor top) path constraint that must be satisfied to reach
@@ -16,19 +17,18 @@
* <p>The state should be interpreted as follows:
*
* <ol>
- * <li>If a path constraint is BOTH in {@link #pathConstraints} AND {@link
- * #negatedPathConstraints} then the path constraint should be IGNORED.
- * <li>If a path constraint is ONLY in {@link #pathConstraints} then the current program point can
- * only be reached if the path constraint is satisfied.
- * <li>If a path constraint is ONLY in {@link #negatedPathConstraints} then the current program
- * point can only be reached if the path constraint is NOT satisfied.
+ * <li>If a path constraint is DISABLED then the path constraint should be IGNORED.
+ * <li>If a path constraint is POSITIVE then the current program point can only be reached if the
+ * path constraint is satisfied.
+ * <li>If a path constraint is NEGATIVE then the current program point can only be reached if the
+ * path constraint is NOT satisfied.
* </ol>
*
- * <p>Example: In the below example, when entering the IF-THEN branch, we add the path constraint
- * [(flags & 1) != 0] to {@link #pathConstraints}. When entering the (empty) IF-ELSE branch, we add
- * the same path constraint to {@link #negatedPathConstraints}. When reaching the return block, the
- * two path constraint states are joined, resulting in the state where {@link #pathConstraints} and
- * {@link #negatedPathConstraints} both contains the constraint [(flags & 1) != 0].
+ * <p>Example: In the below example, when entering the IF-THEN branch, we add [(flags & 1) != 0] as
+ * a POSITIVE path constraint to {@link #pathConstraints}. When entering the (empty) IF-ELSE branch,
+ * we add the same expression as a NEGATIVE path constraint to {@link #pathConstraints}. When
+ * reaching the return block, the two path constraint states are joined, resulting in the state
+ * where the constraint [(flags & 1) != 0] is DISABLED.
*
* <pre>
* static Object Foo(Object o, int flags) {
@@ -41,61 +41,54 @@
*/
public class ConcretePathConstraintAnalysisState extends PathConstraintAnalysisState {
- private final Set<ComputationTreeNode> pathConstraints;
- private final Set<ComputationTreeNode> negatedPathConstraints;
+ // TODO(b/302281503): Consider changing this to an ImmutableMap.
+ private final Map<ComputationTreeNode, PathConstraintKind> pathConstraints;
+
+ public ConcretePathConstraintAnalysisState() {
+ this.pathConstraints = Collections.emptyMap();
+ }
ConcretePathConstraintAnalysisState(
- Set<ComputationTreeNode> pathConstraints, Set<ComputationTreeNode> negatedPathConstraints) {
+ Map<ComputationTreeNode, PathConstraintKind> pathConstraints) {
this.pathConstraints = pathConstraints;
- this.negatedPathConstraints = negatedPathConstraints;
}
static ConcretePathConstraintAnalysisState create(
ComputationTreeNode pathConstraint, boolean negate) {
- Set<ComputationTreeNode> pathConstraints = Collections.singleton(pathConstraint);
- if (negate) {
- return new ConcretePathConstraintAnalysisState(Collections.emptySet(), pathConstraints);
- } else {
- return new ConcretePathConstraintAnalysisState(pathConstraints, Collections.emptySet());
- }
+ return new ConcretePathConstraintAnalysisState(
+ Collections.singletonMap(pathConstraint, PathConstraintKind.get(negate)));
}
@Override
public PathConstraintAnalysisState add(ComputationTreeNode pathConstraint, boolean negate) {
- if (negate) {
- if (negatedPathConstraints.contains(pathConstraint)) {
+ PathConstraintKind previousKind = pathConstraints.get(pathConstraint);
+ if (previousKind != null) {
+ if (previousKind == PathConstraintKind.DISABLED) {
+ // There is a loop.
return this;
}
- return new ConcretePathConstraintAnalysisState(
- pathConstraints, add(pathConstraint, negatedPathConstraints));
- } else {
- if (pathConstraints.contains(pathConstraint)) {
+ if (previousKind == PathConstraintKind.get(negate)) {
+ // This branch is dominated by a previous if-condition that has the same branch condition,
+ // e.g., if (x) { if (x) { ...
return this;
}
- return new ConcretePathConstraintAnalysisState(
- add(pathConstraint, pathConstraints), negatedPathConstraints);
+ // This branch is dominated by a previous if-condition that has the negated branch condition,
+ // e.g., if (x) { if (!x) { ...
+ return bottom();
}
+ // No jumps can dominate the entry of their own block, so when adding the condition of a jump
+ // this cannot currently be active.
+ Map<ComputationTreeNode, PathConstraintKind> newPathConstraints =
+ new HashMap<>(pathConstraints.size() + 1);
+ newPathConstraints.putAll(pathConstraints);
+ newPathConstraints.put(pathConstraint, PathConstraintKind.get(negate));
+ return new ConcretePathConstraintAnalysisState(newPathConstraints);
}
- private static Set<ComputationTreeNode> add(
- ComputationTreeNode pathConstraint, Set<ComputationTreeNode> pathConstraints) {
- if (pathConstraints.isEmpty()) {
- return Collections.singleton(pathConstraint);
- }
- assert !pathConstraints.contains(pathConstraint);
- Set<ComputationTreeNode> newPathConstraints = new HashSet<>(pathConstraints);
- newPathConstraints.add(pathConstraint);
- return newPathConstraints;
- }
-
- public Set<ComputationTreeNode> getPathConstraints() {
+ public Map<ComputationTreeNode, PathConstraintKind> getPathConstraintsForTesting() {
return pathConstraints;
}
- public Set<ComputationTreeNode> getNegatedPathConstraints() {
- return negatedPathConstraints;
- }
-
@Override
public boolean isConcrete() {
return true;
@@ -106,55 +99,87 @@
return this;
}
+ @Override
+ public boolean isGreaterThanOrEquals(AppView<?> appView, PathConstraintAnalysisState state) {
+ if (state.isConcrete()) {
+ return isGreaterThanOrEquals(state.asConcreteState());
+ }
+ return super.isGreaterThanOrEquals(appView, state);
+ }
+
+ private boolean isGreaterThanOrEquals(ConcretePathConstraintAnalysisState other) {
+ // The current state must contain all keys of the other state.
+ if (pathConstraints.size() < other.pathConstraints.size()) {
+ return false;
+ }
+ for (ComputationTreeNode otherPathConstraint : other.pathConstraints.keySet()) {
+ if (!pathConstraints.containsKey(otherPathConstraint)) {
+ return false;
+ }
+ }
+ // The current path constraint kinds must be the join of the kinds.
+ for (Entry<ComputationTreeNode, PathConstraintKind> entry : pathConstraints.entrySet()) {
+ ComputationTreeNode pathConstraint = entry.getKey();
+ PathConstraintKind kind = entry.getValue();
+ PathConstraintKind otherKind = other.pathConstraints.get(pathConstraint);
+ PathConstraintKind joinKind = kind.join(otherKind);
+ if (kind != joinKind) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ public boolean isNegated(ComputationTreeNode pathConstraint) {
+ PathConstraintKind kind = pathConstraints.get(pathConstraint);
+ assert kind != null;
+ assert kind != PathConstraintKind.DISABLED;
+ return kind == PathConstraintKind.NEGATIVE;
+ }
+
// TODO(b/302281503): Consider returning the list of differentiating path constraints.
// For example, if we have the condition X & Y, then we may not know anything about X but we
// could know something about Y. Add a test showing this.
- // TODO(b/302281503): Add a field `inactivePathConstraints` and ensure that pathConstraints and
- // negatedPathConstraints are disjoint. This way (1) we reduce the size of the sets and (2)
- // this does not need to check if each path constraint is not in the negated set.
public ComputationTreeNode getDifferentiatingPathConstraint(
ConcretePathConstraintAnalysisState other) {
- for (ComputationTreeNode pathConstraint : pathConstraints) {
- if (!negatedPathConstraints.contains(pathConstraint)
- && other.negatedPathConstraints.contains(pathConstraint)) {
- return pathConstraint;
+ for (Entry<ComputationTreeNode, PathConstraintKind> entry : pathConstraints.entrySet()) {
+ ComputationTreeNode pathConstraint = entry.getKey();
+ PathConstraintKind kind = entry.getValue();
+ if (kind == PathConstraintKind.DISABLED) {
+ continue;
}
- }
- for (ComputationTreeNode negatedPathConstraint : negatedPathConstraints) {
- if (!pathConstraints.contains(negatedPathConstraint)
- && other.pathConstraints.contains(negatedPathConstraint)) {
- return negatedPathConstraint;
+ PathConstraintKind otherKind = other.pathConstraints.get(pathConstraint);
+ if (otherKind != null && kind.isNegation(otherKind)) {
+ return pathConstraint;
}
}
return null;
}
public ConcretePathConstraintAnalysisState join(ConcretePathConstraintAnalysisState other) {
- Set<ComputationTreeNode> newPathConstraints = join(pathConstraints, other.pathConstraints);
- Set<ComputationTreeNode> newNegatedPathConstraints =
- join(negatedPathConstraints, other.negatedPathConstraints);
- if (identical(newPathConstraints, newNegatedPathConstraints)) {
+ if (isGreaterThanOrEquals(other)) {
return this;
}
- if (other.identical(newPathConstraints, newNegatedPathConstraints)) {
+ if (other.isGreaterThanOrEquals(this)) {
return other;
}
- return new ConcretePathConstraintAnalysisState(newPathConstraints, newNegatedPathConstraints);
+ Map<ComputationTreeNode, PathConstraintKind> newPathConstraints =
+ new HashMap<>(pathConstraints.size() + other.pathConstraints.size());
+ join(other, newPathConstraints);
+ other.join(this, newPathConstraints);
+ return new ConcretePathConstraintAnalysisState(newPathConstraints);
}
- private static Set<ComputationTreeNode> join(
- Set<ComputationTreeNode> pathConstraints, Set<ComputationTreeNode> otherPathConstraints) {
- if (pathConstraints.isEmpty()) {
- return otherPathConstraints;
+ private void join(
+ ConcretePathConstraintAnalysisState other,
+ Map<ComputationTreeNode, PathConstraintKind> newPathConstraints) {
+ for (Entry<ComputationTreeNode, PathConstraintKind> entry : pathConstraints.entrySet()) {
+ ComputationTreeNode pathConstraint = entry.getKey();
+ PathConstraintKind kind = entry.getValue();
+ PathConstraintKind otherKind = other.pathConstraints.get(pathConstraint);
+ PathConstraintKind joinKind = kind.join(otherKind);
+ newPathConstraints.put(pathConstraint, joinKind);
}
- if (otherPathConstraints.isEmpty()) {
- return pathConstraints;
- }
- Set<ComputationTreeNode> newPathConstraints =
- new HashSet<>(pathConstraints.size() + otherPathConstraints.size());
- newPathConstraints.addAll(pathConstraints);
- newPathConstraints.addAll(otherPathConstraints);
- return newPathConstraints;
}
@Override
@@ -166,23 +191,11 @@
return false;
}
ConcretePathConstraintAnalysisState state = (ConcretePathConstraintAnalysisState) obj;
- return equals(state.pathConstraints, state.negatedPathConstraints);
- }
-
- public boolean equals(
- Set<ComputationTreeNode> pathConstraints, Set<ComputationTreeNode> negatedPathConstraints) {
- return this.pathConstraints.equals(pathConstraints)
- && this.negatedPathConstraints.equals(negatedPathConstraints);
- }
-
- public boolean identical(
- Set<ComputationTreeNode> pathConstraints, Set<ComputationTreeNode> negatedPathConstraints) {
- return this.pathConstraints == pathConstraints
- && this.negatedPathConstraints == negatedPathConstraints;
+ return pathConstraints.equals(state.pathConstraints);
}
@Override
public int hashCode() {
- return Objects.hash(pathConstraints, negatedPathConstraints);
+ return pathConstraints.hashCode();
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/path/state/PathConstraintKind.java b/src/main/java/com/android/tools/r8/ir/analysis/path/state/PathConstraintKind.java
new file mode 100644
index 0000000..4e97d27
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/ir/analysis/path/state/PathConstraintKind.java
@@ -0,0 +1,29 @@
+// Copyright (c) 2024, 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.path.state;
+
+public enum PathConstraintKind {
+ POSITIVE,
+ NEGATIVE,
+ DISABLED;
+
+ static PathConstraintKind get(boolean negate) {
+ return negate ? NEGATIVE : POSITIVE;
+ }
+
+ boolean isNegation(PathConstraintKind other) {
+ switch (this) {
+ case POSITIVE:
+ return other == NEGATIVE;
+ case NEGATIVE:
+ return other == POSITIVE;
+ default:
+ return false;
+ }
+ }
+
+ PathConstraintKind join(PathConstraintKind other) {
+ return this == other ? this : DISABLED;
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/value/NumberFromIntervalValue.java b/src/main/java/com/android/tools/r8/ir/analysis/value/NumberFromIntervalValue.java
index 161f1a1..450c351 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/value/NumberFromIntervalValue.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/value/NumberFromIntervalValue.java
@@ -105,7 +105,7 @@
@Override
public int hashCode() {
- int hash = 31 * (31 * (31 + Long.hashCode(minInclusive)) + Long.hashCode(maxInclusive));
+ int hash = 31 * (31 + Long.hashCode(minInclusive)) + Long.hashCode(maxInclusive);
assert hash == Objects.hash(minInclusive, maxInclusive);
return hash;
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
index 9ab29f3..2d4b2ee 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
@@ -204,13 +204,10 @@
TrivialGotosCollapser trivialGotosCollapser = new TrivialGotosCollapser(appView);
timing.begin("BasicBlock peephole optimizations");
- if (code.getConversionOptions().isPeepholeOptimizationsEnabled()) {
- for (int i = 0; i < PEEPHOLE_OPTIMIZATION_PASSES; i++) {
- trivialGotosCollapser.run(code, timing);
- PeepholeOptimizer.removeIdenticalPredecessorBlocks(code, registerAllocator);
- PeepholeOptimizer.shareIdenticalBlockSuffix(
- code, registerAllocator, SUFFIX_SHARING_OVERHEAD);
- }
+ for (int i = 0; i < PEEPHOLE_OPTIMIZATION_PASSES; i++) {
+ trivialGotosCollapser.run(code, timing);
+ PeepholeOptimizer.removeIdenticalPredecessorBlocks(code, registerAllocator);
+ PeepholeOptimizer.shareIdenticalBlockSuffix(code, registerAllocator, SUFFIX_SHARING_OVERHEAD);
}
timing.end();
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRToDexFinalizer.java b/src/main/java/com/android/tools/r8/ir/conversion/IRToDexFinalizer.java
index 205d2bc..edac43f 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRToDexFinalizer.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRToDexFinalizer.java
@@ -76,14 +76,12 @@
registerAllocator.allocateRegisters();
timing.end();
TrivialGotosCollapser trivialGotosCollapser = new TrivialGotosCollapser(appView);
- if (code.getConversionOptions().isPeepholeOptimizationsEnabled()) {
- timing.begin("Peephole optimize");
- for (int i = 0; i < PEEPHOLE_OPTIMIZATION_PASSES; i++) {
- trivialGotosCollapser.run(code, timing);
- PeepholeOptimizer.optimize(appView, code, registerAllocator);
- }
- timing.end();
+ timing.begin("Peephole optimize");
+ for (int i = 0; i < PEEPHOLE_OPTIMIZATION_PASSES; i++) {
+ trivialGotosCollapser.run(code, timing);
+ PeepholeOptimizer.optimize(appView, code, registerAllocator);
}
+ timing.end();
timing.begin("Clean up");
CodeRewriter.removeUnneededMovesOnExitingPaths(code, registerAllocator);
trivialGotosCollapser.run(code, timing);
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java b/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
index 679a884..72e0051 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
@@ -107,7 +107,6 @@
import com.android.tools.r8.ir.code.UnusedArgument;
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.ir.code.ValueType;
-import com.android.tools.r8.ir.conversion.passes.TrivialPhiSimplifier;
import com.android.tools.r8.ir.optimize.AffectedValues;
import com.android.tools.r8.ir.optimize.CustomLensCodeRewriter;
import com.android.tools.r8.optimize.MemberRebindingAnalysis;
@@ -233,15 +232,6 @@
affectedPhis.addAll(
customLensCodeRewriter.rewriteCode(code, methodProcessor, prototypeChanges, graphLens));
}
- if (!unusedArguments.isEmpty()) {
- for (UnusedArgument unusedArgument : unusedArguments) {
- if (unusedArgument.outValue().hasPhiUsers()) {
- // See b/240282988: We can end up in situations where the second round of IR processing
- // introduce phis for irreducible control flow, we need to resolve them.
- TrivialPhiSimplifier.replaceUnusedArgumentTrivialPhis(unusedArgument);
- }
- }
- }
rewritePartialDefault(
code,
method,
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/MethodConversionOptions.java b/src/main/java/com/android/tools/r8/ir/conversion/MethodConversionOptions.java
index fd3c580..970479e 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/MethodConversionOptions.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/MethodConversionOptions.java
@@ -78,25 +78,17 @@
public abstract boolean isGeneratingDex();
- public abstract boolean isPeepholeOptimizationsEnabled();
-
public abstract boolean shouldFinalizeAfterLensCodeRewriter();
public static class MutableMethodConversionOptions extends MethodConversionOptions {
private final Target target;
- private boolean enablePeepholeOptimizations = true;
private boolean finalizeAfterLensCodeRewriter;
private MutableMethodConversionOptions(Target target) {
this.target = target;
}
- public void disablePeepholeOptimizations(MethodProcessor methodProcessor) {
- assert methodProcessor.isPrimaryMethodProcessor();
- enablePeepholeOptimizations = false;
- }
-
public MutableMethodConversionOptions setFinalizeAfterLensCodeRewriter() {
finalizeAfterLensCodeRewriter = true;
return this;
@@ -118,11 +110,6 @@
}
@Override
- public boolean isPeepholeOptimizationsEnabled() {
- return enablePeepholeOptimizations;
- }
-
- @Override
public boolean shouldFinalizeAfterLensCodeRewriter() {
return finalizeAfterLensCodeRewriter;
}
@@ -148,10 +135,5 @@
public boolean isGeneratingDex() {
throw new Unreachable();
}
-
- @Override
- public boolean isPeepholeOptimizationsEnabled() {
- throw new Unreachable();
- }
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/PrimaryR8IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/PrimaryR8IRConverter.java
index 59a9a0e..3b3ba7a 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/PrimaryR8IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/PrimaryR8IRConverter.java
@@ -135,7 +135,6 @@
// Analyze the data collected by the argument propagator, use the analysis result to update
// the parameter optimization infos, and rewrite the application.
// TODO(b/199237357): Automatically rewrite state when lens changes.
- enumUnboxer.rewriteWithLens();
numberUnboxer.rewriteWithLens();
outliner.rewriteWithLens();
appView.withArgumentPropagator(
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/passes/TrivialPhiSimplifier.java b/src/main/java/com/android/tools/r8/ir/conversion/passes/TrivialPhiSimplifier.java
index 27a3cba..eacb3aa 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/passes/TrivialPhiSimplifier.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/passes/TrivialPhiSimplifier.java
@@ -14,7 +14,6 @@
import com.android.tools.r8.ir.code.InvokeDirect;
import com.android.tools.r8.ir.code.NewInstance;
import com.android.tools.r8.ir.code.Phi;
-import com.android.tools.r8.ir.code.UnusedArgument;
import com.android.tools.r8.ir.code.Value;
import com.google.common.collect.Sets;
import java.util.List;
@@ -22,34 +21,22 @@
public class TrivialPhiSimplifier {
- public static void replaceUnusedArgumentTrivialPhis(UnusedArgument unusedArgument) {
- replaceTrivialPhis(unusedArgument.outValue());
- for (Phi phiUser : unusedArgument.outValue().uniquePhiUsers()) {
- phiUser.removeTrivialPhi();
- }
- assert !unusedArgument.outValue().hasPhiUsers();
- }
-
- @SuppressWarnings("ReferenceEquality")
public static void ensureDirectStringNewToInit(AppView<?> appView, IRCode code) {
boolean changed = false;
DexItemFactory dexItemFactory = appView.dexItemFactory();
- for (Instruction instruction : code.instructions()) {
- if (instruction.isInvokeDirect()) {
- InvokeDirect invoke = instruction.asInvokeDirect();
- DexMethod method = invoke.getInvokedMethod();
- if (dexItemFactory.isConstructor(method)
- && method.holder == dexItemFactory.stringType
- && invoke.getReceiver().isPhi()) {
- NewInstance newInstance = findNewInstance(invoke.getReceiver().asPhi());
- replaceTrivialPhis(newInstance.outValue());
- if (invoke.getReceiver().isPhi()) {
- throw new CompilationError(
- "Failed to remove trivial phis between new-instance and <init>");
- }
- newInstance.markNoSpilling();
- changed = true;
+ for (InvokeDirect invoke : code.<InvokeDirect>instructions(Instruction::isInvokeDirect)) {
+ DexMethod method = invoke.getInvokedMethod();
+ if (dexItemFactory.isConstructor(method)
+ && method.getHolderType().isIdenticalTo(dexItemFactory.stringType)
+ && invoke.getReceiver().isPhi()) {
+ NewInstance newInstance = findNewInstance(invoke.getReceiver().asPhi());
+ replaceTrivialPhis(newInstance.outValue());
+ if (invoke.getReceiver().isPhi()) {
+ throw new CompilationError(
+ "Failed to remove trivial phis between new-instance and <init>");
}
+ newInstance.markNoSpilling();
+ changed = true;
}
}
assert !changed || code.isConsistentSSA(appView);
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 d7bf459..520e2d8 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
@@ -38,6 +38,7 @@
import com.android.tools.r8.ir.desugar.ServiceLoaderSourceCode;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.ConsumerUtils;
+import com.android.tools.r8.utils.DominatorChecker;
import com.android.tools.r8.utils.ListUtils;
import com.android.tools.r8.utils.WorkList;
import java.util.ArrayList;
@@ -231,9 +232,9 @@
replacements.put(loadResult.iteratorInvoke, code.createConstNull());
// Iterator.hasNext() --> true / false
- if (directRewriteResult.hasNextInstr != null) {
+ if (directRewriteResult.priorHasNextInstr != null) {
replacements.put(
- directRewriteResult.hasNextInstr,
+ directRewriteResult.priorHasNextInstr,
code.createBooleanConstant(!loadResult.implClasses.isEmpty()));
}
if (directRewriteResult.nextInstr != null) {
@@ -279,6 +280,11 @@
replacementExtras.put(directRewriteResult.nextInstr, List.of(initInstr));
}
}
+ if (directRewriteResult.subsequentHasNextInstr != null) {
+ replacements.put(
+ directRewriteResult.subsequentHasNextInstr,
+ code.createBooleanConstant(loadResult.implClasses.size() > 1));
+ }
}
private ServiceLoaderLoadResult analyzeServiceLoaderLoad(IRCode code, InvokeStatic invokeInstr) {
@@ -428,8 +434,9 @@
if (iteratorInvoke.outValue().hasPhiUsers()) {
return null;
}
- InvokeMethod hasNextInstr = null;
+ InvokeMethod priorHasNextInstr = null;
InvokeMethod nextInstr = null;
+ InvokeMethod subsequentHasNextInstr = null;
// We only bother to support a single call to hasNext() and next(), and they must appear within
// the same try/catch.
for (Instruction user : iteratorInvoke.outValue().aliasedUsers()) {
@@ -447,10 +454,14 @@
return null;
}
if (curCall.getInvokedMethod().isIdenticalTo(iteratorMethods.hasNext)) {
- if (hasNextInstr != null) {
+ if (subsequentHasNextInstr != null) {
return null;
}
- hasNextInstr = curCall;
+ if (priorHasNextInstr != null) {
+ subsequentHasNextInstr = curCall;
+ } else {
+ priorHasNextInstr = curCall;
+ }
} else if (curCall.getInvokedMethod().isIdenticalTo(iteratorMethods.next)) {
if (nextInstr != null) {
return null;
@@ -461,29 +472,58 @@
}
}
+ // Figure out which hasNext is the first one.
BasicBlock iteratorBlock = iteratorInvoke.getBlock();
- BasicBlock hasNextBlock = hasNextInstr != null ? hasNextInstr.getBlock() : null;
+ BasicBlock priorHasNextBlock = priorHasNextInstr != null ? priorHasNextInstr.getBlock() : null;
+ BasicBlock subsequentHasNextBlock =
+ subsequentHasNextInstr != null ? subsequentHasNextInstr.getBlock() : null;
BasicBlock nextBlock = nextInstr != null ? nextInstr.getBlock() : null;
- if (hasNextBlock != null && nextBlock != null) {
- // See if hasNext() is reachable after next().
- if (hasNextBlock == nextBlock) {
- if (nextBlock.iterator(nextInstr).nextUntil(hasNextInstr) != null) {
+ if (priorHasNextBlock != null && nextBlock != null) {
+ if (subsequentHasNextBlock != null) {
+ if (priorHasNextBlock == subsequentHasNextBlock) {
+ // This would be odd, since hasNext() is usually used in conditionals.
return null;
}
- } else if (hasPredecessorPathTo(iteratorBlock, hasNextBlock, nextBlock)) {
+ // Make sure subsequentHasNextBlock and priorHasNextBlock are not reversed.
+ if (hasPredecessorPathTo(iteratorBlock, priorHasNextBlock, subsequentHasNextBlock)) {
+ InvokeMethod tmp = priorHasNextInstr;
+ priorHasNextInstr = subsequentHasNextInstr;
+ subsequentHasNextInstr = tmp;
+ priorHasNextBlock = subsequentHasNextBlock;
+ subsequentHasNextBlock = subsequentHasNextInstr.getBlock();
+ }
+ // Ensure the second hasNext() comes after the call to next().
+ if (nextBlock == subsequentHasNextBlock) {
+ if (nextBlock.iterator(nextInstr).nextUntil(subsequentHasNextInstr) == null) {
+ return null;
+ }
+ } else if (!DominatorChecker.check(iteratorBlock, subsequentHasNextBlock, nextBlock)) {
+ return null;
+ }
+ }
+
+ // Ensure the first hasNext() is not after next().
+ if (priorHasNextBlock == nextBlock) {
+ if (nextBlock.iterator(nextInstr).nextUntil(priorHasNextInstr) != null) {
+ return null;
+ }
+ } else if (hasPredecessorPathTo(iteratorBlock, priorHasNextBlock, nextBlock)) {
return null;
}
}
// Make sure each instruction can be run at most once (no loops).
- if (hasNextBlock != null && loopExists(iteratorBlock, hasNextBlock)) {
+ if (priorHasNextBlock != null && loopExists(iteratorBlock, priorHasNextBlock)) {
return null;
}
if (nextBlock != null && loopExists(iteratorBlock, nextBlock)) {
return null;
}
+ if (subsequentHasNextBlock != null && loopExists(iteratorBlock, subsequentHasNextBlock)) {
+ return null;
+ }
- return new DirectRewriteResult(hasNextInstr, nextInstr);
+ return new DirectRewriteResult(priorHasNextInstr, nextInstr, subsequentHasNextInstr);
}
private static boolean loopExists(BasicBlock subgraphEntryBlock, BasicBlock targetBlock) {
@@ -495,6 +535,9 @@
if (subgraphEntryBlock == subgraphExitBlock) {
return false;
}
+ if (subgraphEntryBlock == targetBlock) {
+ return true;
+ }
WorkList<BasicBlock> workList = WorkList.newIdentityWorkList();
workList.markAsSeen(subgraphEntryBlock);
workList.addIfNotSeen(subgraphExitBlock.getPredecessors());
@@ -636,12 +679,17 @@
}
private static class DirectRewriteResult {
+ public final InvokeMethod priorHasNextInstr;
public final InvokeMethod nextInstr;
- public final InvokeMethod hasNextInstr;
+ public final InvokeMethod subsequentHasNextInstr;
- public DirectRewriteResult(InvokeMethod hasNextInstr, InvokeMethod nextInstr) {
- this.hasNextInstr = hasNextInstr;
+ public DirectRewriteResult(
+ InvokeMethod priorHasNextInstr,
+ InvokeMethod nextInstr,
+ InvokeMethod subsequentHasNextInstr) {
+ this.priorHasNextInstr = priorHasNextInstr;
this.nextInstr = nextInstr;
+ this.subsequentHasNextInstr = subsequentHasNextInstr;
}
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/EmptyEnumUnboxer.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/EmptyEnumUnboxer.java
index 4e07fba..5c31e4c 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/EmptyEnumUnboxer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/EmptyEnumUnboxer.java
@@ -12,7 +12,7 @@
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.conversion.IRConverter;
import com.android.tools.r8.ir.conversion.MethodProcessor;
-import com.android.tools.r8.ir.conversion.PostMethodProcessor.Builder;
+import com.android.tools.r8.ir.conversion.PostMethodProcessor;
import com.android.tools.r8.ir.optimize.info.OptimizationFeedbackDelayed;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.Timing;
@@ -54,17 +54,10 @@
}
@Override
- @SuppressWarnings("BadImport")
- public void rewriteWithLens() {
- // Intentionally empty.
- }
-
- @Override
- @SuppressWarnings("BadImport")
public void unboxEnums(
AppView<AppInfoWithLiveness> appView,
IRConverter converter,
- Builder postMethodProcessorBuilder,
+ PostMethodProcessor.Builder postMethodProcessorBuilder,
ExecutorService executorService,
OptimizationFeedbackDelayed feedback,
Timing timing) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxer.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxer.java
index 081d608..1106670 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxer.java
@@ -12,7 +12,7 @@
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.conversion.IRConverter;
import com.android.tools.r8.ir.conversion.MethodProcessor;
-import com.android.tools.r8.ir.conversion.PostMethodProcessor.Builder;
+import com.android.tools.r8.ir.conversion.PostMethodProcessor;
import com.android.tools.r8.ir.optimize.info.OptimizationFeedbackDelayed;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.Timing;
@@ -41,13 +41,10 @@
public abstract void recordEnumState(DexProgramClass clazz, StaticFieldValues staticFieldValues);
- public abstract void rewriteWithLens();
-
- @SuppressWarnings("BadImport")
public abstract void unboxEnums(
AppView<AppInfoWithLiveness> appView,
IRConverter converter,
- Builder postMethodProcessorBuilder,
+ PostMethodProcessor.Builder postMethodProcessorBuilder,
ExecutorService executorService,
OptimizationFeedbackDelayed feedback,
Timing timing)
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxerImpl.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxerImpl.java
index 7a20f638..1a6404b 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxerImpl.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxerImpl.java
@@ -36,6 +36,7 @@
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexItemFactory;
+import com.android.tools.r8.graph.DexItemFactory.ClassMethods;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexMethodHandle;
import com.android.tools.r8.graph.DexProgramClass;
@@ -79,7 +80,7 @@
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.ir.conversion.IRConverter;
import com.android.tools.r8.ir.conversion.MethodProcessor;
-import com.android.tools.r8.ir.conversion.PostMethodProcessor.Builder;
+import com.android.tools.r8.ir.conversion.PostMethodProcessor;
import com.android.tools.r8.ir.desugar.LambdaDescriptor;
import com.android.tools.r8.ir.optimize.enums.EnumDataMap.EnumData;
import com.android.tools.r8.ir.optimize.enums.EnumInstanceFieldData.EnumInstanceFieldKnownData;
@@ -156,10 +157,6 @@
private final Map<DexType, EnumStaticFieldValues> staticFieldValuesMap =
new ConcurrentHashMap<>();
- // Methods depending on library modelisation need to be reprocessed so they are peephole
- // optimized.
- private LongLivedProgramMethodSetBuilder<ProgramMethodSet> methodsDependingOnLibraryModelisation;
-
// Map from checkNotNull() methods to the enums that use the given method.
private LongLivedProgramMethodMapBuilder<LongLivedClassSetBuilder<DexProgramClass>>
checkNotNullMethodsBuilder;
@@ -219,10 +216,6 @@
return false;
}
- private void markMethodDependsOnLibraryModelisation(ProgramMethod method) {
- methodsDependingOnLibraryModelisation.add(method, appView.graphLens());
- }
-
private DexProgramClass getEnumUnboxingCandidateOrNull(TypeElement lattice) {
if (lattice.isClassType()) {
DexType classType = lattice.asClassType().getClassType();
@@ -322,13 +315,6 @@
enumUnboxingCandidatesInfo.addMethodDependency(eligibleEnum, code.context());
}
}
- // TODO(b/225838009): Remove this when always using LIR.
- if (!appView.testing().canUseLir(appView)) {
- if (methodsDependingOnLibraryModelisation.contains(code.context(), appView.graphLens())) {
- code.mutateConversionOptions(
- conversionOptions -> conversionOptions.disablePeepholeOptimizations(methodProcessor));
- }
- }
}
private void markEnumEligible(DexType type, Set<DexType> eligibleEnums) {
@@ -507,7 +493,6 @@
eligibleEnums.add(enumType);
}
- @SuppressWarnings("ReferenceEquality")
private boolean isLegitimateConstClassUser(
Instruction user, ProgramMethod context, DexProgramClass enumClass) {
if (user.isAssume()) {
@@ -523,11 +508,10 @@
if (singleTarget == null) {
return false;
}
- if (singleTarget.getReference() == factory.enumMembers.valueOf) {
+ if (singleTarget.getReference().isIdenticalTo(factory.enumMembers.valueOf)) {
// The name data is required for the correct mapping from the enum name to the ordinal
// in the valueOf utility method.
addRequiredNameData(enumClass);
- markMethodDependsOnLibraryModelisation(context);
// The out-value must be cast before it is used, or an assume instruction must strengthen
// its dynamic type, so that the out-value is analyzed by the enum unboxing analysis.
if (invoke.hasOutValue()) {
@@ -546,7 +530,7 @@
}
} else if (enumUser.isCheckCast()) {
CheckCast checkCast = enumUser.asCheckCast();
- if (checkCast.getType() == enumClass.getType()) {
+ if (checkCast.getType().isIdenticalTo(enumClass.getType())) {
// OK.
continue;
}
@@ -556,9 +540,9 @@
}
return true;
}
- if (singleTarget.getReference()
- == factory.javaLangReflectArrayMembers.newInstanceMethodWithDimensions) {
- markMethodDependsOnLibraryModelisation(context);
+ if (singleTarget
+ .getReference()
+ .isIdenticalTo(factory.javaLangReflectArrayMembers.newInstanceMethodWithDimensions)) {
return true;
}
}
@@ -566,7 +550,7 @@
if (user.isInvokeVirtual()) {
InvokeVirtual invoke = user.asInvokeVirtual();
DexMethod invokedMethod = invoke.getInvokedMethod();
- if (invokedMethod == factory.classMethods.desiredAssertionStatus) {
+ if (invokedMethod.isIdenticalTo(factory.classMethods.desiredAssertionStatus)) {
// Only valid in the enum's class initializer, since the class constant must be rewritten
// to LocalEnumUtility.class instead of int.class.
return context.getDefinition().isClassInitializer() && context.getHolder() == enumClass;
@@ -584,11 +568,11 @@
enumClass, factory.enumMembers.nameField);
}
- @SuppressWarnings("ReferenceEquality")
private boolean isUnboxableNameMethod(DexMethod method) {
- return method == factory.classMethods.getName
- || method == factory.classMethods.getCanonicalName
- || method == factory.classMethods.getSimpleName;
+ ClassMethods classMethods = factory.classMethods;
+ return method.isIdenticalTo(classMethods.getName)
+ || method.isIdenticalTo(classMethods.getCanonicalName)
+ || method.isIdenticalTo(classMethods.getSimpleName);
}
private void addNullDependencies(IRCode code, Value nullValue, Set<DexType> eligibleEnums) {
@@ -676,24 +660,13 @@
enumUnboxingCandidatesInfo =
new EnumUnboxingCandidateAnalysis(appView, this)
.findCandidates(graphLensForPrimaryOptimizationPass);
- methodsDependingOnLibraryModelisation =
- LongLivedProgramMethodSetBuilder.createConcurrentForIdentitySet(
- graphLensForPrimaryOptimizationPass);
}
@Override
- @SuppressWarnings("BadImport")
- public void rewriteWithLens() {
- methodsDependingOnLibraryModelisation =
- methodsDependingOnLibraryModelisation.rewrittenWithLens(appView.graphLens());
- }
-
- @Override
- @SuppressWarnings("BadImport")
public void unboxEnums(
AppView<AppInfoWithLiveness> appView,
IRConverter converter,
- Builder postMethodProcessorBuilder,
+ PostMethodProcessor.Builder postMethodProcessorBuilder,
ExecutorService executorService,
OptimizationFeedbackDelayed feedback,
Timing timing)
@@ -762,12 +735,7 @@
dependencies
.rewrittenWithLens(appView)
.removeAll(treeFixerResult.getPrunedItems().getRemovedMethods()))
- .merge(
- methodsDependingOnLibraryModelisation
- .rewrittenWithLens(appView)
- .removeAll(treeFixerResult.getPrunedItems().getRemovedMethods()))
.addAll(treeFixerResult.getMethodsToProcess(), appView.graphLens());
- methodsDependingOnLibraryModelisation.clear();
updateOptimizationInfos(executorService, feedback, treeFixerResult, previousLens);
@@ -871,7 +839,6 @@
return enumDataMap;
}
- @SuppressWarnings("ReferenceEquality")
private EnumDataMap analyzeEnumInstances() {
ImmutableMap.Builder<DexType, DexType> enumSubtypes = ImmutableMap.builder();
ImmutableMap.Builder<DexType, EnumData> builder = ImmutableMap.builder();
@@ -888,7 +855,7 @@
builder.put(enumClass.type, data);
if (data.valuesTypes != null) {
for (DexType value : data.valuesTypes.values()) {
- if (value != enumClass.type) {
+ if (value.isNotIdenticalTo(enumClass.type)) {
enumSubtypes.put(value, enumClass.type);
}
}
@@ -1487,20 +1454,13 @@
return Reason.INVALID_INVOKE;
}
- Reason reason =
- analyzeLibraryInvoke(
- invoke,
- context,
- enumClass,
- enumValue,
- singleTarget.getReference(),
- singleTarget.getHolder());
-
- if (reason == Reason.ELIGIBLE) {
- markMethodDependsOnLibraryModelisation(context);
- }
-
- return reason;
+ return analyzeLibraryInvoke(
+ invoke,
+ context,
+ enumClass,
+ enumValue,
+ singleTarget.getReference(),
+ singleTarget.getHolder());
}
private Reason comparableAsUnboxedValues(InvokeMethod invoke) {
@@ -1770,6 +1730,5 @@
@Override
public void onMethodCodePruned(ProgramMethod method) {
enumUnboxingCandidatesInfo.addPrunedMethod(method);
- methodsDependingOnLibraryModelisation.remove(method.getReference(), appView.graphLens());
}
}
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagator.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagator.java
index 1d0edbe..2273dbb 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagator.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagator.java
@@ -10,6 +10,7 @@
import com.android.tools.r8.graph.ImmediateProgramSubtypingInfo;
import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.graph.PrunedItems;
+import com.android.tools.r8.ir.analysis.path.PathConstraintSupplier;
import com.android.tools.r8.ir.code.AbstractValueSupplier;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.conversion.IRConverter;
@@ -17,6 +18,7 @@
import com.android.tools.r8.ir.conversion.PostMethodProcessor;
import com.android.tools.r8.ir.conversion.PrimaryR8IRConverter;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.FieldStateCollection;
+import com.android.tools.r8.optimize.argumentpropagation.codescanner.InFlowComparator;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.MethodState;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.MethodStateCollectionByReference;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.VirtualRootMethodsAnalysis;
@@ -127,7 +129,9 @@
assert methodProcessor.isPrimaryMethodProcessor();
AbstractValueSupplier abstractValueSupplier =
value -> value.getAbstractValue(appView, method);
- codeScanner.scan(method, code, abstractValueSupplier, timing);
+ PathConstraintSupplier pathConstraintSupplier =
+ new PathConstraintSupplier(appView, code, codeScanner.getMethodParameterFactory());
+ codeScanner.scan(method, code, abstractValueSupplier, pathConstraintSupplier, timing);
assert effectivelyUnusedArgumentsAnalysis != null;
effectivelyUnusedArgumentsAnalysis.scan(method, code);
@@ -249,6 +253,7 @@
assert appView.isAllCodeProcessed();
FieldStateCollection fieldStates = codeScanner.getFieldStates();
MethodStateCollectionByReference methodStates = codeScanner.getMethodStates();
+ InFlowComparator inFlowComparator = codeScanner.getInFlowComparator();
appView.testing().argumentPropagatorEventConsumer.acceptCodeScannerResult(methodStates);
codeScanner = null;
@@ -261,6 +266,7 @@
immediateSubtypingInfo,
fieldStates,
methodStates,
+ inFlowComparator,
stronglyConnectedProgramComponents,
interfaceDispatchOutsideProgram)
.propagateOptimizationInfo(
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorCodeScanner.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorCodeScanner.java
index b72657b..ba31341 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorCodeScanner.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorCodeScanner.java
@@ -4,7 +4,6 @@
package com.android.tools.r8.optimize.argumentpropagation;
-
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexClassAndMethod;
@@ -15,10 +14,8 @@
import com.android.tools.r8.graph.MethodResolutionResult.SingleResolutionResult;
import com.android.tools.r8.graph.ProgramField;
import com.android.tools.r8.graph.ProgramMethod;
-import com.android.tools.r8.ir.analysis.framework.intraprocedural.DataflowAnalysisResult.SuccessfulDataflowAnalysisResult;
-import com.android.tools.r8.ir.analysis.path.PathConstraintAnalysis;
+import com.android.tools.r8.ir.analysis.path.PathConstraintSupplier;
import com.android.tools.r8.ir.analysis.path.state.ConcretePathConstraintAnalysisState;
-import com.android.tools.r8.ir.analysis.path.state.PathConstraintAnalysisState;
import com.android.tools.r8.ir.analysis.type.DynamicType;
import com.android.tools.r8.ir.analysis.type.DynamicTypeWithUpperBound;
import com.android.tools.r8.ir.analysis.type.Nullability;
@@ -39,6 +36,7 @@
import com.android.tools.r8.ir.code.InvokeMethod;
import com.android.tools.r8.ir.code.InvokeMethodWithReceiver;
import com.android.tools.r8.ir.code.Phi;
+import com.android.tools.r8.ir.code.Position.SourcePosition;
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.AbstractFunction;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.BaseInFlow;
@@ -58,6 +56,7 @@
import com.android.tools.r8.optimize.argumentpropagation.codescanner.FieldValueFactory;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.IfThenElseAbstractFunction;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.InFlow;
+import com.android.tools.r8.optimize.argumentpropagation.codescanner.InFlowComparator;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.InstanceFieldReadAbstractFunction;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.MethodParameter;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.MethodParameterFactory;
@@ -80,6 +79,8 @@
import com.android.tools.r8.utils.Timing;
import com.android.tools.r8.utils.structural.StructuralItem;
import com.google.common.collect.Sets;
+import it.unimi.dsi.fastutil.objects.Object2IntMap;
+import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
@@ -108,7 +109,7 @@
private final FieldValueFactory fieldValueFactory = new FieldValueFactory();
- private final MethodParameterFactory methodParameterFactory = new MethodParameterFactory();
+ final MethodParameterFactory methodParameterFactory = new MethodParameterFactory();
private final Set<DexMethod> monomorphicVirtualMethods = Sets.newIdentityHashSet();
@@ -134,6 +135,8 @@
private final MethodStateCollectionByReference methodStates =
MethodStateCollectionByReference.createConcurrent();
+ private final InFlowComparator.Builder inFlowComparatorBuilder = InFlowComparator.builder();
+
public ArgumentPropagatorCodeScanner(AppView<AppInfoWithLiveness> appView) {
this(appView, new ArgumentPropagatorReprocessingCriteriaCollection(appView));
}
@@ -158,6 +161,10 @@
return fieldStates;
}
+ public MethodParameterFactory getMethodParameterFactory() {
+ return methodParameterFactory;
+ }
+
public MethodStateCollectionByReference getMethodStates() {
return methodStates;
}
@@ -166,6 +173,10 @@
return virtualRootMethods.get(method.getReference());
}
+ InFlowComparator getInFlowComparator() {
+ return inFlowComparatorBuilder.build();
+ }
+
// TODO(b/296030319): Allow lookups in the FieldStateCollection using DexField keys to avoid the
// need for definitionFor here.
private boolean isFieldValueAlreadyUnknown(DexType staticType, DexField field) {
@@ -219,8 +230,9 @@
ProgramMethod method,
IRCode code,
AbstractValueSupplier abstractValueSupplier,
+ PathConstraintSupplier pathConstraintSupplier,
Timing timing) {
- new CodeScanner(abstractValueSupplier, code, method).scan(timing);
+ new CodeScanner(abstractValueSupplier, code, method, pathConstraintSupplier).scan(timing);
}
protected class CodeScanner {
@@ -228,15 +240,19 @@
protected final AbstractValueSupplier abstractValueSupplier;
protected final IRCode code;
protected final ProgramMethod context;
+ private final PathConstraintSupplier pathConstraintSupplier;
- private SuccessfulDataflowAnalysisResult<BasicBlock, PathConstraintAnalysisState>
- pathConstraintAnalysisResult;
+ private Object2IntMap<Phi> phiNumbering = null;
protected CodeScanner(
- AbstractValueSupplier abstractValueSupplier, IRCode code, ProgramMethod method) {
+ AbstractValueSupplier abstractValueSupplier,
+ IRCode code,
+ ProgramMethod method,
+ PathConstraintSupplier pathConstraintSupplier) {
this.abstractValueSupplier = abstractValueSupplier;
this.code = code;
this.context = method;
+ this.pathConstraintSupplier = pathConstraintSupplier;
}
public void scan(Timing timing) {
@@ -359,6 +375,9 @@
// If the value is an argument of the enclosing method or defined by a field-get, then clearly
// we have no information about its abstract value (yet). Instead of treating this as having an
// unknown runtime value, we instead record a flow constraint.
+ // TODO(b/302281503): Cache computed in flow so that we do not compute the same in flow for the
+ // same value multiple times.
+ // TODO(b/302281503): Canonicalize computed in flow.
private InFlow computeInFlow(
DexType staticType,
Value value,
@@ -406,9 +425,13 @@
return null;
}
ConcretePathConstraintAnalysisState leftPredecessorPathConstraint =
- getPathConstraint(phi.getBlock().getPredecessors().get(0)).asConcreteState();
+ pathConstraintSupplier
+ .getPathConstraint(phi.getBlock().getPredecessors().get(0))
+ .asConcreteState();
ConcretePathConstraintAnalysisState rightPredecessorPathConstraint =
- getPathConstraint(phi.getBlock().getPredecessors().get(1)).asConcreteState();
+ pathConstraintSupplier
+ .getPathConstraint(phi.getBlock().getPredecessors().get(1))
+ .asConcreteState();
if (leftPredecessorPathConstraint == null || rightPredecessorPathConstraint == null) {
return null;
}
@@ -420,10 +443,34 @@
}
NonEmptyValueState leftValue = valueStateSupplier.apply(phi.getOperand(0));
NonEmptyValueState rightValue = valueStateSupplier.apply(phi.getOperand(1));
- if (leftPredecessorPathConstraint.getNegatedPathConstraints().contains(condition)) {
- return new IfThenElseAbstractFunction(condition, rightValue, leftValue);
+ IfThenElseAbstractFunction result =
+ leftPredecessorPathConstraint.isNegated(condition)
+ ? new IfThenElseAbstractFunction(condition, rightValue, leftValue)
+ : new IfThenElseAbstractFunction(condition, leftValue, rightValue);
+ recordIfThenElsePosition(result, phi);
+ return result;
+ }
+
+ private void recordIfThenElsePosition(
+ IfThenElseAbstractFunction ifThenElseAbstractFunction, Phi phi) {
+ inFlowComparatorBuilder.addIfThenElsePosition(
+ ifThenElseAbstractFunction,
+ SourcePosition.builder()
+ .setMethod(code.context().getReference())
+ .setLine(getOrCreatePhiNumbering().getInt(phi))
+ .build());
+ }
+
+ private Object2IntMap<Phi> getOrCreatePhiNumbering() {
+ if (phiNumbering == null) {
+ phiNumbering = new Object2IntOpenHashMap<>();
+ for (BasicBlock block : code.getBlocks()) {
+ for (Phi phi : block.getPhis()) {
+ phiNumbering.put(phi, phiNumbering.size());
+ }
+ }
}
- return new IfThenElseAbstractFunction(condition, leftValue, rightValue);
+ return phiNumbering;
}
private InFlow castBaseInFlow(InFlow inFlow, Value value) {
@@ -876,16 +923,6 @@
}
}
- private PathConstraintAnalysisState getPathConstraint(BasicBlock block) {
- if (pathConstraintAnalysisResult == null) {
- PathConstraintAnalysis analysis =
- new PathConstraintAnalysis(appView, code, methodParameterFactory);
- pathConstraintAnalysisResult = analysis.run(code.entryBlock()).asSuccessfulAnalysisResult();
- assert pathConstraintAnalysisResult != null;
- }
- return pathConstraintAnalysisResult.getBlockExitState(block);
- }
-
@SuppressWarnings("ReferenceEquality")
private DexMethod getRepresentative(InvokeMethod invoke, ProgramMethod resolvedMethod) {
if (resolvedMethod.getDefinition().belongsToDirectPool()) {
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorOptimizationInfoPropagator.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorOptimizationInfoPropagator.java
index 99b3640..800b277 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorOptimizationInfoPropagator.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorOptimizationInfoPropagator.java
@@ -10,6 +10,7 @@
import com.android.tools.r8.graph.ImmediateProgramSubtypingInfo;
import com.android.tools.r8.ir.conversion.PrimaryR8IRConverter;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.FieldStateCollection;
+import com.android.tools.r8.optimize.argumentpropagation.codescanner.InFlowComparator;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.MethodStateCollectionByReference;
import com.android.tools.r8.optimize.argumentpropagation.propagation.InFlowPropagator;
import com.android.tools.r8.optimize.argumentpropagation.propagation.InterfaceMethodArgumentPropagator;
@@ -34,6 +35,7 @@
private final PrimaryR8IRConverter converter;
private final FieldStateCollection fieldStates;
private final MethodStateCollectionByReference methodStates;
+ private final InFlowComparator inFlowComparator;
private final ImmediateProgramSubtypingInfo immediateSubtypingInfo;
private final List<Set<DexProgramClass>> stronglyConnectedProgramComponents;
@@ -47,6 +49,7 @@
ImmediateProgramSubtypingInfo immediateSubtypingInfo,
FieldStateCollection fieldStates,
MethodStateCollectionByReference methodStates,
+ InFlowComparator inFlowComparator,
List<Set<DexProgramClass>> stronglyConnectedProgramComponents,
BiConsumer<Set<DexProgramClass>, DexMethodSignature> interfaceDispatchOutsideProgram) {
this.appView = appView;
@@ -54,6 +57,7 @@
this.immediateSubtypingInfo = immediateSubtypingInfo;
this.fieldStates = fieldStates;
this.methodStates = methodStates;
+ this.inFlowComparator = inFlowComparator;
this.stronglyConnectedProgramComponents = stronglyConnectedProgramComponents;
this.interfaceDispatchOutsideProgram = interfaceDispatchOutsideProgram;
}
@@ -79,7 +83,8 @@
classesWithSingleCallerInlinedInstanceInitializers,
converter,
fieldStates,
- methodStates)
+ methodStates,
+ inFlowComparator)
.run(executorService);
timing.end();
}
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/BottomPrimitiveTypeValueState.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/BottomPrimitiveTypeValueState.java
index ba50708..b429da0 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/BottomPrimitiveTypeValueState.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/BottomPrimitiveTypeValueState.java
@@ -28,7 +28,7 @@
StateCloner cloner,
Action onChangedAction) {
if (inState.isBottom()) {
- assert inState == bottomPrimitiveTypeState();
+ assert inState.identical(bottomPrimitiveTypeState());
return this;
}
if (inState.isUnknown()) {
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/BottomValueState.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/BottomValueState.java
index 609b769..a267042 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/BottomValueState.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/BottomValueState.java
@@ -31,4 +31,14 @@
public ValueState mutableCopyWithoutInFlow() {
return this;
}
+
+ @Override
+ public final boolean equals(Object obj) {
+ return this == obj;
+ }
+
+ @Override
+ public final int hashCode() {
+ return System.identityHashCode(this);
+ }
}
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/CastAbstractFunction.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/CastAbstractFunction.java
index a4eb77e..2586f8d 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/CastAbstractFunction.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/CastAbstractFunction.java
@@ -44,10 +44,12 @@
}
@Override
- public int internalCompareToSameKind(InFlow other) {
+ public int internalCompareToSameKind(InFlow other, InFlowComparator comparator) {
CastAbstractFunction fn = other.asCastAbstractFunction();
if (inFlow != fn.inFlow) {
- return inFlow.compareTo(fn.inFlow);
+ int result = inFlow.compareTo(fn.inFlow, comparator);
+ assert result != 0;
+ return result;
}
return type.compareTo(fn.type);
}
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/ConcreteArrayTypeValueState.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/ConcreteArrayTypeValueState.java
index 26dc74a..ca98915 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/ConcreteArrayTypeValueState.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/ConcreteArrayTypeValueState.java
@@ -14,6 +14,7 @@
import com.android.tools.r8.utils.Action;
import com.android.tools.r8.utils.SetUtils;
import java.util.Collections;
+import java.util.Objects;
import java.util.Set;
import java.util.function.Supplier;
@@ -136,6 +137,23 @@
}
@Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (!(obj instanceof ConcreteArrayTypeValueState)) {
+ return false;
+ }
+ ConcreteArrayTypeValueState state = (ConcreteArrayTypeValueState) obj;
+ return nullability.equals(state.nullability) && getInFlow().equals(state.getInFlow());
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(getClass(), nullability, getInFlow());
+ }
+
+ @Override
public String toString() {
assert !hasInFlow();
return "ArrayState(" + nullability + ")";
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/ConcreteClassTypeValueState.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/ConcreteClassTypeValueState.java
index f6a608d..96e476b 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/ConcreteClassTypeValueState.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/ConcreteClassTypeValueState.java
@@ -15,6 +15,7 @@
import com.android.tools.r8.utils.Action;
import com.android.tools.r8.utils.SetUtils;
import java.util.Collections;
+import java.util.Objects;
import java.util.Set;
import java.util.function.Supplier;
@@ -189,6 +190,25 @@
}
@Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (!(obj instanceof ConcreteClassTypeValueState)) {
+ return false;
+ }
+ ConcreteClassTypeValueState state = (ConcreteClassTypeValueState) obj;
+ return abstractValue.equals(state.abstractValue)
+ && dynamicType.equals(state.dynamicType)
+ && getInFlow().equals(state.getInFlow());
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(getClass(), abstractValue, dynamicType, getInFlow());
+ }
+
+ @Override
public String toString() {
assert !hasInFlow();
return "ClassState(type: " + dynamicType + ", value: " + abstractValue + ")";
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/ConcretePrimitiveTypeValueState.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/ConcretePrimitiveTypeValueState.java
index 864c16e..118c05d 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/ConcretePrimitiveTypeValueState.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/ConcretePrimitiveTypeValueState.java
@@ -12,6 +12,7 @@
import com.android.tools.r8.utils.Action;
import com.android.tools.r8.utils.SetUtils;
import java.util.Collections;
+import java.util.Objects;
import java.util.Set;
import java.util.function.Supplier;
@@ -129,6 +130,23 @@
}
@Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (!(obj instanceof ConcretePrimitiveTypeValueState)) {
+ return false;
+ }
+ ConcretePrimitiveTypeValueState state = (ConcretePrimitiveTypeValueState) obj;
+ return abstractValue.equals(state.abstractValue) && getInFlow().equals(state.getInFlow());
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(getClass(), abstractValue, getInFlow());
+ }
+
+ @Override
public String toString() {
assert !hasInFlow();
return "PrimitiveState(" + abstractValue + ")";
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/ConcreteReceiverValueState.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/ConcreteReceiverValueState.java
index 350d7f9..04711fe 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/ConcreteReceiverValueState.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/ConcreteReceiverValueState.java
@@ -13,6 +13,7 @@
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.Action;
import java.util.Collections;
+import java.util.Objects;
import java.util.Set;
import java.util.function.Supplier;
@@ -147,6 +148,23 @@
}
@Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (!(obj instanceof ConcreteReceiverValueState)) {
+ return false;
+ }
+ ConcreteReceiverValueState state = (ConcreteReceiverValueState) obj;
+ return dynamicType.equals(state.dynamicType) && getInFlow().equals(state.getInFlow());
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(getClass(), dynamicType, getInFlow());
+ }
+
+ @Override
public String toString() {
assert !hasInFlow();
return "ReceiverState(" + dynamicType + ")";
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/FieldValue.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/FieldValue.java
index 0b9e152..99fcf7b 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/FieldValue.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/FieldValue.java
@@ -25,7 +25,7 @@
}
@Override
- public int internalCompareToSameKind(InFlow other) {
+ public int internalCompareToSameKind(InFlow other, InFlowComparator comparator) {
return field.compareTo(other.asFieldValue().getField());
}
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/IdentityAbstractFunction.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/IdentityAbstractFunction.java
index 1b33495b..f31b821 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/IdentityAbstractFunction.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/IdentityAbstractFunction.java
@@ -41,7 +41,7 @@
}
@Override
- public int internalCompareToSameKind(InFlow inFlow) {
+ public int internalCompareToSameKind(InFlow inFlow, InFlowComparator comparator) {
assert this == inFlow;
return 0;
}
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/IfThenElseAbstractFunction.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/IfThenElseAbstractFunction.java
index a6da375..4d21db9 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/IfThenElseAbstractFunction.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/IfThenElseAbstractFunction.java
@@ -6,10 +6,12 @@
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.ir.analysis.value.AbstractValue;
+import com.android.tools.r8.ir.code.Position.SourcePosition;
import com.android.tools.r8.optimize.argumentpropagation.computation.ComputationTreeNode;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.ListUtils;
import java.util.List;
+import java.util.Objects;
/**
* Represents a ternary expression (exp ? u : v). The {@link #condition} is an expression containing
@@ -133,9 +135,11 @@
}
@Override
- public int internalCompareToSameKind(InFlow inFlow) {
- // TODO(b/302281503): Find a way to make this comparable.
- return hashCode() - inFlow.hashCode();
+ public int internalCompareToSameKind(InFlow inFlow, InFlowComparator comparator) {
+ SourcePosition position = comparator.getIfThenElsePosition(this);
+ SourcePosition otherPosition =
+ comparator.getIfThenElsePosition(inFlow.asIfThenElseAbstractFunction());
+ return position.compareTo(otherPosition);
}
@Override
@@ -152,4 +156,23 @@
public InFlowKind getKind() {
return InFlowKind.ABSTRACT_FUNCTION_IF_THEN_ELSE;
}
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (!(obj instanceof IfThenElseAbstractFunction)) {
+ return false;
+ }
+ IfThenElseAbstractFunction fn = (IfThenElseAbstractFunction) obj;
+ return condition.equals(fn.condition)
+ && thenState.equals(fn.thenState)
+ && elseState.equals(fn.elseState);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(getClass(), condition, thenState, elseState);
+ }
}
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/InFlow.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/InFlow.java
index 4590558..3c28c20 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/InFlow.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/InFlow.java
@@ -7,17 +7,16 @@
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.optimize.compose.UpdateChangedFlagsAbstractFunction;
-public interface InFlow extends Comparable<InFlow> {
+public interface InFlow {
- @Override
- default int compareTo(InFlow inFlow) {
+ default int compareTo(InFlow inFlow, InFlowComparator comparator) {
if (getKind() == inFlow.getKind()) {
- return internalCompareToSameKind(inFlow);
+ return internalCompareToSameKind(inFlow, comparator);
}
return getKind().ordinal() - inFlow.getKind().ordinal();
}
- int internalCompareToSameKind(InFlow inFlow);
+ int internalCompareToSameKind(InFlow inFlow, InFlowComparator comparator);
InFlowKind getKind();
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/InFlowComparator.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/InFlowComparator.java
new file mode 100644
index 0000000..82a1cc8
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/InFlowComparator.java
@@ -0,0 +1,53 @@
+// Copyright (c) 2024, 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.optimize.argumentpropagation.codescanner;
+
+import com.android.tools.r8.ir.code.Position.SourcePosition;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.Map;
+
+public class InFlowComparator implements Comparator<InFlow> {
+
+ private final Map<IfThenElseAbstractFunction, SourcePosition> ifThenElsePositions;
+
+ private InFlowComparator(Map<IfThenElseAbstractFunction, SourcePosition> ifThenElsePositions) {
+ this.ifThenElsePositions = ifThenElsePositions;
+ }
+
+ public SourcePosition getIfThenElsePosition(IfThenElseAbstractFunction fn) {
+ SourcePosition position = ifThenElsePositions.get(fn);
+ assert position != null;
+ return position;
+ }
+
+ public static Builder builder() {
+ return new Builder();
+ }
+
+ public void clear() {
+ ifThenElsePositions.clear();
+ }
+
+ @Override
+ public int compare(InFlow inFlow, InFlow other) {
+ return inFlow.compareTo(other, this);
+ }
+
+ public static class Builder {
+
+ private final Map<IfThenElseAbstractFunction, SourcePosition> ifThenElsePositions =
+ new HashMap<>();
+
+ public void addIfThenElsePosition(IfThenElseAbstractFunction fn, SourcePosition position) {
+ synchronized (ifThenElsePositions) {
+ ifThenElsePositions.put(fn, position);
+ }
+ }
+
+ public InFlowComparator build() {
+ return new InFlowComparator(ifThenElsePositions);
+ }
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/InstanceFieldReadAbstractFunction.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/InstanceFieldReadAbstractFunction.java
index d9c23c4..382706b 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/InstanceFieldReadAbstractFunction.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/InstanceFieldReadAbstractFunction.java
@@ -74,9 +74,9 @@
}
@Override
- public int internalCompareToSameKind(InFlow other) {
+ public int internalCompareToSameKind(InFlow other, InFlowComparator comparator) {
InstanceFieldReadAbstractFunction fn = other.asInstanceFieldReadAbstractFunction();
- int result = receiver.compareTo(fn.receiver);
+ int result = receiver.compareTo(fn.receiver, comparator);
if (result == 0) {
result = field.compareTo(fn.field);
}
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/MethodParameter.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/MethodParameter.java
index 138725e..ed49cea 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/MethodParameter.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/MethodParameter.java
@@ -63,7 +63,7 @@
}
@Override
- public int internalCompareToSameKind(InFlow other) {
+ public int internalCompareToSameKind(InFlow other, InFlowComparator comparator) {
MethodParameter methodParameter = other.asMethodParameter();
int result = method.compareTo(methodParameter.method);
if (result == 0) {
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/OrAbstractFunction.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/OrAbstractFunction.java
index 8e84a76..c8294a4 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/OrAbstractFunction.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/OrAbstractFunction.java
@@ -67,9 +67,9 @@
}
@Override
- public int internalCompareToSameKind(InFlow other) {
+ public int internalCompareToSameKind(InFlow other, InFlowComparator comparator) {
OrAbstractFunction fn = other.asOrAbstractFunction();
- int result = inFlow.compareTo(fn.inFlow);
+ int result = inFlow.compareTo(fn.inFlow, comparator);
if (result == 0) {
result = constant.getIntValue() - fn.constant.getIntValue();
}
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/UnknownAbstractFunction.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/UnknownAbstractFunction.java
index dd06a31..726ade5 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/UnknownAbstractFunction.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/UnknownAbstractFunction.java
@@ -41,7 +41,7 @@
}
@Override
- public int internalCompareToSameKind(InFlow inFlow) {
+ public int internalCompareToSameKind(InFlow inFlow, InFlowComparator comparator) {
assert this == inFlow;
return 0;
}
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/UnknownValueState.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/UnknownValueState.java
index 6fdedc4..69c1fc8 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/UnknownValueState.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/UnknownValueState.java
@@ -53,6 +53,16 @@
}
@Override
+ public boolean equals(Object obj) {
+ return this == obj;
+ }
+
+ @Override
+ public int hashCode() {
+ return System.identityHashCode(this);
+ }
+
+ @Override
public String toString() {
return "⊤";
}
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/ValueState.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/ValueState.java
index b88d240..a0ed391 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/ValueState.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/codescanner/ValueState.java
@@ -11,6 +11,7 @@
import com.android.tools.r8.ir.analysis.value.AbstractValue;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.Action;
+import com.android.tools.r8.utils.ObjectUtils;
public abstract class ValueState {
@@ -136,4 +137,14 @@
DexType outStaticType,
StateCloner cloner,
Action onChangedAction);
+
+ @Override
+ public abstract boolean equals(Object obj);
+
+ @Override
+ public abstract int hashCode();
+
+ public final boolean identical(ValueState state) {
+ return ObjectUtils.identical(this, state);
+ }
}
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/propagation/FlowGraph.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/propagation/FlowGraph.java
index c9649f8..98247c0 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/propagation/FlowGraph.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/propagation/FlowGraph.java
@@ -11,6 +11,7 @@
import com.android.tools.r8.ir.conversion.IRConverter;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.FieldStateCollection;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.FlowGraphStateProvider;
+import com.android.tools.r8.optimize.argumentpropagation.codescanner.InFlowComparator;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.MethodParameter;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.MethodStateCollectionByReference;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.ValueState;
@@ -58,8 +59,9 @@
AppView<AppInfoWithLiveness> appView,
IRConverter converter,
FieldStateCollection fieldStates,
- MethodStateCollectionByReference methodStates) {
- return new FlowGraphBuilder(appView, converter, fieldStates, methodStates);
+ MethodStateCollectionByReference methodStates,
+ InFlowComparator inFlowComparator) {
+ return new FlowGraphBuilder(appView, converter, fieldStates, methodStates, inFlowComparator);
}
@Override
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/propagation/FlowGraphBuilder.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/propagation/FlowGraphBuilder.java
index d564575..5830d3f 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/propagation/FlowGraphBuilder.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/propagation/FlowGraphBuilder.java
@@ -22,6 +22,7 @@
import com.android.tools.r8.optimize.argumentpropagation.codescanner.FieldStateCollection;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.FieldValue;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.InFlow;
+import com.android.tools.r8.optimize.argumentpropagation.codescanner.InFlowComparator;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.MethodParameter;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.MethodState;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.MethodStateCollectionByReference;
@@ -32,7 +33,6 @@
import it.unimi.dsi.fastutil.ints.Int2ReferenceMap;
import it.unimi.dsi.fastutil.ints.Int2ReferenceMaps;
import it.unimi.dsi.fastutil.ints.Int2ReferenceOpenHashMap;
-import java.util.Comparator;
import java.util.LinkedHashMap;
import java.util.List;
@@ -42,6 +42,7 @@
private final IRConverter converter;
private final FieldStateCollection fieldStates;
private final MethodStateCollectionByReference methodStates;
+ private final InFlowComparator inFlowComparator;
private final LinkedHashMap<DexField, FlowGraphFieldNode> fieldNodes = new LinkedHashMap<>();
private final LinkedHashMap<DexMethod, Int2ReferenceMap<FlowGraphParameterNode>> parameterNodes =
@@ -51,15 +52,18 @@
AppView<AppInfoWithLiveness> appView,
IRConverter converter,
FieldStateCollection fieldStates,
- MethodStateCollectionByReference methodStates) {
+ MethodStateCollectionByReference methodStates,
+ InFlowComparator inFlowComparator) {
this.appView = appView;
this.converter = converter;
this.fieldStates = fieldStates;
this.methodStates = methodStates;
+ this.inFlowComparator = inFlowComparator;
}
public FlowGraphBuilder addClasses() {
appView.appInfo().classesWithDeterministicOrder().forEach(this::add);
+ inFlowComparator.clear();
return this;
}
@@ -96,7 +100,7 @@
FlowGraphFieldNode node = getOrCreateFieldNode(field, concreteFieldState);
List<InFlow> inFlowWithDeterministicOrder =
- ListUtils.sort(concreteFieldState.getInFlow(), Comparator.naturalOrder());
+ ListUtils.sort(concreteFieldState.getInFlow(), inFlowComparator);
for (InFlow inFlow : inFlowWithDeterministicOrder) {
if (addInFlow(inFlow, node).shouldBreak()) {
assert node.isUnknown();
@@ -143,7 +147,9 @@
}
FlowGraphParameterNode node = getOrCreateParameterNode(method, parameterIndex, methodState);
- for (InFlow inFlow : concreteParameterState.getInFlow()) {
+ List<InFlow> inFlowWithDeterministicOrder =
+ ListUtils.sort(concreteParameterState.getInFlow(), inFlowComparator);
+ for (InFlow inFlow : inFlowWithDeterministicOrder) {
if (addInFlow(inFlow, node).shouldBreak()) {
assert node.isUnknown();
break;
@@ -259,7 +265,7 @@
fieldStates.remove(field);
FlowGraphFieldNode node = getFieldNode(field);
if (node != null && !node.getState().isUnknown()) {
- assert node.getState() == concreteFieldState;
+ assert node.getState().identical(concreteFieldState);
node.setState(concreteFieldStateOrBottom);
}
}
@@ -275,7 +281,7 @@
ValueState concreteParameterStateOrBottom = concreteParameterState.clearInFlow();
FlowGraphParameterNode node = getParameterNode(method, i);
if (node != null && !node.getState().isUnknown()) {
- assert node.getState() == concreteParameterState;
+ assert node.getState().identical(concreteParameterState);
node.setState(concreteParameterStateOrBottom);
}
}
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/propagation/FlowGraphNode.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/propagation/FlowGraphNode.java
index 7f9f0f0..b2e3a53 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/propagation/FlowGraphNode.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/propagation/FlowGraphNode.java
@@ -47,7 +47,7 @@
getStaticType(),
StateCloner.getCloner(),
onChangedAction);
- if (newState != oldState) {
+ if (!newState.identical(oldState)) {
setState(newState);
onChangedAction.execute();
}
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/propagation/InFlowPropagator.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/propagation/InFlowPropagator.java
index 81e3424..82f2350 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/propagation/InFlowPropagator.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/propagation/InFlowPropagator.java
@@ -24,6 +24,7 @@
import com.android.tools.r8.optimize.argumentpropagation.codescanner.ConcreteValueState;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.FieldStateCollection;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.FlowGraphStateProvider;
+import com.android.tools.r8.optimize.argumentpropagation.codescanner.InFlowComparator;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.MethodState;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.MethodStateCollectionByReference;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.NonEmptyValueState;
@@ -48,19 +49,22 @@
final IRConverter converter;
protected final FieldStateCollection fieldStates;
final MethodStateCollectionByReference methodStates;
+ final InFlowComparator inFlowComparator;
public InFlowPropagator(
AppView<AppInfoWithLiveness> appView,
Set<DexProgramClass> classesWithSingleCallerInlinedInstanceInitializers,
IRConverter converter,
FieldStateCollection fieldStates,
- MethodStateCollectionByReference methodStates) {
+ MethodStateCollectionByReference methodStates,
+ InFlowComparator inFlowComparator) {
this.appView = appView;
this.classesWithSingleCallerInlinedInstanceInitializers =
classesWithSingleCallerInlinedInstanceInitializers;
this.converter = converter;
this.fieldStates = fieldStates;
this.methodStates = methodStates;
+ this.inFlowComparator = inFlowComparator;
}
public void run(ExecutorService executorService) throws ExecutionException {
@@ -97,7 +101,7 @@
// Build a graph with an edge from parameter p -> parameter p' if all argument information for p
// must be included in the argument information for p'.
FlowGraph flowGraph =
- FlowGraph.builder(appView, converter, fieldStates, methodStates)
+ FlowGraph.builder(appView, converter, fieldStates, methodStates, inFlowComparator)
.addClasses()
.clearInFlow()
.build();
@@ -342,7 +346,7 @@
ValueState state = node.getState();
ValueState previousState = fieldStates.set(field, state);
assert state.isUnknown()
- || state == previousState
+ || state.identical(previousState)
|| (state.isConcrete() && previousState.isBottom())
: "Expected current state to be >= previous state";
});
diff --git a/src/main/java/com/android/tools/r8/optimize/compose/ArgumentPropagatorCodeScannerForComposableFunctions.java b/src/main/java/com/android/tools/r8/optimize/compose/ArgumentPropagatorCodeScannerForComposableFunctions.java
index 2116f3a..371af3d 100644
--- a/src/main/java/com/android/tools/r8/optimize/compose/ArgumentPropagatorCodeScannerForComposableFunctions.java
+++ b/src/main/java/com/android/tools/r8/optimize/compose/ArgumentPropagatorCodeScannerForComposableFunctions.java
@@ -6,6 +6,7 @@
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.ProgramMethod;
+import com.android.tools.r8.ir.analysis.path.PathConstraintSupplier;
import com.android.tools.r8.ir.code.AbstractValueSupplier;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.InvokeMethod;
@@ -30,8 +31,9 @@
ProgramMethod method,
IRCode code,
AbstractValueSupplier abstractValueSupplier,
+ PathConstraintSupplier pathConstraintSupplier,
Timing timing) {
- new CodeScanner(abstractValueSupplier, code, method).scan(timing);
+ new CodeScanner(abstractValueSupplier, code, method, pathConstraintSupplier).scan(timing);
}
@Override
@@ -44,8 +46,11 @@
private class CodeScanner extends ArgumentPropagatorCodeScanner.CodeScanner {
protected CodeScanner(
- AbstractValueSupplier abstractValueSupplier, IRCode code, ProgramMethod method) {
- super(abstractValueSupplier, code, method);
+ AbstractValueSupplier abstractValueSupplier,
+ IRCode code,
+ ProgramMethod method,
+ PathConstraintSupplier pathConstraintSupplier) {
+ super(abstractValueSupplier, code, method, pathConstraintSupplier);
}
@Override
diff --git a/src/main/java/com/android/tools/r8/optimize/compose/ComposeMethodProcessor.java b/src/main/java/com/android/tools/r8/optimize/compose/ComposeMethodProcessor.java
index 22ed377..3b51e7c 100644
--- a/src/main/java/com/android/tools/r8/optimize/compose/ComposeMethodProcessor.java
+++ b/src/main/java/com/android/tools/r8/optimize/compose/ComposeMethodProcessor.java
@@ -13,6 +13,7 @@
import com.android.tools.r8.graph.ProgramField;
import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.ir.analysis.constant.SparseConditionalConstantPropagation;
+import com.android.tools.r8.ir.analysis.path.PathConstraintSupplier;
import com.android.tools.r8.ir.analysis.value.AbstractValue;
import com.android.tools.r8.ir.code.AbstractValueSupplier;
import com.android.tools.r8.ir.code.IRCode;
@@ -30,6 +31,7 @@
import com.android.tools.r8.optimize.argumentpropagation.codescanner.ConcretePrimitiveTypeValueState;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.ConcreteValueState;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.FieldStateCollection;
+import com.android.tools.r8.optimize.argumentpropagation.codescanner.InFlowComparator;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.MethodParameter;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.MethodState;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.MethodStateCollectionByReference;
@@ -95,9 +97,15 @@
throws ExecutionException {
prepareForInFlowPropagator();
+ InFlowComparator emptyComparator = InFlowComparator.builder().build();
InFlowPropagator inFlowPropagator =
new InFlowPropagator(
- appView, null, converter, codeScanner.getFieldStates(), codeScanner.getMethodStates()) {
+ appView,
+ null,
+ converter,
+ codeScanner.getFieldStates(),
+ codeScanner.getMethodStates(),
+ emptyComparator) {
@Override
protected DefaultFieldValueJoiner createDefaultFieldValueJoiner(
@@ -226,7 +234,9 @@
assert abstractValue != null;
return abstractValue;
};
- codeScanner.scan(method, code, abstractValueSupplier, timing);
+ PathConstraintSupplier pathConstraintSupplier =
+ new PathConstraintSupplier(appView, code, codeScanner.getMethodParameterFactory());
+ codeScanner.scan(method, code, abstractValueSupplier, pathConstraintSupplier, timing);
}
public boolean isProcessed(ComposableCallGraphNode node) {
diff --git a/src/main/java/com/android/tools/r8/optimize/compose/UpdateChangedFlagsAbstractFunction.java b/src/main/java/com/android/tools/r8/optimize/compose/UpdateChangedFlagsAbstractFunction.java
index e57b4fb..91476d3 100644
--- a/src/main/java/com/android/tools/r8/optimize/compose/UpdateChangedFlagsAbstractFunction.java
+++ b/src/main/java/com/android/tools/r8/optimize/compose/UpdateChangedFlagsAbstractFunction.java
@@ -14,6 +14,7 @@
import com.android.tools.r8.optimize.argumentpropagation.codescanner.ConcreteValueState;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.FlowGraphStateProvider;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.InFlow;
+import com.android.tools.r8.optimize.argumentpropagation.codescanner.InFlowComparator;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.InFlowKind;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.OrAbstractFunction;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.ValueState;
@@ -131,8 +132,8 @@
}
@Override
- public int internalCompareToSameKind(InFlow other) {
- return inFlow.compareTo(other.asUpdateChangedFlagsAbstractFunction().inFlow);
+ public int internalCompareToSameKind(InFlow other, InFlowComparator comparator) {
+ return inFlow.compareTo(other.asUpdateChangedFlagsAbstractFunction().inFlow, comparator);
}
@Override
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 147ce09..5ca4dbe 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -2360,20 +2360,25 @@
private void processDeferredAnnotations(
Map<DexType, Map<DexAnnotation, List<ProgramDefinition>>> deferredAnnotations,
Function<ProgramDefinition, AnnotatedKind> kindProvider) {
+ // Collect annotations to process as processing the annotation can modify liveAnnotations.
+ Set<Map<DexAnnotation, List<ProgramDefinition>>> toProcess = Sets.newIdentityHashSet();
for (DexType annotationType : liveAnnotations) {
Map<DexAnnotation, List<ProgramDefinition>> annotations =
deferredAnnotations.remove(annotationType);
if (annotations != null) {
assert annotations.keySet().stream()
.allMatch(annotation -> annotationType.isIdenticalTo(annotation.getAnnotationType()));
- annotations.forEach(
- (annotation, annotatedItems) ->
- annotatedItems.forEach(
- annotatedItem ->
- processAnnotation(
- annotatedItem, annotation, kindProvider.apply(annotatedItem))));
+ toProcess.add(annotations);
}
}
+ toProcess.forEach(
+ annotations ->
+ annotations.forEach(
+ (annotation, annotatedItems) ->
+ annotatedItems.forEach(
+ annotatedItem ->
+ processAnnotation(
+ annotatedItem, annotation, kindProvider.apply(annotatedItem)))));
}
private void ensureMethodsContinueToWidenAccess(ClassDefinition clazz) {
diff --git a/src/main/java/com/android/tools/r8/utils/ResourceShrinkerUtils.java b/src/main/java/com/android/tools/r8/utils/ResourceShrinkerUtils.java
index 41ad64f..47742f0 100644
--- a/src/main/java/com/android/tools/r8/utils/ResourceShrinkerUtils.java
+++ b/src/main/java/com/android/tools/r8/utils/ResourceShrinkerUtils.java
@@ -3,38 +3,37 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.utils;
+import com.android.build.shrinker.NoDebugReporter;
+import com.android.build.shrinker.ShrinkerDebugReporter;
import com.android.build.shrinker.r8integration.R8ResourceShrinkerState;
import com.android.tools.r8.AndroidResourceInput;
+import com.android.tools.r8.AndroidResourceProvider;
+import com.android.tools.r8.DiagnosticsHandler;
import com.android.tools.r8.FeatureSplit;
import com.android.tools.r8.ResourceException;
+import com.android.tools.r8.StringConsumer;
import com.android.tools.r8.graph.AppView;
import java.io.InputStream;
-import java.util.Collection;
+import java.util.function.Supplier;
public class ResourceShrinkerUtils {
public static R8ResourceShrinkerState createResourceShrinkerState(AppView<?> appView) {
+ InternalOptions options = appView.options();
R8ResourceShrinkerState state =
new R8ResourceShrinkerState(
- exception -> appView.reporter().fatalError(new ExceptionDiagnostic(exception)));
- InternalOptions options = appView.options();
+ exception -> appView.reporter().fatalError(new ExceptionDiagnostic(exception)),
+ shrinkerDebugReporterFromStringConsumer(
+ options.resourceShrinkerConfiguration.getDebugConsumer(), appView.reporter()));
if (options.resourceShrinkerConfiguration.isOptimizedShrinking()
&& options.androidResourceProvider != null) {
try {
- addResources(
- appView,
- state,
- options.androidResourceProvider.getAndroidResources(),
- FeatureSplit.BASE);
+ addResources(appView, state, options.androidResourceProvider, FeatureSplit.BASE);
if (options.hasFeatureSplitConfiguration()) {
for (FeatureSplit featureSplit :
options.getFeatureSplitConfiguration().getFeatureSplits()) {
if (featureSplit.getAndroidResourceProvider() != null) {
- addResources(
- appView,
- state,
- featureSplit.getAndroidResourceProvider().getAndroidResources(),
- featureSplit);
+ addResources(appView, state, featureSplit.getAndroidResourceProvider(), featureSplit);
}
}
}
@@ -49,10 +48,10 @@
private static void addResources(
AppView<?> appView,
R8ResourceShrinkerState state,
- Collection<AndroidResourceInput> androidResources,
+ AndroidResourceProvider androidResourceProvider,
FeatureSplit featureSplit)
throws ResourceException {
- for (AndroidResourceInput androidResource : androidResources) {
+ for (AndroidResourceInput androidResource : androidResourceProvider.getAndroidResources()) {
switch (androidResource.getKind()) {
case MANIFEST:
state.addManifestProvider(
@@ -66,6 +65,10 @@
() -> wrapThrowingInputStreamResource(appView, androidResource),
androidResource.getPath().location());
break;
+ case KEEP_RULE_FILE:
+ state.addKeepRuleRileProvider(
+ () -> wrapThrowingInputStreamResource(appView, androidResource));
+ break;
case RES_FOLDER_FILE:
state.addResFileProvider(
() -> wrapThrowingInputStreamResource(appView, androidResource),
@@ -77,6 +80,29 @@
}
}
+ public static ShrinkerDebugReporter shrinkerDebugReporterFromStringConsumer(
+ StringConsumer consumer, DiagnosticsHandler diagnosticsHandler) {
+ if (consumer == null) {
+ return NoDebugReporter.INSTANCE;
+ }
+ return new ShrinkerDebugReporter() {
+ @Override
+ public void debug(Supplier<String> logSupplier) {
+ consumer.accept(logSupplier.get(), diagnosticsHandler);
+ }
+
+ @Override
+ public void info(Supplier<String> logProducer) {
+ consumer.accept(logProducer.get(), diagnosticsHandler);
+ }
+
+ @Override
+ public void close() throws Exception {
+ consumer.finished(diagnosticsHandler);
+ }
+ };
+ }
+
private static InputStream wrapThrowingInputStreamResource(
AppView<?> appView, AndroidResourceInput androidResource) {
try {
diff --git a/src/resourceshrinker/java/com/android/build/shrinker/ShrinkerDebugReporter.kt b/src/resourceshrinker/java/com/android/build/shrinker/ShrinkerDebugReporter.kt
index b592bb8..bcb6af3 100644
--- a/src/resourceshrinker/java/com/android/build/shrinker/ShrinkerDebugReporter.kt
+++ b/src/resourceshrinker/java/com/android/build/shrinker/ShrinkerDebugReporter.kt
@@ -18,16 +18,17 @@
import java.io.File
import java.io.PrintWriter
+import java.util.function.Supplier
interface ShrinkerDebugReporter : AutoCloseable {
- fun debug(f: () -> String)
- fun info(f: () -> String)
+ fun debug(f: Supplier<String>)
+ fun info(f: Supplier<String>)
}
object NoDebugReporter : ShrinkerDebugReporter {
- override fun debug(f: () -> String) = Unit
+ override fun debug(f: Supplier<String>) = Unit
- override fun info(f: () -> String) = Unit
+ override fun info(f: Supplier<String>) = Unit
override fun close() = Unit
}
@@ -36,12 +37,12 @@
reportFile: File
) : ShrinkerDebugReporter {
private val writer: PrintWriter = reportFile.let { PrintWriter(it) }
- override fun debug(f: () -> String) {
- writer.println(f())
+ override fun debug(f: Supplier<String>) {
+ writer.println(f.get())
}
- override fun info(f: () -> String) {
- writer.println(f())
+ override fun info(f: Supplier<String>) {
+ writer.println(f.get())
}
override fun close() {
@@ -56,14 +57,14 @@
) : ShrinkerDebugReporter {
private val writer: PrintWriter? = reportFile?.let { PrintWriter(it) }
- override fun debug(f: () -> String) {
- val message = f()
+ override fun debug(f: Supplier<String>) {
+ val message = f.get()
writer?.println(message)
logDebug(message)
}
- override fun info(f: () -> String) {
- val message = f()
+ override fun info(f: Supplier<String>) {
+ val message = f.get()
writer?.println(message)
logInfo(message)
}
diff --git a/src/resourceshrinker/java/com/android/build/shrinker/r8integration/LegacyResourceShrinker.java b/src/resourceshrinker/java/com/android/build/shrinker/r8integration/LegacyResourceShrinker.java
index 7a82e3f..019a123 100644
--- a/src/resourceshrinker/java/com/android/build/shrinker/r8integration/LegacyResourceShrinker.java
+++ b/src/resourceshrinker/java/com/android/build/shrinker/r8integration/LegacyResourceShrinker.java
@@ -10,9 +10,9 @@
import com.android.aapt.Resources.ResourceTable;
import com.android.aapt.Resources.XmlNode;
-import com.android.build.shrinker.NoDebugReporter;
import com.android.build.shrinker.ResourceShrinkerImplKt;
import com.android.build.shrinker.ResourceTableUtilKt;
+import com.android.build.shrinker.ShrinkerDebugReporter;
import com.android.build.shrinker.graph.ProtoResourcesGraphBuilder;
import com.android.build.shrinker.obfuscation.ProguardMappingsRecorder;
import com.android.build.shrinker.r8integration.R8ResourceShrinkerState.R8ResourceShrinkerModel;
@@ -52,7 +52,9 @@
private final Map<String, byte[]> dexInputs;
private final Collection<PathAndBytes> resFolderInputs;
private final Collection<PathAndBytes> xmlInputs;
+ private final List<byte[]> keepRuleInput;
private List<String> proguardMapStrings;
+ private final ShrinkerDebugReporter debugReporter;
private final List<PathAndBytes> manifest;
private final Map<PathAndBytes, FeatureSplit> resourceTables;
@@ -61,10 +63,12 @@
private final Map<String, byte[]> dexInputs = new HashMap<>();
private final Map<Path, PathAndBytes> resFolderInputs = new HashMap<>();
private final Map<Path, PathAndBytes> xmlInputs = new HashMap<>();
+ private final List<byte[]> keepRuleInput = new ArrayList<>();
private final List<PathAndBytes> manifests = new ArrayList<>();
private final Map<PathAndBytes, FeatureSplit> resourceTables = new HashMap<>();
private List<String> proguardMapStrings;
+ private ShrinkerDebugReporter debugReporter;
private Builder() {}
@@ -89,6 +93,11 @@
return this;
}
+ public Builder addKeepRuleInput(byte[] bytes) {
+ keepRuleInput.add(bytes);
+ return this;
+ }
+
public Builder addResFolderInput(Path path, byte[] bytes) {
PathAndBytes existing = resFolderInputs.get(path);
if (existing != null) {
@@ -117,12 +126,19 @@
manifests,
resourceTables,
xmlInputs.values(),
- proguardMapStrings);
+ keepRuleInput,
+ proguardMapStrings,
+ debugReporter);
}
public void setProguardMapStrings(List<String> proguardMapStrings) {
this.proguardMapStrings = proguardMapStrings;
}
+
+ public Builder setShrinkerDebugReporter(ShrinkerDebugReporter debugReporter) {
+ this.debugReporter = debugReporter;
+ return this;
+ }
}
private LegacyResourceShrinker(
@@ -131,13 +147,17 @@
List<PathAndBytes> manifests,
Map<PathAndBytes, FeatureSplit> resourceTables,
Collection<PathAndBytes> xmlInputs,
- List<String> proguardMapStrings) {
+ List<byte[]> additionalRawXmlInputs,
+ List<String> proguardMapStrings,
+ ShrinkerDebugReporter debugReporter) {
this.dexInputs = dexInputs;
this.resFolderInputs = resFolderInputs;
this.manifest = manifests;
this.resourceTables = resourceTables;
this.xmlInputs = xmlInputs;
+ this.keepRuleInput = additionalRawXmlInputs;
this.proguardMapStrings = proguardMapStrings;
+ this.debugReporter = debugReporter;
}
public static Builder builder() {
@@ -145,7 +165,7 @@
}
public ShrinkerResult run() throws IOException, ParserConfigurationException, SAXException {
- R8ResourceShrinkerModel model = new R8ResourceShrinkerModel(NoDebugReporter.INSTANCE, true);
+ R8ResourceShrinkerModel model = new R8ResourceShrinkerModel(debugReporter, true);
for (PathAndBytes pathAndBytes : resourceTables.keySet()) {
ResourceTable loadedResourceTable = ResourceTable.parseFrom(pathAndBytes.bytes);
model.instantiateFromResourceTable(loadedResourceTable, false);
@@ -156,7 +176,7 @@
}
for (Entry<String, byte[]> entry : dexInputs.entrySet()) {
// The analysis needs an origin for the dex files, synthesize an easy recognizable one.
- Path inMemoryR8 = Paths.get("in_memory_r8_" + entry.getKey() + ".dex");
+ Path inMemoryR8 = Paths.get("in_memory_r8_" + entry.getKey());
R8ResourceShrinker.runResourceShrinkerAnalysis(
entry.getValue(), inMemoryR8, new DexFileAnalysisCallback(inMemoryR8, model));
}
@@ -164,10 +184,8 @@
ProtoAndroidManifestUsageRecorderKt.recordUsagesFromNode(
XmlNode.parseFrom(pathAndBytes.bytes), model);
}
- for (PathAndBytes xmlInput : xmlInputs) {
- if (xmlInput.path.startsWith("res/raw")) {
- ToolsAttributeUsageRecorderKt.processRawXml(getUtfReader(xmlInput.getBytes()), model);
- }
+ for (byte[] keepBytes : keepRuleInput) {
+ ToolsAttributeUsageRecorderKt.processRawXml(getUtfReader(keepBytes), model);
}
ImmutableMap<String, PathAndBytes> resFolderMappings =
@@ -188,11 +206,19 @@
ResourceStore resourceStore = model.getResourceStore();
resourceStore.processToolsAttributes();
model.keepPossiblyReferencedResources();
+ debugReporter.debug(model.getResourceStore()::dumpResourceModel);
// Transitively mark the reachable resources in the model.
// Finds unused resources in provided resources collection.
// Marks all used resources as 'reachable' in original collection.
List<Resource> unusedResources =
- ResourcesUtil.findUnusedResources(model.getResourceStore().getResources(), x -> {});
+ ResourcesUtil.findUnusedResources(
+ model.getResourceStore().getResources(),
+ roots -> {
+ debugReporter.debug(() -> "The root reachable resources are:");
+ roots.forEach(root -> debugReporter.debug(() -> " " + root));
+ });
+ debugReporter.debug(() -> "Unused resources are: ");
+ unusedResources.forEach(unused -> debugReporter.debug(() -> " " + unused));
ImmutableSet.Builder<String> resEntriesToKeep = new ImmutableSet.Builder<>();
for (PathAndBytes xmlInput : Iterables.concat(xmlInputs, resFolderInputs)) {
if (ResourceShrinkerImplKt.isJarPathReachable(resourceStore, xmlInput.path.toString())) {
diff --git a/src/resourceshrinker/java/com/android/build/shrinker/r8integration/R8ResourceShrinkerState.java b/src/resourceshrinker/java/com/android/build/shrinker/r8integration/R8ResourceShrinkerState.java
index 49f3546..5ac9755 100644
--- a/src/resourceshrinker/java/com/android/build/shrinker/r8integration/R8ResourceShrinkerState.java
+++ b/src/resourceshrinker/java/com/android/build/shrinker/r8integration/R8ResourceShrinkerState.java
@@ -16,7 +16,6 @@
import com.android.aapt.Resources.XmlAttribute;
import com.android.aapt.Resources.XmlElement;
import com.android.aapt.Resources.XmlNode;
-import com.android.build.shrinker.NoDebugReporter;
import com.android.build.shrinker.ResourceShrinkerImplKt;
import com.android.build.shrinker.ResourceShrinkerModel;
import com.android.build.shrinker.ResourceTableUtilKt;
@@ -54,6 +53,7 @@
private final Function<Exception, RuntimeException> errorHandler;
private final R8ResourceShrinkerModel r8ResourceShrinkerModel;
private final Map<String, Supplier<InputStream>> xmlFileProviders = new HashMap<>();
+ private final List<Supplier<InputStream>> keepRuleFileProviders = new ArrayList<>();
private final List<Supplier<InputStream>> manifestProviders = new ArrayList<>();
private final Map<String, Supplier<InputStream>> resfileProviders = new HashMap<>();
@@ -69,8 +69,10 @@
boolean tryClass(String possibleClass, Origin xmlFileOrigin);
}
- public R8ResourceShrinkerState(Function<Exception, RuntimeException> errorHandler) {
- r8ResourceShrinkerModel = new R8ResourceShrinkerModel(NoDebugReporter.INSTANCE, true);
+ public R8ResourceShrinkerState(
+ Function<Exception, RuntimeException> errorHandler,
+ ShrinkerDebugReporter shrinkerDebugReporter) {
+ r8ResourceShrinkerModel = new R8ResourceShrinkerModel(shrinkerDebugReporter, true);
this.errorHandler = errorHandler;
}
@@ -140,6 +142,10 @@
this.xmlFileProviders.put(location, inputStreamSupplier);
}
+ public void addKeepRuleRileProvider(Supplier<InputStream> inputStreamSupplier) {
+ this.keepRuleFileProviders.add(inputStreamSupplier);
+ }
+
public void addResFileProvider(Supplier<InputStream> inputStreamSupplier, String location) {
this.resfileProviders.put(location, inputStreamSupplier);
}
@@ -307,11 +313,9 @@
}
public void updateModelWithKeepXmlReferences() throws IOException {
- for (Map.Entry<String, Supplier<InputStream>> entry : xmlFileProviders.entrySet()) {
- if (entry.getKey().startsWith("res/raw")) {
- ToolsAttributeUsageRecorderKt.processRawXml(
- getUtfReader(entry.getValue().get().readAllBytes()), r8ResourceShrinkerModel);
- }
+ for (Supplier<InputStream> keepRuleFileProvider : keepRuleFileProviders) {
+ ToolsAttributeUsageRecorderKt.processRawXml(
+ getUtfReader(keepRuleFileProvider.get().readAllBytes()), r8ResourceShrinkerModel);
}
}
diff --git a/src/test/java/com/android/tools/r8/androidresources/KeepXmlFilesTest.java b/src/test/java/com/android/tools/r8/androidresources/KeepXmlFilesTest.java
index 5d19a6a..3676fd8 100644
--- a/src/test/java/com/android/tools/r8/androidresources/KeepXmlFilesTest.java
+++ b/src/test/java/com/android/tools/r8/androidresources/KeepXmlFilesTest.java
@@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.androidresources;
+import com.android.tools.r8.R8TestBuilder;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.androidresources.AndroidResourceTestingUtils.AndroidTestResource;
@@ -36,6 +37,7 @@
return new AndroidTestResourceBuilder()
.withSimpleManifestAndAppNameString()
.addKeepXmlFor("@string/bar", "@drawable/foobar")
+ .addExplicitKeepRuleFileFor("@drawable/barfoo")
.addRClassInitializeWithDefaultValues(R.string.class, R.drawable.class)
.build(temp);
}
@@ -47,7 +49,7 @@
.addProgramClasses(FooBar.class)
.addAndroidResources(getTestResources(temp))
.addKeepMainRule(FooBar.class)
- .applyIf(optimized, b -> b.enableOptimizedShrinking())
+ .applyIf(optimized, R8TestBuilder::enableOptimizedShrinking)
.compile()
.inspectShrunkenResources(
resourceTableInspector -> {
@@ -57,6 +59,8 @@
resourceTableInspector.assertContainsResourceWithName("string", "foo");
// Referenced from keep.xml
resourceTableInspector.assertContainsResourceWithName("drawable", "foobar");
+ // Referenced from additional keep xml files
+ resourceTableInspector.assertContainsResourceWithName("drawable", "barfoo");
resourceTableInspector.assertDoesNotContainResourceWithName(
"string", "unused_string");
resourceTableInspector.assertDoesNotContainResourceWithName(
@@ -86,6 +90,7 @@
public static class drawable {
public static int foobar;
+ public static int barfoo;
public static int unused_drawable;
}
}
diff --git a/src/test/java/com/android/tools/r8/androidresources/ResourceShrinkerLoggingTest.java b/src/test/java/com/android/tools/r8/androidresources/ResourceShrinkerLoggingTest.java
new file mode 100644
index 0000000..5ed2e09
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/androidresources/ResourceShrinkerLoggingTest.java
@@ -0,0 +1,190 @@
+// Copyright (c) 2024, 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.androidresources;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.androidresources.AndroidResourceTestingUtils.AndroidTestResource;
+import com.android.tools.r8.androidresources.AndroidResourceTestingUtils.AndroidTestResourceBuilder;
+import com.android.tools.r8.utils.BooleanUtils;
+import com.android.tools.r8.utils.StringUtils;
+import com.google.common.collect.ImmutableList;
+import java.util.List;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class ResourceShrinkerLoggingTest extends TestBase {
+
+ @Parameter(0)
+ public TestParameters parameters;
+
+ @Parameter(1)
+ public boolean optimized;
+
+ @Parameters(name = "{0}, optimized: {1}")
+ public static List<Object[]> data() {
+ return buildParameters(
+ getTestParameters().withDefaultDexRuntime().withAllApiLevels().build(),
+ BooleanUtils.values());
+ }
+
+ public static AndroidTestResource getTestResources(TemporaryFolder temp) throws Exception {
+ return new AndroidTestResourceBuilder()
+ .withSimpleManifestAndAppNameString()
+ .addRClassInitializeWithDefaultValues(R.string.class, R.drawable.class)
+ .build(temp);
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ StringBuilder log = new StringBuilder();
+ testForR8(parameters.getBackend())
+ .setMinApi(parameters)
+ .addProgramClasses(FooBar.class)
+ .apply(
+ b ->
+ b.getBuilder()
+ .setResourceShrinkerConfiguration(
+ configurationBuilder -> {
+ if (optimized) {
+ configurationBuilder.enableOptimizedShrinkingWithR8();
+ }
+ configurationBuilder.setDebugConsumer(
+ (string, handler) -> log.append(string + "\n"));
+ return configurationBuilder.build();
+ }))
+ .addAndroidResources(getTestResources(temp))
+ .addKeepMainRule(FooBar.class)
+ .compile()
+ .inspectShrunkenResources(
+ resourceTableInspector -> {
+ resourceTableInspector.assertContainsResourceWithName("string", "bar");
+ resourceTableInspector.assertContainsResourceWithName("string", "foo");
+ resourceTableInspector.assertContainsResourceWithName("drawable", "foobar");
+ resourceTableInspector.assertDoesNotContainResourceWithName(
+ "string", "unused_string");
+ resourceTableInspector.assertDoesNotContainResourceWithName(
+ "drawable", "unused_drawable");
+ })
+ .run(parameters.getRuntime(), FooBar.class)
+ .assertSuccess();
+ // TODO(b/360284664): Add (non compatible) logging for optimized shrinking
+ if (!optimized) {
+ // Consistent with the old AGP embedded shrinker
+ List<String> strings = StringUtils.splitLines(log.toString());
+ // string:bar reachable from code
+ for (String dexReachableString : ImmutableList.of("bar", "foo")) {
+ ensureDexReachableResourcesState(strings, "string", dexReachableString, true);
+ ensureResourceReachabilityState(strings, "string", dexReachableString, true);
+ ensureRootResourceState(strings, "string", dexReachableString, true);
+ ensureUnusedState(strings, "string", dexReachableString, false);
+ }
+ // The app name is only reachable from the manifest, not dex
+ ensureDexReachableResourcesState(strings, "string", "app_name", false);
+ ensureResourceReachabilityState(strings, "string", "app_name", true);
+ ensureRootResourceState(strings, "string", "app_name", true);
+ ensureUnusedState(strings, "string", "app_name", false);
+
+ ensureDexReachableResourcesState(strings, "drawable", "unused_drawable", false);
+ ensureResourceReachabilityState(strings, "drawable", "unused_drawable", false);
+ ensureRootResourceState(strings, "drawable", "unused_drawable", false);
+ ensureUnusedState(strings, "drawable", "unused_drawable", true);
+
+ ensureDexReachableResourcesState(strings, "drawable", "foobar", true);
+ ensureResourceReachabilityState(strings, "drawable", "foobar", true);
+ ensureRootResourceState(strings, "drawable", "foobar", true);
+ ensureUnusedState(strings, "drawable", "foobar", false);
+ }
+ }
+
+ private void ensureDexReachableResourcesState(
+ List<String> logStrings, String type, String name, boolean reachable) {
+ // Example line:
+ // Marking drawable:foobar:2130771968 reachable: referenced from classes.dex
+ assertEquals(
+ logStrings.stream()
+ .anyMatch(
+ s ->
+ s.contains("Marking " + type + ":" + name)
+ && s.contains("reachable: referenced from")),
+ reachable);
+ }
+
+ private void ensureResourceReachabilityState(
+ List<String> logStrings, String type, String name, boolean reachable) {
+ // Example line:
+ // @packagename:string/bar : reachable=true
+ assertTrue(
+ logStrings.stream()
+ .anyMatch(s -> s.contains(type + "/" + name + " : reachable=" + reachable)));
+ }
+
+ private void ensureRootResourceState(
+ List<String> logStrings, String type, String name, boolean isRoot) {
+ assertEquals(isInSection(logStrings, type, name, "The root reachable resources are:"), isRoot);
+ }
+
+ private void ensureUnusedState(
+ List<String> logStrings, String type, String name, boolean isUnused) {
+ assertEquals(isInSection(logStrings, type, name, "Unused resources are: "), isUnused);
+ }
+
+ private static boolean isInSection(
+ List<String> logStrings, String type, String name, String sectionHeader) {
+ // Example for roots
+ // "The root reachable resources are:"
+ // " drawable:foobar:2130771968"
+ boolean isInSection = false;
+ for (String logString : logStrings) {
+ if (logString.equals(sectionHeader)) {
+ isInSection = true;
+ continue;
+ }
+ if (isInSection) {
+ if (!logString.startsWith(" ")) {
+ return false;
+ }
+ if (logString.startsWith(" " + type + ":" + name)) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
+ public static class FooBar {
+
+ public static void main(String[] args) {
+ if (System.currentTimeMillis() == 0) {
+ System.out.println(R.drawable.foobar);
+ System.out.println(R.string.bar);
+ System.out.println(R.string.foo);
+ }
+ }
+ }
+
+ public static class R {
+
+ public static class string {
+
+ public static int bar;
+ public static int foo;
+ public static int unused_string;
+ }
+
+ public static class drawable {
+
+ public static int foobar;
+ public static int unused_drawable;
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/annotations/NestedAnnotationsUnusedTest.java b/src/test/java/com/android/tools/r8/annotations/NestedAnnotationsUnusedTest.java
new file mode 100644
index 0000000..a807a27
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/annotations/NestedAnnotationsUnusedTest.java
@@ -0,0 +1,75 @@
+// Copyright (c) 2024, 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.annotations;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isAbsent;
+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.annotations.NestedAnnotationsUsedTest.A;
+import com.android.tools.r8.annotations.NestedAnnotationsUsedTest.TestClass;
+import com.android.tools.r8.utils.StringUtils;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+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 regression test for b/359385828.
+@RunWith(Parameterized.class)
+public class NestedAnnotationsUnusedTest extends TestBase {
+
+ @Parameter(0)
+ public TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ private static final String EXPECTED_OUTPUT = StringUtils.lines("Found");
+
+ @Test
+ public void testR8() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepClassRules(A.class)
+ .addKeepRuntimeVisibleAnnotations()
+ .addKeepMainRule(TestClass.class)
+ .setMinApi(parameters)
+ .run(parameters.getRuntime(), TestClass.class)
+ .inspect(inspector -> assertThat(inspector.clazz(B.class), isAbsent()))
+ .assertSuccessWithOutput(EXPECTED_OUTPUT);
+ }
+
+ @Retention(RetentionPolicy.RUNTIME)
+ @Target({ElementType.TYPE})
+ public @interface A {
+ B b();
+
+ B[] bs() default {};
+ }
+
+ @Retention(RetentionPolicy.RUNTIME)
+ public @interface B {
+ String x();
+ }
+
+ @A(
+ b = @B(x = "1"),
+ bs = {@B(x = "2"), @B(x = "3")})
+ static class TestClass {
+
+ public static void main(String[] args) {
+ System.out.println(TestClass.class.getAnnotation(A.class) != null ? "Found" : "Not found");
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/annotations/NestedAnnotationsUsedTest.java b/src/test/java/com/android/tools/r8/annotations/NestedAnnotationsUsedTest.java
new file mode 100644
index 0000000..90ae5bf
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/annotations/NestedAnnotationsUsedTest.java
@@ -0,0 +1,78 @@
+// Copyright (c) 2024, 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.annotations;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresentAndRenamed;
+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.StringUtils;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+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 regression test for b/359385828.
+@RunWith(Parameterized.class)
+public class NestedAnnotationsUsedTest extends TestBase {
+
+ @Parameter(0)
+ public TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ private static final String EXPECTED_OUTPUT = StringUtils.lines("1", "2", "3");
+
+ @Test
+ public void testR8() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepClassRules(A.class)
+ .addKeepRuntimeVisibleAnnotations()
+ .addKeepMainRule(TestClass.class)
+ .setMinApi(parameters)
+ .run(parameters.getRuntime(), TestClass.class)
+ .inspect(
+ inspector -> {
+ assertThat(inspector.clazz(B.class), isPresentAndRenamed());
+ })
+ .assertSuccessWithOutput(EXPECTED_OUTPUT);
+ }
+
+ @Retention(RetentionPolicy.RUNTIME)
+ @Target({ElementType.TYPE})
+ public @interface A {
+ B b();
+
+ B[] bs() default {};
+ }
+
+ @Retention(RetentionPolicy.RUNTIME)
+ public @interface B {
+ String x();
+ }
+
+ @A(
+ b = @B(x = "1"),
+ bs = {@B(x = "2"), @B(x = "3")})
+ static class TestClass {
+ public static void main(String[] args) throws Exception {
+ System.out.println(TestClass.class.getAnnotation(A.class).b().x());
+ for (B b : TestClass.class.getAnnotation(A.class).bs()) {
+ System.out.println(b.x());
+ }
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/benchmarks/BenchmarkConfig.java b/src/test/java/com/android/tools/r8/benchmarks/BenchmarkConfig.java
index b04a407..0eab29b 100644
--- a/src/test/java/com/android/tools/r8/benchmarks/BenchmarkConfig.java
+++ b/src/test/java/com/android/tools/r8/benchmarks/BenchmarkConfig.java
@@ -3,8 +3,9 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.benchmarks;
+import com.android.tools.r8.utils.IterableUtils;
import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Sets;
+import com.google.common.collect.Iterables;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
@@ -12,7 +13,6 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
-import java.util.Objects;
import java.util.Set;
import java.util.concurrent.TimeUnit;
@@ -26,20 +26,6 @@
throw new BenchmarkConfigError(
"Inconsistent single/group benchmark setup: " + benchmark + " and " + other);
}
- Set<String> subNames =
- Sets.union(benchmark.getSubBenchmarks().keySet(), other.getSubBenchmarks().keySet());
- for (String subName : subNames) {
- if (!Objects.equals(
- benchmark.getSubBenchmarks().get(subName), other.getSubBenchmarks().get(subName))) {
- throw new BenchmarkConfigError(
- "Inconsistent metrics for sub-benchmark "
- + subName
- + " in benchmarks: "
- + benchmark
- + " and "
- + other);
- }
- }
if (!benchmark.getSuite().equals(other.getSuite())) {
throw new BenchmarkConfigError(
"Inconsistent suite for benchmarks: " + benchmark + " and " + other);
@@ -182,6 +168,11 @@
return this;
}
+ public Builder measureComposableCodeSize() {
+ metrics.add(BenchmarkMetric.ComposableCodeSize);
+ return this;
+ }
+
public Builder measureWarmup() {
measureWarmup = true;
return this;
@@ -250,6 +241,16 @@
this.measureWarmup = measureWarmup;
}
+ public boolean containsMetric(BenchmarkMetric metric) {
+ Iterable<BenchmarkMetric> metrics =
+ isSingleBenchmark() ? getMetrics() : IterableUtils.flatten(getSubBenchmarks().values());
+ return Iterables.contains(metrics, metric);
+ }
+
+ public boolean containsComposableCodeSizeMetric() {
+ return containsMetric(BenchmarkMetric.ComposableCodeSize);
+ }
+
public BenchmarkIdentifier getIdentifier() {
return id;
}
diff --git a/src/test/java/com/android/tools/r8/benchmarks/BenchmarkResultsCollection.java b/src/test/java/com/android/tools/r8/benchmarks/BenchmarkResultsCollection.java
index da3f3d2..8fc8508 100644
--- a/src/test/java/com/android/tools/r8/benchmarks/BenchmarkResultsCollection.java
+++ b/src/test/java/com/android/tools/r8/benchmarks/BenchmarkResultsCollection.java
@@ -24,22 +24,25 @@
@Override
public void addRuntimeResult(long result) {
throw new BenchmarkConfigError(
- "Unexpected attempt to add a runtime result to a the root of a benchmark with"
- + " sub-benchmarks");
+ "Unexpected attempt to add a result to a the root of a benchmark with sub-benchmarks");
}
@Override
public void addCodeSizeResult(long result) {
throw new BenchmarkConfigError(
- "Unexpected attempt to add a runtime result to a the root of a benchmark with"
- + " sub-benchmarks");
+ "Unexpected attempt to add a result to a the root of a benchmark with sub-benchmarks");
+ }
+
+ @Override
+ public void addComposableCodeSizeResult(long result) {
+ throw new BenchmarkConfigError(
+ "Unexpected attempt to add a result to a the root of a benchmark with sub-benchmarks");
}
@Override
public void addResourceSizeResult(long result) {
throw new BenchmarkConfigError(
- "Unexpected attempt to add a runtime result to a the root of a benchmark with"
- + " sub-benchmarks");
+ "Unexpected attempt to add a result to a the root of a benchmark with sub-benchmarks");
}
@Override
diff --git a/src/test/java/com/android/tools/r8/benchmarks/BenchmarkResultsSingle.java b/src/test/java/com/android/tools/r8/benchmarks/BenchmarkResultsSingle.java
index e15dffb..ad6b1b3 100644
--- a/src/test/java/com/android/tools/r8/benchmarks/BenchmarkResultsSingle.java
+++ b/src/test/java/com/android/tools/r8/benchmarks/BenchmarkResultsSingle.java
@@ -9,13 +9,15 @@
import it.unimi.dsi.fastutil.longs.LongList;
import java.io.PrintStream;
import java.util.Set;
+import java.util.function.LongConsumer;
public class BenchmarkResultsSingle implements BenchmarkResults {
- private String name;
+ private final String name;
private final Set<BenchmarkMetric> metrics;
private final LongList runtimeResults = new LongArrayList();
private final LongList codeSizeResults = new LongArrayList();
+ private final LongList composableCodeSizeResults = new LongArrayList();
public BenchmarkResultsSingle(String name, Set<BenchmarkMetric> metrics) {
this.name = name;
@@ -30,6 +32,10 @@
return codeSizeResults;
}
+ public LongList getComposableCodeSizeResults() {
+ return composableCodeSizeResults;
+ }
+
public LongList getRuntimeResults() {
return runtimeResults;
}
@@ -47,6 +53,15 @@
}
@Override
+ public void addComposableCodeSizeResult(long result) {
+ verifyMetric(
+ BenchmarkMetric.ComposableCodeSize,
+ metrics.contains(BenchmarkMetric.ComposableCodeSize),
+ true);
+ composableCodeSizeResults.add(result);
+ }
+
+ @Override
public void addResourceSizeResult(long result) {
addCodeSizeResult(result);
}
@@ -78,6 +93,10 @@
BenchmarkMetric.CodeSize,
metrics.contains(BenchmarkMetric.CodeSize),
!codeSizeResults.isEmpty());
+ verifyMetric(
+ BenchmarkMetric.ComposableCodeSize,
+ metrics.contains(BenchmarkMetric.ComposableCodeSize),
+ !composableCodeSizeResults.isEmpty());
}
private void printRunTime(long duration) {
@@ -89,6 +108,11 @@
System.out.println(BenchmarkResults.prettyMetric(name, BenchmarkMetric.CodeSize, "" + bytes));
}
+ private void printComposableCodeSize(long bytes) {
+ System.out.println(
+ BenchmarkResults.prettyMetric(name, BenchmarkMetric.ComposableCodeSize, "" + bytes));
+ }
+
@Override
public void printResults(ResultMode mode, boolean failOnCodeSizeDifferences) {
verifyConfigAndResults();
@@ -97,6 +121,13 @@
long result = mode == ResultMode.SUM ? sum : sum / runtimeResults.size();
printRunTime(result);
}
+ printCodeSizeResults(codeSizeResults, failOnCodeSizeDifferences, this::printCodeSize);
+ printCodeSizeResults(
+ composableCodeSizeResults, failOnCodeSizeDifferences, this::printComposableCodeSize);
+ }
+
+ private static void printCodeSizeResults(
+ LongList codeSizeResults, boolean failOnCodeSizeDifferences, LongConsumer printer) {
if (!codeSizeResults.isEmpty()) {
long size = codeSizeResults.getLong(0);
if (failOnCodeSizeDifferences) {
@@ -107,7 +138,7 @@
}
}
}
- printCodeSize(size);
+ printer.accept(size);
}
}
diff --git a/src/test/java/com/android/tools/r8/benchmarks/BenchmarkResultsSingleAdapter.java b/src/test/java/com/android/tools/r8/benchmarks/BenchmarkResultsSingleAdapter.java
index 55936d7..68d6fb8 100644
--- a/src/test/java/com/android/tools/r8/benchmarks/BenchmarkResultsSingleAdapter.java
+++ b/src/test/java/com/android/tools/r8/benchmarks/BenchmarkResultsSingleAdapter.java
@@ -22,6 +22,10 @@
(codeSizeResult, iteration) -> {
JsonObject resultObject = new JsonObject();
resultObject.addProperty("code_size", codeSizeResult);
+ if (!result.getComposableCodeSizeResults().isEmpty()) {
+ resultObject.addProperty(
+ "composable_code_size", result.getComposableCodeSizeResults().getLong(iteration));
+ }
resultObject.addProperty("runtime", result.getRuntimeResults().getLong(iteration));
resultsArray.add(resultObject);
});
diff --git a/src/test/java/com/android/tools/r8/benchmarks/BenchmarkResultsWarmup.java b/src/test/java/com/android/tools/r8/benchmarks/BenchmarkResultsWarmup.java
index 1a4e33c..19c1de8 100644
--- a/src/test/java/com/android/tools/r8/benchmarks/BenchmarkResultsWarmup.java
+++ b/src/test/java/com/android/tools/r8/benchmarks/BenchmarkResultsWarmup.java
@@ -13,6 +13,7 @@
private final String name;
private final LongList runtimeResults = new LongArrayList();
private long codeSizeResult = -1;
+ private long composableCodeSizeResult = -1;
private long resourceSizeResult = -1;
public BenchmarkResultsWarmup(String name) {
@@ -36,6 +37,20 @@
}
@Override
+ public void addComposableCodeSizeResult(long result) {
+ if (composableCodeSizeResult == -1) {
+ composableCodeSizeResult = result;
+ }
+ if (composableCodeSizeResult != result) {
+ throw new RuntimeException(
+ "Unexpected Composable code size difference: "
+ + result
+ + " and "
+ + composableCodeSizeResult);
+ }
+ }
+
+ @Override
public void addResourceSizeResult(long result) {
if (resourceSizeResult == -1) {
resourceSizeResult = result;
diff --git a/src/test/java/com/android/tools/r8/benchmarks/appdumps/AppDumpBenchmarkBuilder.java b/src/test/java/com/android/tools/r8/benchmarks/appdumps/AppDumpBenchmarkBuilder.java
index 9454d6e..09934bc 100644
--- a/src/test/java/com/android/tools/r8/benchmarks/appdumps/AppDumpBenchmarkBuilder.java
+++ b/src/test/java/com/android/tools/r8/benchmarks/appdumps/AppDumpBenchmarkBuilder.java
@@ -3,6 +3,8 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.benchmarks.appdumps;
+import static junit.framework.TestCase.assertFalse;
+
import com.android.tools.r8.LibraryDesugaringTestConfiguration;
import com.android.tools.r8.R8FullTestBuilder;
import com.android.tools.r8.TestBase;
@@ -20,7 +22,12 @@
import com.android.tools.r8.benchmarks.BenchmarkTarget;
import com.android.tools.r8.dump.CompilerDump;
import com.android.tools.r8.dump.DumpOptions;
+import com.android.tools.r8.keepanno.annotations.AnnotationPattern;
+import com.android.tools.r8.keepanno.annotations.KeepEdge;
+import com.android.tools.r8.keepanno.annotations.KeepItemKind;
+import com.android.tools.r8.keepanno.annotations.KeepTarget;
import java.io.IOException;
+import java.lang.annotation.RetentionPolicy;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
@@ -31,6 +38,18 @@
public class AppDumpBenchmarkBuilder {
+ @KeepEdge(
+ consequences =
+ @KeepTarget(
+ kind = KeepItemKind.ONLY_METHODS,
+ methodAnnotatedByClassName = "androidx.compose.runtime.Composable",
+ constraints = {},
+ constrainAnnotations =
+ @AnnotationPattern(
+ name = "androidx.compose.runtime.Composable",
+ retention = RetentionPolicy.CLASS)))
+ static class KeepComposableAnnotations {}
+
public static AppDumpBenchmarkBuilder builder() {
return new AppDumpBenchmarkBuilder();
}
@@ -99,6 +118,7 @@
.addDependency(dumpDependency)
.measureRunTime()
.measureCodeSize()
+ .measureComposableCodeSize()
.setTimeout(10, TimeUnit.MINUTES)
.build();
}
@@ -158,6 +178,10 @@
return name + "Code";
}
+ private String nameForComposableCodePart() {
+ return name + "ComposableCode";
+ }
+
private String nameForLibraryPart() {
return name + "Library";
}
@@ -210,6 +234,16 @@
options -> options.getOpenClosedInterfacesOptions().suppressAllOpenInterfaces());
}
+ private static ThrowableConsumer<? super R8FullTestBuilder>
+ getKeepComposableAnnotationsConfiguration() {
+ return testBuilder ->
+ testBuilder
+ .addProgramClasses(KeepComposableAnnotations.class)
+ .addKeepClassAndMembersRules("androidx.compose.runtime.Composable")
+ .addKeepRuntimeVisibleAnnotations()
+ .enableExperimentalKeepAnnotations();
+ }
+
private static BenchmarkMethod runR8(
AppDumpBenchmarkBuilder builder, ThrowableConsumer<? super R8FullTestBuilder> configuration) {
return internalRunR8(builder, false, configuration);
@@ -253,20 +287,26 @@
})
.apply(configuration)
.applyIf(
+ environment.getConfig().containsComposableCodeSizeMetric(),
+ getKeepComposableAnnotationsConfiguration())
+ .applyIf(
enableResourceShrinking,
b ->
b.enableOptimizedShrinking()
.setAndroidResourcesFromPath(dump.getAndroidResources()))
.applyIf(
enableResourceShrinking,
+ r -> {
+ assertFalse(environment.getConfig().containsComposableCodeSizeMetric());
+ r.benchmarkCompile(results.getSubResults(builder.nameForRuntimePart()))
+ .benchmarkCodeSize(results.getSubResults(builder.nameForCodePart()))
+ .benchmarkResourceSize(
+ results.getSubResults(builder.nameForResourcePart()));
+ },
r ->
- r.benchmarkCompile(
- results.getSubResults(builder.nameForRuntimePart()))
- .benchmarkCodeSize(
- results.getSubResults(builder.nameForCodePart()))
- .benchmarkResourceSize(
- results.getSubResults(builder.nameForResourcePart())),
- r -> r.benchmarkCompile(results).benchmarkCodeSize(results));
+ r.benchmarkCompile(results)
+ .benchmarkCodeSize(results)
+ .benchmarkComposableCodeSize(results));
});
}
diff --git a/src/test/java/com/android/tools/r8/ir/analysis/path/PathConstraintAnalysisUnitTest.java b/src/test/java/com/android/tools/r8/ir/analysis/path/PathConstraintAnalysisUnitTest.java
index 597cca6..8107aff 100644
--- a/src/test/java/com/android/tools/r8/ir/analysis/path/PathConstraintAnalysisUnitTest.java
+++ b/src/test/java/com/android/tools/r8/ir/analysis/path/PathConstraintAnalysisUnitTest.java
@@ -15,6 +15,7 @@
import com.android.tools.r8.ir.analysis.framework.intraprocedural.DataflowAnalysisResult.SuccessfulDataflowAnalysisResult;
import com.android.tools.r8.ir.analysis.path.state.ConcretePathConstraintAnalysisState;
import com.android.tools.r8.ir.analysis.path.state.PathConstraintAnalysisState;
+import com.android.tools.r8.ir.analysis.path.state.PathConstraintKind;
import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.MethodParameterFactory;
@@ -23,6 +24,9 @@
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import java.util.Map.Entry;
+import java.util.Set;
+import java.util.stream.Collectors;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -61,7 +65,10 @@
// Inspect ENTRY state.
PathConstraintAnalysisState entryConstraint =
successfulResult.getBlockExitState(code.entryBlock());
- assertTrue(entryConstraint.isBottom());
+ assertTrue(entryConstraint.isConcrete());
+
+ ConcretePathConstraintAnalysisState concreteEntryConstraint = entryConstraint.asConcreteState();
+ assertTrue(concreteEntryConstraint.getPathConstraintsForTesting().isEmpty());
// Inspect THEN state.
PathConstraintAnalysisState thenConstraint =
@@ -69,11 +76,12 @@
assertTrue(thenConstraint.isConcrete());
ConcretePathConstraintAnalysisState concreteThenConstraint = thenConstraint.asConcreteState();
- assertEquals(1, concreteThenConstraint.getPathConstraints().size());
- assertEquals(0, concreteThenConstraint.getNegatedPathConstraints().size());
+ assertEquals(1, getPositivePathConstraints(concreteThenConstraint).size());
+ assertEquals(0, getNegativePathConstraints(concreteThenConstraint).size());
+ assertEquals(0, getDisabledPathConstraints(concreteThenConstraint).size());
ComputationTreeNode thenPathConstraint =
- concreteThenConstraint.getPathConstraints().iterator().next();
+ getPositivePathConstraints(concreteThenConstraint).iterator().next();
assertTrue(thenPathConstraint instanceof ComputationTreeUnopCompareNode);
// Inspect ELSE state.
@@ -82,11 +90,12 @@
assertTrue(elseConstraint.isConcrete());
ConcretePathConstraintAnalysisState concreteElseConstraint = elseConstraint.asConcreteState();
- assertEquals(0, concreteElseConstraint.getPathConstraints().size());
- assertEquals(1, concreteElseConstraint.getNegatedPathConstraints().size());
+ assertEquals(0, getPositivePathConstraints(concreteElseConstraint).size());
+ assertEquals(1, getNegativePathConstraints(concreteElseConstraint).size());
+ assertEquals(0, getDisabledPathConstraints(concreteElseConstraint).size());
ComputationTreeNode elsePathConstraint =
- concreteElseConstraint.getNegatedPathConstraints().iterator().next();
+ getNegativePathConstraints(concreteElseConstraint).iterator().next();
assertEquals(thenPathConstraint, elsePathConstraint);
// Inspect RETURN state.
@@ -96,10 +105,36 @@
ConcretePathConstraintAnalysisState concreteReturnConstraint =
returnConstraint.asConcreteState();
- assertEquals(1, concreteReturnConstraint.getPathConstraints().size());
- assertEquals(
- concreteReturnConstraint.getPathConstraints(),
- concreteReturnConstraint.getNegatedPathConstraints());
+ assertEquals(0, getPositivePathConstraints(concreteReturnConstraint).size());
+ assertEquals(0, getNegativePathConstraints(concreteReturnConstraint).size());
+ assertEquals(1, getDisabledPathConstraints(concreteReturnConstraint).size());
+
+ ComputationTreeNode returnPathConstraint =
+ getDisabledPathConstraints(concreteReturnConstraint).iterator().next();
+ assertEquals(thenPathConstraint, returnPathConstraint);
+ }
+
+ private Set<ComputationTreeNode> getPositivePathConstraints(
+ ConcretePathConstraintAnalysisState state) {
+ return getPathConstraintsOfKind(state, PathConstraintKind.POSITIVE);
+ }
+
+ private Set<ComputationTreeNode> getNegativePathConstraints(
+ ConcretePathConstraintAnalysisState state) {
+ return getPathConstraintsOfKind(state, PathConstraintKind.NEGATIVE);
+ }
+
+ private Set<ComputationTreeNode> getDisabledPathConstraints(
+ ConcretePathConstraintAnalysisState state) {
+ return getPathConstraintsOfKind(state, PathConstraintKind.DISABLED);
+ }
+
+ private Set<ComputationTreeNode> getPathConstraintsOfKind(
+ ConcretePathConstraintAnalysisState state, PathConstraintKind kind) {
+ return state.getPathConstraintsForTesting().entrySet().stream()
+ .filter(entry -> entry.getValue() == kind)
+ .map(Entry::getKey)
+ .collect(Collectors.toSet());
}
static class Main {
diff --git a/src/test/java/com/android/tools/r8/ir/regalloc/IdenticalAfterRegisterAllocationTest.java b/src/test/java/com/android/tools/r8/ir/regalloc/IdenticalAfterRegisterAllocationTest.java
index 181c0c5..6c68d17 100644
--- a/src/test/java/com/android/tools/r8/ir/regalloc/IdenticalAfterRegisterAllocationTest.java
+++ b/src/test/java/com/android/tools/r8/ir/regalloc/IdenticalAfterRegisterAllocationTest.java
@@ -146,11 +146,6 @@
}
@Override
- public boolean isPeepholeOptimizationsEnabled() {
- return false;
- }
-
- @Override
public boolean shouldFinalizeAfterLensCodeRewriter() {
return false;
}
diff --git a/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderRewritingWithAssumeNoSideEffectsTest.java b/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderRewritingWithAssumeNoSideEffectsTest.java
index c57f579..4de5ec7 100644
--- a/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderRewritingWithAssumeNoSideEffectsTest.java
+++ b/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderRewritingWithAssumeNoSideEffectsTest.java
@@ -61,6 +61,28 @@
}
}
+ public static class SubsequentHasNextRunner {
+ public static void main(String[] args) {
+ Iterator<Service> iterator =
+ ServiceLoader.load(Service.class, Service.class.getClassLoader()).iterator();
+ if (iterator.hasNext()) {
+ iterator.next().print();
+ System.out.println("" + iterator.hasNext());
+ }
+ }
+ }
+
+ public static class SubsequentHasNextRunnerNonDominating {
+ public static void main(String[] args) {
+ Iterator<Service> iterator =
+ ServiceLoader.load(Service.class, Service.class.getClassLoader()).iterator();
+ if (iterator.hasNext()) {
+ iterator.next().print();
+ }
+ System.out.println("" + iterator.hasNext());
+ }
+ }
+
public static class MultipleCallsRunner {
public static void main(String[] args) {
Iterator<Service> iterator =
@@ -76,6 +98,7 @@
Iterator<Service> iterator =
ServiceLoader.load(Service.class, Service.class.getClassLoader()).iterator();
+ // We optimize a subsequent hasNext() only when a prior hasNext() exists.
iterator.next().print();
if (iterator.hasNext()) {
System.out.println("not reached");
@@ -187,6 +210,36 @@
}
@Test
+ public void testRewritingWithSubsequentHasNext_oneImpl()
+ throws IOException, CompilationFailedException, ExecutionException {
+ doTest(ServiceImpl.class)
+ .addKeepMainRule(SubsequentHasNextRunner.class)
+ .compile()
+ .run(parameters.getRuntime(), SubsequentHasNextRunner.class)
+ .assertSuccessWithOutput(EXPECTED_OUTPUT + StringUtils.lines("false"))
+ .inspect(
+ inspector -> {
+ assertIteratorPresent(inspector, false);
+ verifyServiceMetaInf(inspector, Service.class, ServiceImpl.class);
+ });
+ }
+
+ @Test
+ public void testRewritingWithSubsequentHasNext_twoImpls()
+ throws IOException, CompilationFailedException, ExecutionException {
+ doTest(ServiceImpl.class, ServiceImpl2.class)
+ .addKeepMainRule(SubsequentHasNextRunner.class)
+ .compile()
+ .run(parameters.getRuntime(), SubsequentHasNextRunner.class)
+ .assertSuccessWithOutput(EXPECTED_OUTPUT + StringUtils.lines("true"))
+ .inspect(
+ inspector -> {
+ assertIteratorPresent(inspector, false);
+ verifyServiceMetaInf(inspector, Service.class, ServiceImpl.class, ServiceImpl2.class);
+ });
+ }
+
+ @Test
public void testDoNotRewriteMultipleCalls()
throws IOException, CompilationFailedException, ExecutionException {
doTest(ServiceImpl.class)
@@ -275,4 +328,19 @@
verifyServiceMetaInf(inspector, Service.class, ServiceImpl.class);
});
}
+
+ @Test
+ public void testDoNotRewriteNonDominatedSubsequentHasNext()
+ throws IOException, CompilationFailedException, ExecutionException {
+ doTest(ServiceImpl.class)
+ .addKeepMainRule(SubsequentHasNextRunnerNonDominating.class)
+ .compile()
+ .run(parameters.getRuntime(), SubsequentHasNextRunnerNonDominating.class)
+ .assertSuccessWithOutput(EXPECTED_OUTPUT + StringUtils.lines("false"))
+ .inspect(
+ inspector -> {
+ assertIteratorPresent(inspector, true);
+ verifyServiceMetaInf(inspector, Service.class, ServiceImpl.class);
+ });
+ }
}
diff --git a/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderTestBase.java b/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderTestBase.java
index 809f782..9ce3ecd 100644
--- a/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderTestBase.java
+++ b/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderTestBase.java
@@ -23,10 +23,10 @@
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.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import java.io.IOException;
import java.util.Arrays;
-import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
@@ -96,16 +96,7 @@
public void verifyServiceMetaInf(
CodeInspector inspector, Class<?> serviceClass, Class<?> serviceImplClass) {
- // Account for renaming, and for the impl to be merged with the interface.
- String finalServiceName = inspector.clazz(serviceClass).getFinalName();
- String finalImplName = inspector.clazz(serviceImplClass).getFinalName();
- if (finalServiceName == null) {
- finalServiceName = finalImplName;
- }
- Map<String, List<String>> actual = getServiceMappings();
- Map<String, List<String>> expected =
- ImmutableMap.of(finalServiceName, Collections.singletonList(finalImplName));
- assertEquals(expected, actual);
+ verifyServiceMetaInf(inspector, serviceClass, serviceImplClass, null);
}
public void verifyServiceMetaInf(
@@ -113,13 +104,20 @@
Class<?> serviceClass,
Class<?> serviceImplClass1,
Class<?> serviceImplClass2) {
- // Account for renaming. No class merging should happen.
+ // Account for renaming & that impls can be removed if unused.
String finalServiceName = inspector.clazz(serviceClass).getFinalName();
String finalImplName1 = inspector.clazz(serviceImplClass1).getFinalName();
- String finalImplName2 = inspector.clazz(serviceImplClass2).getFinalName();
+ String finalImplName2 =
+ serviceImplClass2 == null ? null : inspector.clazz(serviceImplClass2).getFinalName();
+ if (finalServiceName == null) {
+ finalServiceName = finalImplName1;
+ }
Map<String, List<String>> actual = getServiceMappings();
- Map<String, List<String>> expected =
- ImmutableMap.of(finalServiceName, Arrays.asList(finalImplName1, finalImplName2));
+ List<String> expectedImpls =
+ finalImplName2 == null
+ ? ImmutableList.of(finalImplName1)
+ : ImmutableList.of(finalImplName1, finalImplName2);
+ Map<String, List<String>> expected = ImmutableMap.of(finalServiceName, expectedImpls);
assertEquals(expected, actual);
}
diff --git a/src/test/testbase/java/com/android/tools/r8/R8TestBuilder.java b/src/test/testbase/java/com/android/tools/r8/R8TestBuilder.java
index bf3998d..3412d54 100644
--- a/src/test/testbase/java/com/android/tools/r8/R8TestBuilder.java
+++ b/src/test/testbase/java/com/android/tools/r8/R8TestBuilder.java
@@ -47,8 +47,11 @@
import com.android.tools.r8.utils.SemanticVersion;
import com.android.tools.r8.utils.SourceFileTemplateProvider;
import com.android.tools.r8.utils.codeinspector.Matchers;
+import java.io.ByteArrayInputStream;
import java.io.IOException;
+import java.io.InputStream;
import java.lang.annotation.Annotation;
+import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
@@ -982,15 +985,58 @@
AndroidTestResource testResource, Path output, List<byte[]> classFileData) {
Path resources = testResource.getResourceZip();
resourceShrinkerOutput = output;
- getBuilder()
- .setAndroidResourceProvider(
- new ArchiveProtoAndroidResourceProvider(resources, new PathOrigin(resources)));
+ ArchiveProtoAndroidResourceProvider provider = getResourceProvider(testResource);
+ getBuilder().setAndroidResourceProvider(provider);
getBuilder()
.setAndroidResourceConsumer(new ArchiveProtoAndroidResourceConsumer(output, resources));
return addProgramClassFileData(classFileData);
}
+ private static ArchiveProtoAndroidResourceProvider getResourceProvider(
+ AndroidTestResource testResource) {
+ Path resources = testResource.getResourceZip();
+ if (testResource.getAdditionalKeepRuleFiles().isEmpty()) {
+ return new ArchiveProtoAndroidResourceProvider(resources, new PathOrigin(resources));
+ }
+ ArchiveProtoAndroidResourceProvider provider =
+ new ArchiveProtoAndroidResourceProvider(resources, new PathOrigin(resources)) {
+ @Override
+ public Collection<AndroidResourceInput> getAndroidResources() throws ResourceException {
+ ArrayList<AndroidResourceInput> resourceInputs =
+ new ArrayList<>(super.getAndroidResources());
+ resourceInputs.addAll(
+ testResource.getAdditionalKeepRuleFiles().stream()
+ .map(
+ s ->
+ new AndroidResourceInput() {
+ @Override
+ public Origin getOrigin() {
+ return Origin.unknown();
+ }
+
+ @Override
+ public ResourcePath getPath() {
+ return () -> "keep/rule/path";
+ }
+
+ @Override
+ public Kind getKind() {
+ return Kind.KEEP_RULE_FILE;
+ }
+
+ @Override
+ public InputStream getByteStream() throws ResourceException {
+ return new ByteArrayInputStream(s.getBytes(StandardCharsets.UTF_8));
+ }
+ })
+ .collect(Collectors.toList()));
+ return resourceInputs;
+ }
+ };
+ return provider;
+ }
+
public T setAndroidResourcesFromPath(Path input) throws IOException {
return setAndroidResourcesFromPath(
input, getState().getNewTempFile("resourceshrinkeroutput.zip"));
diff --git a/src/test/testbase/java/com/android/tools/r8/R8TestCompileResult.java b/src/test/testbase/java/com/android/tools/r8/R8TestCompileResult.java
index 172fb2b..3ff7487 100644
--- a/src/test/testbase/java/com/android/tools/r8/R8TestCompileResult.java
+++ b/src/test/testbase/java/com/android/tools/r8/R8TestCompileResult.java
@@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
@@ -12,6 +13,8 @@
import com.android.tools.r8.androidresources.AndroidResourceTestingUtils.ResourceTableInspector;
import com.android.tools.r8.benchmarks.BenchmarkResults;
import com.android.tools.r8.dexsplitter.SplitterTestBase.SplitRunner;
+import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.metadata.R8BuildMetadata;
import com.android.tools.r8.profile.art.model.ExternalArtProfile;
import com.android.tools.r8.profile.art.utils.ArtProfileInspector;
@@ -25,7 +28,7 @@
import com.android.tools.r8.utils.ZipUtils;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
-import com.android.tools.r8.utils.codeinspector.Matchers;
+import com.android.tools.r8.utils.codeinspector.FoundClassSubject;
import com.android.tools.r8.utils.graphinspector.GraphInspector;
import java.io.IOException;
import java.io.UncheckedIOException;
@@ -266,21 +269,18 @@
throws IOException {
assert getBackend() == runtime.getBackend();
ClassSubject mainClassSubject = inspector().clazz(SplitRunner.class);
- assertThat(
- "Did you forget a keep rule for the main method?", mainClassSubject, Matchers.isPresent());
+ assertThat("Did you forget a keep rule for the main method?", mainClassSubject, isPresent());
assertThat(
"Did you forget a keep rule for the main method?",
mainClassSubject.mainMethod(),
- Matchers.isPresent());
+ isPresent());
ClassSubject mainFeatureClassSubject = featureInspector(feature).clazz(mainFeatureClass);
assertThat(
- "Did you forget a keep rule for the run method?",
- mainFeatureClassSubject,
- Matchers.isPresent());
+ "Did you forget a keep rule for the run method?", mainFeatureClassSubject, isPresent());
assertThat(
"Did you forget a keep rule for the run method?",
mainFeatureClassSubject.uniqueMethodWithOriginalName("run"),
- Matchers.isPresent());
+ isPresent());
String[] args = new String[2 + featureDependencies.length];
args[0] = mainFeatureClassSubject.getFinalName();
args[1] = feature.toString();
@@ -319,4 +319,29 @@
results.addCodeSizeResult(applicationSizeWithFeatures);
return self();
}
+
+ @Override
+ public R8TestCompileResult benchmarkComposableCodeSize(BenchmarkResults results)
+ throws IOException {
+ int composableCodeSize = getComposableCodeSize(inspector());
+ for (Path feature : features) {
+ composableCodeSize += getComposableCodeSize(featureInspector(feature));
+ }
+ results.addComposableCodeSizeResult(composableCodeSize);
+ return self();
+ }
+
+ private int getComposableCodeSize(CodeInspector inspector) {
+ DexType composableType =
+ inspector.getFactory().createType("Landroidx/compose/runtime/Composable;");
+ int composableCodeSize = 0;
+ for (FoundClassSubject classSubject : inspector.allClasses()) {
+ for (ProgramMethod method : classSubject.getDexProgramClass().directProgramMethods()) {
+ if (method.getAnnotations().hasAnnotation(composableType)) {
+ composableCodeSize += method.getDefinition().getCode().asDexCode().codeSizeInBytes();
+ }
+ }
+ }
+ return composableCodeSize;
+ }
}
diff --git a/src/test/testbase/java/com/android/tools/r8/TestBuilder.java b/src/test/testbase/java/com/android/tools/r8/TestBuilder.java
index a7c2cd8..5f9d323 100644
--- a/src/test/testbase/java/com/android/tools/r8/TestBuilder.java
+++ b/src/test/testbase/java/com/android/tools/r8/TestBuilder.java
@@ -50,7 +50,7 @@
return self();
}
- public T applyIf(boolean value, ThrowableConsumer<T> consumer) {
+ public T applyIf(boolean value, ThrowableConsumer<? super T> consumer) {
T self = self();
if (value) {
consumer.acceptWithRuntimeException(self);
diff --git a/src/test/testbase/java/com/android/tools/r8/TestCompileResult.java b/src/test/testbase/java/com/android/tools/r8/TestCompileResult.java
index 8f32eae..1f50960 100644
--- a/src/test/testbase/java/com/android/tools/r8/TestCompileResult.java
+++ b/src/test/testbase/java/com/android/tools/r8/TestCompileResult.java
@@ -20,6 +20,7 @@
import com.android.tools.r8.ToolHelper.ProcessResult;
import com.android.tools.r8.benchmarks.BenchmarkResults;
import com.android.tools.r8.debug.DebugTestConfig;
+import com.android.tools.r8.errors.Unimplemented;
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.utils.AndroidApiLevel;
@@ -721,4 +722,9 @@
results.addCodeSizeResult(app.applicationSize());
return self();
}
+
+ public CR benchmarkComposableCodeSize(BenchmarkResults results)
+ throws IOException, ResourceException {
+ throw new Unimplemented();
+ }
}
diff --git a/src/test/testbase/java/com/android/tools/r8/androidresources/AndroidResourceTestingUtils.java b/src/test/testbase/java/com/android/tools/r8/androidresources/AndroidResourceTestingUtils.java
index 80333c3..2f9a852 100644
--- a/src/test/testbase/java/com/android/tools/r8/androidresources/AndroidResourceTestingUtils.java
+++ b/src/test/testbase/java/com/android/tools/r8/androidresources/AndroidResourceTestingUtils.java
@@ -173,10 +173,13 @@
public static class AndroidTestResource {
private final AndroidTestRClass rClass;
private final Path resourceZip;
+ private final List<String> additionalKeepRuleFiles;
- AndroidTestResource(AndroidTestRClass rClass, Path resourceZip) {
+ AndroidTestResource(
+ AndroidTestRClass rClass, Path resourceZip, List<String> additionalRawXmlFiles) {
this.rClass = rClass;
this.resourceZip = resourceZip;
+ this.additionalKeepRuleFiles = additionalRawXmlFiles;
}
public AndroidTestRClass getRClass() {
@@ -186,6 +189,10 @@
public Path getResourceZip() {
return resourceZip;
}
+
+ public List<String> getAdditionalKeepRuleFiles() {
+ return additionalKeepRuleFiles;
+ }
}
// Easy traversable resource table.
@@ -296,6 +303,7 @@
// Used for xml files that we hard code to -v24
private final Map<String, String> apiSpecificXmlFiles = new TreeMap<>();
private final Map<String, String> rawXmlFiles = new TreeMap<>();
+ private final List<String> keepRuleFiles = new ArrayList<>();
private final List<Class<?>> classesToRemap = new ArrayList<>();
private int packageId = 0x7f;
private String packageName;
@@ -379,7 +387,16 @@
}
public AndroidTestResourceBuilder addKeepXmlFor(String... resourceReferences) {
- addRawXml("keep.xml", String.format(KEEP_XML, String.join(",", resourceReferences)));
+ addRawXml("keep.xml", getKeepXmlContent(resourceReferences));
+ return this;
+ }
+
+ private static String getKeepXmlContent(String[] resourceReferences) {
+ return String.format(KEEP_XML, String.join(",", resourceReferences));
+ }
+
+ public AndroidTestResourceBuilder addExplicitKeepRuleFileFor(String... resourceReferences) {
+ keepRuleFiles.add(getKeepXmlContent(resourceReferences));
return this;
}
@@ -546,7 +563,7 @@
}
});
return new AndroidTestResource(
- new AndroidTestRClass(rClassJavaFile, rewrittenRClassFiles), output);
+ new AndroidTestRClass(rClassJavaFile, rewrittenRClassFiles), output, keepRuleFiles);
}
private String createOverlayableXml() {
diff --git a/src/test/testbase/java/com/android/tools/r8/benchmarks/BenchmarkMetric.java b/src/test/testbase/java/com/android/tools/r8/benchmarks/BenchmarkMetric.java
index 62bc0ba..abf93ce 100644
--- a/src/test/testbase/java/com/android/tools/r8/benchmarks/BenchmarkMetric.java
+++ b/src/test/testbase/java/com/android/tools/r8/benchmarks/BenchmarkMetric.java
@@ -6,6 +6,7 @@
public enum BenchmarkMetric {
RunTimeRaw,
CodeSize,
+ ComposableCodeSize,
StartupTime;
public String getDartType() {
diff --git a/src/test/testbase/java/com/android/tools/r8/benchmarks/BenchmarkResults.java b/src/test/testbase/java/com/android/tools/r8/benchmarks/BenchmarkResults.java
index 9dcfa97..a925368 100644
--- a/src/test/testbase/java/com/android/tools/r8/benchmarks/BenchmarkResults.java
+++ b/src/test/testbase/java/com/android/tools/r8/benchmarks/BenchmarkResults.java
@@ -13,6 +13,8 @@
// Append a code size result. This is always assumed to be identical if called multiple times.
void addCodeSizeResult(long result);
+ void addComposableCodeSizeResult(long result);
+
// Append a resource size result. This is always assumed to be identical if called multiple times.
void addResourceSizeResult(long result);
diff --git a/tools/perf/index.html b/tools/perf/index.html
index ec09f14..73c2931 100644
--- a/tools/perf/index.html
+++ b/tools/perf/index.html
@@ -3,6 +3,7 @@
<head>
<meta charset="utf-8">
<title>R8 perf</title>
+ <link rel="stylesheet" href="stylesheet.css">
</head>
<body>
<div id="benchmark-selectors"></div>
@@ -20,6 +21,7 @@
</div>
</div>
<script src="https://cdn.jsdelivr.net/npm/chart.js@4.4.3/dist/chart.umd.min.js"></script>
+ <script src="https://cdn.jsdelivr.net/npm/chartjs-plugin-datalabels@2.2.0"></script>
<script type="module">
// Utility methods.
Array.prototype.any = function(predicate) {
@@ -90,7 +92,9 @@
}
}
if (selectedBenchmarks.size == 0) {
- selectedBenchmarks.add(benchmarks.values().next().value);
+ const randomBenchmarkIndex = Math.floor(Math.random() * benchmarks.size);
+ const randomBenchmark = Array.from(benchmarks)[randomBenchmarkIndex];
+ selectedBenchmarks.add(randomBenchmark);
}
for (const benchmark of benchmarks.values()) {
const input = document.createElement('input');
@@ -133,6 +137,16 @@
.first()
.code_size
: NaN);
+ const composableCodeSizeData =
+ filteredCommits.map(
+ (c, i) =>
+ selectedBenchmark in filteredCommits[i].benchmarks
+ ? filteredCommits[i]
+ .benchmarks[selectedBenchmark]
+ .results
+ .first()
+ .composable_code_size
+ : NaN);
const codeSizeScatterData = [];
for (const commit of filteredCommits.values()) {
if (!(selectedBenchmark in commit.benchmarks)) {
@@ -182,7 +196,34 @@
type: 'line',
label: 'Code size',
data: codeSizeData,
+ datalabels: {
+ align: 'end',
+ anchor: 'end'
+ },
tension: 0.1,
+ yAxisID: 'y',
+ segment: {
+ borderColor: ctx =>
+ skipped(
+ ctx,
+ myChart
+ ? myChart.data.datasets[ctx.datasetIndex].backgroundColor
+ : undefined),
+ borderDash: ctx => skipped(ctx, [6, 6]),
+ },
+ spanGaps: true
+ },
+ {
+ benchmark: selectedBenchmark,
+ type: 'line',
+ label: 'Composable size',
+ data: composableCodeSizeData,
+ datalabels: {
+ align: 'start',
+ anchor: 'start'
+ },
+ tension: 0.1,
+ yAxisID: 'y_composable_code_size',
segment: {
borderColor: ctx =>
skipped(
@@ -199,6 +240,11 @@
type: 'scatter',
label: 'Nondeterminism',
data: codeSizeScatterData,
+ datalabels: {
+ labels: {
+ value: null
+ }
+ },
radius: 6,
pointBackgroundColor: 'red'
},
@@ -207,8 +253,13 @@
type: 'line',
label: 'Runtime',
data: runtimeData,
+ datalabels: {
+ labels: {
+ value: null
+ }
+ },
tension: 0.1,
- yAxisID: 'y2',
+ yAxisID: 'y_runtime',
segment: {
borderColor: ctx =>
skipped(
@@ -225,7 +276,12 @@
type: 'scatter',
label: 'Runtime variance',
data: runtimeScatterData,
- yAxisID: 'y2'
+ datalabels: {
+ labels: {
+ value: null
+ }
+ },
+ yAxisID: 'y_runtime'
}
]);
}
@@ -238,7 +294,7 @@
// Legend tracking.
const legends =
- new Set(['Code size', 'Nondeterminism', 'Runtime', 'Runtime variance']);
+ new Set(['Code size', 'Composable size', 'Nondeterminism', 'Runtime', 'Runtime variance']);
const selectedLegends =
new Set(
unescape(window.location.hash.substring(1))
@@ -246,6 +302,21 @@
.filter(l => legends.has(l)));
if (selectedLegends.size == 0) {
legends.forEach(l => selectedLegends.add(l));
+ selectedLegends.delete('Runtime variance')
+ }
+
+ function getDataPercentageChange(context) {
+ var i = context.dataIndex;
+ var value = context.dataset.data[i];
+ var j = i;
+ var previousValue;
+ do {
+ if (j == 0) {
+ return null;
+ }
+ previousValue = context.dataset.data[--j];
+ } while (previousValue === undefined || isNaN(previousValue));
+ return (value - previousValue) / previousValue * 100;
}
// Chart options.
@@ -254,9 +325,32 @@
event.native.target.style.cursor =
chartElement[0] ? 'pointer' : 'default',
plugins: {
+ datalabels: {
+ backgroundColor: 'rgba(255, 255, 255, 0.7)',
+ borderColor: 'rgba(128, 128, 128, 0.7)',
+ borderRadius: 4,
+ borderWidth: 1,
+ color: context => getDataPercentageChange(context) < 0 ? 'green' : 'red',
+ display: context => {
+ var percentageChange = getDataPercentageChange(context);
+ return percentageChange !== null && Math.abs(percentageChange) >= 0.1;
+ },
+ font: {
+ size: 20,
+ weight: 'bold'
+ },
+ offset: 8,
+ formatter: (value, context) => {
+ var percentageChange = getDataPercentageChange(context);
+ var percentageChangeTwoDecimals = Math.round(percentageChange * 100) / 100;
+ var glyph = percentageChange < 0 ? '▼' : '▲';
+ return glyph + ' ' + percentageChangeTwoDecimals + '%';
+ },
+ padding: 6
+ },
legend: {
labels: {
- filter: function(legendItem, data) {
+ filter: (legendItem, data) => {
// Only retain the legends for the first selected benchmark. If
// multiple benchmarks are selected, then use the legends of the
// first selected benchmark to control all selected benchmarks.
@@ -265,7 +359,7 @@
return legendItem.datasetIndex < numUniqueLegends;
},
},
- onClick: function legendClickHandler(e, legendItem, legend) {
+ onClick: (e, legendItem, legend) => {
const clickedLegend = legendItem.text;
if (selectedLegends.has(clickedLegend)) {
selectedLegends.delete(clickedLegend);
@@ -277,7 +371,7 @@
},
tooltip: {
callbacks: {
- title: (context) => {
+ title: context => {
const elementInfo = context[0];
var commit;
if (elementInfo.dataset.type == 'line') {
@@ -288,7 +382,7 @@
}
return commit.title;
},
- footer: (context) => {
+ footer: context => {
const elementInfo = context[0];
var commit;
if (elementInfo.dataset.type == 'line') {
@@ -317,18 +411,25 @@
text: 'Code size (bytes)'
}
},
- y2: {
+ y_runtime: {
position: 'right',
title: {
display: true,
text: 'Runtime (seconds)'
}
- }
+ },
+ y_composable_code_size: {
+ position: 'left',
+ title: {
+ display: true,
+ text: 'Composable code size (bytes)'
+ }
+ },
}
};
// Setup click handler.
- canvas.onclick = function (event) {
+ canvas.onclick = event => {
const points =
myChart.getElementsAtEventForMode(
event, 'nearest', { intersect: true }, true);
@@ -340,7 +441,7 @@
};
// Setup chart navigation.
- var zoom = { left: 0, right: commits.length };
+ var zoom = { left: Math.max(0, commits.length - 75), right: commits.length };
for (const urlOption of urlOptions.values()) {
if (urlOption.startsWith('L')) {
var left = parseInt(urlOption.substring(1));
@@ -358,7 +459,7 @@
}
}
- showMoreLeft.onclick = function (event) {
+ showMoreLeft.onclick = event => {
if (zoom.left == 0) {
return;
}
@@ -370,7 +471,7 @@
updateChart(true, false);
};
- showLessLeft.onclick = function (event) {
+ showLessLeft.onclick = event => {
const currentSize = zoom.right - zoom.left;
zoom.left = zoom.left + Math.floor(currentSize / 2);
if (zoom.left >= zoom.right) {
@@ -379,7 +480,7 @@
updateChart(true, false);
};
- showLessRight.onclick = function (event) {
+ showLessRight.onclick = event => {
if (zoom.right == 0) {
return;
}
@@ -391,7 +492,7 @@
updateChart(true, false);
};
- showMoreRight.onclick = function (event) {
+ showMoreRight.onclick = event => {
const currentSize = zoom.right - zoom.left;
zoom.right = zoom.right + currentSize;
if (zoom.right > commits.length) {
@@ -419,6 +520,13 @@
const datasetMeta = myChart.getDatasetMeta(datasetIndex);
datasetMeta.hidden = !selectedLegends.has(datasetMeta.label);
}
+
+ // Update scales.
+ options.scales.y.display = selectedLegends.has('Code size');
+ options.scales.y_composable_code_size.display = selectedLegends.has('Composable size');
+ options.scales.y_runtime.display =
+ selectedLegends.has('Runtime') || selectedLegends.has('Runtime variance');
+
// Update chart.
myChart.update();
}
@@ -466,7 +574,8 @@
// Create chart.
const myChart = new Chart(canvas, {
data: getData(),
- options: options
+ options: options,
+ plugins: [ChartDataLabels]
});
// Hide disabled legends.
diff --git a/tools/perf/stylesheet.css b/tools/perf/stylesheet.css
new file mode 100644
index 0000000..d458469
--- /dev/null
+++ b/tools/perf/stylesheet.css
@@ -0,0 +1,5 @@
+@import url('https://fonts.googleapis.com/css2?family=Roboto:ital,wght@0,100;0,300;0,400;0,500;0,700;0,900;1,100;1,300;1,400;1,500;1,700;1,900&family=Source+Sans+3:ital,wght@0,200..900;1,200..900&display=swap');
+
+body {
+ font-family: 'Roboto';
+}
\ No newline at end of file
diff --git a/tools/run_benchmark.py b/tools/run_benchmark.py
index 6bf0882..f01535f 100755
--- a/tools/run_benchmark.py
+++ b/tools/run_benchmark.py
@@ -18,7 +18,9 @@
utils.GRADLE_TASK_TESTBASE_WITH_APPLY_MAPPING_JAR,
utils.GRADLE_TASK_TEST_DEPS_JAR
]
-GOLEM_BUILD_TARGETS = [utils.GRADLE_TASK_R8LIB] + GOLEM_BUILD_TARGETS_TESTS
+GOLEM_BUILD_TARGETS = [
+ utils.GRADLE_TASK_R8LIB, utils.GRADLE_TASK_KEEP_ANNO_JAR
+] + GOLEM_BUILD_TARGETS_TESTS
def get_golem_resource_path(benchmark):
@@ -46,6 +48,13 @@
required=True,
# These should 1:1 with benchmarks/BenchmarkTarget.java
choices=['d8', 'r8-full', 'r8-force', 'r8-compat'])
+ result.add_argument(
+ '--debug-agent',
+ '--debug_agent',
+ help=
+ 'Enable Java debug agent and suspend compilation (default disabled)',
+ default=False,
+ action='store_true')
result.add_argument('--nolib',
'--no-lib',
'--no-r8lib',
@@ -119,7 +128,8 @@
utils.GRADLE_TASK_TEST_JAR, utils.GRADLE_TASK_TEST_DEPS_JAR,
utils.GRADLE_TASK_TEST_UNZIP_TESTBASE
]
- buildTargets = [utils.GRADLE_TASK_R8] + testBuildTargets
+ buildTargets = [utils.GRADLE_TASK_R8, utils.GRADLE_TASK_KEEP_ANNO_JAR
+ ] + testBuildTargets
r8jar = utils.R8_JAR
testjars = [
utils.R8_TESTS_JAR, utils.R8_TESTS_DEPS_JAR, utils.R8_TESTBASE_JAR
@@ -157,7 +167,8 @@
cmd = [
jdk.GetJavaExecutable(jdkhome), '-Xms8g', '-Xmx8g',
'-XX:+TieredCompilation', '-XX:TieredStopAtLevel=4',
- '-DBENCHMARK_IGNORE_CODE_SIZE_DIFFERENCES'
+ '-DBENCHMARK_IGNORE_CODE_SIZE_DIFFERENCES',
+ f'-DBUILD_PROP_KEEPANNO_RUNTIME_PATH={utils.REPO_ROOT}/d8_r8/keepanno/build/classes/java/main'
]
if options.enable_assertions:
cmd.append('-ea')
@@ -175,6 +186,10 @@
if options.output:
cmd.append(f'-DBENCHMARK_OUTPUT={options.output}')
cmd.extend(['-cp', ':'.join([r8jar] + testjars)])
+ if options.debug_agent:
+ cmd.append(
+ '-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=*:5005'
+ )
cmd.extend([
'com.android.tools.r8.benchmarks.BenchmarkMainEntryRunner',
options.benchmark,
