Remove modeling of default parameters of Composable functions
Bug: b/302483644
Change-Id: Ie5f875d2b8b2747e3eddbf317d704a49ac339d6e
diff --git a/src/main/java/com/android/tools/r8/graph/AppView.java b/src/main/java/com/android/tools/r8/graph/AppView.java
index 1290cc8..85a71d4 100644
--- a/src/main/java/com/android/tools/r8/graph/AppView.java
+++ b/src/main/java/com/android/tools/r8/graph/AppView.java
@@ -542,7 +542,7 @@
}
public ComposeReferences getComposeReferences() {
- assert testing().modelUnknownChangedAndDefaultArgumentsToComposableFunctions;
+ assert testing().modelChangedArgumentsToComposableFunctions;
if (composeReferences == null) {
composeReferences = new ComposeReferences(dexItemFactory());
}
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/value/AbstractValueFactory.java b/src/main/java/com/android/tools/r8/ir/analysis/value/AbstractValueFactory.java
index 567d39a..2e59422 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/value/AbstractValueFactory.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/value/AbstractValueFactory.java
@@ -86,7 +86,7 @@
// Account for the temporary hack in the Compose modeling where we create a
// DefiniteBitsNumberValue with set bits=0b1^32 and unset bits = 0b1^(31)0. This value is used
// to simulate the effect of `x | 1` in joins.
- if (testingOptions.modelUnknownChangedAndDefaultArgumentsToComposableFunctions) {
+ if (testingOptions.modelChangedArgumentsToComposableFunctions) {
boolean overlappingSetAndUnsetBits = (definitelySetBits & definitelyUnsetBits) != 0;
if (overlappingSetAndUnsetBits) {
allBitsSet = false;
@@ -95,7 +95,7 @@
if (allBitsSet) {
return createUncheckedSingleNumberValue(definitelySetBits);
}
- return new DefiniteBitsNumberValue(definitelySetBits, definitelyUnsetBits, testingOptions);
+ return new DefiniteBitsNumberValue(definitelySetBits, definitelyUnsetBits);
}
return AbstractValue.unknown();
}
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/value/DefiniteBitsNumberValue.java b/src/main/java/com/android/tools/r8/ir/analysis/value/DefiniteBitsNumberValue.java
index a18281e..778e61b 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/value/DefiniteBitsNumberValue.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/value/DefiniteBitsNumberValue.java
@@ -8,7 +8,6 @@
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.lens.GraphLens;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
-import com.android.tools.r8.utils.InternalOptions.TestingOptions;
import com.android.tools.r8.utils.OptionalBool;
import java.util.Objects;
@@ -17,10 +16,8 @@
private final int definitelySetBits;
private final int definitelyUnsetBits;
- public DefiniteBitsNumberValue(
- int definitelySetBits, int definitelyUnsetBits, TestingOptions testingOptions) {
- assert (definitelySetBits & definitelyUnsetBits) == 0
- || testingOptions.modelUnknownChangedAndDefaultArgumentsToComposableFunctions;
+ public DefiniteBitsNumberValue(int definitelySetBits, int definitelyUnsetBits) {
+ assert (definitelySetBits & definitelyUnsetBits) == 0;
this.definitelySetBits = definitelySetBits;
this.definitelyUnsetBits = definitelyUnsetBits;
}
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorCodeScannerModeling.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorCodeScannerModeling.java
index 3e5e433..c5c3476 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorCodeScannerModeling.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorCodeScannerModeling.java
@@ -17,7 +17,7 @@
ArgumentPropagatorCodeScannerModeling(AppView<AppInfoWithLiveness> appView) {
this.composeModeling =
- appView.testing().modelUnknownChangedAndDefaultArgumentsToComposableFunctions
+ appView.testing().modelChangedArgumentsToComposableFunctions
? new ArgumentPropagatorComposeModeling(appView)
: null;
}
diff --git a/src/main/java/com/android/tools/r8/optimize/compose/ArgumentPropagatorComposeModeling.java b/src/main/java/com/android/tools/r8/optimize/compose/ArgumentPropagatorComposeModeling.java
index 332b695..8450fe3 100644
--- a/src/main/java/com/android/tools/r8/optimize/compose/ArgumentPropagatorComposeModeling.java
+++ b/src/main/java/com/android/tools/r8/optimize/compose/ArgumentPropagatorComposeModeling.java
@@ -38,7 +38,7 @@
private final DexString invokeName;
public ArgumentPropagatorComposeModeling(AppView<AppInfoWithLiveness> appView) {
- assert appView.testing().modelUnknownChangedAndDefaultArgumentsToComposableFunctions;
+ assert appView.testing().modelChangedArgumentsToComposableFunctions;
this.appView = appView;
this.rewrittenComposeReferences =
appView
@@ -145,92 +145,83 @@
return null;
}
- // We only model the two last arguments ($$changed and $$default) to the @Composable function.
- // If this argument is not on of these two int arguments, then don't apply any modeling.
- if (argumentIndex <= composerParameterIndex) {
+ // We only model the $$changed argument to the @Composable function.
+ if (argumentIndex != composerParameterIndex + 1) {
return null;
}
assert argument.getType().isInt();
- DexString expectedFieldName =
- !hasDefaultParameter || argumentIndex == invokedMethod.getArity() - 2
- ? rewrittenComposeReferences.changedFieldName
- : rewrittenComposeReferences.defaultFieldName;
- DexField expectedField =
+ DexField changedField =
appView
.dexItemFactory()
.createField(
- context.getHolderType(), appView.dexItemFactory().intType, expectedFieldName);
+ context.getHolderType(),
+ appView.dexItemFactory().intType,
+ rewrittenComposeReferences.changedFieldName);
UpdateChangedFlagsAbstractFunction inFlow = null;
- if (expectedFieldName.isIdenticalTo(rewrittenComposeReferences.changedFieldName)) {
- // We are looking at an argument to the $$changed parameter of the @Composable function.
- // We generally expect this argument to be defined by a call to updateChangedFlags().
- if (argument.isDefinedByInstructionSatisfying(Instruction::isInvokeStatic)) {
- InvokeStatic invokeStatic = argument.getDefinition().asInvokeStatic();
- SingleResolutionResult<?> resolutionResult =
- invokeStatic.resolveMethod(appView, context).asSingleResolution();
- if (resolutionResult == null) {
- return null;
- }
- DexClassAndMethod invokeSingleTarget =
- resolutionResult
- .lookupDispatchTarget(appView, invokeStatic, context)
- .getSingleDispatchTarget();
- if (invokeSingleTarget == null) {
- return null;
- }
- inFlow =
- invokeSingleTarget
- .getOptimizationInfo()
- .getAbstractFunction()
- .asUpdateChangedFlagsAbstractFunction();
- if (inFlow == null) {
- return null;
- }
- // By accounting for the abstract function we can safely strip the call.
- argument = invokeStatic.getFirstArgument();
+ // We are looking at an argument to the $$changed parameter of the @Composable function.
+ // We generally expect this argument to be defined by a call to updateChangedFlags().
+ if (argument.isDefinedByInstructionSatisfying(Instruction::isInvokeStatic)) {
+ InvokeStatic invokeStatic = argument.getDefinition().asInvokeStatic();
+ SingleResolutionResult<?> resolutionResult =
+ invokeStatic.resolveMethod(appView, context).asSingleResolution();
+ if (resolutionResult == null) {
+ return null;
}
- // Allow the argument to be defined by `this.$$changed | 1`.
- if (argument.isDefinedByInstructionSatisfying(Instruction::isOr)) {
- Or or = argument.getDefinition().asOr();
- Value maybeNumberOperand =
- or.leftValue().isConstNumber() ? or.leftValue() : or.rightValue();
- Value otherOperand = or.getOperand(1 - or.inValues().indexOf(maybeNumberOperand));
- if (!maybeNumberOperand.isConstNumber(1)) {
- return null;
- }
- // Strip the OR instruction.
- argument = otherOperand;
- // Update the model from bottom to a special value that effectively throws away any known
- // information about the lowermost bit of $$changed.
- SingleNumberValue one =
- appView.abstractValueFactory().createSingleNumberValue(1, TypeElement.getInt());
- inFlow =
- new UpdateChangedFlagsAbstractFunction(
- new OrAbstractFunction(new FieldValue(expectedField), one));
- } else {
- inFlow = new UpdateChangedFlagsAbstractFunction(new FieldValue(expectedField));
+ DexClassAndMethod invokeSingleTarget =
+ resolutionResult
+ .lookupDispatchTarget(appView, invokeStatic, context)
+ .getSingleDispatchTarget();
+ if (invokeSingleTarget == null) {
+ return null;
}
+ inFlow =
+ invokeSingleTarget
+ .getOptimizationInfo()
+ .getAbstractFunction()
+ .asUpdateChangedFlagsAbstractFunction();
+ if (inFlow == null) {
+ return null;
+ }
+ // By accounting for the abstract function we can safely strip the call.
+ argument = invokeStatic.getFirstArgument();
+ }
+ // Allow the argument to be defined by `this.$$changed | 1`.
+ if (argument.isDefinedByInstructionSatisfying(Instruction::isOr)) {
+ Or or = argument.getDefinition().asOr();
+ Value maybeNumberOperand = or.leftValue().isConstNumber() ? or.leftValue() : or.rightValue();
+ Value otherOperand = or.getOperand(1 - or.inValues().indexOf(maybeNumberOperand));
+ if (!maybeNumberOperand.isConstNumber(1)) {
+ return null;
+ }
+ // Strip the OR instruction.
+ argument = otherOperand;
+ // Update the model from bottom to a special value that effectively throws away any known
+ // information about the lowermost bit of $$changed.
+ SingleNumberValue one =
+ appView.abstractValueFactory().createSingleNumberValue(1, TypeElement.getInt());
+ inFlow =
+ new UpdateChangedFlagsAbstractFunction(
+ new OrAbstractFunction(new FieldValue(changedField), one));
+ } else {
+ inFlow = new UpdateChangedFlagsAbstractFunction(new FieldValue(changedField));
}
- // At this point we expect that the restart lambda is reading either this.$$changed or
- // this.$$default using an instance-get.
+ // At this point we expect that the restart lambda is reading this.$$changed using an
+ // instance-get.
if (!argument.isDefinedByInstructionSatisfying(Instruction::isInstanceGet)) {
return null;
}
// Check that the instance-get is reading the capture field that we expect it to.
InstanceGet instanceGet = argument.getDefinition().asInstanceGet();
- if (!instanceGet.getField().isIdenticalTo(expectedField)) {
+ if (!instanceGet.getField().isIdenticalTo(changedField)) {
return null;
}
- // Return the argument model. Note that, for the $$default field, this is always bottom, which
- // is equivalent to modeling that this call does not contribute any new argument information.
- return inFlow != null
- ? new ConcretePrimitiveTypeValueState(inFlow)
- : ValueState.bottomPrimitiveTypeParameter();
+ // Return the argument model.
+ return new ConcretePrimitiveTypeValueState(inFlow);
}
}
diff --git a/src/main/java/com/android/tools/r8/optimize/compose/ComposableOptimizationPass.java b/src/main/java/com/android/tools/r8/optimize/compose/ComposableOptimizationPass.java
index 1c23e06..3da6f3b 100644
--- a/src/main/java/com/android/tools/r8/optimize/compose/ComposableOptimizationPass.java
+++ b/src/main/java/com/android/tools/r8/optimize/compose/ComposableOptimizationPass.java
@@ -41,7 +41,7 @@
}
TestingOptions testingOptions = options.getTestingOptions();
if (!testingOptions.enableComposableOptimizationPass
- || !testingOptions.modelUnknownChangedAndDefaultArgumentsToComposableFunctions) {
+ || !testingOptions.modelChangedArgumentsToComposableFunctions) {
return;
}
timing.time(
diff --git a/src/main/java/com/android/tools/r8/optimize/compose/ComposeReferences.java b/src/main/java/com/android/tools/r8/optimize/compose/ComposeReferences.java
index e441188..e38f9a4 100644
--- a/src/main/java/com/android/tools/r8/optimize/compose/ComposeReferences.java
+++ b/src/main/java/com/android/tools/r8/optimize/compose/ComposeReferences.java
@@ -12,7 +12,6 @@
public class ComposeReferences {
public final DexString changedFieldName;
- public final DexString defaultFieldName;
public final DexType composableType;
public final DexType composerType;
@@ -21,7 +20,6 @@
public ComposeReferences(DexItemFactory factory) {
changedFieldName = factory.createString("$$changed");
- defaultFieldName = factory.createString("$$default");
composableType = factory.createType("Landroidx/compose/runtime/Composable;");
composerType = factory.createType("Landroidx/compose/runtime/Composer;");
@@ -35,12 +33,10 @@
public ComposeReferences(
DexString changedFieldName,
- DexString defaultFieldName,
DexType composableType,
DexType composerType,
DexMethod updatedChangedFlagsMethod) {
this.changedFieldName = changedFieldName;
- this.defaultFieldName = defaultFieldName;
this.composableType = composableType;
this.composerType = composerType;
this.updatedChangedFlagsMethod = updatedChangedFlagsMethod;
@@ -49,7 +45,6 @@
public ComposeReferences rewrittenWithLens(GraphLens graphLens, GraphLens codeLens) {
return new ComposeReferences(
changedFieldName,
- defaultFieldName,
graphLens.lookupClassType(composableType, codeLens),
graphLens.lookupClassType(composerType, codeLens),
graphLens.getRenamedMethodSignature(updatedChangedFlagsMethod, codeLens));
diff --git a/src/main/java/com/android/tools/r8/shaking/AnnotationRemover.java b/src/main/java/com/android/tools/r8/shaking/AnnotationRemover.java
index 40e2beb..4254412 100644
--- a/src/main/java/com/android/tools/r8/shaking/AnnotationRemover.java
+++ b/src/main/java/com/android/tools/r8/shaking/AnnotationRemover.java
@@ -334,7 +334,7 @@
AnnotatedKind kind,
Mode mode,
InternalOptions options) {
- return options.testing.modelUnknownChangedAndDefaultArgumentsToComposableFunctions
+ return options.testing.modelChangedArgumentsToComposableFunctions
&& mode.isInitialTreeShaking()
&& kind.isMethod()
&& annotation
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index 7b83b69..a00cd5b 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -2392,10 +2392,9 @@
public boolean enableComposableOptimizationPass =
SystemPropertyUtils.parseSystemPropertyForDevelopmentOrDefault(
"com.android.tools.r8.enableComposableOptimizationPass", false);
- public boolean modelUnknownChangedAndDefaultArgumentsToComposableFunctions =
+ public boolean modelChangedArgumentsToComposableFunctions =
SystemPropertyUtils.parseSystemPropertyForDevelopmentOrDefault(
- "com.android.tools.r8.modelUnknownChangedAndDefaultArgumentsToComposableFunctions",
- false);
+ "com.android.tools.r8.modelChangedArgumentsToComposableFunctions", false);
// Flag to allow processing of resources in D8. A data resource consumer still needs to be
// specified.
diff --git a/src/test/java/com/android/tools/r8/compose/NestedComposableArgumentPropagationTest.java b/src/test/java/com/android/tools/r8/compose/NestedComposableArgumentPropagationTest.java
index d056f65..7ca99c9 100644
--- a/src/test/java/com/android/tools/r8/compose/NestedComposableArgumentPropagationTest.java
+++ b/src/test/java/com/android/tools/r8/compose/NestedComposableArgumentPropagationTest.java
@@ -117,7 +117,7 @@
options -> {
options.getOpenClosedInterfacesOptions().suppressAllOpenInterfaces();
options.testing.enableComposableOptimizationPass = enableComposeOptimizations;
- options.testing.modelUnknownChangedAndDefaultArgumentsToComposableFunctions =
+ options.testing.modelChangedArgumentsToComposableFunctions =
enableComposeOptimizations;
})
.setMinApi(AndroidApiLevel.N)
