Add support for pushing resource values into static field values
This was originally disabled, but in cases where we have keeps on R
classes this is increasing the code size of using non final fields
Bug: b/287398085
Change-Id: Ia20a45916d67b60d817ac32d25735a25a387d2c1
diff --git a/src/main/java/com/android/tools/r8/graph/DexValue.java b/src/main/java/com/android/tools/r8/graph/DexValue.java
index fe532fc..0bd18ff 100644
--- a/src/main/java/com/android/tools/r8/graph/DexValue.java
+++ b/src/main/java/com/android/tools/r8/graph/DexValue.java
@@ -30,6 +30,7 @@
import com.android.tools.r8.utils.structural.StructuralMapping;
import it.unimi.dsi.fastutil.objects.Reference2IntMap;
import java.util.Arrays;
+import java.util.Objects;
import java.util.function.Consumer;
import java.util.function.Supplier;
import org.objectweb.asm.ConstantDynamic;
@@ -57,7 +58,8 @@
ANNOTATION(0x1d),
NULL(0x1e),
BOOLEAN(0x1f),
- CONST_DYNAMIC(-1);
+ CONST_DYNAMIC(-1),
+ RESOURCE_NUMBER(-2);
public static DexValueKind fromId(int id) {
switch (id) {
@@ -99,6 +101,8 @@
return BOOLEAN;
case -1:
return CONST_DYNAMIC;
+ case -2:
+ return RESOURCE_NUMBER;
default:
throw new Unreachable();
}
@@ -266,6 +270,14 @@
return null;
}
+ public boolean isDexValueResourceNumber() {
+ return false;
+ }
+
+ public DexValueResourceNumber asDexValueResourceNumber() {
+ return null;
+ }
+
public boolean isDexValueLong() {
return false;
}
@@ -886,6 +898,96 @@
}
}
+ public static class DexValueResourceNumber extends DexValueNumber {
+ private final int value;
+
+ private DexValueResourceNumber(int value) {
+ this.value = value;
+ }
+
+ public static DexValueResourceNumber create(int value) {
+ return new DexValueResourceNumber(value);
+ }
+
+ public int getValue() {
+ return value;
+ }
+
+ @Override
+ public AbstractValue toAbstractValue(AbstractValueFactory factory) {
+ return factory.createSingleResourceNumberValue(getValue());
+ }
+
+ @Override
+ public void writeTo(DexOutputBuffer dest, ObjectToOffsetMapping mapping) {
+ writeIntegerTo(DexValueKind.INT, value, Integer.BYTES, dest);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(value, getValueKind());
+ }
+
+ @Override
+ public boolean equals(Object other) {
+ if (other == this) {
+ return true;
+ }
+ return other instanceof DexValueResourceNumber
+ && value == ((DexValueResourceNumber) other).value;
+ }
+
+ @Override
+ public String toString() {
+ return "ResourceNumber " + value;
+ }
+
+ @Override
+ public DexType getType(DexItemFactory factory) {
+ return factory.intType;
+ }
+
+ @Override
+ public long getRawValue() {
+ return value;
+ }
+
+ @Override
+ public Object getBoxedValue() {
+ return value;
+ }
+
+ @Override
+ public Object asAsmEncodedObject() {
+ return Integer.valueOf(value);
+ }
+
+ @Override
+ void internalAcceptHashing(HashingVisitor visitor) {
+ visitor.visitInt(value);
+ }
+
+ @Override
+ int internalAcceptCompareTo(DexValue other, CompareToVisitor visitor) {
+ return visitor.visitInt(value, other.asDexValueResourceNumber().value);
+ }
+
+ @Override
+ public DexValueKind getValueKind() {
+ return DexValueKind.RESOURCE_NUMBER;
+ }
+
+ @Override
+ public boolean isDexValueResourceNumber() {
+ return true;
+ }
+
+ @Override
+ public DexValueResourceNumber asDexValueResourceNumber() {
+ return this;
+ }
+ }
+
public static class DexValueLong extends DexValueNumber {
public static final DexValueLong DEFAULT = new DexValueLong(0);
diff --git a/src/main/java/com/android/tools/r8/graph/UseRegistry.java b/src/main/java/com/android/tools/r8/graph/UseRegistry.java
index b69adc0..986267c 100644
--- a/src/main/java/com/android/tools/r8/graph/UseRegistry.java
+++ b/src/main/java/com/android/tools/r8/graph/UseRegistry.java
@@ -277,7 +277,9 @@
|| arg.isDexValueLong()
|| arg.isDexValueFloat()
|| arg.isDexValueDouble()
- || arg.isDexValueString();
+ || arg.isDexValueString()
+ || arg.isDexValueResourceNumber();
+ break;
}
if (continuation.shouldBreak()) {
break;
diff --git a/src/main/java/com/android/tools/r8/graph/analysis/ResourceAccessAnalysis.java b/src/main/java/com/android/tools/r8/graph/analysis/ResourceAccessAnalysis.java
index fa4979c..3d1cb69 100644
--- a/src/main/java/com/android/tools/r8/graph/analysis/ResourceAccessAnalysis.java
+++ b/src/main/java/com/android/tools/r8/graph/analysis/ResourceAccessAnalysis.java
@@ -15,6 +15,7 @@
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.DexValue;
import com.android.tools.r8.graph.FieldResolutionResult.SingleFieldResolutionResult;
+import com.android.tools.r8.graph.ProgramDefinition;
import com.android.tools.r8.graph.ProgramField;
import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.ir.code.IRCode;
@@ -46,9 +47,26 @@
public static void register(
AppView<? extends AppInfoWithClassHierarchy> appView, Enqueuer enqueuer) {
- if (enabled(appView, enqueuer)) {
+ if (fieldAccessAnalysisEnabled(appView, enqueuer)) {
enqueuer.registerFieldAccessAnalysis(new ResourceAccessAnalysis(appView, enqueuer));
}
+ if (liveFieldAnalysisEnabled(appView, enqueuer)) {
+ enqueuer.registerAnalysis(
+ new EnqueuerAnalysis() {
+ @Override
+ public void processNewlyLiveField(
+ ProgramField field, ProgramDefinition context, EnqueuerWorklist worklist) {
+ DexEncodedField definition = field.getDefinition();
+ if (field.getAccessFlags().isStatic()
+ && definition.hasExplicitStaticValue()
+ && definition.getStaticValue().isDexValueResourceNumber()) {
+ appView
+ .getResourceShrinkerState()
+ .trace(definition.getStaticValue().asDexValueResourceNumber().getValue());
+ }
+ }
+ });
+ }
}
@Override
@@ -56,7 +74,14 @@
EnqueuerFieldAccessAnalysis.super.done(enqueuer);
}
- private static boolean enabled(
+ private static boolean liveFieldAnalysisEnabled(
+ AppView<? extends AppInfoWithClassHierarchy> appView, Enqueuer enqueuer) {
+ return appView.options().androidResourceProvider != null
+ && appView.options().isOptimizedResourceShrinking()
+ && enqueuer.getMode().isFinalTreeShaking();
+ }
+
+ private static boolean fieldAccessAnalysisEnabled(
AppView<? extends AppInfoWithClassHierarchy> appView, Enqueuer enqueuer) {
return appView.options().androidResourceProvider != null
&& appView.options().resourceShrinkerConfiguration.isOptimizedShrinking()
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/ClassInitializerDefaultsOptimization.java b/src/main/java/com/android/tools/r8/ir/optimize/ClassInitializerDefaultsOptimization.java
index afaf0c7..c6d8ac4 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/ClassInitializerDefaultsOptimization.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/ClassInitializerDefaultsOptimization.java
@@ -28,6 +28,7 @@
import com.android.tools.r8.graph.DexValue.DexValueInt;
import com.android.tools.r8.graph.DexValue.DexValueLong;
import com.android.tools.r8.graph.DexValue.DexValueNull;
+import com.android.tools.r8.graph.DexValue.DexValueResourceNumber;
import com.android.tools.r8.graph.DexValue.DexValueShort;
import com.android.tools.r8.graph.DexValue.DexValueString;
import com.android.tools.r8.graph.FieldResolutionResult;
@@ -188,6 +189,9 @@
} else {
throw new Unreachable("Unexpected default value for field type " + fieldType + ".");
}
+ } else if (value.isConstResourceNumber()) {
+ int resourceValue = value.getDefinition().asResourceConstNumber().getValue();
+ fieldsWithStaticValues.put(field, DexValueResourceNumber.create(resourceValue));
} else {
ConstNumber cnst = value.getConstInstruction().asConstNumber();
if (fieldType == dexItemFactory.booleanType) {
@@ -442,7 +446,7 @@
isWrittenBefore.remove(fieldReference);
}
continue;
- } else if ((fieldReference.type.isPrimitiveType() && !hasPutOfConstResource(put))
+ } else if (fieldReference.type.isPrimitiveType()
|| fieldReference.type == dexItemFactory.stringType) {
finalFieldPuts.put(field, put);
unnecessaryStaticPuts.add(put);
@@ -509,10 +513,6 @@
return validateFinalFieldPuts(finalFieldPuts, isWrittenBefore);
}
- private boolean hasPutOfConstResource(StaticPut put) {
- return put.value().isConstResourceNumber();
- }
-
private Map<DexEncodedField, StaticPut> validateFinalFieldPuts(
Map<DexEncodedField, StaticPut> finalFieldPuts,
Map<DexField, Set<StaticPut>> isWrittenBefore) {
diff --git a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
index 4a8af9c..530ec46 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -3333,6 +3333,11 @@
// Already live.
return;
}
+
+ assert !field.getAccessFlags().isStatic()
+ || !field.getDefinition().hasExplicitStaticValue()
+ || !field.getDefinition().getStaticValue().isDexValueResourceNumber()
+ || mode.isFinalTreeShaking();
addEffectivelyLiveOriginalField(field);
// Mark the field as targeted.
diff --git a/src/test/java/com/android/tools/r8/androidresources/ConstResourceValueToStaticFieldTest.java b/src/test/java/com/android/tools/r8/androidresources/ConstResourceValueToStaticFieldTest.java
new file mode 100644
index 0000000..82459aa
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/androidresources/ConstResourceValueToStaticFieldTest.java
@@ -0,0 +1,97 @@
+// Copyright (c) 2024, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package com.android.tools.r8.androidresources;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isAbsent;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.android.tools.r8.R8FullTestBuilder;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.androidresources.AndroidResourceTestingUtils.AndroidTestResource;
+import com.android.tools.r8.androidresources.AndroidResourceTestingUtils.AndroidTestResourceBuilder;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class ConstResourceValueToStaticFieldTest extends TestBase {
+
+ @Parameter(0)
+ public TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection parameters() {
+ return getTestParameters().withDefaultDexRuntime().withAllApiLevels().build();
+ }
+
+ public static AndroidTestResource getTestResources(TemporaryFolder temp) throws Exception {
+ return new AndroidTestResourceBuilder()
+ .withSimpleManifestAndAppNameString()
+ .addRClassInitializeWithDefaultValues(R.string.class)
+ .build(temp);
+ }
+
+ private R8FullTestBuilder getSharedBuilder() throws Exception {
+ return testForR8(parameters.getBackend())
+ .setMinApi(parameters)
+ .addProgramClasses(FooBar.class)
+ .addAndroidResources(getTestResources(temp))
+ .addKeepMainRule(FooBar.class)
+ .enableOptimizedShrinking();
+ }
+
+ @Test
+ public void testR8KeepAll() throws Exception {
+ getSharedBuilder()
+ .addKeepAllClassesRule()
+ .compile()
+ .inspect(
+ codeInspector -> {
+ ClassSubject rStringClass = codeInspector.clazz(R.string.class);
+ // Since we have the keep all we should still have the class and the init.
+ assertThat(rStringClass, isPresent());
+ assertThat(rStringClass.uniqueInstanceInitializer(), isPresent());
+ assertThat(rStringClass.field("int", "foo"), isPresent());
+ // Even with the keep all we should have removed the field write, and hence the
+ // clinit.
+ assertThat(rStringClass.clinit(), isAbsent());
+ })
+ .run(parameters.getRuntime(), FooBar.class)
+ .assertSuccessWithEmptyOutput();
+ }
+
+ @Test
+ public void testR8NoKeep() throws Exception {
+ getSharedBuilder()
+ .compile()
+ .inspect(
+ codeInspector -> {
+ ClassSubject rStringClass = codeInspector.clazz(R.string.class);
+ assertThat(rStringClass, isAbsent());
+ })
+ .run(parameters.getRuntime(), FooBar.class)
+ .assertSuccessWithEmptyOutput();
+ }
+
+ public static class FooBar {
+ public static void main(String[] args) {
+ if (System.currentTimeMillis() == 0) {
+ System.out.println(R.string.foo);
+ }
+ }
+ }
+
+ public static class R {
+ public static class string {
+ public static int foo = 0x7f110004;
+ }
+ }
+}
