Reland "Include default field values for kept classes"
This reverts commit 97d3c57db8d41771213107d75e641c71c08095b7.
Reland "Analyze written-before-read property at allocation sites"
This reverts commit 334da78fd9db078e556bf0e069c213ade19ea3da.
Change-Id: Ib97c17af15fa74793c1569102be3dbf0284ca441
diff --git a/src/main/java/com/android/tools/r8/ir/code/Phi.java b/src/main/java/com/android/tools/r8/ir/code/Phi.java
index a6f39aa..7bafe1b 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Phi.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Phi.java
@@ -200,6 +200,15 @@
return operands;
}
+ public boolean hasOperandThatMatches(Predicate<Value> predicate) {
+ for (Value operand : operands) {
+ if (predicate.test(operand)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
public void removeOperand(int index) {
removeOperand(index, null, alwaysFalse());
}
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/propagation/DefaultFieldValueJoiner.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/propagation/DefaultFieldValueJoiner.java
index 80cf637..45848ad 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/propagation/DefaultFieldValueJoiner.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/propagation/DefaultFieldValueJoiner.java
@@ -7,6 +7,8 @@
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.Code;
+import com.android.tools.r8.graph.DefaultUseRegistryWithResult;
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.ProgramField;
@@ -17,6 +19,8 @@
import com.android.tools.r8.ir.analysis.value.AbstractValue;
import com.android.tools.r8.ir.analysis.value.AbstractValueFactory;
import com.android.tools.r8.ir.code.IRCode;
+import com.android.tools.r8.ir.code.Instruction;
+import com.android.tools.r8.ir.code.NewInstance;
import com.android.tools.r8.ir.conversion.MethodConversionOptions;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.ConcreteArrayTypeValueState;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.ConcreteClassTypeValueState;
@@ -26,7 +30,6 @@
import com.android.tools.r8.optimize.argumentpropagation.codescanner.NonEmptyValueState;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.ValueState;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
-import com.android.tools.r8.utils.IterableUtils;
import com.android.tools.r8.utils.MapUtils;
import com.android.tools.r8.utils.Pair;
import com.android.tools.r8.utils.ThreadUtils;
@@ -35,6 +38,7 @@
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.Collections;
import java.util.Deque;
import java.util.IdentityHashMap;
import java.util.List;
@@ -44,6 +48,7 @@
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.function.Consumer;
+import java.util.function.Predicate;
public class DefaultFieldValueJoiner {
@@ -69,6 +74,9 @@
// Find all the fields where we need to determine if each field read is guaranteed to be
// dominated by a write.
Map<DexProgramClass, List<ProgramField>> fieldsOfInterest = getFieldsOfInterest();
+ if (fieldsOfInterest.isEmpty()) {
+ return Collections.emptyMap();
+ }
// If constructor inlining is disabled, then we focus on whether each instance initializer
// definitely assigns the given field before it is read. We do the same for final and static
@@ -94,12 +102,12 @@
// constructor inlining, we find all new-instance instructions (including subtype allocations)
// and check if the field is written on each allocation before it is possibly read.
analyzeNewInstanceInstructions(
- fieldsNotSubjectToInitializerAnalysis, fieldsWithLiveDefaultValue::add);
+ fieldsNotSubjectToInitializerAnalysis, fieldsWithLiveDefaultValue::add, executorService);
return updateFlowGraphs(fieldsWithLiveDefaultValue, executorService);
}
- private Map<DexProgramClass, List<ProgramField>> getFieldsOfInterest() {
+ protected Map<DexProgramClass, List<ProgramField>> getFieldsOfInterest() {
Map<DexProgramClass, List<ProgramField>> fieldsOfInterest = new IdentityHashMap<>();
for (DexProgramClass clazz : appView.appInfo().classes()) {
clazz.forEachProgramField(
@@ -240,13 +248,101 @@
private void analyzeNewInstanceInstructions(
Map<DexType, ProgramFieldSet> nonFinalInstanceFields,
- Consumer<ProgramField> liveDefaultValueConsumer) {
- // Conservatively treat all fields as maybe read before written.
- // TODO(b/296030319): Implement analysis by building IR for all methods that instantiate the
- // relevant classes and analyzing the puts to the newly created instances.
- for (ProgramField field : IterableUtils.flatten(nonFinalInstanceFields.values())) {
- liveDefaultValueConsumer.accept(field);
+ Consumer<ProgramField> liveDefaultValueConsumer,
+ ExecutorService executorService)
+ throws ExecutionException {
+ // To simplify the analysis, we currently bail out for non-final classes.
+ // TODO(b/296030319): Handle non-final classes.
+ MapUtils.removeIf(
+ nonFinalInstanceFields,
+ (holderType, fields) -> {
+ assert !fields.isEmpty();
+ DexProgramClass holder = fields.iterator().next().getHolder();
+ // If the class is kept it could be instantiated directly, in which case all default field
+ // values could be live.
+ if (appView.getKeepInfo(holder).isPinned(appView.options())) {
+ fields.forEach(liveDefaultValueConsumer);
+ return true;
+ }
+ if (holder.isFinal() || !appView.appInfo().isInstantiatedIndirectly(holder)) {
+ // When the class is not explicitly marked final, the class could in principle have
+ // injected subclasses if it is pinned. However, none of the fields are pinned, so we
+ // should be allowed to reason about the field assignments in the program.
+ assert fields.stream()
+ .allMatch(
+ field -> appView.getKeepInfo(field).isValuePropagationAllowed(appView, field));
+ return false;
+ }
+ fields.forEach(liveDefaultValueConsumer);
+ return true;
+ });
+
+ // We analyze all allocations of the classes that declare one of the given fields.
+ ThreadUtils.processMethods(
+ appView,
+ method ->
+ analyzeNewInstanceInstructionsInMethod(
+ nonFinalInstanceFields, liveDefaultValueConsumer, method),
+ appView.options().getThreadingModule(),
+ executorService);
+ }
+
+ private void analyzeNewInstanceInstructionsInMethod(
+ Map<DexType, ProgramFieldSet> nonFinalInstanceFields,
+ Consumer<ProgramField> liveDefaultValueConsumer,
+ ProgramMethod method) {
+ if (!maybeHasNewInstanceThatMatches(method, nonFinalInstanceFields::containsKey)) {
+ return;
}
+ IRCode code = method.buildIR(appView, MethodConversionOptions.nonConverting());
+ for (NewInstance newInstance : code.<NewInstance>instructions(Instruction::isNewInstance)) {
+ ProgramFieldSet fieldsOfInterest = nonFinalInstanceFields.get(newInstance.getType());
+ if (fieldsOfInterest == null) {
+ continue;
+ }
+ FieldReadBeforeWriteDfsAnalysis analysis =
+ new FieldReadBeforeWriteDfsAnalysis(appView, code, fieldsOfInterest, newInstance) {
+
+ @Override
+ public AnalysisContinuation acceptFieldMaybeReadBeforeWrite(ProgramField field) {
+ // Remove this field from the `fieldsOfInterest`, so that we do not spend more time
+ // analyzing it.
+ if (fieldsOfInterest.remove(field)) {
+ liveDefaultValueConsumer.accept(field);
+ }
+ return AnalysisContinuation.abortIf(fieldsOfInterest.isEmpty());
+ }
+ };
+ analysis.run();
+ if (fieldsOfInterest.isEmpty()) {
+ nonFinalInstanceFields.remove(newInstance.getType());
+ }
+ }
+ }
+
+ private boolean maybeHasNewInstanceThatMatches(
+ ProgramMethod method, Predicate<DexType> predicate) {
+ Code code = method.getDefinition().getCode();
+ if (code == null || code.isSharedCodeObject()) {
+ return false;
+ }
+ if (code.isLirCode()) {
+ return code.asLirCode()
+ .hasConstantItemThatMatches(
+ constant -> constant instanceof DexType && predicate.test((DexType) constant));
+ }
+ assert appView.isCfByteCodePassThrough(method);
+ assert code.isCfCode();
+ return method.registerCodeReferencesWithResult(
+ new DefaultUseRegistryWithResult<>(appView, method, false) {
+
+ @Override
+ public void registerNewInstance(DexType type) {
+ if (predicate.test(type)) {
+ setResult(true);
+ }
+ }
+ });
}
private Map<FlowGraph, Deque<FlowGraphNode>> updateFlowGraphs(
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/propagation/FieldReadBeforeWriteDfsAnalysis.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/propagation/FieldReadBeforeWriteDfsAnalysis.java
new file mode 100644
index 0000000..1fdfb27
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/propagation/FieldReadBeforeWriteDfsAnalysis.java
@@ -0,0 +1,387 @@
+// 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.propagation;
+
+import static com.android.tools.r8.ir.code.Opcodes.ASSUME;
+import static com.android.tools.r8.ir.code.Opcodes.CHECK_CAST;
+import static com.android.tools.r8.ir.code.Opcodes.CONST_NUMBER;
+import static com.android.tools.r8.ir.code.Opcodes.CONST_STRING;
+import static com.android.tools.r8.ir.code.Opcodes.GOTO;
+import static com.android.tools.r8.ir.code.Opcodes.INSTANCE_PUT;
+import static com.android.tools.r8.ir.code.Opcodes.INVOKE_DIRECT;
+import static com.android.tools.r8.ir.code.Opcodes.INVOKE_INTERFACE;
+import static com.android.tools.r8.ir.code.Opcodes.INVOKE_STATIC;
+import static com.android.tools.r8.ir.code.Opcodes.INVOKE_SUPER;
+import static com.android.tools.r8.ir.code.Opcodes.INVOKE_VIRTUAL;
+import static com.android.tools.r8.ir.code.Opcodes.RETURN;
+import static com.android.tools.r8.ir.code.Opcodes.THROW;
+
+import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.DexClassAndMethod;
+import com.android.tools.r8.graph.DexItemFactory;
+import com.android.tools.r8.graph.ProgramField;
+import com.android.tools.r8.ir.code.Assume;
+import com.android.tools.r8.ir.code.BasicBlock;
+import com.android.tools.r8.ir.code.BasicBlockInstructionIterator;
+import com.android.tools.r8.ir.code.CheckCast;
+import com.android.tools.r8.ir.code.ConstNumber;
+import com.android.tools.r8.ir.code.ConstString;
+import com.android.tools.r8.ir.code.Goto;
+import com.android.tools.r8.ir.code.IRCode;
+import com.android.tools.r8.ir.code.InstancePut;
+import com.android.tools.r8.ir.code.Instruction;
+import com.android.tools.r8.ir.code.InvokeDirect;
+import com.android.tools.r8.ir.code.InvokeMethod;
+import com.android.tools.r8.ir.code.NewInstance;
+import com.android.tools.r8.ir.code.Phi;
+import com.android.tools.r8.ir.code.Return;
+import com.android.tools.r8.ir.code.Throw;
+import com.android.tools.r8.shaking.AppInfoWithLiveness;
+import com.android.tools.r8.utils.TraversalContinuation;
+import com.android.tools.r8.utils.WorkList;
+import com.android.tools.r8.utils.collections.ProgramFieldSet;
+
+/**
+ * Analysis that is given an allocation site (a {@link NewInstance} instruction) and a set of fields
+ * that belong to that newly allocated instance.
+ *
+ * <p>The analysis computes the subset of the given fields which are maybe read before they are
+ * written. The default value of these fields is potentially read, whereas the default value of the
+ * complement field set are guaranteed to never be read.
+ *
+ * <p>The analysis works by exploring all possible paths starting from the given allocation site to
+ * the normal and the exceptional exits of the method, keeping track of which fields are definitely
+ * written before they are read and which fields have maybe been read.
+ */
+public abstract class FieldReadBeforeWriteDfsAnalysis extends FieldReadBeforeWriteDfsAnalysisState {
+
+ private final AppView<AppInfoWithLiveness> appView;
+ private final IRCode code;
+ private final DexItemFactory dexItemFactory;
+ // The set of fields to consider. Note that this is a concurrent set and that the caller may
+ // concurrently remove fields from the set. This may happen if we concurrently find a
+ // read-before-write of one of the fields.
+ private final ProgramFieldSet fields;
+ private final WorkList<WorkItem> worklist = WorkList.newIdentityWorkList();
+
+ private final FieldReadBeforeWriteDfsAnalysis self = this;
+
+ public FieldReadBeforeWriteDfsAnalysis(
+ AppView<AppInfoWithLiveness> appView,
+ IRCode code,
+ ProgramFieldSet fields,
+ NewInstance newInstance) {
+ super(newInstance);
+ this.appView = appView;
+ this.code = code;
+ this.dexItemFactory = appView.dexItemFactory();
+ this.fields = fields;
+ }
+
+ // Returns ABORT if all fields of interest are now maybe-read-before-written.
+ // Otherwise returns CONTINUE.
+ public abstract AnalysisContinuation acceptFieldMaybeReadBeforeWrite(ProgramField field);
+
+ public void run() {
+ worklist.addIfNotSeen(new InitialWorkItem());
+ worklist.run(WorkItem::process);
+ }
+
+ public enum AnalysisContinuation {
+ // Signals to abort the analysis completely (i.e., to break out of the DFS). This is used when
+ // we've reported all fields as being maybe read before written.
+ ABORT,
+ // Signals to continue the current DFS.
+ CONTINUE,
+ // Signals that all fields have been written before they are read on the current program path,
+ // meaning that the algorithm does not need to explore any further. The algorithm should instead
+ // backtrack and explore any other program paths.
+ BREAK;
+
+ static AnalysisContinuation abortIf(boolean condition) {
+ if (condition) {
+ return ABORT;
+ }
+ return CONTINUE;
+ }
+
+ boolean isAbort() {
+ return this == ABORT;
+ }
+
+ boolean isAbortOrContinue() {
+ return isAbort() || isContinue();
+ }
+
+ boolean isBreak() {
+ return this == BREAK;
+ }
+
+ boolean isContinue() {
+ return this == CONTINUE;
+ }
+
+ TraversalContinuation<?, ?> toTraversalContinuation() {
+ assert isAbortOrContinue();
+ return TraversalContinuation.breakIf(isAbort());
+ }
+ }
+
+ abstract class WorkItem {
+
+ abstract TraversalContinuation<?, ?> process();
+
+ void applyPhis(BasicBlock block) {
+ // TODO(b/339210038): When adding support for non-linear control flow, we need to implement
+ // backtracking of this (i.e., we should remove the out value from the object aliases again).
+ for (Phi phi : block.getPhis()) {
+ if (phi.hasOperandThatMatches(self::isMaybeInstance)) {
+ addInstanceAlias(phi);
+ }
+ }
+ }
+
+ AnalysisContinuation applyInstructions(BasicBlockInstructionIterator instructionIterator) {
+ while (instructionIterator.hasNext()) {
+ Instruction instruction = instructionIterator.next();
+ assert !instruction.hasOutValue() || !isMaybeInstance(instruction.outValue());
+ AnalysisContinuation continuation;
+ // TODO(b/339210038): Extend this to many other instructions, such as ConstClass,
+ // InstanceOf, *Binop, etc.
+ switch (instruction.opcode()) {
+ case ASSUME:
+ continuation = applyAssumeInstruction(instruction.asAssume());
+ break;
+ case CHECK_CAST:
+ continuation = applyCheckCastInstruction(instruction.asCheckCast());
+ break;
+ case CONST_NUMBER:
+ continuation = applyConstNumber(instruction.asConstNumber());
+ break;
+ case CONST_STRING:
+ continuation = applyConstString(instruction.asConstString());
+ break;
+ case GOTO:
+ continuation = applyGotoInstruction(instruction.asGoto());
+ break;
+ case INSTANCE_PUT:
+ continuation = applyInstancePut(instruction.asInstancePut());
+ break;
+ case INVOKE_DIRECT:
+ case INVOKE_INTERFACE:
+ case INVOKE_STATIC:
+ case INVOKE_SUPER:
+ case INVOKE_VIRTUAL:
+ continuation = applyInvokeMethodInstruction(instruction.asInvokeMethod());
+ break;
+ case RETURN:
+ continuation = applyReturnInstruction(instruction.asReturn());
+ break;
+ case THROW:
+ continuation = applyThrowInstruction(instruction.asThrow());
+ break;
+ default:
+ continuation = applyUnhandledInstruction();
+ break;
+ }
+ if (continuation.isAbort()) {
+ return continuation;
+ }
+ if (continuation.isBreak()) {
+ break;
+ }
+ }
+ return AnalysisContinuation.CONTINUE;
+ }
+
+ // TODO(b/339210038): When adding support for non-linear control flow, we need to implement
+ // backtracking of this (i.e., we should remove the out value from the object aliases again).
+ private AnalysisContinuation applyAssumeInstruction(Assume assume) {
+ if (isMaybeInstance(assume.src())) {
+ addInstanceAlias(assume.outValue());
+ }
+ return AnalysisContinuation.CONTINUE;
+ }
+
+ // TODO(b/339210038): When adding support for non-linear control flow, we need to implement
+ // backtracking of this (i.e., we should remove the out value from the object aliases again).
+ private AnalysisContinuation applyCheckCastInstruction(CheckCast checkCast) {
+ if (isMaybeInstance(checkCast.object())) {
+ addInstanceAlias(checkCast.outValue());
+ }
+ // If the instance has escaped to the heap and this check-cast instruction throws, then it is
+ // possible that the instance is retrieved from the heap and all fields are read.
+ return markRemainingFieldsAsMaybeReadBeforeWrittenIfInstanceIsEscaped();
+ }
+
+ private AnalysisContinuation applyConstNumber(ConstNumber unusedConstNumber) {
+ return AnalysisContinuation.CONTINUE;
+ }
+
+ private AnalysisContinuation applyConstString(ConstString unusedConstString) {
+ return AnalysisContinuation.CONTINUE;
+ }
+
+ private AnalysisContinuation applyGotoInstruction(Goto gotoInstruction) {
+ BasicBlock targetBlock = gotoInstruction.getTarget();
+ if (isBlockOnStack(targetBlock)) {
+ // Bail out in case of cycles.
+ return markRemainingFieldsAsMaybeReadBeforeWritten();
+ } else {
+ // Continue exploration into the successor block.
+ worklist.addIgnoringSeenSet(new ProcessBlockWorkItem(targetBlock));
+ return AnalysisContinuation.CONTINUE;
+ }
+ }
+
+ private AnalysisContinuation applyInstancePut(InstancePut instancePut) {
+ // If the instance has escaped and this instance-put instruction can throw, then the program
+ // can get the instance from the heap and read any field. Give up in this case.
+ if (isEscaped() && instancePut.instructionInstanceCanThrow(appView, code.context())) {
+ return markRemainingFieldsAsMaybeReadBeforeWritten();
+ }
+
+ // Record if this is a definite write to one of the fields of interest.
+ if (isDefinitelyInstance(instancePut.object())) {
+ ProgramField resolvedField =
+ instancePut.resolveField(appView, code.context()).getProgramField();
+ if (resolvedField != null && fields.contains(resolvedField)) {
+ addWrittenBeforeRead(resolvedField);
+ }
+
+ // If all fields of interest are written before read, then stop the exploration of the
+ // current program path (but continue to explore any program paths from previous unexplored
+ // branches).
+ if (fields.allMatch(self::isWrittenBeforeRead)) {
+ return AnalysisContinuation.BREAK;
+ }
+ }
+
+ // Record if the instance has escaped as a result of this instance-put.
+ if (!isEscaped() && isMaybeInstance(instancePut.value())) {
+ setEscaped(instancePut);
+ }
+ return AnalysisContinuation.CONTINUE;
+ }
+
+ private AnalysisContinuation applyInvokeMethodInstruction(InvokeMethod invoke) {
+ // Allow calls to java.lang.Object.<init>().
+ // TODO(b/339210038): Generalize this to other constructors.
+ if (invoke.isInvokeConstructor(dexItemFactory)
+ && isDefinitelyInstance(invoke.getFirstArgument())) {
+ DexClassAndMethod resolvedMethod =
+ invoke.resolveMethod(appView, code.context()).getResolutionPair();
+ if (resolvedMethod != null
+ && resolvedMethod
+ .getReference()
+ .isIdenticalTo(dexItemFactory.objectMembers.constructor)) {
+ return AnalysisContinuation.CONTINUE;
+ }
+ }
+
+ // Conservatively treat calls as reading any field if the receiver has escaped or is escaping.
+ if (!isEscaped()
+ && invoke.hasInValueThatMatches(self::isMaybeInstance)
+ && invoke.instructionMayHaveSideEffects(appView, code.context())) {
+ setEscaped(invoke);
+ }
+
+ if (isEscaped()) {
+ return markRemainingFieldsAsMaybeReadBeforeWritten();
+ }
+
+ // Otherwise, this is a call to a method where none of the arguments is an alias of the
+ // instance, and the instance has not escaped. Therefore, this call cannot read any of fields
+ // from the instance.
+ return AnalysisContinuation.CONTINUE;
+ }
+
+ private AnalysisContinuation applyReturnInstruction(Return unusedReturnInstruction) {
+ return markRemainingFieldsAsMaybeReadBeforeWritten();
+ }
+
+ private AnalysisContinuation applyThrowInstruction(Throw unusedThrowInstruction) {
+ return markRemainingFieldsAsMaybeReadBeforeWrittenIfInstanceIsEscaped();
+ }
+
+ private AnalysisContinuation applyUnhandledInstruction() {
+ return markRemainingFieldsAsMaybeReadBeforeWritten();
+ }
+
+ AnalysisContinuation markRemainingFieldsAsMaybeReadBeforeWritten() {
+ for (ProgramField field : fields) {
+ if (!isWrittenBeforeRead(field)) {
+ AnalysisContinuation continuation = acceptFieldMaybeReadBeforeWrite(field);
+ assert continuation.isAbortOrContinue();
+ if (continuation.isAbort()) {
+ return continuation;
+ }
+ }
+ }
+ // At this point we could also CONTINUE, but we check if the fields of interest have become
+ // empty as a result of concurrent modification.
+ return AnalysisContinuation.abortIf(fields.isEmpty());
+ }
+
+ AnalysisContinuation markRemainingFieldsAsMaybeReadBeforeWrittenIfInstanceIsEscaped() {
+ if (isEscaped()) {
+ return markRemainingFieldsAsMaybeReadBeforeWritten();
+ }
+ return AnalysisContinuation.CONTINUE;
+ }
+ }
+
+ class InitialWorkItem extends WorkItem {
+
+ @Override
+ TraversalContinuation<?, ?> process() {
+ // We start the analysis from the unique constructor invoke instead of from the NewInstance
+ // instruction, since no instructions before the constructor call can read any fields from the
+ // uninitialized this.
+ // TODO(b/339210038): In principle it may be possible for the NewInstance value to flow into a
+ // phi before the unique constructor invoke. If this happens we would not record the phi as
+ // an alias when starting the analysis from the invoke-direct.
+ InvokeDirect uniqueConstructorInvoke =
+ getNewInstance().getUniqueConstructorInvoke(dexItemFactory);
+ if (uniqueConstructorInvoke == null) {
+ return markRemainingFieldsAsMaybeReadBeforeWritten().toTraversalContinuation();
+ }
+ BasicBlock block = uniqueConstructorInvoke.getBlock();
+ // TODO(b/339210038): Maybe allow exceptional control flow.
+ if (block.hasCatchHandlers()) {
+ return markRemainingFieldsAsMaybeReadBeforeWritten().toTraversalContinuation();
+ }
+ addBlockToStack(block);
+ addInstanceAlias(getNewInstance().outValue());
+ BasicBlockInstructionIterator instructionIterator = block.iterator(uniqueConstructorInvoke);
+ // Start the analysis from the invoke-direct instruction. This is important if we can tell
+ // that the constructor definitely writes some fields.
+ instructionIterator.previous();
+ return applyInstructions(instructionIterator).toTraversalContinuation();
+ }
+ }
+
+ class ProcessBlockWorkItem extends WorkItem {
+
+ private final BasicBlock block;
+
+ ProcessBlockWorkItem(BasicBlock block) {
+ this.block = block;
+ }
+
+ @Override
+ TraversalContinuation<?, ?> process() {
+ // TODO(b/339210038): Maybe allow exceptional control flow.
+ if (block.hasCatchHandlers()) {
+ return TraversalContinuation.breakIf(
+ markRemainingFieldsAsMaybeReadBeforeWritten().isAbort());
+ }
+ addBlockToStack(block);
+ applyPhis(block);
+ AnalysisContinuation continuation = applyInstructions(block.iterator());
+ assert continuation.isAbortOrContinue();
+ return TraversalContinuation.breakIf(continuation.isAbort());
+ }
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/propagation/FieldReadBeforeWriteDfsAnalysisState.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/propagation/FieldReadBeforeWriteDfsAnalysisState.java
new file mode 100644
index 0000000..7e24c03
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/propagation/FieldReadBeforeWriteDfsAnalysisState.java
@@ -0,0 +1,84 @@
+// 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.propagation;
+
+import com.android.tools.r8.graph.ProgramField;
+import com.android.tools.r8.ir.code.BasicBlock;
+import com.android.tools.r8.ir.code.Instruction;
+import com.android.tools.r8.ir.code.NewInstance;
+import com.android.tools.r8.ir.code.Value;
+import com.android.tools.r8.utils.collections.ProgramFieldSet;
+import com.google.common.collect.Sets;
+import java.util.Set;
+
+/**
+ * The state we track during the field-maybe-read-before-write/field-never-read-before-written
+ * analysis.
+ */
+public class FieldReadBeforeWriteDfsAnalysisState {
+
+ // The current allocation we are analyzing.
+ private final NewInstance newInstance;
+
+ // The first instruction on the current program path starting from the `newInstance` instruction
+ // from which the `newInstance` value escapes.
+ private Instruction escape = null;
+
+ // The set of values that *may* be aliases of the `newInstance` value.
+ private final Set<Value> instanceAliases = Sets.newIdentityHashSet();
+
+ // The set of blocks on the current program path.
+ private final Set<BasicBlock> stack = Sets.newIdentityHashSet();
+
+ // The set of fields that are guaranteed to be written before they are read on the current program
+ // path.
+ private final ProgramFieldSet writtenBeforeRead = ProgramFieldSet.create();
+
+ FieldReadBeforeWriteDfsAnalysisState(NewInstance newInstance) {
+ this.newInstance = newInstance;
+ }
+
+ void addInstanceAlias(Value instanceAlias) {
+ boolean changed = instanceAliases.add(instanceAlias);
+ assert changed;
+ }
+
+ void addBlockToStack(BasicBlock block) {
+ boolean changed = stack.add(block);
+ assert changed;
+ }
+
+ void addWrittenBeforeRead(ProgramField field) {
+ writtenBeforeRead.add(field);
+ }
+
+ NewInstance getNewInstance() {
+ return newInstance;
+ }
+
+ boolean isBlockOnStack(BasicBlock block) {
+ return stack.contains(block);
+ }
+
+ boolean isEscaped() {
+ return escape != null;
+ }
+
+ boolean isDefinitelyInstance(Value value) {
+ return value.getAliasedValue() == newInstance.outValue();
+ }
+
+ boolean isMaybeInstance(Value value) {
+ return instanceAliases.contains(value);
+ }
+
+ boolean isWrittenBeforeRead(ProgramField field) {
+ return writtenBeforeRead.contains(field);
+ }
+
+ void setEscaped(Instruction escape) {
+ assert !isEscaped();
+ this.escape = escape;
+ }
+}
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 d62328d..81e3424 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
@@ -46,7 +46,7 @@
final AppView<AppInfoWithLiveness> appView;
final Set<DexProgramClass> classesWithSingleCallerInlinedInstanceInitializers;
final IRConverter converter;
- final FieldStateCollection fieldStates;
+ protected final FieldStateCollection fieldStates;
final MethodStateCollectionByReference methodStates;
public InFlowPropagator(
@@ -113,12 +113,15 @@
private Map<FlowGraph, Deque<FlowGraphNode>> includeDefaultValuesInFieldStates(
List<FlowGraph> flowGraphs, ExecutorService executorService) throws ExecutionException {
- DefaultFieldValueJoiner joiner =
- new DefaultFieldValueJoiner(
- appView, classesWithSingleCallerInlinedInstanceInitializers, fieldStates, flowGraphs);
+ DefaultFieldValueJoiner joiner = createDefaultFieldValueJoiner(flowGraphs);
return joiner.joinDefaultFieldValuesForFieldsWithReadBeforeWrite(executorService);
}
+ protected DefaultFieldValueJoiner createDefaultFieldValueJoiner(List<FlowGraph> flowGraphs) {
+ return new DefaultFieldValueJoiner(
+ appView, classesWithSingleCallerInlinedInstanceInitializers, fieldStates, flowGraphs);
+ }
+
private void processFlowGraphs(List<FlowGraph> flowGraphs, ExecutorService executorService)
throws ExecutionException {
ThreadUtils.processItems(
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 8d7498c..22ed377 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
@@ -9,6 +9,7 @@
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.ProgramField;
import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.ir.analysis.constant.SparseConditionalConstantPropagation;
@@ -33,6 +34,8 @@
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.ValueState;
+import com.android.tools.r8.optimize.argumentpropagation.propagation.DefaultFieldValueJoiner;
+import com.android.tools.r8.optimize.argumentpropagation.propagation.FlowGraph;
import com.android.tools.r8.optimize.argumentpropagation.propagation.InFlowPropagator;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.IterableUtils;
@@ -42,6 +45,8 @@
import com.android.tools.r8.utils.collections.ProgramMethodSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
+import java.util.Collections;
+import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ExecutionException;
@@ -92,7 +97,22 @@
InFlowPropagator inFlowPropagator =
new InFlowPropagator(
- appView, null, converter, codeScanner.getFieldStates(), codeScanner.getMethodStates());
+ appView, null, converter, codeScanner.getFieldStates(), codeScanner.getMethodStates()) {
+
+ @Override
+ protected DefaultFieldValueJoiner createDefaultFieldValueJoiner(
+ List<FlowGraph> flowGraphs) {
+ return new DefaultFieldValueJoiner(appView, null, fieldStates, flowGraphs) {
+
+ @Override
+ protected Map<DexProgramClass, List<ProgramField>> getFieldsOfInterest() {
+ // We do not rely on the optimization of any fields in the Composable optimization
+ // pass.
+ return Collections.emptyMap();
+ }
+ };
+ }
+ };
inFlowPropagator.run(executorService);
ArgumentPropagatorOptimizationInfoPopulator optimizationInfoPopulator =
diff --git a/src/main/java/com/android/tools/r8/utils/collections/ProgramFieldSet.java b/src/main/java/com/android/tools/r8/utils/collections/ProgramFieldSet.java
index c778df3..891593a 100644
--- a/src/main/java/com/android/tools/r8/utils/collections/ProgramFieldSet.java
+++ b/src/main/java/com/android/tools/r8/utils/collections/ProgramFieldSet.java
@@ -8,14 +8,15 @@
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.ProgramField;
+import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
import java.util.IdentityHashMap;
import java.util.Iterator;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
-import java.util.function.Predicate;
import java.util.stream.Stream;
public class ProgramFieldSet implements Iterable<ProgramField> {
@@ -54,6 +55,10 @@
backing.putAll(fields.backing);
}
+ public boolean allMatch(Predicate<? super ProgramField> predicate) {
+ return Iterables.all(this, predicate);
+ }
+
public boolean createAndAdd(DexProgramClass clazz, DexEncodedField definition) {
return add(new ProgramField(clazz, definition));
}
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/PackagePrivateMembersAccessedTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/PackagePrivateMembersAccessedTest.java
index 0d912b3..fa46f4d 100644
--- a/src/test/java/com/android/tools/r8/classmerging/horizontal/PackagePrivateMembersAccessedTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/PackagePrivateMembersAccessedTest.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.classmerging.horizontal;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isAbsentIf;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static org.hamcrest.MatcherAssert.assertThat;
@@ -35,7 +36,9 @@
.assertSuccessWithOutputLines("foo", "hello", "5", "foobar")
.inspect(
codeInspector -> {
- assertThat(codeInspector.clazz(C.class), isPresent());
+ assertThat(
+ codeInspector.clazz(C.class),
+ isAbsentIf(parameters.canInitNewInstanceUsingSuperclassConstructor()));
assertThat(codeInspector.clazz(D.class), isPresent());
assertThat(codeInspector.clazz(E.class), isPresent());
});
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/membervaluepropagation/InstanceFieldWithConstructorInliningTest.java b/src/test/java/com/android/tools/r8/ir/optimize/membervaluepropagation/InstanceFieldWithConstructorInliningTest.java
index f952fa9..142d565 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/membervaluepropagation/InstanceFieldWithConstructorInliningTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/membervaluepropagation/InstanceFieldWithConstructorInliningTest.java
@@ -3,10 +3,10 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.ir.optimize.membervaluepropagation;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isAbsent;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
-import static com.android.tools.r8.utils.codeinspector.Matchers.isPresentIf;
import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
import com.android.tools.r8.NeverClassInline;
import com.android.tools.r8.TestBase;
@@ -42,17 +42,11 @@
.inspect(
inspector -> {
ClassSubject aClassSubject = inspector.clazz(A.class);
- // TODO(b/339210038): Should always be absent.
- assertThat(
- aClassSubject,
- isPresentIf(parameters.canInitNewInstanceUsingSuperclassConstructor()));
+ assertThat(aClassSubject, isAbsent());
MethodSubject mainMethodSubject = inspector.clazz(Main.class).mainMethod();
assertThat(mainMethodSubject, isPresent());
- // TODO(b/339210038): Should always contain 42.
- assertEquals(
- parameters.canInitNewInstanceUsingSuperclassConstructor(),
- mainMethodSubject.streamInstructions().noneMatch(i -> i.isConstNumber(42)));
+ assertTrue(mainMethodSubject.streamInstructions().anyMatch(i -> i.isConstNumber(42)));
})
.run(parameters.getRuntime(), Main.class)
.assertSuccessWithOutputLines("42");
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/membervaluepropagation/LiveDefaultFieldValueOfReflectivelyInstantiatedClassTest.java b/src/test/java/com/android/tools/r8/ir/optimize/membervaluepropagation/LiveDefaultFieldValueOfReflectivelyInstantiatedClassTest.java
new file mode 100644
index 0000000..358227d
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/membervaluepropagation/LiveDefaultFieldValueOfReflectivelyInstantiatedClassTest.java
@@ -0,0 +1,60 @@
+// 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.optimize.membervaluepropagation;
+
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import 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;
+
+@RunWith(Parameterized.class)
+public class LiveDefaultFieldValueOfReflectivelyInstantiatedClassTest extends TestBase {
+
+ @Parameter(0)
+ public TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepMainRule(Main.class)
+ .addKeepClassAndDefaultConstructor(BooleanBox.class)
+ .enableInliningAnnotations()
+ .setMinApi(parameters)
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines("false");
+ }
+
+ static class Main {
+
+ public static void main(String[] args) throws Exception {
+ Class<?> clazz = System.currentTimeMillis() > 0 ? BooleanBox.class : Object.class;
+ BooleanBox box = (BooleanBox) clazz.getDeclaredConstructor().newInstance();
+ System.out.println(box.value);
+ box.set();
+ }
+ }
+
+ static class BooleanBox {
+
+ boolean value;
+
+ BooleanBox() {}
+
+ @NeverInline
+ void set() {
+ value = true;
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/membervaluepropagation/fields/FieldInitializedByConstantArgumentMultipleConstructorsTest.java b/src/test/java/com/android/tools/r8/ir/optimize/membervaluepropagation/fields/FieldInitializedByConstantArgumentMultipleConstructorsTest.java
index fca4473..862fbc9 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/membervaluepropagation/fields/FieldInitializedByConstantArgumentMultipleConstructorsTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/membervaluepropagation/fields/FieldInitializedByConstantArgumentMultipleConstructorsTest.java
@@ -4,8 +4,8 @@
package com.android.tools.r8.ir.optimize.membervaluepropagation.fields;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isAbsent;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
-import static com.android.tools.r8.utils.codeinspector.Matchers.isPresentIf;
import static org.hamcrest.MatcherAssert.assertThat;
import com.android.tools.r8.NeverClassInline;
@@ -18,21 +18,20 @@
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;
@RunWith(Parameterized.class)
public class FieldInitializedByConstantArgumentMultipleConstructorsTest extends TestBase {
- private final TestParameters parameters;
+ @Parameter(0)
+ public TestParameters parameters;
- @Parameterized.Parameters(name = "{0}")
+ @Parameters(name = "{0}")
public static TestParametersCollection data() {
return getTestParameters().withAllRuntimesAndApiLevels().build();
}
- public FieldInitializedByConstantArgumentMultipleConstructorsTest(TestParameters parameters) {
- this.parameters = parameters;
- }
-
@Test
public void test() throws Exception {
testForR8(parameters.getBackend())
@@ -51,10 +50,7 @@
ClassSubject testClassSubject = inspector.clazz(TestClass.class);
assertThat(testClassSubject, isPresent());
assertThat(testClassSubject.uniqueMethodWithOriginalName("live"), isPresent());
- // TODO(b/280275115): Constructor inlining regresses instance field value analysis.
- assertThat(
- testClassSubject.uniqueMethodWithOriginalName("dead"),
- isPresentIf(parameters.canInitNewInstanceUsingSuperclassConstructor()));
+ assertThat(testClassSubject.uniqueMethodWithOriginalName("dead"), isAbsent());
}
static class TestClass {
