Revert "Reland "Analyze written-before-read property at allocation sites""
This reverts commit 18d01e1b55289d810cc99d49081b02e8bc828835.
Reason for revert: fails on platform, see
https://android-build.corp.google.com/test_investigate/invocation/I04800010289154153/test/TR56429348958799735/?blocking=blocking&status=none
Change-Id: I98e43dc5b7e149a25f1538028619599dbcd5c757
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 7bafe1b..a6f39aa 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,15 +200,6 @@
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 e750309..80cf637 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,8 +7,6 @@
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;
@@ -19,8 +17,6 @@
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;
@@ -30,6 +26,7 @@
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;
@@ -38,7 +35,6 @@
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;
@@ -48,7 +44,6 @@
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.function.Consumer;
-import java.util.function.Predicate;
public class DefaultFieldValueJoiner {
@@ -74,9 +69,6 @@
// 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
@@ -102,12 +94,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, executorService);
+ fieldsNotSubjectToInitializerAnalysis, fieldsWithLiveDefaultValue::add);
return updateFlowGraphs(fieldsWithLiveDefaultValue, executorService);
}
- protected Map<DexProgramClass, List<ProgramField>> getFieldsOfInterest() {
+ private Map<DexProgramClass, List<ProgramField>> getFieldsOfInterest() {
Map<DexProgramClass, List<ProgramField>> fieldsOfInterest = new IdentityHashMap<>();
for (DexProgramClass clazz : appView.appInfo().classes()) {
clazz.forEachProgramField(
@@ -248,95 +240,13 @@
private void analyzeNewInstanceInstructions(
Map<DexType, ProgramFieldSet> nonFinalInstanceFields,
- 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 (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;
+ 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);
}
- 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
deleted file mode 100644
index 1fdfb27..0000000
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/propagation/FieldReadBeforeWriteDfsAnalysis.java
+++ /dev/null
@@ -1,387 +0,0 @@
-// 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
deleted file mode 100644
index 7e24c03..0000000
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/propagation/FieldReadBeforeWriteDfsAnalysisState.java
+++ /dev/null
@@ -1,84 +0,0 @@
-// 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 81e3424..d62328d 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;
- protected final FieldStateCollection fieldStates;
+ final FieldStateCollection fieldStates;
final MethodStateCollectionByReference methodStates;
public InFlowPropagator(
@@ -113,15 +113,12 @@
private Map<FlowGraph, Deque<FlowGraphNode>> includeDefaultValuesInFieldStates(
List<FlowGraph> flowGraphs, ExecutorService executorService) throws ExecutionException {
- DefaultFieldValueJoiner joiner = createDefaultFieldValueJoiner(flowGraphs);
+ DefaultFieldValueJoiner joiner =
+ new DefaultFieldValueJoiner(
+ appView, classesWithSingleCallerInlinedInstanceInitializers, fieldStates, 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 22ed377..8d7498c 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,7 +9,6 @@
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;
@@ -34,8 +33,6 @@
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;
@@ -45,8 +42,6 @@
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;
@@ -97,22 +92,7 @@
InFlowPropagator inFlowPropagator =
new InFlowPropagator(
- 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();
- }
- };
- }
- };
+ appView, null, converter, codeScanner.getFieldStates(), codeScanner.getMethodStates());
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 891593a..c778df3 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,15 +8,14 @@
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> {
@@ -55,10 +54,6 @@
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 fa46f4d..0d912b3 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,7 +4,6 @@
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;
@@ -36,9 +35,7 @@
.assertSuccessWithOutputLines("foo", "hello", "5", "foobar")
.inspect(
codeInspector -> {
- assertThat(
- codeInspector.clazz(C.class),
- isAbsentIf(parameters.canInitNewInstanceUsingSuperclassConstructor()));
+ assertThat(codeInspector.clazz(C.class), isPresent());
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 142d565..f952fa9 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.assertTrue;
+import static org.junit.Assert.assertEquals;
import com.android.tools.r8.NeverClassInline;
import com.android.tools.r8.TestBase;
@@ -42,11 +42,17 @@
.inspect(
inspector -> {
ClassSubject aClassSubject = inspector.clazz(A.class);
- assertThat(aClassSubject, isAbsent());
+ // TODO(b/339210038): Should always be absent.
+ assertThat(
+ aClassSubject,
+ isPresentIf(parameters.canInitNewInstanceUsingSuperclassConstructor()));
MethodSubject mainMethodSubject = inspector.clazz(Main.class).mainMethod();
assertThat(mainMethodSubject, isPresent());
- assertTrue(mainMethodSubject.streamInstructions().anyMatch(i -> i.isConstNumber(42)));
+ // TODO(b/339210038): Should always contain 42.
+ assertEquals(
+ parameters.canInitNewInstanceUsingSuperclassConstructor(),
+ mainMethodSubject.streamInstructions().noneMatch(i -> i.isConstNumber(42)));
})
.run(parameters.getRuntime(), Main.class)
.assertSuccessWithOutputLines("42");
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 862fbc9..fca4473 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,20 +18,21 @@
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 {
- @Parameter(0)
- public TestParameters parameters;
+ private final TestParameters parameters;
- @Parameters(name = "{0}")
+ @Parameterized.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())
@@ -50,7 +51,10 @@
ClassSubject testClassSubject = inspector.clazz(TestClass.class);
assertThat(testClassSubject, isPresent());
assertThat(testClassSubject.uniqueMethodWithOriginalName("live"), isPresent());
- assertThat(testClassSubject.uniqueMethodWithOriginalName("dead"), isAbsent());
+ // TODO(b/280275115): Constructor inlining regresses instance field value analysis.
+ assertThat(
+ testClassSubject.uniqueMethodWithOriginalName("dead"),
+ isPresentIf(parameters.canInitNewInstanceUsingSuperclassConstructor()));
}
static class TestClass {
